New issue
Advanced search Search tips

Issue 890794 link

Starred by 2 users

Issue metadata

Status: Duplicate
Merged: issue 890796
Owner: ----
Closed: Nov 20
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Task

Blocked on:
issue 889899

Blocking:
issue 883330
issue 898810



Sign in to add a comment

Convert chrome/browser/signin/signin_ui_util.cc to IdentityManager

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

Issue description

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

 
Blocking: 887461
Blocking: 898810
Blocking: -887461
Mergedinto: 890796
Status: Duplicate (was: Available)
Project Member

Comment 5 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 6 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 7 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

Sign in to add a comment