[sync] Separate sign in from sync on Desktop Chrome

chrome/browser/ui/webui/sync_setup_handler.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/webui/sync_setup_handler.h"
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
@@ skipping 47 matching lines @@
 
 namespace {
 
 // A structure which contains all the configuration information for sync.
 struct SyncConfigInfo {
   SyncConfigInfo();
   ~SyncConfigInfo();
 
   bool encrypt_all;
   bool sync_everything;
+  bool sync_nothing;
   syncer::ModelTypeSet data_types;
   std::string passphrase;
   bool passphrase_is_gaia;
 };
 
 SyncConfigInfo::SyncConfigInfo()
     : encrypt_all(false),
       sync_everything(false),
+      sync_nothing(false),
       passphrase_is_gaia(false) {
 }
 
 SyncConfigInfo::~SyncConfigInfo() {}
 
 // Note: The order of these types must match the ordering of
 // the respective types in ModelType
 const char* kDataTypeNames[] = {
   "bookmarks",
   "preferences",
@@ skipping 52 matching lines @@
   if (!parsed_value.get() || !parsed_value->GetAsDictionary(&result)) {
     DLOG(ERROR) << "GetConfiguration() not passed a Dictionary";
     return false;
   }
 
   if (!result->GetBoolean("syncAllDataTypes", &config->sync_everything)) {
     DLOG(ERROR) << "GetConfiguration() not passed a syncAllDataTypes value";
     return false;
   }
 
+  if (!result->GetBoolean("syncNothing", &config->sync_nothing)) {
+    DLOG(ERROR) << "GetConfiguration() not passed a syncNothing value";
+    return false;
+  }
+
+  DCHECK(!(config->sync_everything && config->sync_nothing))
+      << "syncAllDataTypes and syncNothing cannot both be true";
+
   ModelTypeNameMap type_names = GetSelectableTypeNameMap();
 
   for (ModelTypeNameMap::const_iterator it = type_names.begin();
        it != type_names.end(); ++it) {
     std::string key_name = it->second + std::string("Synced");
     bool sync_value;
     if (!result->GetBoolean(key_name, &sync_value)) {
       DLOG(ERROR) << "GetConfiguration() not passed a value for " << key_name;
       return false;
     }
@@ skipping 191 matching lines @@
     { "invalidAccessCode", IDS_SYNC_INVALID_ACCESS_CODE_LABEL },
     { "enterAccessCode", IDS_SYNC_ENTER_ACCESS_CODE_LABEL },
     { "getAccessCodeHelp", IDS_SYNC_ACCESS_CODE_HELP_LABEL },
     { "getAccessCodeURL", IDS_SYNC_GET_ACCESS_CODE_URL },
     { "invalidOtp", IDS_SYNC_INVALID_OTP_LABEL },
     { "enterOtp", IDS_SYNC_ENTER_OTP_LABEL },
     { "getOtpHelp", IDS_SYNC_OTP_HELP_LABEL },
     { "getOtpURL", IDS_SYNC_GET_OTP_URL },
     { "syncAllDataTypes", IDS_SYNC_EVERYTHING },
     { "chooseDataTypes", IDS_SYNC_CHOOSE_DATATYPES },
+    { "syncNothing", IDS_SYNC_NOTHING },
     { "bookmarks", IDS_SYNC_DATATYPE_BOOKMARKS },
     { "preferences", IDS_SYNC_DATATYPE_PREFERENCES },
     { "autofill", IDS_SYNC_DATATYPE_AUTOFILL },
     { "themes", IDS_SYNC_DATATYPE_THEMES },
     { "passwords", IDS_SYNC_DATATYPE_PASSWORDS },
     { "extensions", IDS_SYNC_DATATYPE_EXTENSIONS },
     { "typedURLs", IDS_SYNC_DATATYPE_TYPED_URLS },
     { "apps", IDS_SYNC_DATATYPE_APPS },
     { "openTabs", IDS_SYNC_DATATYPE_TABS },
     { "syncZeroDataTypesError", IDS_SYNC_ZERO_DATA_TYPES_ERROR },
@@ skipping 65 matching lines @@
   // Should only be called if user is signed in, so no longer need our
   // SigninTracker.
   signin_tracker_.reset();
   configuring_sync_ = true;
   DCHECK(service->sync_initialized()) <<
       "Cannot configure sync until the sync backend is initialized";
 
   // Setup args for the sync configure screen:
   //   showSyncEverythingPage: false to skip directly to the configure screen
   //   syncAllDataTypes: true if the user wants to sync everything
+  //   syncNothing: true if the user wants to sync nothing
   //   <data_type>Registered: true if the associated data type is supported
   //   <data_type>Synced: true if the user wants to sync that specific data type
   //   encryptionEnabled: true if sync supports encryption
   //   encryptAllData: true if user wants to encrypt all data (not just
   //       passwords)
   //   usePassphrase: true if the data is encrypted with a secondary passphrase
   //   show_passphrase: true if a passphrase is needed to decrypt the sync data
   DictionaryValue args;
 
   // Tell the UI layer which data types are registered/enabled by the user.
   const syncer::ModelTypeSet registered_types =
       service->GetRegisteredDataTypes();
   const syncer::ModelTypeSet preferred_types =
       service->GetPreferredDataTypes();
   ModelTypeNameMap type_names = GetSelectableTypeNameMap();
   for (ModelTypeNameMap::const_iterator it = type_names.begin();
        it != type_names.end(); ++it) {
     syncer::ModelType sync_type = it->first;
     const std::string key_name = it->second;
     args.SetBoolean(key_name + "Registered",
                     registered_types.Has(sync_type));
     args.SetBoolean(key_name + "Synced", preferred_types.Has(sync_type));
   }
   browser_sync::SyncPrefs sync_prefs(GetProfile()->GetPrefs());
   args.SetBoolean("passphraseFailed", passphrase_failed);
   args.SetBoolean("showSyncEverythingPage", !show_advanced);
   args.SetBoolean("syncAllDataTypes", sync_prefs.HasKeepEverythingSynced());
+  args.SetBoolean("syncNothing", false);  // Always false during initial setup.
   args.SetBoolean("encryptAllData", service->EncryptEverythingEnabled());
 
   // We call IsPassphraseRequired() here, instead of calling
   // IsPassphraseRequiredForDecryption(), because we want to show the passphrase
   // UI even if no encrypted data types are enabled.
   args.SetBoolean("showPassphrase", service->IsPassphraseRequired());
   // Keystore encryption is behind a flag. Only show the new encryption settings
   // if keystore encryption is enabled.
   args.SetBoolean("keystoreEncryptionEnabled", IsKeystoreEncryptionEnabled());
 
@@ skipping 70 matching lines @@
 
   ProfileSyncService* service = GetSyncService();
   DCHECK(service);
   if (!service->HasSyncSetupCompleted()) {
     // This is the first time configuring sync, so log it.
     base::FilePath profile_file_path = GetProfile()->GetPath();
     ProfileMetrics::LogProfileSyncSignIn(profile_file_path);
 
     // We're done configuring, so notify ProfileSyncService that it is OK to
     // start syncing.
+    service->SetSetupInProgress(false);

[Andrew T Wilson (Slow), 2013/05/16 08:39:30]

Where was this done previously (i.e. why is this necessary now)?

[Raghu Simha, 2013/05/17 05:14:17]

Earlier, this was being called in CloseSyncSetup as part of the
sign-out-if-not-fully-setup block, and was being run irrespective of whether the
user canceled sign in or not. The above call is necessary now that the
CloseSyncSetup has been refactored out to disable sync only in the cancel case,
since we want to leave the backend initialized in the user-needs-to-re-auth
case.

     service->SetSyncSetupCompleted();
   }
 }
 
 bool SyncSetupHandler::IsActiveLogin() const {
   // LoginUIService can be NULL if page is brought up in incognito mode
   // (i.e. if the user is running in guest mode in cros and brings up settings).
   LoginUIService* service = GetLoginUIService();
   return service && (service->current_login_ui() == this);
 }
@@ skipping 314 matching lines @@
   // Gaia credentials are valid - update the UI.
   DisplayGaiaSuccessAndSettingUp();
 }
 
 void SyncSetupHandler::SigninFailed(const GoogleServiceAuthError& error) {
   // Stop a timer to handle timeout in waiting for checking network connection.
   backend_start_timer_.reset();
 
   last_signin_error_ = error;
 
-  // If using web-based sign in flow, don't show the gaia sign in page again
-  // since there is no way to show the user an error message.
-  if (SyncPromoUI::UseWebBasedSigninFlow()) {
+  if (!retry_on_signin_failure_ &&
+      (error.state() == GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS ||
+       error.state() == GoogleServiceAuthError::ACCOUNT_DELETED ||
+       error.state() == GoogleServiceAuthError::ACCOUNT_DISABLED)) {
+    // The user received an auth error while re-enabling sync after having
+    // chosen to "Sync nothing". This can happen if the gaia password has been
+    // changed since the user last signed in, or if the account has been deleted
+    // or disabled. Leave the sync backend initialized, and close the setup
+    // dialog. An error message will be shown on the settings page.
+    CloseOverlay();

[Andrew T Wilson (Slow), 2013/05/16 08:39:30]

It seems dangerous to just call CloseOverlay() - CloseSyncSetup() does lots of
good cleanup like freeing the SigninTracker. I think the code below that calls
CloseOverlay() is probably incorrect. In fact, I think there's lots of code that
calls CloseOverlay() (like DisplayGaiaSuccessAndClose(), etc that are doing the
wrong thing here too - but that's for another CL :)

Can we just fall through to use the existing cases below? Especially since roger
is driving a CL to remove all of the non-web-based-signin-flow code, so all of
this could just turn into a single call to CloseSyncSetup().

If CloseSyncSetup() does something you don't want to happen here, then we should
change it to do the right thing.

[Raghu Simha, 2013/05/17 05:14:17]

As a matter of fact, CloseOverlay() calls CloseSyncSetup(), and therefore, does
all the good cleanup you mentioned. In addition to calling CloseSyncSetup(), it
also closes the overlay window. This has been true since this CL:
https://chromiumcodereview.appspot.com/9295044/diff/38001/chrome/browser/ui/w...

The case I am trying to handle here is when sync has been stopped, and the user
tries to re-enable it after the gaia password has changed. If we merely call
CloseSyncSetup without calling CloseOverlay, all the cleanup is done, but the
spinner window remains visible, and hides the "sign in again" link that shows up
on the main settings page.

I've collapsed the if block I originally added into one of the existing cases.
I've also retained the call to CloseOverlay(), which explicitly calls
CloseSyncSetup(). I've updated the description of CloseOverlay() in the header
file. If you believe CloseOverlay() shouldn't be calling CloseSyncSetup(), let
me know, and I'll change it in another CL.

[Andrew T Wilson (Slow), 2013/05/17 07:26:10]

Hah, apparently I forgot about my own refactorings :)

+  } else if (SyncPromoUI::UseWebBasedSigninFlow()) {
+    // If using web-based sign in flow, don't show the gaia sign in page again
+    // since there is no way to show the user an error message.
     CloseSyncSetup();
   } else if (retry_on_signin_failure_) {
     // Got a failed signin - this is either just a typical auth error, or a
     // sync error (treat sync errors as "fatal errors" - i.e. non-auth errors).
     // On ChromeOS, this condition can happen when auth token is invalid and
     // cannot start sync backend.
-    // If using web-based sign in flow, don't show the gaia sign in page again
-    // since there is no way to show the user an error message.
     ProfileSyncService* service = GetSyncService();
     DisplayGaiaLogin(service && service->HasUnrecoverableError());
   } else {
     // TODO(peria): Show error dialog for prompting sign in and out on
     // Chrome OS. http://crbug.com/128692
     CloseOverlay();
   }
 }
 
 Profile* SyncSetupHandler::GetProfile() const {
@@ skipping 15 matching lines @@
   // If we have signed in while sync is already setup, it must be due to some
   // kind of re-authentication flow. In that case, just close the signin dialog
   // rather than forcing the user to go through sync configuration.
   if (!service || service->HasSyncSetupCompleted())
     DisplayGaiaSuccessAndClose();
   else
     DisplayConfigureSync(false, false);
 }
 
 void SyncSetupHandler::HandleConfigure(const ListValue* args) {
   std::string json;

[Andrew T Wilson (Slow), 2013/05/16 08:39:30]

Can you please add a DCHECK(!signin_tracker.get()) here - it's an error to ever
get here with a SigninTracker active.

[Raghu Simha, 2013/05/17 05:14:17]

Done.

   if (!args->GetString(0, &json)) {
     NOTREACHED() << "Could not read JSON argument";
     return;
   }
   if (json.empty()) {
     NOTREACHED();
     return;
   }
 
   SyncConfigInfo configuration;
   if (!GetConfiguration(json, &configuration)) {
     // The page sent us something that we didn't understand.
     // This probably indicates a programming error.
     NOTREACHED();
     return;
   }
 
   // Start configuring the ProfileSyncService using the configuration passed
   // to us from the JS layer.
   ProfileSyncService* service = GetSyncService();
 
   // If the sync engine has shutdown for some reason, just close the sync
   // dialog.
   if (!service || !service->sync_initialized()) {
     CloseOverlay();
     return;
   }
 
+  // Disable sync, but remain signed in if the user selected "Sync nothing" in
+  // the advanced settings dialog. Note: In order to disable sync across
+  // restarts on Chrome OS, we must call OnStopSyncingPermanently(), which
+  // suppresses sync startup in addition to disabling it.
+  if (configuration.sync_nothing) {
+    ProfileSyncService::SyncEvent(
+        ProfileSyncService::STOP_FROM_ADVANCED_DIALOG);
+    CloseOverlay();
+    service->OnStopSyncingPermanently();
+    service->SetSetupInProgress(false);
+    return;
+  }
+
   // Note: Data encryption will not occur until configuration is complete
   // (when the PSS receives its CONFIGURE_DONE notification from the sync
   // backend), so the user still has a chance to cancel out of the operation
   // if (for example) some kind of passphrase error is encountered.
   if (configuration.encrypt_all)
     service->EnableEncryptEverything();
 
   bool passphrase_failed = false;
   if (!configuration.passphrase.empty()) {
     // We call IsPassphraseRequired() here (instead of
@@ skipping 116 matching lines @@
 }
 
 void SyncSetupHandler::HandleCloseTimeout(const ListValue* args) {
   CloseSyncSetup();
 }
 
 void SyncSetupHandler::CloseSyncSetup() {
   // TODO(atwilson): Move UMA tracking of signin events out of sync module.
   ProfileSyncService* sync_service = GetSyncService();
   if (IsActiveLogin()) {
-    if (!sync_service || !sync_service->HasSyncSetupCompleted()) {
+    // Don't log a cancel event if the sync setup dialog is being
+    // automatically closed due to an auth error.
+    if ((!sync_service || !sync_service->HasSyncSetupCompleted()) &&
+        last_signin_error_.state() == GoogleServiceAuthError::NONE) {
       if (signin_tracker_.get()) {
         ProfileSyncService::SyncEvent(
             ProfileSyncService::CANCEL_DURING_SIGNON);
       } else if (configuring_sync_) {
         ProfileSyncService::SyncEvent(
             ProfileSyncService::CANCEL_DURING_CONFIGURE);
       } else {
         ProfileSyncService::SyncEvent(
             ProfileSyncService::CANCEL_FROM_SIGNON_WITHOUT_AUTH);
       }
+
+      // If the user clicked "Cancel" while setting up sync, disable sync
+      // because we don't want the sync backend to remain in the initialized
+      // state. Note: In order to disable sync across restarts on Chrome OS, we
+      // must call OnStopSyncingPermanently(), which suppresses sync startup in
+      // addition to disabling it.
+      if (sync_service) {
+        DVLOG(1) << "Sync setup aborted by user action";
+        sync_service->OnStopSyncingPermanently();
+        sync_service->SetSetupInProgress(false);
+      }
     }
 
     // Let the various services know that we're no longer active.
     if (SyncPromoUI::UseWebBasedSigninFlow())
       CloseGaiaSigninPage();
 
     GetLoginUIService()->LoginUIClosed(this);
   }
 
-  if (sync_service) {
-    // Make sure user isn't left half-logged-in (signed in, but without sync
-    // started up). If the user hasn't finished setting up sync, then sign out
-    // and shut down sync.
-    if (!sync_service->HasSyncSetupCompleted()) {
-      DVLOG(1) << "Signin aborted by user action";
-      // We can get here on Chrome OS (e.g dashboard clear), but "do nothing"
-      // is the correct behavior.
-#if !defined(OS_CHROMEOS)
-      if (signin_tracker_.get() || sync_service->FirstSetupInProgress()) {
-        // User was still in the process of signing in, so sign him out again.
-        // This makes sure that the user isn't left signed in but with sync
-        // un-configured.
-        //
-        // This has the side-effect of signing out the user in the following
-        // scenario:
-        //   * User signs in while sync is disabled by policy.
-        //   * Sync is re-enabled by policy.
-        //   * User brings up sync setup dialog to do initial sync config.
-        //   * User cancels out of the dialog.
-        //
-        // This case is indistinguishable from the "one click signin" case where
-        // the user checks the "advanced setup" checkbox, then cancels out of
-        // the setup box, which is a much more common scenario, so we do the
-        // right thing for the one-click case.
-        SigninManagerFactory::GetForProfile(GetProfile())->SignOut();
-      }
-#endif
-      sync_service->DisableForUser();
-      browser_sync::SyncPrefs sync_prefs(GetProfile()->GetPrefs());
-      sync_prefs.SetStartSuppressed(true);
-    }
-    sync_service->SetSetupInProgress(false);
-  }
-
   // Reset the attempted email address and error, otherwise the sync setup
   // overlay in the settings page will stay in whatever error state it was last
   // when it is reopened.
   last_attempted_user_email_.clear();
   last_signin_error_ = GoogleServiceAuthError::AuthErrorNone();
 
   configuring_sync_ = false;
   signin_tracker_.reset();
 
   // Stop a timer to handle timeout in waiting for checking network connection.
   backend_start_timer_.reset();
 }
 
 void SyncSetupHandler::OpenSyncSetup() {
   if (!PrepareSyncSetup())
     return;
 
   // There are several different UI flows that can bring the user here:
   // 1) Signin promo.
   // 2) Normal signin through settings page (GetAuthenticatedUsername() is
   //    empty).
   // 3) Previously working credentials have expired.
   // 4) User is signed in, but has stopped sync via the google dashboard, and
   //    signout is prohibited by policy so we need to force a re-auth.
   // 5) User clicks [Advanced Settings] button on options page while already
   //    logged in.
   // 6) One-click signin (credentials are already available, so should display
   //    sync configure UI, not login UI).
-  // 7) ChromeOS re-enable after disabling sync.
+  // 7) User re-enables sync after disabling it via advanced settings.
   SigninManagerBase* signin =
       SigninManagerFactory::GetForProfile(GetProfile());
   if (signin->GetAuthenticatedUsername().empty() ||
-#if !defined(OS_CHROMEOS)
-      (GetSyncService() && GetSyncService()->IsStartSuppressed()) ||
-#endif
       signin->signin_global_error()->HasMenuItem()) {
     // User is not logged in, or login has been specially requested - need to
     // display login UI (cases 1-3).
     DisplayGaiaLogin(false);
   } else {
     if (!GetSyncService()) {
       // This can happen if the user directly navigates to /settings/syncSetup.
       DLOG(WARNING) << "Cannot display sync UI when sync is disabled";
       CloseOverlay();
       return;
     }
 
     // User is already logged in. They must have brought up the config wizard
     // via the "Advanced..." button or through One-Click signin (cases 4-6), or
-    // they are re-enabling sync on Chrome OS.
+    // they are re-enabling sync after having disabled it (case 7).
     DisplayConfigureSync(true, false);
   }
 
   if (!SyncPromoUI::UseWebBasedSigninFlow())
     ShowSetupUI();
 }
 
 void SyncSetupHandler::OpenConfigureSync() {
   if (!PrepareSyncSetup())
     return;
@@ skipping 127 matching lines @@
       *error_message = l10n_util::GetStringUTF16(
           IDS_SYNC_USER_NAME_IN_USE_ERROR);
       return false;
     }
   }
 
   return true;
 }
 
 #endif  // !defined(OS_CHROMEOS)