Convert GCM to talk to Identity Service client library |
|||||||
Issue descriptionCurrently //components/gcm_driver interacts with the user's Google identity via ProfileIdentityProvider and AccountTracker (the latter being backed by the former). GCMAccountTracker uses these classes to make access token requests for all accounts known to AccountTracker. This should change to interacting with the Identity Service client library.
,
Feb 7 2018
,
Feb 7 2018
,
Apr 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/021e3e4d708a695a7714daf1eae89cbfa4a07708 commit 021e3e4d708a695a7714daf1eae89cbfa4a07708 Author: Colin Blundell <blundell@chromium.org> Date: Thu Apr 19 01:03:24 2018 Move gaia::AccountTracker into //components/gcm_driver This class is deprecated and slated for removal. It now has only one remaining consumer. This CL moves it into the directory of that consumer, with followup merging it into the code of that consumer. This CL also excludes AccountTracker and GCMAccountTracker from the build on Android, as they're not used on that platform. Bug: 729590, 809923 Change-Id: I225edf86a3af293779a8cc231ffffac711f22246 Reviewed-on: https://chromium-review.googlesource.com/1002845 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Reviewed-by: Mihai Sardarescu <msarda@chromium.org> Cr-Commit-Position: refs/heads/master@{#551904} [modify] https://crrev.com/021e3e4d708a695a7714daf1eae89cbfa4a07708/components/gcm_driver/BUILD.gn [rename] https://crrev.com/021e3e4d708a695a7714daf1eae89cbfa4a07708/components/gcm_driver/account_tracker.cc [rename] https://crrev.com/021e3e4d708a695a7714daf1eae89cbfa4a07708/components/gcm_driver/account_tracker.h [rename] https://crrev.com/021e3e4d708a695a7714daf1eae89cbfa4a07708/components/gcm_driver/account_tracker_unittest.cc [modify] https://crrev.com/021e3e4d708a695a7714daf1eae89cbfa4a07708/components/gcm_driver/gcm_account_tracker.cc [modify] https://crrev.com/021e3e4d708a695a7714daf1eae89cbfa4a07708/components/gcm_driver/gcm_account_tracker.h [modify] https://crrev.com/021e3e4d708a695a7714daf1eae89cbfa4a07708/components/gcm_driver/gcm_account_tracker_unittest.cc [modify] https://crrev.com/021e3e4d708a695a7714daf1eae89cbfa4a07708/components/gcm_driver/gcm_profile_service.cc [modify] https://crrev.com/021e3e4d708a695a7714daf1eae89cbfa4a07708/google_apis/BUILD.gn
,
May 7 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dee2d72ad9cc43c9e8362f32186490c8c9283e83 commit dee2d72ad9cc43c9e8362f32186490c8c9283e83 Author: Colin Blundell <blundell@chromium.org> Date: Mon May 07 10:46:52 2018 [GCM] Use SigninManager in GCMProfileService::IdentityObserver As a first step toward completely eliminating the usage of ProfileIdentityProvider in //components/gcm_driver, this CL threads in SigninManager and replaces direct usage of ProfileIdentityProvider in GCMProfileService::IdentityObserver with equivalent usage of SigninManager. Note that it is easy to verify that the usage is equivalent: The ProfileIdentityProvider method calls are replaced with their implementations in profile_identity_provider.cc. Full design doc here: https://docs.google.com/document/d/1OmNrIiMDkF7eOYVB4RIiDvY7pQz3zUrOf-D4TVwdRQA/edit?ts=5aa6b936# TBR=zea@chromium.org, rockot@chromium.org Bug: 809923 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: I20515d7431f0e8f5370a47cd04c8b21eb0874d6f Reviewed-on: https://chromium-review.googlesource.com/1004999 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Colin Blundell <blundell@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Cr-Commit-Position: refs/heads/master@{#556408} [modify] https://crrev.com/dee2d72ad9cc43c9e8362f32186490c8c9283e83/chrome/browser/extensions/extension_gcm_app_handler_unittest.cc [modify] https://crrev.com/dee2d72ad9cc43c9e8362f32186490c8c9283e83/chrome/browser/gcm/gcm_profile_service_factory.cc [modify] https://crrev.com/dee2d72ad9cc43c9e8362f32186490c8c9283e83/chrome/browser/gcm/gcm_profile_service_unittest.cc [modify] https://crrev.com/dee2d72ad9cc43c9e8362f32186490c8c9283e83/components/gcm_driver/gcm_profile_service.cc [modify] https://crrev.com/dee2d72ad9cc43c9e8362f32186490c8c9283e83/components/gcm_driver/gcm_profile_service.h [modify] https://crrev.com/dee2d72ad9cc43c9e8362f32186490c8c9283e83/ios/chrome/browser/services/gcm/ios_chrome_gcm_profile_service_factory.cc
,
May 8 2018
,
May 8 2018
,
May 14 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/04720563e44ad0aaf1388917bf1bb79d74fa563e commit 04720563e44ad0aaf1388917bf1bb79d74fa563e Author: Colin Blundell <blundell@chromium.org> Date: Mon May 14 09:19:40 2018 Rationalize (GCM)AccountTracker unittests for ChromeOS The GCMAccountTracker and AccountTracker classes listen for the GoogleSigninSucceeded/GoogleSignedOut callbacks indirectly via ProfileIdentityProvider. Their unittests cause these callbacks to fire via calls to FakeIdentityProvider::{LogIn, LogOut}. These classes are also used on ChromeOS, where the callbacks will never fire (ChromeOS does not use SigninManager, which fires the callbacks). The structure of these unittests presents a challenge to refactoring the production code to depend on SigninManager directly: the SigninManagerBase test infrastructure, which is all that can be used on ChromeOS, rightfully has no mechanism for firing the callbacks. This CL rationalizes these tests for ChromeOS via: - Commenting out the tests that actually exercise the callbacks firing - Changing the rest of the tests to simply set the primary account info *without* causing the callbacks to fire - Renaming utility functions as relevant to make the structure clear Making this change paves the way for a followup change that ports these classes (and their tests) to depend directly on SigninManager. Bug: 809923 Change-Id: Ia6c067d845f7bbb7bd78928ac163bda2fdd77a20 Reviewed-on: https://chromium-review.googlesource.com/1042391 Reviewed-by: Peter Beverloo <peter@chromium.org> Reviewed-by: Roger Tawa <rogerta@chromium.org> Commit-Queue: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#558235} [modify] https://crrev.com/04720563e44ad0aaf1388917bf1bb79d74fa563e/components/gcm_driver/account_tracker_unittest.cc [modify] https://crrev.com/04720563e44ad0aaf1388917bf1bb79d74fa563e/components/gcm_driver/gcm_account_tracker_unittest.cc [modify] https://crrev.com/04720563e44ad0aaf1388917bf1bb79d74fa563e/google_apis/gaia/fake_identity_provider.cc [modify] https://crrev.com/04720563e44ad0aaf1388917bf1bb79d74fa563e/google_apis/gaia/fake_identity_provider.h
,
May 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bf6d4c5f3a083a5f5ea7264b8e5f47083887b6cf commit bf6d4c5f3a083a5f5ea7264b8e5f47083887b6cf Author: Colin Blundell <blundell@chromium.org> Date: Wed May 16 11:12:29 2018 [GCM] Use SigninManager in AccountTracker As a step toward completely eliminating the usage of ProfileIdentityProvider in //components/gcm_driver, this CL replaces direct usage of ProfileIdentityProvider in AccountTracker with equivalent usage of SigninManager. Note that it is easy to verify that the usage is equivalent: - The IdentityProvider instance passed in to AccountTracker is always a ProfileIdentityProvider instance. - The IdentityProvider method calls are replaced with their implementations in profile_identity_provider.cc. The production changes are straightforward, but substantial changes are required to the GCMAccountTracker and AccountTracker unittests to have them use FakeSigninManager(Base) where they were previously using FakeIdentityProvider. In particular, the GCMAccountTracker unittest now requires an explicit separation of when an account being added is the primary account vs. a secondary account (whereas FakeIdentityProvider was happy to simply override its value for the active account, SigninManager does not allow for signing in when there is already a signed-in account). The goop required to set up this fake infrastructure in the unittests will be substantially simplified when we complete the process of converting the production code to talk entirely to IdentityManager. Note that for the moment IdentityProvider is still used by AccountTracker to access the token service. A followup CL will pass in the token service directly and completely eliminate AccountTracker's usage of IdentityProvider. Full design doc here: https://docs.google.com/document/d/1OmNrIiMDkF7eOYVB4RIiDvY7pQz3zUrOf-D4TVwdRQA/edit?ts=5aa6b936# Bug: 809923 Change-Id: Ifef69551a215571362610167c3a4cb38c932ff70 Reviewed-on: https://chromium-review.googlesource.com/1046587 Reviewed-by: David Roger <droger@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Commit-Queue: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#559036} [modify] https://crrev.com/bf6d4c5f3a083a5f5ea7264b8e5f47083887b6cf/components/gcm_driver/BUILD.gn [modify] https://crrev.com/bf6d4c5f3a083a5f5ea7264b8e5f47083887b6cf/components/gcm_driver/DEPS [modify] https://crrev.com/bf6d4c5f3a083a5f5ea7264b8e5f47083887b6cf/components/gcm_driver/account_tracker.cc [modify] https://crrev.com/bf6d4c5f3a083a5f5ea7264b8e5f47083887b6cf/components/gcm_driver/account_tracker.h [modify] https://crrev.com/bf6d4c5f3a083a5f5ea7264b8e5f47083887b6cf/components/gcm_driver/account_tracker_unittest.cc [modify] https://crrev.com/bf6d4c5f3a083a5f5ea7264b8e5f47083887b6cf/components/gcm_driver/gcm_account_tracker_unittest.cc [modify] https://crrev.com/bf6d4c5f3a083a5f5ea7264b8e5f47083887b6cf/components/gcm_driver/gcm_profile_service.cc
,
May 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f878993246cf9636795648f4e0183328dd835e6d commit f878993246cf9636795648f4e0183328dd835e6d Author: Colin Blundell <blundell@chromium.org> Date: Tue May 22 11:35:56 2018 [GCM] Use PO2TS directly rather than via ProfileIdentityProvider To complete the elimination of usage of ProfileIdentityProvider in //components/gcm_driver, this CL replaces usage of ProfileIdentityProvider in (GCM)AccountTracker to obtain the Oauth2TokenService with direct usage of the ProfileOAuth2TokenService instance that is backing the ProfileIdentityProvider instance. This CL is a step on the road to eventually using IdentityManager in //components/gcm_driver. The CL is completely straightforward. Full design doc here: https://docs.google.com/document/d/1OmNrIiMDkF7eOYVB4RIiDvY7pQz3zUrOf-D4TVwdRQA/edit?ts=5aa6b936# TBR=rockot@chromium.org, zea@chromium.org Bug: 809923 Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: I4a8c443851b0d4bc660656f528d0cd5a32a62bc6 Reviewed-on: https://chromium-review.googlesource.com/1046969 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Cr-Commit-Position: refs/heads/master@{#560535} [modify] https://crrev.com/f878993246cf9636795648f4e0183328dd835e6d/chrome/browser/extensions/extension_gcm_app_handler_unittest.cc [modify] https://crrev.com/f878993246cf9636795648f4e0183328dd835e6d/chrome/browser/gcm/gcm_profile_service_factory.cc [modify] https://crrev.com/f878993246cf9636795648f4e0183328dd835e6d/chrome/browser/gcm/gcm_profile_service_unittest.cc [modify] https://crrev.com/f878993246cf9636795648f4e0183328dd835e6d/components/gcm_driver/account_tracker.cc [modify] https://crrev.com/f878993246cf9636795648f4e0183328dd835e6d/components/gcm_driver/account_tracker.h [modify] https://crrev.com/f878993246cf9636795648f4e0183328dd835e6d/components/gcm_driver/account_tracker_unittest.cc [modify] https://crrev.com/f878993246cf9636795648f4e0183328dd835e6d/components/gcm_driver/gcm_account_tracker.cc [modify] https://crrev.com/f878993246cf9636795648f4e0183328dd835e6d/components/gcm_driver/gcm_account_tracker.h [modify] https://crrev.com/f878993246cf9636795648f4e0183328dd835e6d/components/gcm_driver/gcm_account_tracker_unittest.cc [modify] https://crrev.com/f878993246cf9636795648f4e0183328dd835e6d/components/gcm_driver/gcm_profile_service.cc [modify] https://crrev.com/f878993246cf9636795648f4e0183328dd835e6d/components/gcm_driver/gcm_profile_service.h [modify] https://crrev.com/f878993246cf9636795648f4e0183328dd835e6d/ios/chrome/browser/services/gcm/ios_chrome_gcm_profile_service_factory.cc
,
Jun 5 2018
,
Jun 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e197b29404a4db3a68fc0ff08da484feeb0700ca commit e197b29404a4db3a68fc0ff08da484feeb0700ca Author: Colin Blundell <blundell@chromium.org> Date: Fri Jun 22 14:06:39 2018 Refactor how GCMAccountTracker stores pending token requests In preparation for porting GCM to use IdentityManager to fetch access tokens rather than ProfileOAuth2TokenService, this CL refactors how GCMAccountTracker internally stores pending requests. Currently it stores them as a vector of raw OAuth2TokenService::Request pointers; when such a pointer gets passed in OnGetToken{Success, Failure}, it looks it up in the vector to delete it. However, the IdentityManager facilities do not pass pointers to the analogous "request" objects (called AccessTokenFetcher), while the client is still responsible for deleting those objects when desired. This CL changes GCMAccountTracker to maintain a map of account IDs to OAuth2TokenService::Request unique_ptrs. When a request is complete, it simply erases the entry in the map by key. One hiccup is that GCMAccountTracker currently can make two concurrent requests for the same account: - It makes token requests for accounts in AccountTracker's list of accounts that are in GCMAccountTracker's TOKEN_NEEDED state. - When it makes a request, it transitions the state of the account to GETTING_TOKEN. - When it receives a notification that an account has signed out, it transitions that account to ACCOUNT_REMOVED state. - When it receives a notification that an account has signed in, if it has the account in ACCOUNT_REMOVED state it transitions that account to GETTING_TOKEN state and refetches access tokens. However, GCMAccountTracker already short-circuits out of taking any action on completion of access token requests if the account is in ACCOUNT_REMOVED state (beyond deleting the request). This CL simply changes GCMAccountTracker to never make two concurrent token requests for the same account by deleting a token request when transitioning an account to ACCOUNT_REMOVED state. Bug: 809923 Change-Id: I2bae540133ae4db3f2cfea47332b2907dd88dd6d Reviewed-on: https://chromium-review.googlesource.com/1104689 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Cr-Commit-Position: refs/heads/master@{#569599} [modify] https://crrev.com/e197b29404a4db3a68fc0ff08da484feeb0700ca/components/gcm_driver/gcm_account_tracker.cc [modify] https://crrev.com/e197b29404a4db3a68fc0ff08da484feeb0700ca/components/gcm_driver/gcm_account_tracker.h
,
Jun 28 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/aa0f4f090b990d9498bbafd3b8387d20a0fea9bc commit aa0f4f090b990d9498bbafd3b8387d20a0fea9bc Author: Colin Blundell <blundell@chromium.org> Date: Thu Jun 28 14:41:45 2018 Fix GCMAccountTracker unittest Noticed while preparing https://chromium-review.googlesource.com/c/chromium/src/+/1113548: this test currently does not test what it is intended to. It is intended to test that when an ongoing access token request is fulfilled with an expired token, GCMAccountTracker notices that the token is no longer valid and marks the account as needing a token request. However, the current structure of the test prevents any access token request ever being made for the newly-added account: the account is added when the driver isn't connected, in which case GCMAccountTracker doesn't make access token requests. Hence, the test passes even if the call to IssueExpiredAccessToken() is replaced by a call to IssueAccessToken(). This CL changes the test to match its intention: we add the account with the driver connected and check that the request was started, then disconnect the driver before fulfilling the request (so that another token request isn't made immediately), and *then* check that a new token request is now marked as being needed. In particular, the test now fails if the replacement mentioned in the previous paragraph is made. Bug: 809923 Change-Id: Idc2458d4a0d8e9e702ac8a61cdff9ad138e16fa3 Reviewed-on: https://chromium-review.googlesource.com/1113677 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Cr-Commit-Position: refs/heads/master@{#571118} [modify] https://crrev.com/aa0f4f090b990d9498bbafd3b8387d20a0fea9bc/components/gcm_driver/gcm_account_tracker_unittest.cc
,
Jun 29 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a1b7c909f68e09954e8716e12170755256108818 commit a1b7c909f68e09954e8716e12170755256108818 Author: Colin Blundell <blundell@chromium.org> Date: Fri Jun 29 06:28:06 2018 Make account components explicit in GCMAccountTracker unittest The GCMAccountTracker unittest currently plays fast-and-loose with the notion of an "account ID", which can be either the Gaia ID or the email address for a given account depending on the platform. This is a blocker to porting GCMAccountTracker to use IdentityManager, as the IdentityManager test infrastructure deliberately makes a much more explicit and enforced separation between email address, Gaia ID, and account ID. This CL paves the way for that porting by changing the GCMAccountTracker unittest to be explicit about its usage of the email address, Gaia ID, and account ID of a given account. Interestingly, this change exposed an actual production bug (https://crbug.com/856170). Bug: 809923 Change-Id: I46e2f632bacd89ba55132255a95bcb089d4fb3c8 Reviewed-on: https://chromium-review.googlesource.com/1113548 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Cr-Commit-Position: refs/heads/master@{#571408} [modify] https://crrev.com/a1b7c909f68e09954e8716e12170755256108818/components/gcm_driver/gcm_account_tracker_unittest.cc
,
Jul 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/354f2210efffdbe578d4342f625b00e76332d360 commit 354f2210efffdbe578d4342f625b00e76332d360 Author: Colin Blundell <blundell@chromium.org> Date: Mon Jul 16 14:45:08 2018 [GCM] Port GCMAccountTracker to use IdentityManager The change is straightforward: Rather than using ProfileOAuth2TokenService to make access token requests, GCMAccountTracker now constructs AccessTokenFetchers via IdentityManager. The substantive changes in the CL are the ones in gcm_account_tracker.*. Everything else is just various types of plumbing. Note that in the near-term I will be removing the need for GCMProfileService to know about SigninManager/PO2TS at all, leaving it taking in only IdentityManager. However, that change is blocked on converting the rest of the code in //components/gcm_driver to talk to IdentityManager. TBR=zea@chromium.org, rockot@chromium.org Bug: 809923 Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: I12d2f1998d398a9081355c55fb56761cf660a753 Reviewed-on: https://chromium-review.googlesource.com/1113740 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Cr-Commit-Position: refs/heads/master@{#575256} [modify] https://crrev.com/354f2210efffdbe578d4342f625b00e76332d360/chrome/browser/extensions/extension_gcm_app_handler_unittest.cc [modify] https://crrev.com/354f2210efffdbe578d4342f625b00e76332d360/chrome/browser/gcm/gcm_profile_service_factory.cc [modify] https://crrev.com/354f2210efffdbe578d4342f625b00e76332d360/chrome/browser/gcm/gcm_profile_service_unittest.cc [modify] https://crrev.com/354f2210efffdbe578d4342f625b00e76332d360/components/gcm_driver/BUILD.gn [modify] https://crrev.com/354f2210efffdbe578d4342f625b00e76332d360/components/gcm_driver/DEPS [modify] https://crrev.com/354f2210efffdbe578d4342f625b00e76332d360/components/gcm_driver/gcm_account_tracker.cc [modify] https://crrev.com/354f2210efffdbe578d4342f625b00e76332d360/components/gcm_driver/gcm_account_tracker.h [modify] https://crrev.com/354f2210efffdbe578d4342f625b00e76332d360/components/gcm_driver/gcm_account_tracker_unittest.cc [modify] https://crrev.com/354f2210efffdbe578d4342f625b00e76332d360/components/gcm_driver/gcm_profile_service.cc [modify] https://crrev.com/354f2210efffdbe578d4342f625b00e76332d360/components/gcm_driver/gcm_profile_service.h [modify] https://crrev.com/354f2210efffdbe578d4342f625b00e76332d360/ios/chrome/browser/gcm/ios_chrome_gcm_profile_service_factory.cc
,
Jul 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f1e3e4808476c11a75a9341c8fae3726fec86d30 commit f1e3e4808476c11a75a9341c8fae3726fec86d30 Author: Colin Blundell <blundell@chromium.org> Date: Tue Jul 17 07:29:47 2018 [GCM] Port GCMProfileService to use IdentityManager Part of the porting of //components/gcm_driver to use IdentityManager rather than SigninManager and ProfileOAuth2TokenService. This particular change is completely straightforward, with GCMProfileService's helper class IdentityObserver observing IdentityManager rather than SigninManager. Bug: 809923 Change-Id: I821893895b2b122cec852a5bbe0c62f1a0b52670 Reviewed-on: https://chromium-review.googlesource.com/1113926 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Cr-Commit-Position: refs/heads/master@{#575571} [modify] https://crrev.com/f1e3e4808476c11a75a9341c8fae3726fec86d30/components/gcm_driver/gcm_profile_service.cc
,
Jul 24
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/39ba6f46c46947a80d2c36026bedbbec8d5e50fd commit 39ba6f46c46947a80d2c36026bedbbec8d5e50fd Author: Colin Blundell <blundell@chromium.org> Date: Tue Jul 24 07:11:31 2018 Make account components explicit in AccountTracker unittest The AccountTracker unittest currently plays fast-and-loose with the notion of an "account ID", which can be either the Gaia ID or the email address for a given account depending on the platform. This is a blocker to porting AccountTracker to use IdentityManager, as the IdentityManager test infrastructure deliberately makes a much more explicit and enforced separation between email address, Gaia ID, and account ID, reflecting how these elements are used in production. In particular: - Adding an account takes in the gaia ID and email and returns an account ID. - Calls that interact with the token service take in the account ID. This CL paves the way for that porting by changing the AccountTracker unittest to be explicit about its usage of the email address, Gaia ID, and account ID of a given account. The test now adds accounts by passing the gaia ID/email, obtains the account IDs of the added accounts, and interacts with the token service via those account IDs. Bug: 809923 Change-Id: Ia0bf7fc90c035976f8c578563e7d81c597d4d635 Reviewed-on: https://chromium-review.googlesource.com/1138082 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Cr-Commit-Position: refs/heads/master@{#577444} [modify] https://crrev.com/39ba6f46c46947a80d2c36026bedbbec8d5e50fd/components/gcm_driver/account_tracker_unittest.cc
,
Jul 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/98dd473e0e0bcb8e8f4510aae824d6f1a876715d commit 98dd473e0e0bcb8e8f4510aae824d6f1a876715d Author: Colin Blundell <blundell@chromium.org> Date: Fri Jul 27 10:41:45 2018 AccountTracker: Minor code cleanup Currently, AccountTracker::GoogleSigninSucceeded() calls AccountTracker::OnRefreshTokenAvailable() for each account that the token service knows about. However, this is conceptually somewhat confusing, as it's not the case that the refresh tokens for those accounts have suddenly become available. Moreover, an upcoming conversion of this class to use IdentityManager will make it even less suitable, as the corresponding observer callback passes information that AccountTracker won't have (and doesn't need) when invoking it manually. This CL changes AccountTracker::GoogleSigninSucceeded() to directly call through to AccountTracker::UpdateSigninState(), better reflecting the actual flow. Bug: 809923 Change-Id: Ic7673659cb9383d55eb8eda5cfb84a22f3a72214 Reviewed-on: https://chromium-review.googlesource.com/1150223 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Cr-Commit-Position: refs/heads/master@{#578585} [modify] https://crrev.com/98dd473e0e0bcb8e8f4510aae824d6f1a876715d/components/gcm_driver/account_tracker.cc
,
Jul 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b5e688dcc4bffc28d177f53d4c2a17cafac3bc28 commit b5e688dcc4bffc28d177f53d4c2a17cafac3bc28 Author: Colin Blundell <blundell@chromium.org> Date: Fri Jul 27 12:09:07 2018 IdentityManager test APIs: Add flexibility when clearing primary account When clearing the primary account, three behaviors are possible: 1. Explicitly remove all accounts. 2. Explicitly keep all accounts. 3. Let the signin code internally decide between 1 and 2 based on its own default policy. The IdentityManager test APIS (IdentityTestEnvironment and identity_test_utils.h) currently support only 3. However, an upcoming conversion will require 1 and 2. This CL adds that support in to the APIS for clearing the primary account. Bug: 798699, 809923 Change-Id: Idb717836c843a14a5f6d33aab5d60d7e14ff4d1c Reviewed-on: https://chromium-review.googlesource.com/1149861 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Cr-Commit-Position: refs/heads/master@{#578605} [modify] https://crrev.com/b5e688dcc4bffc28d177f53d4c2a17cafac3bc28/services/identity/public/cpp/identity_test_environment.cc [modify] https://crrev.com/b5e688dcc4bffc28d177f53d4c2a17cafac3bc28/services/identity/public/cpp/identity_test_environment.h [modify] https://crrev.com/b5e688dcc4bffc28d177f53d4c2a17cafac3bc28/services/identity/public/cpp/identity_test_utils.cc [modify] https://crrev.com/b5e688dcc4bffc28d177f53d4c2a17cafac3bc28/services/identity/public/cpp/identity_test_utils.h
,
Jul 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/66ef2a877f14c2fa7b15b070c529e570eca617e6 commit 66ef2a877f14c2fa7b15b070c529e570eca617e6 Author: Colin Blundell <blundell@chromium.org> Date: Fri Jul 27 13:00:48 2018 IdentityManager: Relax some constraints on test APIs Various test APIs around IdentityManager currently DCHECK preconditions when invoked (e.g., clearing the primary account DCHECKs that there is a primary account). However, unittests for GCM that will shortly be converted intentionally stress-test the system in various exceptional ways, e.g., invoking the flow that the primary account was cleared when there is no primary account. To accomodate porting those unittests without having to reduce their coverage, this CL relaxes the constraints on the relevant IdentityManager test APIs. Bug: 798699, 809923 Change-Id: Ibe2db519d0e231ea707ad09b329d6abc6ad7a9d2 Reviewed-on: https://chromium-review.googlesource.com/1149864 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Cr-Commit-Position: refs/heads/master@{#578615} [modify] https://crrev.com/66ef2a877f14c2fa7b15b070c529e570eca617e6/services/identity/public/cpp/identity_test_environment.h [modify] https://crrev.com/66ef2a877f14c2fa7b15b070c529e570eca617e6/services/identity/public/cpp/identity_test_utils.cc [modify] https://crrev.com/66ef2a877f14c2fa7b15b070c529e570eca617e6/services/identity/public/cpp/identity_test_utils.h
,
Jul 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4bf47a559863ac429f4c61d31abe827670390d15 commit 4bf47a559863ac429f4c61d31abe827670390d15 Author: Colin Blundell <blundell@chromium.org> Date: Fri Jul 27 15:25:11 2018 IdentityTestEnv: Add API to wait for token requests for given account The current IdentityTestEnvironment APIs to wait for access token requests and issue responses operate in a shotgun manner: when the next access token request is received, the test API implementation issues a response for *all* pending access token requests. However, unittests that is targeted in an upcoming conversion requires more flexibility: as the production code that it is testing interacts with multiple accounts, the test issues access token requests for multiple accounts, and then sequentially issues responses and checks that a specific flow for each account in question was initiated in response to the specific response. Moreover, one of these unittests issues responses in an order different to the order in which the requests were made. To accommodate porting of these tests, this CL adds the ability to wait for an access token request for a given account and then issue a response only for that account. If while waiting for a request from a specific account a request for a *different* account is seen, the waiting code forwards on the handling of the request for that other account to the next iteration of the runloop; this allows for the case where a test wants to handle requests for multiple accounts in an order different to the order in which those requests were made. We leave in a variant that does not take in the account ID and operates as before, as this is convenient for the common case of tests that don't care about this level of detail (usually because they are testing a production flow that operates on the primary account). Bug: 798699, 809923 Change-Id: I6b1d93efd47b1eba128ac0b62c833736700e5196 Reviewed-on: https://chromium-review.googlesource.com/1149876 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Cr-Commit-Position: refs/heads/master@{#578647} [modify] https://crrev.com/4bf47a559863ac429f4c61d31abe827670390d15/services/identity/public/cpp/identity_test_environment.cc [modify] https://crrev.com/4bf47a559863ac429f4c61d31abe827670390d15/services/identity/public/cpp/identity_test_environment.h
,
Aug 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2b8d7478856e0f025ce3afd70ea874d7e07c9bb3 commit 2b8d7478856e0f025ce3afd70ea874d7e07c9bb3 Author: Colin Blundell <blundell@chromium.org> Date: Thu Aug 02 07:27:52 2018 [GCM] Port AccountTracker to use IdentityManager This CL completes the conversion of GCM's production code to talk to IdentityManager rather than legacy signin classes by converting AccountTracker. The GCMAccountTracker unittest remains to be fully converted away from the legacy signin classes; this will be done in a followup CL. TBR=rohitrao@chromium.org, rockot@chromium.org Bug: 809923 Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: I1513b33850f52b532d83ac389a78464b68ed1a45 Reviewed-on: https://chromium-review.googlesource.com/1148200 Reviewed-by: Ken Rockot <rockot@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Commit-Queue: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#580101} [modify] https://crrev.com/2b8d7478856e0f025ce3afd70ea874d7e07c9bb3/chrome/browser/extensions/extension_gcm_app_handler_unittest.cc [modify] https://crrev.com/2b8d7478856e0f025ce3afd70ea874d7e07c9bb3/chrome/browser/gcm/gcm_profile_service_factory.cc [modify] https://crrev.com/2b8d7478856e0f025ce3afd70ea874d7e07c9bb3/chrome/browser/gcm/gcm_profile_service_unittest.cc [modify] https://crrev.com/2b8d7478856e0f025ce3afd70ea874d7e07c9bb3/components/gcm_driver/BUILD.gn [modify] https://crrev.com/2b8d7478856e0f025ce3afd70ea874d7e07c9bb3/components/gcm_driver/account_tracker.cc [modify] https://crrev.com/2b8d7478856e0f025ce3afd70ea874d7e07c9bb3/components/gcm_driver/account_tracker.h [modify] https://crrev.com/2b8d7478856e0f025ce3afd70ea874d7e07c9bb3/components/gcm_driver/account_tracker_unittest.cc [modify] https://crrev.com/2b8d7478856e0f025ce3afd70ea874d7e07c9bb3/components/gcm_driver/gcm_account_tracker_unittest.cc [modify] https://crrev.com/2b8d7478856e0f025ce3afd70ea874d7e07c9bb3/components/gcm_driver/gcm_profile_service.cc [modify] https://crrev.com/2b8d7478856e0f025ce3afd70ea874d7e07c9bb3/components/gcm_driver/gcm_profile_service.h [modify] https://crrev.com/2b8d7478856e0f025ce3afd70ea874d7e07c9bb3/ios/chrome/browser/gcm/ios_chrome_gcm_profile_service_factory.cc [modify] https://crrev.com/2b8d7478856e0f025ce3afd70ea874d7e07c9bb3/ios/web_view/internal/sync/web_view_gcm_profile_service_factory.mm
,
Aug 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/efba0776724d1b88d14a9856a7f78487a5a68e52 commit efba0776724d1b88d14a9856a7f78487a5a68e52 Author: Colin Blundell <blundell@chromium.org> Date: Thu Aug 02 13:40:44 2018 Port GCMAccountTracker unittest to IdentityTestEnvironment This CL ports the GCMAccountTracker unittest to IdentityTestEnvironment; this is possible now that AccountTracker is no longer using the internal signin classes, and hence the GCMAccountTracker unittest does not need to be aware of those internal signin classes to construct an AccountTracker instance. This CL completes the conversion of GCM away from the internal signin classes and rips out the no-longer-needed dependencies. TBR=msarda@chromium.org Bug: 809923 Change-Id: I78f999fc66543b8d64b85053fbcf72973ce8ac2e Reviewed-on: https://chromium-review.googlesource.com/1152968 Commit-Queue: Colin Blundell <blundell@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Cr-Commit-Position: refs/heads/master@{#580153} [modify] https://crrev.com/efba0776724d1b88d14a9856a7f78487a5a68e52/components/gcm_driver/BUILD.gn [modify] https://crrev.com/efba0776724d1b88d14a9856a7f78487a5a68e52/components/gcm_driver/DEPS [modify] https://crrev.com/efba0776724d1b88d14a9856a7f78487a5a68e52/components/gcm_driver/gcm_account_tracker_unittest.cc
,
Aug 3
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by blundell@chromium.org
, Feb 7 2018