Provide equivalent IdentityManager API to OAuth2TokenService::GetAccounts() |
|||||||||
Issue descriptionThis should be implemented by having IdentityManager listen for refresh tokens being available and revoked, and then having IdentityManager cache the information of the available accounts locally. This implementation will map easily to eventual interaction of IdentityManager with the IdentityService. Note that unlike the primary account, these accounts are not made available synchronously on startup. Clients that want to know when all the tokens are loaded must listen for the O2TS::Observer callback that all tokens were loaded. Enabling that usage model to transition to the Identity Service is crbug.com/740117.
,
Feb 5 2018
,
Feb 6 2018
,
Apr 16 2018
,
May 8 2018
,
May 25 2018
,
Jun 14 2018
,
Jun 14 2018
Yay for "Started"! One additional requirement that we previously talked about, but that isn't mentioned here yet: We'd like the accounts to be sorted in content area/cookie order, i.e. the same order in which they'll show up in the OneGoogleBar account switcher. More precisely: We care which account is the default (first) content area account, i.e. authuser=0.
,
Jun 14 2018
I'm looking at that as well. At this point, I'm thinking about adding that as a separate API. I'm going to start writing up a short design doc for it.
,
Jun 28 2018
,
Jun 29 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cb05af1b8e952afa7bc2729ed16f838b278dbcd4 commit cb05af1b8e952afa7bc2729ed16f838b278dbcd4 Author: Colin Blundell <blundell@chromium.org> Date: Fri Jun 29 13:13:53 2018 IdentityManager: Add APIs for querying state of refresh tokens This CL adds APIs to IdentityManager for obtaining the set of all accounts with refresh tokens and querying whether the primary account is available with a refresh token. The design follows that of IdentityManager's caching of the primary account information: - IdentityManager initializes its state with the current state of ProfileOAuth2TokenService. - IdentityManager updates this state in response to notifications from PO2TS that an account's refresh token was updated/removed. TBR=bsazonov@chromium.org Bug: 806774 Change-Id: Idc4a37a15ced2e7d32f012a50e4e0d1fdb1aecdf Reviewed-on: https://chromium-review.googlesource.com/1098668 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Mihai Sardarescu <msarda@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Cr-Commit-Position: refs/heads/master@{#571451} [modify] https://crrev.com/cb05af1b8e952afa7bc2729ed16f838b278dbcd4/chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceIntegrationTest.java [modify] https://crrev.com/cb05af1b8e952afa7bc2729ed16f838b278dbcd4/services/identity/public/cpp/identity_manager.cc [modify] https://crrev.com/cb05af1b8e952afa7bc2729ed16f838b278dbcd4/services/identity/public/cpp/identity_manager.h [modify] https://crrev.com/cb05af1b8e952afa7bc2729ed16f838b278dbcd4/services/identity/public/cpp/identity_manager_unittest.cc
,
Jul 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f62594528efd0a3e606a7c9da9ebc4efbbe2452c commit f62594528efd0a3e606a7c9da9ebc4efbbe2452c Author: Robert Liao <robliao@chromium.org> Date: Mon Jul 02 23:50:45 2018 Revert "IdentityManager: Add APIs for querying state of refresh tokens" This reverts commit cb05af1b8e952afa7bc2729ed16f838b278dbcd4. Reason for revert: Causes a DCHECK followed by a crash. http://crbug.com/859697 Original change's description: > IdentityManager: Add APIs for querying state of refresh tokens > > This CL adds APIs to IdentityManager for obtaining the set of all > accounts with refresh tokens and querying whether the primary account > is available with a refresh token. > > The design follows that of IdentityManager's caching of the primary > account information: > - IdentityManager initializes its state with the current state of > ProfileOAuth2TokenService. > - IdentityManager updates this state in response to notifications from > PO2TS that an account's refresh token was updated/removed. > > TBR=bsazonov@chromium.org > > Bug: 806774 > Change-Id: Idc4a37a15ced2e7d32f012a50e4e0d1fdb1aecdf > Reviewed-on: https://chromium-review.googlesource.com/1098668 > Commit-Queue: Colin Blundell <blundell@chromium.org> > Reviewed-by: Mihai Sardarescu <msarda@chromium.org> > Reviewed-by: Marc Treib <treib@chromium.org> > Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> > Cr-Commit-Position: refs/heads/master@{#571451} TBR=blundell@chromium.org,msarda@chromium.org,sdefresne@chromium.org,treib@chromium.org,bsazonov@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 806774 , 859697 Change-Id: I636d458101a5a579202dc4683faddfc0643cb19c Reviewed-on: https://chromium-review.googlesource.com/1123119 Reviewed-by: Robert Liao <robliao@chromium.org> Commit-Queue: Robert Liao <robliao@chromium.org> Cr-Commit-Position: refs/heads/master@{#572048} [modify] https://crrev.com/f62594528efd0a3e606a7c9da9ebc4efbbe2452c/chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceIntegrationTest.java [modify] https://crrev.com/f62594528efd0a3e606a7c9da9ebc4efbbe2452c/services/identity/public/cpp/identity_manager.cc [modify] https://crrev.com/f62594528efd0a3e606a7c9da9ebc4efbbe2452c/services/identity/public/cpp/identity_manager.h [modify] https://crrev.com/f62594528efd0a3e606a7c9da9ebc4efbbe2452c/services/identity/public/cpp/identity_manager_unittest.cc
,
Jul 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/225bf60cc1383b273a55eb6a414caa358eecad06 commit 225bf60cc1383b273a55eb6a414caa358eecad06 Author: Colin Blundell <blundell@chromium.org> Date: Tue Jul 03 13:12:42 2018 IdentityManager: Add APIs for querying state of refresh tokens This CL adds APIs to IdentityManager for obtaining the set of all accounts with refresh tokens and querying whether the primary account is available with a refresh token. The design follows that of IdentityManager's caching of the primary account information: - IdentityManager initializes its state with the current state of ProfileOAuth2TokenService. - IdentityManager updates this state in response to notifications from PO2TS that an account's refresh token was updated/removed. This CL is a reland of https://chromium-review.googlesource.com/c/chromium/src/+/1098668 with the following addition: In https://chromium-review.googlesource.com/c/chromium/src/+/1098668, we made the assumption that when IdentityManager receives a notification from ProfileOAuth2TokenService that a token will be revoked, it had previously known about that account (either because the account was available at IdentityManager startup or because IdentityManager previously received a notification from PO2TS that a refresh token had been made available for the account). However, as demonstrated in the linked bug, it turns out that this assumption is not valid in one case: while loading tokens during startup, PO2TS sometimes revokes tokens without having previously made them available. This violation of IdentityManager's assumption causes a crash. It is not feasible at the current time to change this behavior on the part of PO2TS, as AccountTrackerService should be made aware of these events so that it removes the accounts. However, this situation poses a challenge for IdentityManager: Should it forward on the callback to its own observers or not? In this CL, we fix the crash and additionally nail down the semantics that IdentityManager sends token removed callbacks only for accounts of which it previously knew the existence. The reasons for making this choice on the semantics are twofold: (1) Sending an observer callback that a token was removed for an account that was not previously present in IdentityManager::GetAccountsWithRefreshTokens() seems like a violation of the principle of least surprise. (2) It's not clear that AccountTrackerService will always know about the account in this case, in which case we wouldn't have the full information to forward on in the observer callback. This does mean that IdentityManager::Observer::OnRefreshTokenRemovedForAccount() is not strictly identical semantically to ProfileOAuth2TokenService::Observer::OnRefreshTokenRevoked(). However, we strongly believe that the only consumer that needs to know about revocations in this corner case is AccountTrackerService, which will not be converted to depend on IdentityManager as it lives below IdentityManager. TBR=bsazonov@chromium.org Bug: 806774 Change-Id: I9f77525e26825ab418b15c7d1c2a8e4f8e3b4910 Reviewed-on: https://chromium-review.googlesource.com/1124320 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Mihai Sardarescu <msarda@chromium.org> Cr-Commit-Position: refs/heads/master@{#572185} [modify] https://crrev.com/225bf60cc1383b273a55eb6a414caa358eecad06/chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceIntegrationTest.java [modify] https://crrev.com/225bf60cc1383b273a55eb6a414caa358eecad06/services/identity/public/cpp/identity_manager.cc [modify] https://crrev.com/225bf60cc1383b273a55eb6a414caa358eecad06/services/identity/public/cpp/identity_manager.h [modify] https://crrev.com/225bf60cc1383b273a55eb6a414caa358eecad06/services/identity/public/cpp/identity_manager_unittest.cc
,
Jul 3
|
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by blundell@chromium.org
, Jan 29 2018Components: Internals>Services>Identity
Status: Available (was: Untriaged)