[sync] Hide "Sync nothing" UI; Go back to signing out on dashboard clears and abandoned initial set…

[sync] Hide "Sync nothing" UI; Go back to signing out on dashboard clears and abandoned initial setup

A new "Sync nothing" option was recently added to the advanced sync
settings dialog. It allowed the user to stop syncing, but stay signed
in. In addition, sign in and sync were separated, with the result that
on a dashboard clear, the user would remain signed in, but would no
longer have sync enabled.

For M29, we've decided to hide this UI and revert to old sign-in/out
behavior because there are other services that depend on the sync
engine being up and running.

This patch undoes a small number of these changes as listed below:

1) Hides the "Sync nothing" item in the advanced sync settings drop-down
2) Signs out the user if the user clicks "Choose what to sync" during
   initial setup, and then clicks cancel on the advanced dialog
3) Signs out the user when a dashboard clear is done
4) Signs out the user if the user clicks cancel while re-setting up sync
   after the dashboard clear
5) Reverts the status string shown when sync is cleared via the dashboard

BUG=235633, 252049, 248877
TEST=No "Sync nothing" UI in advanced settings; user is signed out when sync is disabled on Desktop; Chrome OS strings back to M28

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=209267

[Raghu Simha, 2013-06-20 00:04:11]

Drew, please review. Thanks.

[Andrew T Wilson (Slow), 2013-06-20 21:49:37]

Mostly looks good - one question though.

https://chromiumcodereview.appspot.com/17392009/diff/27002/chrome/browser/ui/...
File chrome/browser/ui/webui/sync_setup_handler.cc (right):

https://chromiumcodereview.appspot.com/17392009/diff/27002/chrome/browser/ui/...
chrome/browser/ui/webui/sync_setup_handler.cc:939: if (was_signing_in ||
sync_service->FirstSetupInProgress())
Not sure we should be checking was_signing_in here - what's the case where we
want to sign out the user even though FirstSetupInProgress() == false?

[Raghu Simha, 2013-06-26 21:48:15]

Drew, PTAL. Thanks.

cc Roger, FYI.

https://codereview.chromium.org/17392009/diff/27002/chrome/browser/ui/webui/s...
File chrome/browser/ui/webui/sync_setup_handler.cc (right):

https://codereview.chromium.org/17392009/diff/27002/chrome/browser/ui/webui/s...
chrome/browser/ui/webui/sync_setup_handler.cc:939: if (was_signing_in ||
sync_service->FirstSetupInProgress())
On 2013/06/20 21:49:37, Andrew T Wilson wrote:
> Not sure we should be checking was_signing_in here - what's the case where we
> want to sign out the user even though FirstSetupInProgress() == false?

I don't believe there should be any case where FirstSetupInProgress() == false
and was_signing_in is true, and we still want to sign out the user. I included a
check for was_signing_in for 2 reasons:

1) I didn't want to let fall through the cracks, any case where was_signing_in
is true, but for some reason, FirstSetupInProgress() is not yet true. After
taking another look at the code, I noticed that we call SetSetupInProgress(true)
prior to displaying any sync setup UI (or cancel buttons), so we should be good
with the single check.
2) The original implementation (prior to r200867) was checking both conditions.
See line 1150 of the left half of 
https://codereview.chromium.org/14691004/diff/202002/chrome/browser/ui/webui/....

I've removed the check based on the above. Done.

[Raghu Simha, 2013-06-27 17:16:45]

Ping.

[Andrew T Wilson (Slow), 2013-06-28 15:23:58]

Sorry for the delay - I've been offsite for most of the last 2 weeks. LGTM.

[commit-bot: I haz the power, 2013-06-28 17:36:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsimha@chromium.org/17392009/78001

[commit-bot: I haz the power, 2013-06-28 23:52:24]

Retried try job too often on win_rel for step(s) chrome_frame_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-06-29 01:09:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsimha@chromium.org/17392009/78001

[commit-bot: I haz the power, 2013-06-29 01:55:25]

Change committed as 209267