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

Issue 694762 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

MD Settings: Chrome sync gets automatically reenabled after "Reset sync" if chrome://md-settings is open [ChromeOS]

Project Member Reported by jnaveen@chromium.org, Feb 21 2017

Issue description

ENVIRONMENT and STATS
Chrome version: 58.0.3015.0
Platform: 9304.0.0 dev-channel Winky

REPRO STEPS
1. Sign in to Chrome.
2. Open chrome://sync-internals and make sure sync is working correctly.
3. Reset sync via the dashboard (https://www.google.com/settings/chrome/sync)
4. Open chrome://sync-internals and observe for few seconds.

ACTUAL RESULTS
Sync is disabled and then enabled automatically with in few seconds.

EXPECTED RESULTS
Sync is disabled and is not automatically enabled.

ADDITIONAL INFO
This looks like a regression with the new MD settings page. Worked fine in last weeks build 58.0.3007.0 (which has old settings page)
 
 

Comment 1 by pav...@chromium.org, Feb 22 2017

Status: Assigned (was: Untriaged)
I reproduced it with local build from ToT.

Comment 2 by pav...@chromium.org, Feb 23 2017

Components: UI>Settings
Summary: MD Settings: Chrome sync gets automatically reenabled after "Reset sync" if chrome://md-settings is open [ChromeOS] (was: Chrome sync gets disabled and then automatically enabled after sync reset [ChromeOS])
The issue is that ProfileSyncService::RequestStart() gets called when PeopleHandler::OnStateChanged() receives notification about ProfileSyncService shutdown. Here is the stack:
#4 0x7fb260d3e3a6 browser_sync::ProfileSyncService::RequestStart()
#5 0x7fb261e3a6af settings::PeopleHandler::PushSyncPrefs()
#6 0x7fb261e3be09 settings::PeopleHandler::OnStateChanged()
#7 0x7fb2637cdaac syncer::SyncServiceBase::NotifyObservers()
#8 0x7fb260d3289c browser_sync::ProfileSyncService::ShutdownImpl()
#9 0x7fb260d2eb95 browser_sync::ProfileSyncService::StopImpl()
#10 0x7fb260d3e0dd browser_sync::ProfileSyncService::RequestStop()
#11 0x7fb260d3553d browser_sync::ProfileSyncService::OnActionableError()

in this stack ProfileSyncService::RequestStop() sets "sync.suppress_start" preference to true to prevent sync engine from starting after restart where ProfileSyncService::RequestStart() sets it to false and restarts sync engine immediately. 

A better way is for PeopleHandler to call ProfileSyncService::RequestStart() only after explicit user action to configure sync.
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 28 2017

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

commit a6f225f3f8a8fb31228e038217484ffff259463c
Author: pavely <pavely@chromium.org>
Date: Tue Feb 28 02:15:30 2017

Ensure PeopleHandler only calls PSS::RequestStart in response to intention to configure sync

Currently PSS::RequestStart is called from PushSyncPrefs() which can be
triggered by internal sync state changes. This call unsuppresses sync.

Instead PSS::RequestStart should be called only in response to user actions to
configure sync. I think OpenSyncSetup() is a good place.

BUG= 694762 
R=tommycli@chromium.org

Review-Url: https://codereview.chromium.org/2711993002
Cr-Commit-Position: refs/heads/master@{#453457}

[modify] https://crrev.com/a6f225f3f8a8fb31228e038217484ffff259463c/chrome/browser/ui/webui/settings/people_handler.cc
[modify] https://crrev.com/a6f225f3f8a8fb31228e038217484ffff259463c/chrome/browser/ui/webui/settings/people_handler_unittest.cc

Comment 4 by pav...@chromium.org, Feb 28 2017

Status: Fixed (was: Assigned)
Status: Verified (was: Fixed)
59.0.3051.3/9404.0.0

Sign in to add a comment