Add support for temporarily disabling sync.

Add support for temporarily disabling sync.

Adds functionality to the ProfileSyncService to temporarily disable
sync and later be able to enable it again.

This is achieved by stopping the sync backend and suppressing future
startups when sync is disabled. The flag for suppressing startup is
removed when enabling sync again, and the sync backend is started.

Contributed by nyquist@chromium.org

TEST=unit_tests


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=108148

[Andrew T Wilson (Slow), 2011-10-26 01:23:26]

LGTM with one nit.

http://codereview.chromium.org/8332023/diff/1/chrome/browser/sync/profile_syn...
File chrome/browser/sync/profile_sync_service.cc (right):

http://codereview.chromium.org/8332023/diff/1/chrome/browser/sync/profile_syn...
chrome/browser/sync/profile_sync_service.cc:196: if (!HasSyncSetupCompleted()) {
Should we combine this whole if statement into:

if (auto_start_enabled_ || HasSyncSetupCompleted())
  StartUp();

That seems cleaner (with appropriate merging of comments).

[qsr, 2011-10-26 07:07:00]

http://codereview.chromium.org/8332023/diff/1/chrome/browser/sync/profile_syn...
File chrome/browser/sync/profile_sync_service.cc (right):

http://codereview.chromium.org/8332023/diff/1/chrome/browser/sync/profile_syn...
chrome/browser/sync/profile_sync_service.cc:196: if (!HasSyncSetupCompleted()) {
On 2011/10/26 01:23:26, Andrew T Wilson wrote:
> Should we combine this whole if statement into:
> 
> if (auto_start_enabled_ || HasSyncSetupCompleted())
>   StartUp();
> 
> That seems cleaner (with appropriate merging of comments).

 That would need changing the else statement and make it a if. As the else
statement is right now just HasSyncSetupCompleted.

 Probably rewritting this all if as:

if (HasSyncSetupCompleted() || auto_start_enabled_) {
  Startup();
}

is easier to understand, but the comment would have to be rewritten.

[nyquist, 2011-10-26 14:55:35]

Fixed condition for startup of sync and updated comment. PTAL

[akalin, 2011-10-26 18:53:09]

http://codereview.chromium.org/8332023/diff/6001/chrome/browser/sync/profile_...
File chrome/browser/sync/profile_sync_service.cc (right):

http://codereview.chromium.org/8332023/diff/6001/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:1527:
signin_->SetUsername(profile_->GetProfileName());
What is the username used for?  Isn't the profile name just some arbitrary name
(like "Profile0") and not related to the sync account?  If it's something
platform-specific maybe that should be mentioned, and/or this should be put in
an ifdef.

http://codereview.chromium.org/8332023/diff/6001/chrome/browser/sync/profile_...
File chrome/browser/sync/profile_sync_service.h (right):

http://codereview.chromium.org/8332023/diff/6001/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.h:484: virtual void
StopSyncAndSuppressStartUp();
Does this need to be virtual?

What do you think about the name "StopAndSuppress()"?

http://codereview.chromium.org/8332023/diff/6001/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.h:487: virtual void
DisableSyncSuppressionAndStartUpSync();
Does this need to be virtual?

What do you think about the name "UnsuppressAndStart()"?

http://codereview.chromium.org/8332023/diff/6001/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.h:552: void StartUpIfNotSuppressed();
what about just "StartIfNotSuppressed()" or maybe "TryStart()" (with the comment
expanded to say that sync is started unless it's suppressed)

[nyquist, 2011-10-27 18:03:31]

Renamed methods for stopping/starting sync and added google services username to
sync prefs.
PTAL

[akalin, 2011-10-31 20:28:10]

LGTM

[commit-bot: I haz the power, 2011-11-01 18:06:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nyquist@chromium.org/8332023/8002

[commit-bot: I haz the power, 2011-11-01 19:15:30]

Change committed as 108148