suppress user/password dialog when re-enabling sync

chrome/browser/ui/webui/sync_setup_handler.cc

Index: chrome/browser/ui/webui/sync_setup_handler.cc
diff --git a/chrome/browser/ui/webui/sync_setup_handler.cc b/chrome/browser/ui/webui/sync_setup_handler.cc
index 2663f5f8a90e502a7f09b9e5e1f1a0a9f710b84b..9c2c548747a4116568cf3d8b9fdfd67aba3e909b 100644
--- a/chrome/browser/ui/webui/sync_setup_handler.cc
+++ b/chrome/browser/ui/webui/sync_setup_handler.cc
@@ -20,6 +20,8 @@
 #include "chrome/browser/profiles/profile_metrics.h"
 #include "chrome/browser/signin/signin_manager.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
+#include "chrome/browser/signin/token_service.h"
+#include "chrome/browser/signin/token_service_factory.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/ui/browser_list.h"
@@ -490,6 +492,14 @@ void SyncSetupHandler::DisplayGaiaLoginWithErrorMessage(
       "SyncSetupOverlay.showSyncSetupPage", page, args);
 }
 
+// TODO(kochi): Handle error conditions (timeout, other failures).
+void SyncSetupHandler::DisplaySpinner() {
+  configuring_sync_ = true;
+  StringValue page("spinner");
+  web_ui()->CallJavascriptFunction(
+      "SyncSetupOverlay.showSyncSetupPage", page, DictionaryValue());
+}
+
 void SyncSetupHandler::RecordSignin() {
   // By default, do nothing - subclasses can override.
 }
@@ -586,7 +596,11 @@ void SyncSetupHandler::SigninFailed(const GoogleServiceAuthError& error) {
   last_signin_error_ = error;
   // 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 should trigger the orange badge on wrench menu
+  // and prompt to sign out.
+#if !defined(OS_CHROMEOS)
   DisplayGaiaLogin(GetSyncService()->unrecoverable_error_detected());
+#endif

[Andrew T Wilson (Slow), 2012/04/03 17:45:13]

We should probably close the overlay here though, to make sure the spinner is
hidden?

[kochi, 2012/04/03 21:55:29]

Done.

 }
 
 Profile* SyncSetupHandler::GetProfile() const {
@@ -598,14 +612,21 @@ ProfileSyncService* SyncSetupHandler::GetSyncService() const {
 }
 
 void SyncSetupHandler::SigninSuccess() {
+  LOG(WARNING) << "SigninSuccess()";
   DCHECK(GetSyncService()->sync_initialized());
   // 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 (GetSyncService()->HasSyncSetupCompleted())
+  if (GetSyncService()->HasSyncSetupCompleted()) {
     DisplayGaiaSuccessAndClose();
-  else
+  } else {
+#if !defined(OS_CHROMEOS)
     DisplayConfigureSync(false, false);
+#else
+    // On ChromeOS, clicking on "Set up Sync" should show advanced settings.
+    DisplayConfigureSync(true, false);

[Andrew T Wilson (Slow), 2012/04/03 17:45:13]

Are you sure that's what we want here (not the "Sync Everything" button
instead)?

[kochi, 2012/04/03 21:55:29]

I thought this was the intention (before enabling sync,
give the user an opportunity to configure which data types
to be synced), but could be okay as the user can configure
data types anyway.  To make the diversity less, removed this
ifdef.

+#endif
+  }
 }
 
 void SyncSetupHandler::HandleConfigure(const ListValue* args) {
@@ -756,7 +777,6 @@ void SyncSetupHandler::CloseSyncSetup() {
     // 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 && !sync_service->HasSyncSetupCompleted()) {
       DVLOG(1) << "Signin aborted by user action";
       sync_service->DisableForUser();
@@ -795,8 +815,37 @@ void SyncSetupHandler::OpenSyncSetup(bool force_login) {
     // via the "Advanced..." button or the wrench menu.
     DisplayConfigureSync(true, false);
   } else {
+#if !defined(OS_CHROMEOS)
     // User is not logged in - need to display login UI.
     DisplayGaiaLogin(false);
+#else
+    // On Chrome OS user is always logged in. Instead of showing login dialog,
+    // show spinner until the backend gets ready to configure sync.
+
+    // 1. To make PSS:AreCredentialsAvailable() to return true, first we need
+    //    to unsppress the flag.
+    browser_sync::SyncPrefs sync_prefs(GetProfile()->GetPrefs());

[Andrew T Wilson (Slow), 2012/04/03 17:45:13]

Should probably just move this into a helper function.

Also, if you never want to display a GAIA login on cros, you should probably #if
it out of the header and .cc file to make sure calls don't sneak back in.

[kochi, 2012/04/03 21:55:29]

Split this part into a helper function.

For #ifdef'ing the non-ChromeOS part makes this
CL bigger, so i'd like to defer it for later CLs.

On 2012/04/03 17:45:13, Andrew T Wilson wrote:

+    sync_prefs.SetStartSuppressed(false);
+
+    // 2. To listen to the token available notifications, start SigninTracker.
+    signin_tracker_.reset(new SigninTracker(GetProfile(), this));

[Andrew T Wilson (Slow), 2012/04/03 17:45:13]

We don't need a signin tracker in the case that sync is already initialized.

[kochi, 2012/04/03 21:55:29]

But for the case sync backend is stopped, we need to have
SigninTracker.

+    TokenService* token_service =
+        TokenServiceFactory::GetForProfile(GetProfile());
+    DCHECK(token_service);
+    if (token_service->AreCredentialsValid()) {
+      token_service->StartFetchingTokens();

[Andrew T Wilson (Slow), 2012/04/03 17:45:13]

Shouldn't need to do this once you remove SigninManager::SignOut() on cros.

[kochi, 2012/04/03 21:55:29]

Removed.

+    } else {
+      LOG(WARNING) << "TokenService: credentials are invalid!";
+      token_service->LoadTokensFromDB();
+    }
+    if (!service->sync_initialized()) {
+      service->set_setup_in_progress(true);

[Andrew T Wilson (Slow), 2012/04/03 17:45:13]

This looks good. In fact, we might want to have this in general (not just for
os_chromeos) in case someone tries to bring up the configure sync dialog before
the backend is initialized.

[kochi, 2012/04/03 21:55:29]

So as not to break non-ChromeOS for now, I'd like to
keep this as is...

On 2012/04/03 17:45:13, Andrew T Wilson wrote:

+      service->UnsuppressAndStart();
+      DisplaySpinner();
+    } else {
+      DisplayConfigureSync(true, false);
+    }
+#endif
   }
 
   ShowSetupUI();