Rationalize the fake signin flow in ProfileSyncServiceHarness |
||
Issue descriptionThis flow signs the user in with fake signin information. Ideally we would be using identity_test_utils::MakePrimaryAccountAvailable() to do so (and then in the long run IdentityTestEnvironment, once the production code being exercised is no longer directly using SigninManager/ProfileOAuth2TokenService). However, there are complexities that make this difficult: - The user can already be authenticated with SigninManager in some uses of this flow. Re-authenticating via MakePrimaryAccountAvailable() causes a DCHECK failure in the non-ChromeOS case, as SigninManager asserts that signin can only occur when not already signed in. - On ChromeOS, the user can sometimes be authenticated with SigninManager but not have a refresh token set. We don't currently have a flow for just updating the refresh token of the primary account in identity_test_utils.h, and I'd like to avoid adding one. - Even if the user is already authenticated, a call to ProfileSyncService needs to be made to ensure that sync is kicked off. Hence for now we are just stuffing the account info synchronously into the signin classes and then explicitly calling ProfileSyncService::GoogleSigninSucceeded(). In the long term it would be great to fix all of the above and then remove the direct usage of IdentityManager::SetPrimaryAccountInfoSynchronouslyForTests().
,
Apr 25 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d697bdd75db3145a2d02278f1a435f1349449319 commit d697bdd75db3145a2d02278f1a435f1349449319 Author: Marc Treib <treib@chromium.org> Date: Wed Apr 25 15:58:24 2018 Migrate MultiProfileFileManagerBrowserTest to IdentityManager identity::IdentityManager is the new API that replaces SigninManager[Base]. Bug: 825190 , 814307 Change-Id: I3cf7261fad64a63bdc449c190f7d175025408d7c Reviewed-on: https://chromium-review.googlesource.com/1025751 Reviewed-by: Kazuhiro Inaba <kinaba@chromium.org> Reviewed-by: Mihai Sardarescu <msarda@chromium.org> Commit-Queue: Marc Treib <treib@chromium.org> Cr-Commit-Position: refs/heads/master@{#553581} [modify] https://crrev.com/d697bdd75db3145a2d02278f1a435f1349449319/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc [modify] https://crrev.com/d697bdd75db3145a2d02278f1a435f1349449319/services/identity/public/cpp/identity_manager.h
,
Apr 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a69dac0d9ec7cbae1f079b6d04bc2a2d6c1cf2cc commit a69dac0d9ec7cbae1f079b6d04bc2a2d6c1cf2cc Author: Marc Treib <treib@chromium.org> Date: Thu Apr 26 09:39:52 2018 Migrate MultiProfileDownloadNotificationTest to IdentityManager identity::IdentityManager is the new API that replaces SigninManager[Base]. The changes here are copied almost verbatim from crrev.com/c/1025751. Bug: 825190 , 814307 Change-Id: If68afd361fd457284fdb0ed7ef41a4d121055a84 Reviewed-on: https://chromium-review.googlesource.com/1025693 Commit-Queue: Marc Treib <treib@chromium.org> Reviewed-by: Mihai Sardarescu <msarda@chromium.org> Reviewed-by: David Trainor <dtrainor@chromium.org> Cr-Commit-Position: refs/heads/master@{#553964} [modify] https://crrev.com/a69dac0d9ec7cbae1f079b6d04bc2a2d6c1cf2cc/chrome/browser/download/notification/download_notification_interactive_uitest.cc [modify] https://crrev.com/a69dac0d9ec7cbae1f079b6d04bc2a2d6c1cf2cc/services/identity/public/cpp/identity_manager.h
,
Aug 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2849f1206375345053d977ea6f8d8648bfcacf90 commit 2849f1206375345053d977ea6f8d8648bfcacf90 Author: Mikel Astiz <mastiz@chromium.org> Date: Tue Aug 07 05:07:24 2018 Sync test changes in preparation for transport-only sync These should allow tests in upcoming patches to mimic the experimentally supported scenario where sync the feature is disabled, but the sync machinery (the engine acting as a transport mechanism) is active as soon as the user is signed in, in a special mode (subset of datatypes, with ephemeral storage). As first user, we introduce a test for USER_CONSENTS. Bug: 856179, 866814 , 814307 Change-Id: Ie3b6c2da470dde691862a379977b4e01c4f2ead2 Reviewed-on: https://chromium-review.googlesource.com/1160852 Commit-Queue: Mikel Astiz <mastiz@chromium.org> Reviewed-by: Steven Holte <holte@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Reviewed-by: Mihai Sardarescu <msarda@chromium.org> Cr-Commit-Position: refs/heads/master@{#581137} [modify] https://crrev.com/2849f1206375345053d977ea6f8d8648bfcacf90/chrome/browser/metrics/ukm_browsertest.cc [modify] https://crrev.com/2849f1206375345053d977ea6f8d8648bfcacf90/chrome/browser/sync/test/integration/profile_sync_service_harness.cc [modify] https://crrev.com/2849f1206375345053d977ea6f8d8648bfcacf90/chrome/browser/sync/test/integration/profile_sync_service_harness.h [modify] https://crrev.com/2849f1206375345053d977ea6f8d8648bfcacf90/chrome/browser/sync/test/integration/quiesce_status_change_checker.cc [modify] https://crrev.com/2849f1206375345053d977ea6f8d8648bfcacf90/chrome/browser/sync/test/integration/single_client_user_consents_sync_test.cc [modify] https://crrev.com/2849f1206375345053d977ea6f8d8648bfcacf90/chrome/browser/sync/test/integration/sync_errors_test.cc [modify] https://crrev.com/2849f1206375345053d977ea6f8d8648bfcacf90/chrome/browser/sync/test/integration/sync_test.cc [modify] https://crrev.com/2849f1206375345053d977ea6f8d8648bfcacf90/chrome/browser/sync/test/integration/updated_progress_marker_checker.cc [modify] https://crrev.com/2849f1206375345053d977ea6f8d8648bfcacf90/components/sync/driver/async_directory_type_controller.cc [modify] https://crrev.com/2849f1206375345053d977ea6f8d8648bfcacf90/services/identity/public/cpp/identity_manager.h
,
Aug 7
Marking this as fixed after identity_test_utils was adopted in ProfileSyncServiceHarness. Please reopen if there's still follow-up work to do. |
||
►
Sign in to add a comment |
||
Comment 1 by treib@chromium.org
, Apr 24 2018