New issue
Advanced search Search tips

Issue 771908 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 807826



Sign in to add a comment

[Dice] Migration path for enabling Dice

Project Member Reported by droger@chromium.org, Oct 5 2017

Issue description

Detect when the user is in a consistent signin state and migrate them at that time.

This must take multiprofile into account: either migrate each profile independently (preferred), or wait until all profiles are consistent before migrating.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a430566cda04991c24e3c12a631217c829b11d46

commit a430566cda04991c24e3c12a631217c829b11d46
Author: David Roger <droger@chromium.org>
Date: Fri Oct 20 09:36:22 2017

[Dice] Add feature flag for Dice migration

This CL introduces a new feature flag for the Dice migration.
The existing flag for Dice enabled is kept, to make testing easier.

The IsAccountConsistencyDiceEnabed() function is renamed to
IsAccountConsistencyDiceAvailable() and returns true if Dice is enabled
or if the migration is in process.
A new function to test if Dice is actually enabled will be introduced
in a follow up CL.

TBR=jochen

Bug:  771908 
Change-Id: Ieb8278318759794b8ccd3d47379fc94dfd30471a
Reviewed-on: https://chromium-review.googlesource.com/720805
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510389}
[modify] https://crrev.com/a430566cda04991c24e3c12a631217c829b11d46/chrome/browser/about_flags.cc
[modify] https://crrev.com/a430566cda04991c24e3c12a631217c829b11d46/chrome/browser/signin/chrome_signin_helper.cc
[modify] https://crrev.com/a430566cda04991c24e3c12a631217c829b11d46/chrome/browser/signin/dice_browsertest.cc
[modify] https://crrev.com/a430566cda04991c24e3c12a631217c829b11d46/chrome/browser/signin/dice_response_handler.cc
[modify] https://crrev.com/a430566cda04991c24e3c12a631217c829b11d46/chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.cc
[modify] https://crrev.com/a430566cda04991c24e3c12a631217c829b11d46/chrome/browser/signin/process_dice_header_observer_impl.cc
[modify] https://crrev.com/a430566cda04991c24e3c12a631217c829b11d46/chrome/browser/ui/sync/one_click_signin_sync_starter.cc
[modify] https://crrev.com/a430566cda04991c24e3c12a631217c829b11d46/chrome/browser/ui/webui/signin_internals_ui.cc
[modify] https://crrev.com/a430566cda04991c24e3c12a631217c829b11d46/components/browser_sync/profile_sync_service.cc
[modify] https://crrev.com/a430566cda04991c24e3c12a631217c829b11d46/components/signin/core/browser/about_signin_internals.cc
[modify] https://crrev.com/a430566cda04991c24e3c12a631217c829b11d46/components/signin/core/browser/account_reconcilor.cc
[modify] https://crrev.com/a430566cda04991c24e3c12a631217c829b11d46/components/signin/core/browser/dice_header_helper.cc
[modify] https://crrev.com/a430566cda04991c24e3c12a631217c829b11d46/components/signin/core/browser/profile_management_switches_unittest.cc
[modify] https://crrev.com/a430566cda04991c24e3c12a631217c829b11d46/components/signin/core/browser/scoped_account_consistency.cc
[modify] https://crrev.com/a430566cda04991c24e3c12a631217c829b11d46/components/signin/core/browser/scoped_account_consistency.h
[modify] https://crrev.com/a430566cda04991c24e3c12a631217c829b11d46/components/signin/core/browser/signin_manager.cc
[modify] https://crrev.com/a430566cda04991c24e3c12a631217c829b11d46/components/signin/core/common/profile_management_switches.cc
[modify] https://crrev.com/a430566cda04991c24e3c12a631217c829b11d46/components/signin/core/common/profile_management_switches.h

Project Member

Comment 2 by bugdroid1@chromium.org, Oct 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/67a5cb348acffb905c0b8a4de39b1942ce968dd2

commit 67a5cb348acffb905c0b8a4de39b1942ce968dd2
Author: David Roger <droger@chromium.org>
Date: Tue Oct 24 12:45:03 2017

[Dice] Add a user pref to control the migration status

This CL adds functions to enable Dice on a profile and check the
migration status. The status can be checked both from the UI thread and
the IO thread.
The migration status is backed by a user preference.

Bug:  771908 
Change-Id: If51d55ff80a6d67f6cb4258e74cc417812c109a0
Reviewed-on: https://chromium-review.googlesource.com/725340
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511120}
[modify] https://crrev.com/67a5cb348acffb905c0b8a4de39b1942ce968dd2/chrome/browser/prefs/browser_prefs.cc
[modify] https://crrev.com/67a5cb348acffb905c0b8a4de39b1942ce968dd2/components/signin/core/browser/BUILD.gn
[modify] https://crrev.com/67a5cb348acffb905c0b8a4de39b1942ce968dd2/components/signin/core/browser/profile_management_switches_unittest.cc
[modify] https://crrev.com/67a5cb348acffb905c0b8a4de39b1942ce968dd2/components/signin/core/common/BUILD.gn
[modify] https://crrev.com/67a5cb348acffb905c0b8a4de39b1942ce968dd2/components/signin/core/common/profile_management_switches.cc
[modify] https://crrev.com/67a5cb348acffb905c0b8a4de39b1942ce968dd2/components/signin/core/common/profile_management_switches.h

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/303414355223590373cfd0dfc483cdced041c436

commit 303414355223590373cfd0dfc483cdced041c436
Author: David Roger <droger@chromium.org>
Date: Thu Oct 26 13:56:59 2017

[Dice] Implement Dice migration flow

When the migration is in progress, the AccountReconcilor is partially
enabled: it lists the Gaia accounts and compares them against Chrome
accounts. If the reconciliation is a no-op (i.e. the accounts are
consistent), the profile is migrated to Dice.
If the accounts are not consistent, the reconcilor just returns without
touching the accounts.

Other specific behavior when the migration is in progress:
- on Gaia signin, tokens are added to the token service (but the
  Sync promo is not shown)
- on Gaia signout tokens are deleted from the token service, except
  the main account (signing out of Gaia cannot turn off Sync)
- the Gaia signout confirmation dialog is never shown

TBR: bauerb, skym
Bug:  771908 
Change-Id: Ia8c21046e1c9a3a745153b5a1204b6ee1271e71e
Reviewed-on: https://chromium-review.googlesource.com/720815
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511811}
[modify] https://crrev.com/303414355223590373cfd0dfc483cdced041c436/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/303414355223590373cfd0dfc483cdced041c436/chrome/browser/profiles/profile_io_data.h
[modify] https://crrev.com/303414355223590373cfd0dfc483cdced041c436/chrome/browser/signin/account_reconcilor_unittest.cc
[modify] https://crrev.com/303414355223590373cfd0dfc483cdced041c436/chrome/browser/signin/chrome_signin_helper.cc
[modify] https://crrev.com/303414355223590373cfd0dfc483cdced041c436/chrome/browser/signin/dice_response_handler.cc
[modify] https://crrev.com/303414355223590373cfd0dfc483cdced041c436/chrome/browser/signin/dice_response_handler_unittest.cc
[modify] https://crrev.com/303414355223590373cfd0dfc483cdced041c436/chrome/browser/signin/process_dice_header_observer_impl.cc
[modify] https://crrev.com/303414355223590373cfd0dfc483cdced041c436/chrome/browser/ui/sync/one_click_signin_sync_starter.cc
[modify] https://crrev.com/303414355223590373cfd0dfc483cdced041c436/chrome/browser/ui/webui/signin_internals_ui.cc
[modify] https://crrev.com/303414355223590373cfd0dfc483cdced041c436/components/browser_sync/profile_sync_test_util.cc
[modify] https://crrev.com/303414355223590373cfd0dfc483cdced041c436/components/ntp_snippets/breaking_news/subscription_manager_impl_unittest.cc
[modify] https://crrev.com/303414355223590373cfd0dfc483cdced041c436/components/signin/core/browser/about_signin_internals.cc
[modify] https://crrev.com/303414355223590373cfd0dfc483cdced041c436/components/signin/core/browser/about_signin_internals.h
[modify] https://crrev.com/303414355223590373cfd0dfc483cdced041c436/components/signin/core/browser/access_token_fetcher_unittest.cc
[modify] https://crrev.com/303414355223590373cfd0dfc483cdced041c436/components/signin/core/browser/account_reconcilor.cc
[modify] https://crrev.com/303414355223590373cfd0dfc483cdced041c436/components/signin/core/browser/account_reconcilor.h
[modify] https://crrev.com/303414355223590373cfd0dfc483cdced041c436/components/signin/core/browser/dice_header_helper.cc
[modify] https://crrev.com/303414355223590373cfd0dfc483cdced041c436/components/signin/core/browser/dice_header_helper.h
[modify] https://crrev.com/303414355223590373cfd0dfc483cdced041c436/components/signin/core/browser/signin_header_helper.cc
[modify] https://crrev.com/303414355223590373cfd0dfc483cdced041c436/components/signin/core/browser/signin_header_helper.h
[modify] https://crrev.com/303414355223590373cfd0dfc483cdced041c436/components/signin/core/browser/signin_header_helper_unittest.cc
[modify] https://crrev.com/303414355223590373cfd0dfc483cdced041c436/components/signin/core/browser/signin_manager.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/af95e7d0070b4b2e18a7c9ce012ebf7938c293d0

commit af95e7d0070b4b2e18a7c9ce012ebf7938c293d0
Author: David Roger <droger@chromium.org>
Date: Mon Oct 30 13:12:41 2017

[signin] Add coverage for Dice migration in dice_browser_test

Bug:  771908 
Change-Id: Iebf43cebfb10719f61322c51df88b8cb40ecdeac
Reviewed-on: https://chromium-review.googlesource.com/738189
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512482}
[modify] https://crrev.com/af95e7d0070b4b2e18a7c9ce012ebf7938c293d0/chrome/browser/signin/dice_browsertest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e63413d15a7b080394c42c4722483c23b3ef840e

commit e63413d15a7b080394c42c4722483c23b3ef840e
Author: David Roger <droger@chromium.org>
Date: Fri Nov 03 15:59:54 2017

[signin] New profiles are automatically migrated to Dice if possible

This migration happens right at the AccountReconcilor creation, before the
first run UI may be shown. The first run UI can use
IsDiceEnabledForProfile() to customize the user experience for Dice.

Bug:  771908 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I74499be52ed0471370459dae9d9f4156bf94fbea
Reviewed-on: https://chromium-review.googlesource.com/751621
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513792}
[modify] https://crrev.com/e63413d15a7b080394c42c4722483c23b3ef840e/chrome/browser/signin/account_reconcilor_factory.cc
[modify] https://crrev.com/e63413d15a7b080394c42c4722483c23b3ef840e/chrome/browser/signin/account_reconcilor_factory.h
[modify] https://crrev.com/e63413d15a7b080394c42c4722483c23b3ef840e/chrome/browser/signin/account_reconcilor_unittest.cc
[modify] https://crrev.com/e63413d15a7b080394c42c4722483c23b3ef840e/chrome/browser/signin/dice_browsertest.cc
[modify] https://crrev.com/e63413d15a7b080394c42c4722483c23b3ef840e/chrome/browser/signin/dice_response_handler_unittest.cc
[modify] https://crrev.com/e63413d15a7b080394c42c4722483c23b3ef840e/components/signin/core/browser/account_reconcilor.cc
[modify] https://crrev.com/e63413d15a7b080394c42c4722483c23b3ef840e/components/signin/core/browser/account_reconcilor.h
[modify] https://crrev.com/e63413d15a7b080394c42c4722483c23b3ef840e/components/signin/ios/browser/account_consistency_service_unittest.mm
[modify] https://crrev.com/e63413d15a7b080394c42c4722483c23b3ef840e/ios/chrome/browser/signin/account_reconcilor_factory.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/32e219d348b9fe922303132e7c146ec1d6de65ea

commit 32e219d348b9fe922303132e7c146ec1d6de65ea
Author: Matt Menke <mmenke@chromium.org>
Date: Wed Nov 08 14:09:00 2017

Disable some browser_tests on the network mojo bot.

In particular, disable recently added DicePrepareMigrationBrowserTests,
and SSLUITestCommittedInterstitials.ErrorPage (Which likely regressed
in https://chromium-review.googlesource.com/c/chromium/src/+/752002).

TBR=jam@chromium.org
NOTRY=true

Bug:  771908 , 751951 
Change-Id: Id02fbf7d3e8ca10859de521ca2b7ecffdc4cf45e
Reviewed-on: https://chromium-review.googlesource.com/758598
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514813}
[modify] https://crrev.com/32e219d348b9fe922303132e7c146ec1d6de65ea/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Comment 9 by msarda@chromium.org, Jan 16 2018

Is this bug now done? I think we have implemented everything that we needed for the migration, right?

Comment 10 by ew...@chromium.org, Feb 12 2018

Blocking: -716053 807826
Friendly ping. Can this be marked as Fixed for M3?

Comment 11 by ew...@chromium.org, Feb 16 2018

Pinging this question again. Anything left here?
Status: Fixed (was: Started)
We can probably close this in the context of M3.

The current migration strategy is not very aggressive, and we may want to improve it. This can be done later in another bug though, once we have more data from the field and see how quickly people migrate.

Closing this for now.

Sign in to add a comment