Signing in to sync as a different user is redirecting to sync settings

chrome/browser/ui/sync/one_click_signin_helper.cc

Index: chrome/browser/ui/sync/one_click_signin_helper.cc
diff --git a/chrome/browser/ui/sync/one_click_signin_helper.cc b/chrome/browser/ui/sync/one_click_signin_helper.cc
index 90b0f3be5c68e5db1139c945060b91b06fd2de50..e83190a8a702f241a50d28aae77c027d1571ccea 100644
--- a/chrome/browser/ui/sync/one_click_signin_helper.cc
+++ b/chrome/browser/ui/sync/one_click_signin_helper.cc
@@ -136,14 +136,14 @@ void StartSync(const StartSyncArgs& args,
   int action = one_click_signin::HISTOGRAM_MAX;
   switch (args.auto_accept) {
     case OneClickSigninHelper::AUTO_ACCEPT_EXPLICIT:
-      action = one_click_signin::HISTOGRAM_AUTO_WITH_DEFAULTS;
-      break;
-    case OneClickSigninHelper::AUTO_ACCEPT_ACCEPTED:
       action =
           start_mode == OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS ?
               one_click_signin::HISTOGRAM_AUTO_WITH_DEFAULTS :
               one_click_signin::HISTOGRAM_AUTO_WITH_ADVANCED;
       break;
+    case OneClickSigninHelper::AUTO_ACCEPT_ACCEPTED:
+      action = one_click_signin::HISTOGRAM_AUTO_WITH_DEFAULTS;
+      break;

[Roger Tawa OOO till Jul 10th, 2013/02/13 19:39:29]

This change is not related to this bug, but I noticed that the actions here were
reversed.  This would cause the wrong  histogram action to be recorded at line
162 below.

     case OneClickSigninHelper::AUTO_ACCEPT_NONE:
       action =
           start_mode == OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS ?
@@ -993,16 +993,15 @@ void OneClickSigninHelper::DidStopLoading(
         return;
       }
 
-      // In explicit sign ins, the user may have checked the box
+      // In explicit sign ins, the user may have changed the box
       // "Let me choose what to sync".  This is reflected as a change in the
       // source of the continue URL.  Make one last check of the current URL
-      // to see if there is a valid source and its set to settings.  If so,
-      // it overrides the current source.
+      // to see if there is a valid source.  If so, it overrides the
+      // current source.
       SyncPromoUI::Source source =
           SyncPromoUI::GetSourceForSyncPromoURL(url);
-      if (source == SyncPromoUI::SOURCE_SETTINGS &&
-          source_ != SyncPromoUI::SOURCE_SETTINGS) {
-        source_ = SyncPromoUI::SOURCE_SETTINGS;
+      if (source != source_) {

[guohui, 2013/02/13 21:59:55]

Why do we care about source changes other than from non-settings to settings?

[Roger Tawa OOO till Jul 10th, 2013/02/14 15:53:36]

As mentioned in the description, it is now possible to go from settings to
non-settings, due to a change in the gaia sign in page.  When this code was
originally written, it was not possible for this to happen.

+        source_ = source;
         last_minute_source_change = true;
       }
     }
@@ -1053,8 +1052,8 @@ void OneClickSigninHelper::DidStopLoading(
       break;
     case AUTO_ACCEPT_EXPLICIT: {
       OneClickSigninSyncStarter::StartSyncMode start_mode =
-          source_ == SyncPromoUI::SOURCE_SETTINGS ?
-              OneClickSigninSyncStarter::CONFIGURE_SYNC_FIRST :
+          source_ == SyncPromoUI::SOURCE_SETTINGS ?
+              OneClickSigninSyncStarter::CONFIGURE_SYNC_FIRST :
               OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS;
 
       // If the new email address is different from the email address that