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 47cf1eb34c9816ad0a7162463fe108c3fda99ec9..0cdd6bd62dcd4a5a41098d912ef63d0f3d9365af 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,23 @@ bool UseWebBasedSigninFlow() {
       switches::kUseWebBasedSigninFlow);
 }
 
+void BringTabToFront(WebContents* web_contents) {
+  Browser* browser = browser::FindBrowserWithWebContents(web_contents);
+  TabStripModel* tab_strip_model = browser->tab_strip_model();
+  int index = tab_strip_model->GetIndexOfWebContents(web_contents);
+  tab_strip_model->ActivateTabAt(index, false);
+}
+
 }  // namespace
 
+SyncSetupHandler* SyncSetupHandler::active_sync_setup_handler_ = NULL;
+
 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 +500,11 @@ 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 this tab.

[tim (not reviewing), 2012/11/29 06:39:07]

They're separate tabs? 

[Roger Tawa OOO till Jul 10th, 2012/11/29 20:20:56]

yes

+    BringTabToFront(web_ui()->GetWebContents());
+  }
 }
 
 void SyncSetupHandler::ConfigureSyncDone() {
@@ -513,10 +529,15 @@ void SyncSetupHandler::ConfigureSyncDone() {
 }
 
 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);
+  if (UseWebBasedSigninFlow()) {
+    return active_sync_setup_handler_ == this;
+  } else {
+    // 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);
+  }
 }
 
 void SyncSetupHandler::RegisterMessages() {
@@ -566,13 +587,21 @@ SigninManager* SyncSetupHandler::GetSignin() const {
 
 void SyncSetupHandler::DisplayGaiaLogin(bool fatal_error) {
   if (UseWebBasedSigninFlow()) {
+    DCHECK(active_sync_setup_handler_ == this);
+    DCHECK(!active_gaia_signin_tab_);
+
     GURL url(SyncPromoUI::GetSyncPromoURL(GURL(),
         SyncPromoUI::SOURCE_SETTINGS, false));
     Browser* browser = browser::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);
@@ -662,7 +691,10 @@ bool SyncSetupHandler::PrepareSyncSetup() {
     return false;
   }
 
-  if (!UseWebBasedSigninFlow()) {
+  if (UseWebBasedSigninFlow()) {
+    DCHECK(active_sync_setup_handler_ == NULL);

[tim (not reviewing), 2012/11/29 06:39:07]

DCHECK_EQ, here and elsewhere

[Roger Tawa OOO till Jul 10th, 2012/11/29 20:20:56]

Done.

+    active_sync_setup_handler_ = this;
+  } else {
     // Notify services that login UI is now active.
     GetLoginUIService()->SetLoginUI(this);
     service->SetSetupInProgress(true);
@@ -1013,8 +1045,13 @@ void SyncSetupHandler::CloseSyncSetup() {
             ProfileSyncService::CANCEL_FROM_SIGNON_WITHOUT_AUTH);
       }
     }
+
     // Let the various services know that we're no longer active.
-    GetLoginUIService()->LoginUIClosed(this);
+    if (UseWebBasedSigninFlow()) {
+      CloseGaiaSigninPage();
+    } else {
+      GetLoginUIService()->LoginUIClosed(this);
+    }
   }
 
   if (sync_service) {
@@ -1098,13 +1135,34 @@ void SyncSetupHandler::CloseUI() {
   CloseOverlay();
 }
 
+void SyncSetupHandler::WebContentsDestroyed(
+    content::WebContents* web_contents) {
+  DCHECK(active_sync_setup_handler_ == this);
+  DCHECK(active_gaia_signin_tab_);
+
+  content::WebContentsObserver::Observe(NULL);
+  active_sync_setup_handler_ = NULL;
+  active_gaia_signin_tab_ = NULL;
+}
+
 // Private member functions.
 
 bool SyncSetupHandler::FocusExistingWizardIfPresent() {
-  LoginUIService* service = GetLoginUIService();
-  if (!service->current_login_ui())
-    return false;
-  service->current_login_ui()->FocusUI();
+  if (UseWebBasedSigninFlow()) {
+    if (active_sync_setup_handler_ == NULL)
+      return false;
+
+    // |configuring_sync_| true means we are focusing to bring the configure
+    // popup window to the top, not the Gaia sign in page.  Therefore don't
+    // bring Gaia to the front.
+    if (!configuring_sync_)
+      BringTabToFront(active_sync_setup_handler_->active_gaia_signin_tab_);

[tim (not reviewing), 2012/11/29 06:39:07]

There can be multiple active_sync_setup_handler_'s?  Why wouldn't this be
this->active_gaia_signin_tab_?

[Roger Tawa OOO till Jul 10th, 2012/11/29 20:20:56]

No, there can be only one.  However, if you open a second settings page, you can
still click the "sign in to chrome" button on that one.  So
this->active_gaia_signin_tab_ may be null even though there is an active sign in
ui.

This is actually exactly the same as before.  You could have one settings page
with a popup sign in window, and press sign in on another setting pages.  It
brings focus to the first.  See IsActiveLogin() for more details.

+  } else {
+    LoginUIService* service = GetLoginUIService();
+    if (!service->current_login_ui())
+      return false;
+    service->current_login_ui()->FocusUI();
+  }
   return true;
 }
 
@@ -1120,6 +1178,22 @@ void SyncSetupHandler::CloseOverlay() {
   web_ui()->CallJavascriptFunction("OptionsPage.closeOverlay");
 }
 
+void SyncSetupHandler::CloseGaiaSigninPage() {
+  if (active_sync_setup_handler_ == this && active_gaia_signin_tab_) {
+    content::WebContentsObserver::Observe(NULL);
+
+    Browser* browser = browser::FindBrowserWithWebContents(
+        active_sync_setup_handler_->active_gaia_signin_tab_);
+    TabStripModel* tab_strip_model = browser->tab_strip_model();
+    int index = tab_strip_model->GetIndexOfWebContents(
+        active_sync_setup_handler_->active_gaia_signin_tab_);
+    tab_strip_model->CloseTabContentsAt(index, TabStripModel::CLOSE_NONE);
+  }
+
+  active_gaia_signin_tab_ = NULL;
+  active_sync_setup_handler_ = NULL;
+}
+
 bool SyncSetupHandler::IsLoginAuthDataValid(const std::string& username,
                                             string16* error_message) {
   if (username.empty())