Setup from settings should allow configuration first

chrome/browser/ui/sync/one_click_signin_sync_starter.h

 // 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.
 
 #ifndef CHROME_BROWSER_UI_SYNC_ONE_CLICK_SIGNIN_SYNC_STARTER_H_
 #define CHROME_BROWSER_UI_SYNC_ONE_CLICK_SIGNIN_SYNC_STARTER_H_
 
 #include <string>
 
 #include "chrome/browser/signin/signin_tracker.h"
 
 class Browser;
 
 // Waits for successful singin notification from the signin manager and then
 // starts the sync machine.  Instances of this class delete themselves once
 // the job is done.
 class OneClickSigninSyncStarter : public SigninTracker::Observer {
  public:
-  enum StartSyncMode {SYNC_WITH_DEFAULT_SETTINGS, CONFIGURE_SYNC_FIRST };
+  enum StartSyncMode {
+    // Starts the process of signing the user in with the SigninManager, and
+    // once completed automatically starts sync with all data types enabled.
+    SYNC_WITH_DEFAULT_SETTINGS,
+
+    // Starts the process of signing the user in with the SigninManager, and
+    // once completed redirects the user to the settings page to allow them
+    // to configure which data types to sync before sync is enabled.
+    CONFIGURE_SYNC_FIRST,
+
+    // Starts the process of signing the user in with the SigninManager, and
+    // once completed don't do anything wrt sync.  This mode is used when using
+    // the web-based sign in flow directly from the settings page.  This mode

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

I guess SyncSetupHandler uses SigninTracker to determine when signin is done and
continues the login by itself?

What internal state gets messed up? It would obviously be nice to combine this
with CONFIGURE_SYNC_FIRST.

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

I tried it again and the state is OK. I suspect when I cleaned the handling of
the LoginUI calls, I also fixed this up.  Thanks for persisting on this :-)

+    // is distinct from the previous one because redirecting to the settings
+    // page from here messes up the internal state of SyncSetupHander.
+    SIGNIN_ONLY};

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

nit: close paren on its own line.

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

Done.

 
   OneClickSigninSyncStarter(Browser* browser,
                             const std::string& session_index,
                             const std::string& email,
                             const std::string& password,
                             StartSyncMode start_mode);
 
  private:
   virtual ~OneClickSigninSyncStarter();
 
   // SigninTracker::Observer override.
   virtual void GaiaCredentialsValid() OVERRIDE;
   virtual void SigninFailed(const GoogleServiceAuthError& error) OVERRIDE;
   virtual void SigninSuccess() OVERRIDE;
 
   Browser* browser_;
   SigninTracker signin_tracker_;
   StartSyncMode start_mode_;
 
   DISALLOW_COPY_AND_ASSIGN(OneClickSigninSyncStarter);
 };
 
 
 #endif  // CHROME_BROWSER_UI_SYNC_ONE_CLICK_SIGNIN_SYNC_STARTER_H_