Setup from settings should allow configuration first

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 4d60906ac9eae93bc1af7b200521bceee01d75a8..a1d87785102357cb699743a060214fbad96279a2 100644
--- a/chrome/browser/ui/webui/sync_setup_handler.cc
+++ b/chrome/browser/ui/webui/sync_setup_handler.cc
@@ -28,6 +28,7 @@
 #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"
@@ -191,13 +192,26 @@ bool UseWebBasedSigninFlow() {
       switches::kUseWebBasedSigninFlow);
 }
 
+void BringTabToFront(WebContents* web_contents) {
+  Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
+  if (browser) {
+    TabStripModel* tab_strip_model = browser->tab_strip_model();
+    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() {
@@ -489,6 +503,12 @@ void SyncSetupHandler::DisplayConfigureSync(bool show_advanced,
   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 the settings
+    // tab.
+    BringTabToFront(web_ui()->GetWebContents());
+  }
 }
 
 void SyncSetupHandler::ConfigureSyncDone() {
@@ -566,13 +586,25 @@ SigninManager* SyncSetupHandler::GetSignin() const {
 
 void SyncSetupHandler::DisplayGaiaLogin(bool fatal_error) {
   if (UseWebBasedSigninFlow()) {
+    DCHECK(!active_gaia_signin_tab_);
+
+    // Advanced options are no longer being configured if the login screen is
+    // visible. 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);
@@ -581,9 +613,9 @@ void SyncSetupHandler::DisplayGaiaLogin(bool 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,
-  // CloseSyncSetup() will ensure they are logged out.
+  // Advanced options are no longer being configured if the login screen is
+  // visible. If the user exits the signin wizard after this without
+  // configuring sync, CloseSyncSetup() will ensure they are logged out.
   configuring_sync_ = false;
 
   string16 local_error_message(error_message);
@@ -662,11 +694,9 @@ bool SyncSetupHandler::PrepareSyncSetup() {
     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;
 }
@@ -1013,7 +1043,11 @@ void SyncSetupHandler::CloseSyncSetup() {
             ProfileSyncService::CANCEL_FROM_SIGNON_WITHOUT_AUTH);
       }
     }
+
     // Let the various services know that we're no longer active.
+    if (UseWebBasedSigninFlow())
+      CloseGaiaSigninPage();
+
     GetLoginUIService()->LoginUIClosed(this);
   }
 
@@ -1090,8 +1124,15 @@ void SyncSetupHandler::OpenConfigureSync() {
 
 void SyncSetupHandler::FocusUI() {
   DCHECK(IsActiveLogin());
-  WebContents* web_contents = web_ui()->GetWebContents();
-  web_contents->GetDelegate()->ActivateContents(web_contents);
+  // if |signin_tracker_| is valid and web-based sign flow is enabled, bring

[James Hawkins, 2012/12/04 01:02:24]

Optional nit: I don't think this comment adds much value since it's almost
verbatim what the code says.

[Roger Tawa OOO till Jul 10th, 2012/12/04 01:10:14]

True, done.

+  // the tab containing the active gaia signing to the top.  Otherwise activate
+  // the settings tab.
+  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() {
@@ -1099,6 +1140,12 @@ void SyncSetupHandler::CloseUI() {
   CloseOverlay();
 }
 
+void SyncSetupHandler::WebContentsDestroyed(
+    content::WebContents* web_contents) {
+  DCHECK(active_gaia_signin_tab_);
+  CloseSyncSetup();
+}
+
 // Private member functions.
 
 bool SyncSetupHandler::FocusExistingWizardIfPresent() {
@@ -1121,6 +1168,28 @@ void SyncSetupHandler::CloseOverlay() {
   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())