Setup from settings should allow configuration first

chrome/browser/ui/sync/one_click_signin_helper.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/sync/one_click_signin_helper.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/metrics/histogram.h"
@@ skipping 91 matching lines @@
     default:
       NOTREACHED() << "Invalid auto_accept: " << auto_accept;
       break;
   }
 
   UMA_HISTOGRAM_ENUMERATION("AutoLogin.Reverse", action,
                             one_click_signin::HISTOGRAM_MAX);
 }
 
 bool UseWebBasedSigninFlow() {
-  static const bool use_web_based_singin_flow =
+  const bool use_web_based_singin_flow =
       CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kUseWebBasedSigninFlow);

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

Why do we have a separate variable here?

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

No need.  Removed.

   return use_web_based_singin_flow;
 }
 
 // Determines the source of the sign in.  Its either one of the known sign in
 // access point (first run, NTP, menu, settings) or its an implicit sign in
 // via another Google property.  In the former case, "service" is also
 // checked to make sure its "chromiumsync".
 SyncPromoUI::Source GetSigninSource(const GURL& url) {
   std::string value;
   chrome_common_net::GetValueForKeyInQuery(url, "service", &value);
@@ skipping 309 matching lines @@
           prefs::kReverseAutologinRejectedEmailList);
       if (!rejected_emails->empty()) {
         const scoped_ptr<Value> email_value(new base::StringValue(email));
         base::ListValue::const_iterator iter = rejected_emails->Find(
             *email_value);
         if (iter != rejected_emails->end())
           return false;
       }
     }
 
-    // If we're about to show a one-click infobar but the user has started
-    // a concurrent signin flow (perhaps via the promo), we may not have yet
-    // established an authenticated username but we still shouldn't move
-    // forward with two simultaneous signin processes.  This is a bit
-    // contentious as the one-click flow is a much smoother flow from the user
-    // perspective, but it's much more difficult to hijack the other flow from
-    // here as it is to bail.
-    ProfileSyncService* service =
-        ProfileSyncServiceFactory::GetForProfile(profile);
-    if (!service)
-      return false;
+    if (!UseWebBasedSigninFlow()) {
+      // If we're about to show a one-click infobar but the user has started
+      // a concurrent signin flow (perhaps via the promo), we may not have yet
+      // established an authenticated username but we still shouldn't move
+      // forward with two simultaneous signin processes.  This is a bit
+      // contentious as the one-click flow is a much smoother flow from the user
+      // perspective, but it's much more difficult to hijack the other flow from
+      // here as it is to bail.
+      ProfileSyncService* service =
+          ProfileSyncServiceFactory::GetForProfile(profile);
+      if (!service)
+        return false;
 
-    if (service->FirstSetupInProgress())
-      return false;
+      if (service->FirstSetupInProgress())
+        return false;
+    }
   }
 
   return true;
 }
 
 // static
 OneClickSigninHelper::Offer OneClickSigninHelper::CanOfferOnIOThread(
     net::URLRequest* request,
     ProfileIOData* io_data) {
   return CanOfferOnIOThreadImpl(request->url(), request->referrer(),
@@ skipping 251 matching lines @@
       infobar_tab_helper->AddInfoBar(
           new OneClickInfoBarDelegateImpl(infobar_tab_helper, session_index_,
                                           email_, password_));
       break;
     case AUTO_ACCEPT_CONFIGURE:
       StartSync(browser, auto_accept_, session_index_, email_, password_,
                 OneClickSigninSyncStarter::CONFIGURE_SYNC_FIRST);
       break;
     case AUTO_ACCEPT_EXPLICIT:
       StartSync(browser, auto_accept_, session_index_, email_, password_,
-                OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS);
+                source_ == SyncPromoUI::SOURCE_SETTINGS ?
+                    OneClickSigninSyncStarter::SIGNIN_ONLY :
+                    OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS);
       break;
     case REJECTED_FOR_PROFILE:
       UMA_HISTOGRAM_ENUMERATION("AutoLogin.Reverse",
                                 one_click_signin::HISTOGRAM_REJECTED,
                                 one_click_signin::HISTOGRAM_MAX);
       break;
     default:
       NOTREACHED() << "Invalid auto_accept=" << auto_accept_;
       break;
   }
 
-  AutoAccept local_auto_accept = auto_accept_;
-  SyncPromoUI::Source local_source = source_;
+  // If this explicit sign in is not from settings page, show the NTP after
+  // sign in completes.  In the case of the settings page, it will get closed
+  // by SyncSetupHandler.
+  if (auto_accept_ == AUTO_ACCEPT_EXPLICIT &&
+      source_ != SyncPromoUI::SOURCE_SETTINGS) {
+    Profile* profile =
+        Profile::FromBrowserContext(contents->GetBrowserContext());
+    signin_tracker_.reset(new SigninTracker(profile, this));
+  }
 
   email_.clear();
   password_.clear();
   auto_accept_ = NO_AUTO_ACCEPT;
   source_ = SyncPromoUI::SOURCE_UNKNOWN;
-
-  // If this is an explicit sign in by the user, then redirect them to the
-  // NTP.
-  if (local_auto_accept == AUTO_ACCEPT_EXPLICIT) {
-    // If this is an explicit sign in from settings page, then close the
-    // tab.  Otherwise show the NTP after sign in completes.
-    if (local_source == SyncPromoUI::SOURCE_SETTINGS) {
-      contents->Close();
-    } else {
-      Profile* profile =
-          Profile::FromBrowserContext(contents->GetBrowserContext());
-      signin_tracker_.reset(new SigninTracker(profile, this));
-    }
-  }
-
-  // |this| may have been deleted due to the contents->Close() call above.
-  // No members should be accessed at this point.
 }
 
 void OneClickSigninHelper::GaiaCredentialsValid() {
   // Redirect to NTP with sign in bubble visible.
   content::WebContents* contents = web_contents();
   Profile* profile =
       Profile::FromBrowserContext(contents->GetBrowserContext());
   PrefService* pref_service = profile->GetPrefs();
   pref_service->SetBoolean(prefs::kSyncPromoShowNTPBubble, true);
 
   contents->GetController().LoadURL(GURL(chrome::kChromeUINewTabURL),
                                     content::Referrer(),
                                     content::PAGE_TRANSITION_AUTO_TOPLEVEL,
                                     std::string());
 }
 
 void OneClickSigninHelper::SigninFailed(const GoogleServiceAuthError& error) {
   signin_tracker_.reset();
 }
 
 void OneClickSigninHelper::SigninSuccess() {
   signin_tracker_.reset();
 }