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();
+    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.
+    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).
+  // (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.
+    // 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(

[Andrew T Wilson (Slow), 2012/12/03 13:19:02]

So, who closes this GAIA tab when signin is complete? It looks like we keep it
around until sync config is finished (at which point CloseSyncSetup() is called?
Shouldn't we close it once we get SigninSuccess?

[Roger Tawa OOO till Jul 10th, 2012/12/03 19:08:28]

Right its closed by CloseSyncSetup().  It always gets closed on all cases,
success or failure.  No problems as is.

         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
+  // Gaia sign in page.
+  if (UseWebBasedSigninFlow() && !configuring_sync_) {

[Andrew T Wilson (Slow), 2012/12/03 13:19:02]

So previously, configuring_sync_ was just used to log a UMA event (i.e. if the
user cancels out of signin, it lets us log CANCEL_DURING_CONFIGURE instead of
CANCEL_FROM_SIGNON_WITHOUT_AUTH).

I've been reticent to start using it for something else. Perhaps you could gate
this on signin_tracker_ != NULL instead?

[Roger Tawa OOO till Jul 10th, 2012/12/03 19:08:28]

Done.

+      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;
 }