Sync client's that are signed in but do not have FirstSetupComplete set will always resync from scratch |
|||||||||||||||||||||||||
Issue descriptionIf a client is signed in, but kFirstSetupComplete is false, then the client theoretically needs to confirm its settings. But, since the refactoring in https://codereview.chromium.org/1575153004, these clients will still start up Sync and download the control types. At that point, with the backend initialized, the client will get stuck, and will not make further progress. On the next restart, the client will purge its directory, including its sync guid, and resync from scratch again, repeating the process. In this way, clients will be continuously performing a new sync with a new client id on every restart.
,
Jun 30 2016
I'm open to try :) Hoping to have a fix out for review by EOD
,
Jul 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/68623971be0cfc492a2cb0427d7f478e7b214c24 commit 68623971be0cfc492a2cb0427d7f478e7b214c24 Author: zea <zea@chromium.org> Date: Fri Jul 01 01:33:57 2016 [Sync] Stop purging sync data on restart for users who haven't finished setup. Users who don't have first setup complete, but are signed in without sync suppressed, are in an invalid state, likely from previous issues in the sign in flow. We were purging their sync data over-aggressively on startup previously, causing a resync of the control types on each restart. Now we simply leave the data around, and only purge if the user is actually signed out. BUG= 624915 Review-Url: https://codereview.chromium.org/2113453006 Cr-Commit-Position: refs/heads/master@{#403382} [modify] https://crrev.com/68623971be0cfc492a2cb0427d7f478e7b214c24/components/browser_sync/browser/profile_sync_service.cc [modify] https://crrev.com/68623971be0cfc492a2cb0427d7f478e7b214c24/components/browser_sync/browser/profile_sync_service_unittest.cc
,
Jul 5 2016
@zea: Could you please provide us the steps to reproduce which would help us in verifying this from TE side. Thank you.
,
Jul 6 2016
Also, once this has been verified, can we request a merge for M-52?
,
Jul 6 2016
The only way to verify this manually is to set up a new profile, sign in to sync, look at chrome://sync-internals, write down the Sync Client Id. Then exit Chrome and manually edit the Preferences file for that profile. In the Preferences file you'll need to change the value of "has_setup_completed", within "sync", to false. On restart, check to see if Sync Client Id has changed.
,
Jul 6 2016
Updating the label to M52 and requesting a merge for M52, so that our sync client counting metrics aren't completely off for two full versions.
,
Jul 6 2016
Your change meets the bar and is auto-approved for M52 (branch: 2743)
,
Jul 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/97128793647d0829e901967a6524563f6796fed2 commit 97128793647d0829e901967a6524563f6796fed2 Author: Nicolas Zea <zea@chromium.org> Date: Wed Jul 06 20:45:28 2016 [Sync] Stop purging sync data on restart for users who haven't finished setup. Users who don't have first setup complete, but are signed in without sync suppressed, are in an invalid state, likely from previous issues in the sign in flow. We were purging their sync data over-aggressively on startup previously, causing a resync of the control types on each restart. Now we simply leave the data around, and only purge if the user is actually signed out. BUG= 624915 Review-Url: https://codereview.chromium.org/2113453006 Cr-Commit-Position: refs/heads/master@{#403382} (cherry picked from commit 68623971be0cfc492a2cb0427d7f478e7b214c24) Review URL: https://codereview.chromium.org/2126923002 . Cr-Commit-Position: refs/branch-heads/2743@{#589} Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939} [modify] https://crrev.com/97128793647d0829e901967a6524563f6796fed2/components/browser_sync/browser/profile_sync_service.cc [modify] https://crrev.com/97128793647d0829e901967a6524563f6796fed2/components/browser_sync/browser/profile_sync_service_unittest.cc
,
Jul 6 2016
,
Jul 7 2016
Tried to verify the CL as per C#6 on Windows-7,latest M-53 chrome version: 53.0.2785.8.
Observed that the sync client id before(t46dbQ3q3kKO7yOkA8ES+Q==) and after relaunch(rEKTSQPFT1KCYZhbh458zg==) of chrome was different when changed the preference file("has_setup_completed" within "sync" to false).
,
Jul 13 2016
Tested the issue on windows 7 using chrome version 52.0.2743.75.Observed the sync client ids as below Before relaunch:: FTfwKzPaMHsP7UU3DbwH/w== After relaunch :: M4JOKKgluQBFNOwN5PWiDQ== Adding TE-Verified label. Thanks,
,
Jul 15 2016
Reopening. I should have clarified, in the fixed client, the two ids are supposed to be the same. It turns out I fixed one scenario where we would trigger a clear, but there was a second that I didn't cover. Unfortunateley this will require a second patch with some extra changes (which is in review).
,
Jul 15 2016
Please note that we've Desktop-M52 stable stable release next week.This bug is labelled as Stable ReleaseBlock, pls request a merge and make sure to land the fix before 5:00 PM PST, Monday (07/18/16) to release branch once it is baked/verified in canary. Thank you. Also is this change applicable to all OS or any specific OS?
,
Jul 15 2016
,
Jul 15 2016
All OS. Will land and request merge ASAP.
,
Jul 16 2016
Looks like there's an issue with some tests, didn't manage to land the patch on time yesterday. Going to keep working on it, but pushing back to 53 for now.
,
Jul 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d1de1f77e44072018f41b1f54ffca27a7069edb4 commit d1de1f77e44072018f41b1f54ffca27a7069edb4 Author: zea <zea@chromium.org> Date: Sat Jul 16 21:55:47 2016 [Sync] Don't start up sync when FirstSetupCompleted is false and no setup in progress Instead of starting up the backend, we now don't even start it up if the first setup completed flag is false, unless a setup is in progress. On auto start platforms, first setup completed is manually set to true as part of init. BUG= 624915 Review-Url: https://codereview.chromium.org/2159453002 Cr-Commit-Position: refs/heads/master@{#405950} [modify] https://crrev.com/d1de1f77e44072018f41b1f54ffca27a7069edb4/components/browser_sync/browser/profile_sync_service.cc [modify] https://crrev.com/d1de1f77e44072018f41b1f54ffca27a7069edb4/components/browser_sync/browser/profile_sync_service_startup_unittest.cc [modify] https://crrev.com/d1de1f77e44072018f41b1f54ffca27a7069edb4/components/browser_sync/browser/profile_sync_service_unittest.cc [modify] https://crrev.com/d1de1f77e44072018f41b1f54ffca27a7069edb4/components/sync_driver/startup_controller.cc [modify] https://crrev.com/d1de1f77e44072018f41b1f54ffca27a7069edb4/components/sync_driver/startup_controller.h [modify] https://crrev.com/d1de1f77e44072018f41b1f54ffca27a7069edb4/components/sync_driver/startup_controller_unittest.cc
,
Jul 18 2016
Tested the issue on windows 7, Linux Ubuntu 14.04 and Mac 10.11.5 using chrome version 54.0.2799.0.with the below steps 1.Log into chrome and write down the Sync Client Id, 2.Exited chrome 3.Changed the "has_setup_completed" to false from preferences file 4.Relaunch chrome and oberved the Sync Client Id is same as previous one. Please find the below client ids on ALL OS Sync Client ID(Windows) rOyQ3CE7qql7Y3l9u0pAOQ== Sync Client ID(Linux) b3THIDo5KLmABzuDuKkPFw== Sync Client ID(Mac) GPh9MXWKot1RyCYbephk4g== Adding TE-Verified labels. Thanks,
,
Jul 18 2016
,
Jul 18 2016
Your change meets the bar and is auto-approved for M53 (branch: 2785)
,
Jul 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/027be2623107e04e521d56f903e0c7c1e73757e2 commit 027be2623107e04e521d56f903e0c7c1e73757e2 Author: Nicolas Zea <zea@chromium.org> Date: Mon Jul 18 23:02:26 2016 [Sync] Don't start up sync when FirstSetupCompleted is false and no setup in progress Instead of starting up the backend, we now don't even start it up if the first setup completed flag is false, unless a setup is in progress. On auto start platforms, first setup completed is manually set to true as part of init. BUG= 624915 Review-Url: https://codereview.chromium.org/2159453002 Cr-Commit-Position: refs/heads/master@{#405950} (cherry picked from commit d1de1f77e44072018f41b1f54ffca27a7069edb4) Review URL: https://codereview.chromium.org/2161863002 . Cr-Commit-Position: refs/branch-heads/2785@{#207} Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382} [modify] https://crrev.com/027be2623107e04e521d56f903e0c7c1e73757e2/components/browser_sync/browser/profile_sync_service.cc [modify] https://crrev.com/027be2623107e04e521d56f903e0c7c1e73757e2/components/browser_sync/browser/profile_sync_service_startup_unittest.cc [modify] https://crrev.com/027be2623107e04e521d56f903e0c7c1e73757e2/components/browser_sync/browser/profile_sync_service_unittest.cc [modify] https://crrev.com/027be2623107e04e521d56f903e0c7c1e73757e2/components/sync_driver/startup_controller.cc [modify] https://crrev.com/027be2623107e04e521d56f903e0c7c1e73757e2/components/sync_driver/startup_controller.h [modify] https://crrev.com/027be2623107e04e521d56f903e0c7c1e73757e2/components/sync_driver/startup_controller_unittest.cc
,
Jul 19 2016
Tested the issue on windows 7, Linux Ubuntu 14.04 and Mac 10.11.5 using chrome version 53.0.2785.21.Observed the Sync Client Id is same after changing the "has_setup_completed" to false from preferences file. Adding TE-Verified label. Thanks,
,
Jul 22 2016
Requesting merge for the next 52 refresh now that it's baked on canary/dev for the week.
,
Jul 22 2016
[Automated comment] Less than a week to go before stable on M52, we might already have a stable candidate build. Manual review required.
,
Jul 22 2016
Approving merge to M52 branch 2743 based on comment #24. Please merge ASAP. Thank you.
,
Jul 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fc73af1efc8567989b54e27b53977bd92937d0de commit fc73af1efc8567989b54e27b53977bd92937d0de Author: Nicolas Zea <zea@chromium.org> Date: Fri Jul 22 19:24:22 2016 [Sync] Don't start up sync when FirstSetupCompleted is false and no setup in progress Instead of starting up the backend, we now don't even start it up if the first setup completed flag is false, unless a setup is in progress. On auto start platforms, first setup completed is manually set to true as part of init. BUG= 624915 Review-Url: https://codereview.chromium.org/2159453002 Cr-Commit-Position: refs/heads/master@{#405950} (cherry picked from commit d1de1f77e44072018f41b1f54ffca27a7069edb4) Review URL: https://codereview.chromium.org/2172373002 . Cr-Commit-Position: refs/branch-heads/2743@{#692} Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939} [modify] https://crrev.com/fc73af1efc8567989b54e27b53977bd92937d0de/components/browser_sync/browser/profile_sync_service.cc [modify] https://crrev.com/fc73af1efc8567989b54e27b53977bd92937d0de/components/browser_sync/browser/profile_sync_service_startup_unittest.cc [modify] https://crrev.com/fc73af1efc8567989b54e27b53977bd92937d0de/components/browser_sync/browser/profile_sync_service_unittest.cc [modify] https://crrev.com/fc73af1efc8567989b54e27b53977bd92937d0de/components/sync_driver/startup_controller.cc [modify] https://crrev.com/fc73af1efc8567989b54e27b53977bd92937d0de/components/sync_driver/startup_controller.h [modify] https://crrev.com/fc73af1efc8567989b54e27b53977bd92937d0de/components/sync_driver/startup_controller_unittest.cc
,
Jul 22 2016
,
Jul 25 2016
Tested the issue on windows 7 and Mac 10.11.5 using chrome version 52.0.2743.88.Observed the Sync Client Id is same after changing the "has_setup_completed" to false from preferences file on Mac OS.So its working fine on MAC. But on windows 7 after changing the "has_setup_completed" to false from preferences file observed that the Sync Client ID is "Uninitialized: state. Sync client id on windows (c9EsLYRNs0EtsDYeMZoN6Q==) Zea @ Please find the attached screen shot and suggest to very the issue. Thanks,
,
Jul 25 2016
That behavior is expected. As long as the client id doesn't change, it's fine.
,
Jul 26 2016
Verified as per #29 & 30.
,
Aug 1 2016
Tested the issue on windows 7 using chrome version 52.0.2743.113.Observed the sync client id is uninitialized after changing the "has_setup_completed" to false.It is working as intended as per comment #30. Adding TE-Verified label. Thnaks, |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by ew...@chromium.org
, Jun 30 2016