New issue
Advanced search Search tips

Issue 890815 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Task

Blocked on:
issue 889863
issue 889899

Blocking:
issue 883330



Sign in to add a comment

Convert chrome/browser/ui/webui/signin/signin_utils_desktop.cc to IdentityManager

Project Member Reported by sdefresne@chromium.org, Oct 1

Issue description

API used:
- SigninManager::IsSigninAllowed()
- SigninManager::IsAllowedUsername()
- SigninManagerBase::GetAuthenticatedAccountInfo()

 
Labels: Proj-Servicification-VendorBug
Status: Started (was: Available)
Starting to work on this
The IsAllowedUsername bit will be part of CL1346409 (now landing)
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 28

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/198eca95746e419a01cc619e8f063b84a2cbdedc

commit 198eca95746e419a01cc619e8f063b84a2cbdedc
Author: Mario Sanchez Prada <mario@igalia.com>
Date: Wed Nov 28 15:45:09 2018

Migrate consumers of SigninManager::IsAllowedUsername()

Use identity::IsUsernameAllowedByPattern() instead so that we can migrate
those consumers entirely to the IdentityManager in follow-up patches.

Also, in order to make it explicit that this API should not be used any
more, make SigninManager::IsAllowedUsername() a private method and friend
it with the relevant test cases from SigninManagerTest, so that it can
only be used from there. And while at it, also remove some stale lines
"friending" SigninManager with test cases that no longer exist.

Bug:  890794 ,  890815 ,  906085 
Change-Id: Id8a72b0efd89818e02c6d1e73aac1451207b9625
Reviewed-on: https://chromium-review.googlesource.com/c/1346409
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Mario Sanchez Prada <mario@igalia.com>
Cr-Commit-Position: refs/heads/master@{#611704}
[modify] https://crrev.com/198eca95746e419a01cc619e8f063b84a2cbdedc/chrome/browser/signin/signin_ui_util.cc
[modify] https://crrev.com/198eca95746e419a01cc619e8f063b84a2cbdedc/chrome/browser/ui/webui/signin/signin_utils_desktop.cc
[modify] https://crrev.com/198eca95746e419a01cc619e8f063b84a2cbdedc/components/signin/core/browser/signin_manager.h

Project Member

Comment 5 by bugdroid1@chromium.org, Nov 28

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aabacf9ed204501b1766a505753a3a6a41ce503c

commit aabacf9ed204501b1766a505753a3a6a41ce503c
Author: Ned Nguyen <nednguyen@google.com>
Date: Wed Nov 28 16:08:37 2018

Revert "Migrate consumers of SigninManager::IsAllowedUsername()"

This reverts commit 198eca95746e419a01cc619e8f063b84a2cbdedc.

Reason for revert: breaking GPU Linux Builder

Original change's description:
> Migrate consumers of SigninManager::IsAllowedUsername()
> 
> Use identity::IsUsernameAllowedByPattern() instead so that we can migrate
> those consumers entirely to the IdentityManager in follow-up patches.
> 
> Also, in order to make it explicit that this API should not be used any
> more, make SigninManager::IsAllowedUsername() a private method and friend
> it with the relevant test cases from SigninManagerTest, so that it can
> only be used from there. And while at it, also remove some stale lines
> "friending" SigninManager with test cases that no longer exist.
> 
> Bug:  890794 ,  890815 ,  906085 
> Change-Id: Id8a72b0efd89818e02c6d1e73aac1451207b9625
> Reviewed-on: https://chromium-review.googlesource.com/c/1346409
> Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
> Commit-Queue: Mario Sanchez Prada <mario@igalia.com>
> Cr-Commit-Position: refs/heads/master@{#611704}

TBR=blundell@chromium.org,msarda@chromium.org,sdefresne@chromium.org,mario@igalia.com

Change-Id: I8172674e32d7671868fa5374a9457abd6413321e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  890794 ,  890815 ,  906085 
Reviewed-on: https://chromium-review.googlesource.com/c/1354121
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#611710}
[modify] https://crrev.com/aabacf9ed204501b1766a505753a3a6a41ce503c/chrome/browser/signin/signin_ui_util.cc
[modify] https://crrev.com/aabacf9ed204501b1766a505753a3a6a41ce503c/chrome/browser/ui/webui/signin/signin_utils_desktop.cc
[modify] https://crrev.com/aabacf9ed204501b1766a505753a3a6a41ce503c/components/signin/core/browser/signin_manager.h

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 28

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/482335f01cab4681a33b8b29bba3d1434b14fd09

commit 482335f01cab4681a33b8b29bba3d1434b14fd09
Author: Mario Sanchez Prada <mario@igalia.com>
Date: Wed Nov 28 17:48:51 2018

Migrate consumers of SigninManager::IsAllowedUsername()

Use identity::IsUsernameAllowedByPattern() instead so that we can migrate
those consumers entirely to the IdentityManager in follow-up patches.

Also, in order to make it explicit that this API should not be used any
more, make SigninManager::IsAllowedUsername() a private method and friend
it with the relevant test cases from SigninManagerTest, so that it can
only be used from there. And while at it, also remove some stale lines
"friending" SigninManager with test cases that no longer exist.

Bug:  890794 ,  890815 ,  906085 
Change-Id: I2a5be231ff70d3e390aa1718c795aef9d8b255ff
Reviewed-on: https://chromium-review.googlesource.com/c/1353894
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Mario Sanchez Prada <mario@igalia.com>
Cr-Commit-Position: refs/heads/master@{#611764}
[modify] https://crrev.com/482335f01cab4681a33b8b29bba3d1434b14fd09/chrome/browser/signin/signin_ui_util.cc
[modify] https://crrev.com/482335f01cab4681a33b8b29bba3d1434b14fd09/chrome/browser/signin/signin_util.cc
[modify] https://crrev.com/482335f01cab4681a33b8b29bba3d1434b14fd09/chrome/browser/ui/webui/signin/signin_utils_desktop.cc
[modify] https://crrev.com/482335f01cab4681a33b8b29bba3d1434b14fd09/components/signin/core/browser/signin_manager.h

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ed5ef6618829a7172f230e1b17c06cc508af7bcc

commit ed5ef6618829a7172f230e1b17c06cc508af7bcc
Author: Sergio Villar Senin <svillar@igalia.com>
Date: Thu Nov 29 17:04:04 2018

Convert signin_utils_desktop.cc to IdentityManager

Migrated the SigninManager APIs to the identity APIs.

Bug:  890815 
Change-Id: Id145953cdece6f58222bedaadbf3358ac3491665
Reviewed-on: https://chromium-review.googlesource.com/c/1354198
Commit-Queue: Sergio Villar <svillar@igalia.com>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612235}
[modify] https://crrev.com/ed5ef6618829a7172f230e1b17c06cc508af7bcc/chrome/browser/ui/webui/signin/signin_utils_desktop.cc

Status: Fixed (was: Started)
Owner: svil...@igalia.com

Sign in to add a comment