Port PrimaryAccountAccessTokenFetcher to be based on IdentityManager |
||
Issue descriptionPrimaryAccountAccessTokenFetcher currently talks directly to SigninManager/ProfileOAuth2TokenService. We need to instead have it talk to IdentityManager so that it is not coupled to these signin-internal classes.
,
Jun 4 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/abfc646f04fe2bff1be432de354a92bbf29c7622 commit abfc646f04fe2bff1be432de354a92bbf29c7622 Author: Colin Blundell <blundell@chromium.org> Date: Mon Jun 04 09:49:19 2018 Make accountID explicit in PrimaryAccountAccessTokenFetcher unittest The PrimaryAccountAccessTokenFetcher currently passes in a string to SigninManager(Base)'s signin APIs and makes the assumption that this string will end up being the account ID on all platforms. It validates this assumption by passing in the string as the email address when signing in on ChromeOS and as the Gaia ID when signing in on other platforms (since in fact the account ID is defined as the email address on ChromeOS and the Gaia ID on all other platforms). However, this flow won't work when porting this code to use IdentityManager. The corresponding IdentityTestEnvironment API takes in only the email address (by design), thus disallowing one string from playing two different roles depending on platform. To facilitate this porting, this CL changes the PrimaryAccountAccessTokenFetcher unittest to explicitly work with the account ID that SigninManager(Base) has for the primary account after the signin process has completed. Bug: 848642 Change-Id: I30807e8e7c771899726260c845e9adba5ab35381 Reviewed-on: https://chromium-review.googlesource.com/1082194 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Cr-Commit-Position: refs/heads/master@{#564040} [modify] https://crrev.com/abfc646f04fe2bff1be432de354a92bbf29c7622/services/identity/public/cpp/primary_account_access_token_fetcher_unittest.cc
,
Jun 4 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0605d7411404879ba447ba889f31201d51d23579 commit 0605d7411404879ba447ba889f31201d51d23579 Author: Colin Blundell <blundell@chromium.org> Date: Mon Jun 04 11:13:17 2018 IdentityManager: Add test APIs for setting primary account/refresh token To date, IdentityManager's test APIs (IdentityTestEnvironment/identity_test_utils.h) have intentionally only supported making the primary account available, i.e., signed in with a refresh token. However, PrimaryAccountAccessTokenFetcher unittests have a legitimate use case for wanting these steps separate: there are tests that test PrimaryAccountAccessTokenFetcher's correct operation in the case where a fetcher is started with the primary account set but with no token available for the primary account (with the token potentially being made available later). To prepare for porting PrimaryAccountAccessTokenFetcher, this CL adds IdentityManager test APIs for setting just the primary account and adding a refresh token for a given account. This CL also takes the opportunity to make the test APIs that set the primary account return the account ID, as this is useful to consumers that want to sign in the primary account and then do followup operations on that account; these consumers will generally need to know the primary account ID. Bug: 848642 , 798699 Change-Id: I249e6851779545346b5f521afe18150bec20e69f Reviewed-on: https://chromium-review.googlesource.com/1082197 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Cr-Commit-Position: refs/heads/master@{#564055} [modify] https://crrev.com/0605d7411404879ba447ba889f31201d51d23579/services/identity/public/cpp/identity_manager.h [modify] https://crrev.com/0605d7411404879ba447ba889f31201d51d23579/services/identity/public/cpp/identity_test_environment.cc [modify] https://crrev.com/0605d7411404879ba447ba889f31201d51d23579/services/identity/public/cpp/identity_test_environment.h [modify] https://crrev.com/0605d7411404879ba447ba889f31201d51d23579/services/identity/public/cpp/identity_test_utils.cc [modify] https://crrev.com/0605d7411404879ba447ba889f31201d51d23579/services/identity/public/cpp/identity_test_utils.h
,
Jul 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/463a901e90aeeab8d8b4ea019abb778248acf63f commit 463a901e90aeeab8d8b4ea019abb778248acf63f Author: Colin Blundell <blundell@chromium.org> Date: Mon Jul 16 08:35:50 2018 Port PrimaryAccountAccessTokenFetcher to talk to IdentityManager PAATF is currently built directly on top of SigninManager and ProfileOAuth2TokenService. It instead should be layered on top of IdentityManager, so that when the latter is converted to be backed by the Identity Service PAATF automatically also becomes a true part of the Identity Service client library. This CL makes that change, after a rather large amount of prepatory work giving IdentityManager all the necessary API surfaces. The actual change is straightforward after that prepatory work. The only wrinkle is the following: - In the codebase currently clients create PAATF instances via an IdentityManager API, necessitating that identity_manager.h include primary_account_access_token_fetcher.h for some type definitions in that API. - However, to port PAATF it is necessary for primary_account_access_token_fetcher.h to include identity_manager.h to be an IdentityManager::Observer. - Thus, it is necessary *in this CL* to change all clients to construct PAATF instances directly rather than via IdentityManager. This is a completely straightforward change, but results in this CL touching a bunch more files than it otherwise would. The substantive changes in this CL are the ones in //services/identity. TBR=jochen@chromium.org Bug: 848642 Change-Id: Ic8ecb00864b1c44f848f9daae03e01dd4f06dd05 Reviewed-on: https://chromium-review.googlesource.com/1095098 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Colin Blundell <blundell@chromium.org> Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Kyle Horimoto <khorimoto@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Cr-Commit-Position: refs/heads/master@{#575204} [modify] https://crrev.com/463a901e90aeeab8d8b4ea019abb778248acf63f/chrome/browser/feedback/feedback_uploader_chrome.cc [modify] https://crrev.com/463a901e90aeeab8d8b4ea019abb778248acf63f/chrome/browser/printing/cloud_print/gcd_api_flow_impl.cc [modify] https://crrev.com/463a901e90aeeab8d8b4ea019abb778248acf63f/chrome/browser/search/background/ntp_background_service.cc [modify] https://crrev.com/463a901e90aeeab8d8b4ea019abb778248acf63f/chrome/browser/ui/desktop_ios_promotion/sms_service.cc [modify] https://crrev.com/463a901e90aeeab8d8b4ea019abb778248acf63f/components/autofill/core/browser/payments/payments_client.cc [modify] https://crrev.com/463a901e90aeeab8d8b4ea019abb778248acf63f/components/browser_sync/sync_auth_manager.cc [modify] https://crrev.com/463a901e90aeeab8d8b4ea019abb778248acf63f/components/cryptauth/cryptauth_client_impl.cc [modify] https://crrev.com/463a901e90aeeab8d8b4ea019abb778248acf63f/components/feed/core/feed_networking_host.cc [modify] https://crrev.com/463a901e90aeeab8d8b4ea019abb778248acf63f/components/history/core/browser/web_history_service.cc [modify] https://crrev.com/463a901e90aeeab8d8b4ea019abb778248acf63f/components/ntp_snippets/breaking_news/subscription_manager_impl.cc [modify] https://crrev.com/463a901e90aeeab8d8b4ea019abb778248acf63f/components/ntp_snippets/remote/remote_suggestions_fetcher_impl.cc [modify] https://crrev.com/463a901e90aeeab8d8b4ea019abb778248acf63f/components/omnibox/browser/contextual_suggestions_service.cc [modify] https://crrev.com/463a901e90aeeab8d8b4ea019abb778248acf63f/components/suggestions/suggestions_service_impl.cc [modify] https://crrev.com/463a901e90aeeab8d8b4ea019abb778248acf63f/services/identity/public/cpp/identity_manager.cc [modify] https://crrev.com/463a901e90aeeab8d8b4ea019abb778248acf63f/services/identity/public/cpp/identity_manager.h [modify] https://crrev.com/463a901e90aeeab8d8b4ea019abb778248acf63f/services/identity/public/cpp/identity_manager_unittest.cc [modify] https://crrev.com/463a901e90aeeab8d8b4ea019abb778248acf63f/services/identity/public/cpp/primary_account_access_token_fetcher.cc [modify] https://crrev.com/463a901e90aeeab8d8b4ea019abb778248acf63f/services/identity/public/cpp/primary_account_access_token_fetcher.h [modify] https://crrev.com/463a901e90aeeab8d8b4ea019abb778248acf63f/services/identity/public/cpp/primary_account_access_token_fetcher_unittest.cc
,
Jul 16
|
||
►
Sign in to add a comment |
||
Comment 1 by bugdroid1@chromium.org
, Jun 4 2018