New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 852374 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Task



Sign in to add a comment

Evaluate SyncService's "Sync requested" flag and RequestStart/RequestStop

Project Member Reported by treib@chromium.org, Jun 13 2018

Issue description

SyncService currently exposes RequestStart(), RequestStop(), and IsSyncRequested(). The "Sync requested" flag is stored in prefs, and RequestStart/RequestStop set it to true/false. It's also true by default.

It's not clear to me what purpose all this serves: RequestStart/RequestStop supposedly correspond to the user requesting sync startup/shutdown, but as far as I can tell there's no actual UI for that. In practice, it seems that Sync gets started iff there is a signed-in-to-Chrome user - there's no signed-in-to-Chrome-but-Sync-disabled state. (You can turn off the "Sync everything" toggle and a bunch of individual data types, but the Sync machinery will still be running - there's no actual master toggle.)

[Caveat: The above is mostly about desktop and ChromeOS - maybe there actually is a toggle on Android?]

Somewhat related to  bug 839834 .
 

Comment 1 by treib@chromium.org, Jun 13 2018

Quick survey of all RequestStart/RequestStop call sites outside of ProfileSyncService itself:

RequestStart call in PeopleHandler::HandleShowSetupUI [1]: This seems redundant (maybe modulo setting "Sync requested" back to true if it was previously set to false), since Sync will start up after setup anyway.

RequestStop call in PeopleHandler::CloseSyncSetup [2]: Looks like this call site can only be reached if !IsFirstSetupComplete, in which case Sync shouldn't be running in the first place. So maybe this can just be removed?

RequestStop call in ProfileManager::OnLoadProfileForProfileDeletion [3]: This seems to be specifically about clearing any Sync data, not really about stopping Sync, so maybe this could be replaced by an explicit NukeAllData().

[1] https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/settings/people_handler.cc?rcl=57b8e5c1489ede04930ad1d8f5691ff4768d5b9b&l=665
[2] https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/settings/people_handler.cc?rcl=ed731074b350ddb4f9f06bccd625a32e04d38380&l=781
[3] https://cs.chromium.org/chromium/src/chrome/browser/profiles/profile_manager.cc?rcl=ed731074b350ddb4f9f06bccd625a32e04d38380&l=1561

Comment 2 by treib@chromium.org, Jun 13 2018

Okay, I found *one* situation in which the user can be signed in, but have Sync disabled: After a "clear all Sync data" on the dashboard. This currently only applies to ChromeOS, since on all other platforms we also sign out the user. (That might change though, see bug 246839.)

Comment 3 by treib@chromium.org, Jun 19 2018

Status: WontFix (was: Assigned)
It's also possible to get into this state on Android via the OS-level "ChromeSync" flag. So nothing to be done here :-/

Sign in to add a comment