Setup from settings should allow configuration first

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 10 matching lines @@
 #include "chrome/browser/profiles/profile_info_cache.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/profiles/profile_metrics.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/signin/signin_manager.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_navigator.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/webui/signin/login_ui_service.h"
 #include "chrome/browser/ui/webui/signin/login_ui_service_factory.h"
 #include "chrome/browser/ui/webui/sync_promo/sync_promo_ui.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_delegate.h"
 #include "google_apis/gaia/gaia_constants.h"
@@ skipping 143 matching lines @@
 bool IsKeystoreEncryptionEnabled() {
   return CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kSyncKeystoreEncryption);
 }
 
 bool UseWebBasedSigninFlow() {
   return CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kUseWebBasedSigninFlow);
 }
 
+void BringTabToFront(WebContents* web_contents) {
+  Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
+  if (browser) {
+    TabStripModel* tab_strip_model = browser->tab_strip_model();

[James Hawkins, 2012/12/04 00:14:48]

Optional nit: Save indentation by reversing the logic and returning early.

[Roger Tawa OOO till Jul 10th, 2012/12/04 00:36:37]

I think I'll keep it as is.

+    if (tab_strip_model) {
+      int index = tab_strip_model->GetIndexOfWebContents(web_contents);
+      if (index != TabStripModel::kNoTab)
+        tab_strip_model->ActivateTabAt(index, false);
+    }
+  }
+}
+
 }  // namespace
 
 SyncSetupHandler::SyncSetupHandler(ProfileManager* profile_manager)
     : configuring_sync_(false),
       profile_manager_(profile_manager),
       last_signin_error_(GoogleServiceAuthError::NONE),
-      retry_on_signin_failure_(true) {
+      retry_on_signin_failure_(true),
+      active_gaia_signin_tab_(NULL) {
 }
 
 SyncSetupHandler::~SyncSetupHandler() {
   // Just exit if running unit tests (no actual WebUI is attached).
   if (!web_ui())
     return;
 
   // This case is hit when the user performs a back navigation.
   CloseSyncSetup();
 }
@@ skipping 271 matching lines @@
           "fullEncryptionBody",
           GetStringUTF16(IDS_SYNC_FULL_ENCRYPTION_DATA));
     }
   } else {
     args.SetBoolean("usePassphrase", service->IsUsingSecondaryPassphrase());
   }
 
   StringValue page("configure");
   web_ui()->CallJavascriptFunction(
       "SyncSetupOverlay.showSyncSetupPage", page, args);
+
+  if (UseWebBasedSigninFlow()) {
+    // Make sure the tab used for the Gaia sign in does not cover this tab.

[James Hawkins, 2012/12/04 00:14:48]

nit: What is 'this' tab?

[Roger Tawa OOO till Jul 10th, 2012/12/04 00:36:37]

Done.

+    BringTabToFront(web_ui()->GetWebContents());
+  }
 }
 
 void SyncSetupHandler::ConfigureSyncDone() {
   StringValue page("done");
   web_ui()->CallJavascriptFunction(
       "SyncSetupOverlay.showSyncSetupPage", page);
 
   // Suppress the sync promo once the user signs into sync. This way the user
   // doesn't see the sync promo even if they sign out of sync later on.
   SyncPromoUI::SetUserSkippedSyncPromo(GetProfile());
 
   ProfileSyncService* service = GetSyncService();
   if (!service->HasSyncSetupCompleted()) {
     // This is the first time configuring sync, so log it.
     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->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).

[James Hawkins, 2012/12/04 00:14:48]

Did this not fine on one line?

[Roger Tawa OOO till Jul 10th, 2012/12/04 00:36:37]

Done.

+  // (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);
 }
 
 void SyncSetupHandler::RegisterMessages() {
   web_ui()->RegisterMessageCallback(
       "SyncSetupDidClosePage",
       base::Bind(&SyncSetupHandler::OnDidClosePage,
                  base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
@@ skipping 31 matching lines @@
       base::Bind(&SyncSetupHandler::HandleStopSyncing,
                  base::Unretained(this)));
 }
 
 SigninManager* SyncSetupHandler::GetSignin() const {
   return SigninManagerFactory::GetForProfile(GetProfile());
 }
 
 void SyncSetupHandler::DisplayGaiaLogin(bool fatal_error) {
   if (UseWebBasedSigninFlow()) {
+    DCHECK(!active_gaia_signin_tab_);
+
+    // We are no longer configuring sync if the login screen is visible.

[James Hawkins, 2012/12/04 00:14:48]

nit: Who is 'we' here?

[Roger Tawa OOO till Jul 10th, 2012/12/04 00:36:37]

Done.

+    // If the user exits the signin wizard after this without configuring sync,
+    // CloseSyncSetup() will ensure they are logged out.
+    configuring_sync_ = false;
+
     GURL url(SyncPromoUI::GetSyncPromoURL(GURL(),
         SyncPromoUI::SOURCE_SETTINGS, false));
     Browser* browser = chrome::FindBrowserWithWebContents(
         web_ui()->GetWebContents());
-    browser->OpenURL(
+    active_gaia_signin_tab_ = browser->OpenURL(
         content::OpenURLParams(url, content::Referrer(), SINGLETON_TAB,
-                               content::PAGE_TRANSITION_AUTO_BOOKMARK, false));
+                                content::PAGE_TRANSITION_AUTO_BOOKMARK,
+                                false));
+    content::WebContentsObserver::Observe(active_gaia_signin_tab_);
+    signin_tracker_.reset(
+        new SigninTracker(GetProfile(), this,
+                          SigninTracker::WAITING_FOR_GAIA_VALIDATION));
   } else {
     retry_on_signin_failure_ = true;
     DisplayGaiaLoginWithErrorMessage(string16(), fatal_error);
   }
 }
 
 void SyncSetupHandler::DisplayGaiaLoginWithErrorMessage(
     const string16& error_message, bool fatal_error) {
   // We are no longer configuring sync if the login screen is visible.
   // If the user exits the signin wizard after this without configuring sync,
@@ skipping 69 matching lines @@
     return false;
   }
 
   // If the wizard is already visible, just focus that one.
   if (FocusExistingWizardIfPresent()) {
     if (!IsActiveLogin())
       CloseOverlay();
     return false;
   }
 
-  if (!UseWebBasedSigninFlow()) {
-    // Notify services that login UI is now active.
-    GetLoginUIService()->SetLoginUI(this);
-    service->SetSetupInProgress(true);
-  }
+  // Notify services that login UI is now active.
+  GetLoginUIService()->SetLoginUI(this);
+  service->SetSetupInProgress(true);
 
   return true;
 }
 
 void SyncSetupHandler::DisplaySpinner() {
   configuring_sync_ = true;
   StringValue page("spinner");
   DictionaryValue args;
 
   const int kTimeoutSec = 30;
@@ skipping 326 matching lines @@
         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);
       }
     }
+
     // Let the various services know that we're no longer active.
+    if (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";
       sync_service->DisableForUser();
@@ skipping 56 matching lines @@
 void SyncSetupHandler::OpenConfigureSync() {
   if (!PrepareSyncSetup())
     return;
 
   DisplayConfigureSync(true, false);
   ShowSetupUI();
 }
 
 void SyncSetupHandler::FocusUI() {
   DCHECK(IsActiveLogin());
-  WebContents* web_contents = web_ui()->GetWebContents();
-  web_contents->GetDelegate()->ActivateContents(web_contents);
+  // In the case of the web-based sign in flow, |configuring_sync_| true means
+  // we are focusing to bring the configure popup window to the top, not the

[James Hawkins, 2012/12/04 00:14:48]

nit: This sentence can use some clarification:

if |configuring_sync_| is true, bring the tab containing the active gaia signing
to the top.

Note that it appears the comment does not match the code.  The comment talks
about |!sign in page| and the code references the |active_gaia_signin_tab_|. 
The comment also talks about |configure_sync_| which I don't see in the code.

[Roger Tawa OOO till Jul 10th, 2012/12/04 00:36:37]

Good catch.  Done.

+  // Gaia sign in page.
+  if (UseWebBasedSigninFlow() && signin_tracker_) {
+      BringTabToFront(active_gaia_signin_tab_);
+  } else {
+    WebContents* web_contents = web_ui()->GetWebContents();
+    web_contents->GetDelegate()->ActivateContents(web_contents);
+  }
 }
 
 void SyncSetupHandler::CloseUI() {
   DCHECK(IsActiveLogin());
   CloseOverlay();
 }
 
+void SyncSetupHandler::WebContentsDestroyed(
+    content::WebContents* web_contents) {
+  DCHECK(active_gaia_signin_tab_);
+  CloseSyncSetup();
+}
+
 // Private member functions.
 
 bool SyncSetupHandler::FocusExistingWizardIfPresent() {
   LoginUIService* service = GetLoginUIService();
   if (!service->current_login_ui())
     return false;
   service->current_login_ui()->FocusUI();
   return true;
 }
 
 LoginUIService* SyncSetupHandler::GetLoginUIService() const {
   return LoginUIServiceFactory::GetForProfile(GetProfile());
 }
 
 void SyncSetupHandler::CloseOverlay() {
   // Stop a timer to handle timeout in waiting for sync setup.
   backend_start_timer_.reset();
 
   CloseSyncSetup();
   web_ui()->CallJavascriptFunction("OptionsPage.closeOverlay");
 }
 
+void SyncSetupHandler::CloseGaiaSigninPage() {
+  if (active_gaia_signin_tab_) {
+    content::WebContentsObserver::Observe(NULL);
+
+    Browser* browser = chrome::FindBrowserWithWebContents(
+        active_gaia_signin_tab_);
+    if (browser) {
+      TabStripModel* tab_strip_model = browser->tab_strip_model();
+      if (tab_strip_model) {
+        int index = tab_strip_model->GetIndexOfWebContents(
+            active_gaia_signin_tab_);
+        if (index != TabStripModel::kNoTab) {
+          tab_strip_model->ExecuteContextMenuCommand(
+              index, TabStripModel::CommandCloseTab);
+        }
+      }
+    }
+  }
+
+  active_gaia_signin_tab_ = NULL;
+}
+
 bool SyncSetupHandler::IsLoginAuthDataValid(const std::string& username,
                                             string16* error_message) {
   if (username.empty())
     return true;
 
   // Can be null during some unit tests.
   if (!web_ui())
     return true;
 
   if (!GetSignin()->IsAllowedUsername(username)) {
@@ skipping 15 matching lines @@
     if (i != current_profile_index && AreUserNamesEqual(
         cache.GetUserNameOfProfileAtIndex(i), username_utf16)) {
       *error_message = l10n_util::GetStringUTF16(
           IDS_SYNC_USER_NAME_IN_USE_ERROR);
       return false;
     }
   }
 
   return true;
 }