Delay loading the NTP after sign in until MergeSession has been performed in

the content area.

Also fixes a bug where the account reconcilor would run concurrent
MergeSessions on startup if there are multiple accounts connected to the
profile.  This led to a race condition that could mess up the gaia cookie.

Added more tests to  GoogleAutoLoginHelper.

BUG=315269

Review URL: https://codereview.chromium.org/110373007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@244044 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/android/signin/signin_manager_android.cc

@@ -18,7 +18,6 @@
 #include "chrome/browser/browsing_data/browsing_data_remover.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/signin/android_profile_oauth2_token_service.h"
-#include "chrome/browser/signin/google_auto_login_helper.h"
 #include "chrome/browser/signin/profile_oauth2_token_service.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_manager.h"
@@ -206,6 +205,12 @@ void SigninManagerAndroid::OnBrowsingDataRemoverDone() {
                                         java_signin_manager_.obj());
 }
 
+void SigninManagerAndroid::MergeSessionCompleted(
+    const std::string& account_id,
+    const GoogleServiceAuthError& error) {
+  merge_session_helper_.reset();
+}
+
 void SigninManagerAndroid::LogInSignedInUser(JNIEnv* env, jobject obj) {
   if (switches::IsNewProfileManagement()) {
     // New Mirror code path that just fires the events and let the
@@ -223,9 +228,8 @@ void SigninManagerAndroid::LogInSignedInUser(JNIEnv* env, jobject obj) {
     // Old code path that doesn't depend on the new Account Reconcilor.
     // We manually login.
 
-    // AutoLogin deletes itself.
-    GoogleAutoLoginHelper* autoLogin = new GoogleAutoLoginHelper(profile_);
-    autoLogin->LogIn();
+    merge_session_helper_.reset(new GoogleAutoLoginHelper(profile_, this));
+    merge_session_helper_->LogIn();
   }
 }
 

chrome/browser/android/signin/signin_manager_android.h

@@ -13,7 +13,9 @@
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
+#include "chrome/browser/signin/google_auto_login_helper.h"
 
+class GoogleServiceAuthError;
 class Profile;
 
 namespace policy {
@@ -28,7 +30,7 @@ class CloudPolicyClient;
 //
 // This class implements parts of the sign-in flow, to make sure that policy
 // is available before sign-in completes.
-class SigninManagerAndroid {
+class SigninManagerAndroid : public GoogleAutoLoginHelper::Observer {
  public:
   SigninManagerAndroid(JNIEnv* env, jobject obj);
 
@@ -61,6 +63,11 @@ class SigninManagerAndroid {
 
   void OnBrowsingDataRemoverDone();
 
+  // GoogleAutoLoginHelper::Observer implementation.
+  virtual void MergeSessionCompleted(
+      const std::string& account_id,
+      const GoogleServiceAuthError& error) OVERRIDE;
+
   Profile* profile_;
 
   // Java-side SigninManager object.
@@ -77,6 +84,9 @@ class SigninManagerAndroid {
   std::string username_;
 #endif
 
+  // Helper to merge the signed into account into the cookie jar session.
+  scoped_ptr<GoogleAutoLoginHelper> merge_session_helper_;
+
   base::WeakPtrFactory<SigninManagerAndroid> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(SigninManagerAndroid);

chrome/browser/extensions/api/webstore_private/webstore_private_api.cc

@@ -451,6 +451,13 @@ void WebstorePrivateBeginInstallWithManifest3Function::SigninSuccess() {
   SigninCompletedOrNotNeeded();
 }
 
+void WebstorePrivateBeginInstallWithManifest3Function::MergeSessionComplete(
+    const GoogleServiceAuthError& error) {
+  // TODO(rogerta): once the embeded inline flow is enabled, the code in
+  // WebstorePrivateBeginInstallWithManifest3Function::SigninSuccess()
+  // should move to here.
+}
+
 void WebstorePrivateBeginInstallWithManifest3Function::
     SigninCompletedOrNotNeeded() {
   content::WebContents* web_contents = GetAssociatedWebContents();

chrome/browser/extensions/api/webstore_private/webstore_private_api.h

@@ -144,6 +144,8 @@ class WebstorePrivateBeginInstallWithManifest3Function
   // SigninTracker::Observer override.
   virtual void SigninFailed(const GoogleServiceAuthError& error) OVERRIDE;
   virtual void SigninSuccess() OVERRIDE;
+  virtual void MergeSessionComplete(
+      const GoogleServiceAuthError& error) OVERRIDE;
 
   // Called when signin is complete or not needed.
   void SigninCompletedOrNotNeeded();

chrome/browser/signin/account_reconcilor.cc

@@ -10,7 +10,6 @@
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/net/chrome_cookie_notification_details.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/signin/google_auto_login_helper.h"
 #include "chrome/browser/signin/profile_oauth2_token_service.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_manager.h"
@@ -79,9 +78,9 @@ void AccountReconcilor::UserIdFetcher::OnNetworkError(int response_code) {
 AccountReconcilor::AccountReconcilor(Profile* profile)
     : OAuth2TokenService::Consumer("account_reconcilor"),
       profile_(profile),
+      merge_session_helper_(profile, NULL),
       registered_with_token_service_(false),
-      are_gaia_accounts_set_(false),
-      requests_(NULL) {
+      are_gaia_accounts_set_(false),requests_(NULL) {
   DVLOG(1) << "AccountReconcilor::AccountReconcilor";
   RegisterWithSigninManager();
   RegisterWithCookieMonster();
@@ -105,6 +104,7 @@ AccountReconcilor::~AccountReconcilor() {
 
 void AccountReconcilor::Shutdown() {
   DVLOG(1) << "AccountReconcilor::Shutdown";
+  merge_session_helper_.CancelAll();
   DeleteAccessTokenRequestsAndUserIdFetchers();
   UnregisterWithSigninManager();
   UnregisterWithTokenService();
@@ -112,6 +112,16 @@ void AccountReconcilor::Shutdown() {
   StopPeriodicReconciliation();
 }
 
+void AccountReconcilor::AddMergeSessionObserver(
+    GoogleAutoLoginHelper::Observer* observer) {
+  merge_session_helper_.AddObserver(observer);
+}
+
+void AccountReconcilor::RemoveMergeSessionObserver(
+    GoogleAutoLoginHelper::Observer* observer) {
+  merge_session_helper_.RemoveObserver(observer);
+}
+
 void AccountReconcilor::DeleteAccessTokenRequestsAndUserIdFetchers() {
   delete[] requests_;
   requests_ = NULL;
@@ -237,9 +247,7 @@ void AccountReconcilor::OnRefreshTokenRevoked(const std::string& account_id) {
 void AccountReconcilor::OnRefreshTokensLoaded() {}
 
 void AccountReconcilor::PerformMergeAction(const std::string& account_id) {
-  // GoogleAutoLoginHelper deletes itself upon success / failure.
-  GoogleAutoLoginHelper* helper = new GoogleAutoLoginHelper(profile_);
-  helper->LogIn(account_id);
+  merge_session_helper_.LogIn(account_id);
 }
 
 void AccountReconcilor::PerformRemoveAction(const std::string& account_id) {

chrome/browser/signin/account_reconcilor.h

@@ -8,6 +8,7 @@
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
+#include "chrome/browser/signin/google_auto_login_helper.h"
 #include "components/browser_context_keyed_service/browser_context_keyed_service.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
@@ -19,10 +20,10 @@ class Profile;
 struct ChromeCookieDetails;
 
 class AccountReconcilor : public BrowserContextKeyedService,
-                                 content::NotificationObserver,
-                                 GaiaAuthConsumer,
-                                 OAuth2TokenService::Consumer,
-                                 OAuth2TokenService::Observer {
+                          public content::NotificationObserver,
+                          public GaiaAuthConsumer,
+                          public OAuth2TokenService::Consumer,
+                          public OAuth2TokenService::Observer {
  public:
   explicit AccountReconcilor(Profile* profile);
   virtual ~AccountReconcilor();
@@ -30,6 +31,10 @@ class AccountReconcilor : public BrowserContextKeyedService,
   // BrowserContextKeyedService implementation.
   virtual void Shutdown() OVERRIDE;
 
+  // Add or remove observers for the merge session notification.
+  void AddMergeSessionObserver(GoogleAutoLoginHelper::Observer* observer);
+  void RemoveMergeSessionObserver(GoogleAutoLoginHelper::Observer* observer);
+
   Profile* profile() { return profile_; }
 
   bool IsPeriodicReconciliationRunning() const {
@@ -126,6 +131,7 @@ class AccountReconcilor : public BrowserContextKeyedService,
   Profile* profile_;
   content::NotificationRegistrar registrar_;
   base::RepeatingTimer<AccountReconcilor> reconciliation_timer_;
+  GoogleAutoLoginHelper merge_session_helper_;
   bool registered_with_token_service_;
 
   // Used during reconcile action.

chrome/browser/signin/account_reconcilor_unittest.cc

@@ -274,4 +274,3 @@ TEST_F(AccountReconcilorTest, StartReconcileAction) {
   base::RunLoop().RunUntilIdle();
   ASSERT_TRUE(reconcilor->AreAllRefreshTokensChecked());
 }
-

chrome/browser/signin/google_auto_login_helper.cc

@@ -5,86 +5,108 @@
 #include "chrome/browser/signin/google_auto_login_helper.h"
 
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/signin/profile_oauth2_token_service.h"
+#include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
+#include "content/public/browser/notification_service.h"
 #include "google_apis/gaia/gaia_auth_fetcher.h"
 #include "google_apis/gaia/gaia_constants.h"
 
-GoogleAutoLoginHelper::GoogleAutoLoginHelper(Profile* profile)
-    : profile_(profile) {}
+GoogleAutoLoginHelper::GoogleAutoLoginHelper(Profile* profile,
+                                             Observer* observer)
+    : profile_(profile) {
+  if (observer)
+    AddObserver(observer);
+}
 
 GoogleAutoLoginHelper::~GoogleAutoLoginHelper() {
   DCHECK(accounts_.empty());
 }
 
 void GoogleAutoLoginHelper::LogIn() {
-  // This is the code path for non-mirror world.
-  uber_token_fetcher_.reset(CreateNewUbertokenFetcher());
-  uber_token_fetcher_->StartFetchingToken();
-  DCHECK(accounts_.empty());
+  ProfileOAuth2TokenService* token_service =
+      ProfileOAuth2TokenServiceFactory::GetForProfile(profile_);
+  LogIn(token_service->GetPrimaryAccountId());
 }
 
 void GoogleAutoLoginHelper::LogIn(const std::string& account_id) {
-  if (uber_token_fetcher_.get()) {
-    accounts_.push_back(account_id);
-  } else {
-    uber_token_fetcher_.reset(CreateNewUbertokenFetcher());
-    uber_token_fetcher_->StartFetchingToken(account_id);
-  }
+  accounts_.push_back(account_id);
+  if (accounts_.size() == 1)
+    StartFetching();
+}
+
+void GoogleAutoLoginHelper::AddObserver(Observer* observer) {
+  observer_list_.AddObserver(observer);
+}
+
+void GoogleAutoLoginHelper::RemoveObserver(Observer* observer) {
+  observer_list_.RemoveObserver(observer);
+}
+
+void GoogleAutoLoginHelper::CancelAll() {
+  gaia_auth_fetcher_.reset();
+  uber_token_fetcher_.reset();
+  accounts_.clear();
 }
 
 void GoogleAutoLoginHelper::OnUbertokenSuccess(const std::string& uber_token) {
-  VLOG(1) << "GoogleAutoLoginHelper::OnUbertokenSuccess";
-  gaia_auth_fetcher_.reset(CreateNewGaiaAuthFetcher());
+  VLOG(1) << "GoogleAutoLoginHelper::OnUbertokenSuccess"
+          << " account=" << accounts_.front();
+  gaia_auth_fetcher_.reset(
+      new GaiaAuthFetcher(this, GaiaConstants::kChromeSource,
+                          profile_->GetRequestContext()));
   gaia_auth_fetcher_->StartMergeSession(uber_token);
 }
 
 void GoogleAutoLoginHelper::OnUbertokenFailure(
     const GoogleServiceAuthError& error) {
-  VLOG(1) << "Failed to retrieve ubertoken, error: " << error.ToString();
-  if (base::MessageLoop::current()) {
-    base::MessageLoop::current()->DeleteSoon(FROM_HERE, this);
-  } else {
-    delete this;
-  }
+  VLOG(1) << "Failed to retrieve ubertoken"
+          << " account=" << accounts_.front()
+          << " error=" << error.ToString();
+  const std::string account_id = accounts_.front();
+  MergeNextAccount();
+  SignalComplete(account_id, error);
 }
 
 void GoogleAutoLoginHelper::OnMergeSessionSuccess(const std::string& data) {
-  DVLOG(1) << "MergeSession successful." << data;
-  if (accounts_.empty()) {
-    if (base::MessageLoop::current()) {
-      base::MessageLoop::current()->DeleteSoon(FROM_HERE, this);
-    } else {
-      delete this;
-    }
-  } else {
-    uber_token_fetcher_.reset(CreateNewUbertokenFetcher());
-    uber_token_fetcher_->StartFetchingToken(accounts_.front());
-    accounts_.pop_front();
-  }
+  DVLOG(1) << "MergeSession successful account=" << accounts_.front();
+  const std::string account_id = accounts_.front();
+  MergeNextAccount();
+  SignalComplete(account_id, GoogleServiceAuthError::AuthErrorNone());
 }
 
 void GoogleAutoLoginHelper::OnMergeSessionFailure(
     const GoogleServiceAuthError& error) {
-  VLOG(1) << "Failed MergeSession request, error: " << error.ToString();
-  // TODO(acleung): Depending on the return error we should probably
-  // take different actions instead of just throw our hands up.
-
-  // Clearning pending accounts for now.
-  accounts_.clear();
-
-  if (base::MessageLoop::current()) {
-    base::MessageLoop::current()->DeleteSoon(FROM_HERE, this);
-  } else {
-    delete this;
-  }
+  VLOG(1) << "Failed MergeSession"
+          << " account=" << accounts_.front()
+          << " error=" << error.ToString();
+  const std::string account_id = accounts_.front();
+  MergeNextAccount();
+  SignalComplete(account_id, error);
 }
 
-UbertokenFetcher* GoogleAutoLoginHelper::CreateNewUbertokenFetcher() {
-  return new UbertokenFetcher(profile_, this);
+void GoogleAutoLoginHelper::StartFetching() {
+  uber_token_fetcher_.reset(new UbertokenFetcher(profile_, this));
+  uber_token_fetcher_->StartFetchingToken(accounts_.front());
 }
 
-GaiaAuthFetcher* GoogleAutoLoginHelper::CreateNewGaiaAuthFetcher() {
- return new GaiaAuthFetcher(
-      this, GaiaConstants::kChromeSource, profile_->GetRequestContext());
+void GoogleAutoLoginHelper::SignalComplete(
+    const std::string& account_id,
+    const GoogleServiceAuthError& error) {
+  // Its possible for the observer to delete |this| object.  Don't access
+  // access any members after this calling the observer.  This method should
+  // be the last call in any other method.
+  FOR_EACH_OBSERVER(Observer, observer_list_,
+                    MergeSessionCompleted(account_id, error));
 }
 
+void GoogleAutoLoginHelper::MergeNextAccount() {
+  gaia_auth_fetcher_.reset();
+  accounts_.pop_front();
+  if (accounts_.empty()) {
+    uber_token_fetcher_.reset();
+  } else {
+    StartFetching();
+  }
+}

chrome/browser/signin/google_auto_login_helper.h

@@ -2,48 +2,84 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_ANDROID_SIGNIN_GOOGLE_AUTO_LOGIN_HELPER_H_
-#define CHROME_BROWSER_ANDROID_SIGNIN_GOOGLE_AUTO_LOGIN_HELPER_H_
+#ifndef CHROME_BROWSER_SIGNIN_GOOGLE_AUTO_LOGIN_HELPER_H_
+#define CHROME_BROWSER_SIGNIN_GOOGLE_AUTO_LOGIN_HELPER_H_
 
 #include <deque>
 
+#include "base/observer_list.h"
 #include "chrome/browser/signin/ubertoken_fetcher.h"
 #include "google_apis/gaia/gaia_auth_consumer.h"
 
 class GaiaAuthFetcher;
+class GoogleServiceAuthError;
 class Profile;
 
-// Logs in the current signed in user into Google services. Populates the cookie
-// jar with Google credentials of the signed in user.
+// Merges a Google account known to Chrome into the cookie jar.  Once the
+// account is merged, successfully or not, a NOTIFICATION_MERGE_SESSION_COMPLETE
+// notification is sent out.  When merging multiple accounts, one instance of
+// the helper is better than multiple instances if there is the possibility
+// that they run concurrently, since changes to the cookie must be serialized.
+//
+// By default instances of GoogleAutoLoginHelper delete themselves when done.
 class GoogleAutoLoginHelper : public GaiaAuthConsumer,
                               public UbertokenConsumer {
  public:
-  explicit GoogleAutoLoginHelper(Profile* profile);
+  class Observer {
+   public:
+    // Called whenever a merge session is completed.  The account that was
+    // merged is given by |account_id|.  If |error| is equal to
+    // GoogleServiceAuthError::AuthErrorNone() then the merge succeeeded.
+    virtual void MergeSessionCompleted(const std::string& account_id,
+                                       const GoogleServiceAuthError& error) = 0;
+   protected:
+    virtual ~Observer() {}
+  };
+
+  GoogleAutoLoginHelper(Profile* profile, Observer* observer);
   virtual ~GoogleAutoLoginHelper();
 
   void LogIn();
   void LogIn(const std::string& account_id);
 
+  // Add or remove observers of this helper.
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
+  // Cancel all login requests.
+  void CancelAll();
+
+ private:
+  // Overridden from UbertokenConsumer.
+  virtual void OnUbertokenSuccess(const std::string& token) OVERRIDE;
+  virtual void OnUbertokenFailure(const GoogleServiceAuthError& error) OVERRIDE;
+
   // Overridden from GaiaAuthConsumer.
   virtual void OnMergeSessionSuccess(const std::string& data) OVERRIDE;
   virtual void OnMergeSessionFailure(const GoogleServiceAuthError& error)
       OVERRIDE;
 
-  // Overridden from UbertokenConsumer.
-  virtual void OnUbertokenSuccess(const std::string& token) OVERRIDE;
-  virtual void OnUbertokenFailure(const GoogleServiceAuthError& error) OVERRIDE;
+  // Starts the proess of fetching the uber token and performing a merge session
+  // for the next account.  Virtual so that it can be overriden in tests.
+  virtual void StartFetching();
 
-  // For testing.
-  virtual UbertokenFetcher* CreateNewUbertokenFetcher();
-  virtual GaiaAuthFetcher* CreateNewGaiaAuthFetcher();
+  // Call observer when merge session completes.
+  void SignalComplete(const std::string& account_id,
+                      const GoogleServiceAuthError& error);
+
+  // Start the next merge session, if needed.
+  void MergeNextAccount();
 
- private:
   Profile* profile_;
   scoped_ptr<GaiaAuthFetcher> gaia_auth_fetcher_;
   scoped_ptr<UbertokenFetcher> uber_token_fetcher_;
   std::deque<std::string> accounts_;
 
+  // List of observers to notify when merge session completes.
+  // Makes sure list is empty on destruction.
+  ObserverList<Observer, true> observer_list_;
+
   DISALLOW_COPY_AND_ASSIGN(GoogleAutoLoginHelper);
 };
 
-#endif  // CHROME_BROWSER_ANDROID_SIGNIN_GOOGLE_AUTO_LOGIN_HELPER_H_
+#endif  // CHROME_BROWSER_SIGNIN_GOOGLE_AUTO_LOGIN_HELPER_H_

chrome/browser/signin/google_auto_login_helper_unittest.cc

@@ -7,27 +7,32 @@
 #include <vector>
 
 #include "base/message_loop/message_loop.h"
+#include "base/strings/stringprintf.h"
 #include "chrome/browser/signin/google_auto_login_helper.h"
 #include "chrome/test/base/testing_profile.h"
+#include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 
-class MockUbertokenFetcher : public UbertokenFetcher {
+class MockObserver : public GoogleAutoLoginHelper::Observer {
  public:
-  MockUbertokenFetcher(Profile* profile, UbertokenConsumer* consumer) :
-      UbertokenFetcher(profile, consumer), account_id("") {}
-
-  void completePendingRequest() {
-    OnUberAuthTokenSuccess("mock token: " + account_id);
+  explicit MockObserver(GoogleAutoLoginHelper* helper) : helper_(helper) {
+    helper_->AddObserver(this);
   }
 
-  virtual void StartFetchingToken(const std::string& id) OVERRIDE {
-    account_id = id;
+  ~MockObserver() {
+    helper_->RemoveObserver(this);
   }
 
-  std::string account_id;
+  MOCK_METHOD2(MergeSessionCompleted,
+               void(const std::string&,
+                    const GoogleServiceAuthError& ));
+ private:
+  GoogleAutoLoginHelper* helper_;
+
+  DISALLOW_COPY_AND_ASSIGN(MockObserver);
 };
 
 // Counts number of InstrumentedGoogleAutoLoginHelper created.
@@ -38,12 +43,8 @@ int total = 0;
 
 class InstrumentedGoogleAutoLoginHelper : public GoogleAutoLoginHelper {
  public:
-  std::vector<MockUbertokenFetcher*>* mock_uber_fetchers_;
-  TestingProfile profile_;
-
-  InstrumentedGoogleAutoLoginHelper(
-    std::vector<MockUbertokenFetcher*>* fetchers) :
-    GoogleAutoLoginHelper(0), mock_uber_fetchers_(fetchers) {
+  explicit InstrumentedGoogleAutoLoginHelper(Profile* profile) :
+    GoogleAutoLoginHelper(profile, NULL) {
     total++;
   }
 
@@ -51,66 +52,142 @@ class InstrumentedGoogleAutoLoginHelper : public GoogleAutoLoginHelper {
     total--;
   }
 
-  virtual void OnUbertokenSuccess(const std::string& token) OVERRIDE {
-    OnMergeSessionSuccess(token);
-  }
+  MOCK_METHOD0(StartFetching, void());
 
-  virtual UbertokenFetcher* CreateNewUbertokenFetcher() OVERRIDE {
-    MockUbertokenFetcher* m = new MockUbertokenFetcher(&profile_, this);
-    mock_uber_fetchers_->push_back(m);
-    return m;
-  }
+ private:
+  DISALLOW_COPY_AND_ASSIGN(InstrumentedGoogleAutoLoginHelper);
+};
+
+class GoogleAutoLoginHelperTest : public testing::Test {
+ public:
+   GoogleAutoLoginHelperTest()
+       : no_error_(GoogleServiceAuthError::NONE),
+         error_(GoogleServiceAuthError::SERVICE_ERROR) {}
+
+  TestingProfile* profile() { return &profile_; }
 
-  void completePendingFetchers() {
-    while (!mock_uber_fetchers_->empty()) {
-      MockUbertokenFetcher* fetcher = mock_uber_fetchers_->back();
-      mock_uber_fetchers_->pop_back();
-      fetcher->completePendingRequest();
-    }
+  void SimulateUbertokenFailure(UbertokenConsumer* consumer,
+                                const GoogleServiceAuthError& error) {
+    consumer->OnUbertokenFailure(error);
   }
 
-  MOCK_METHOD1(OnMergeSessionSuccess, void(const std::string& uber_token));
+  void SimulateMergeSessionSuccess(GaiaAuthConsumer* consumer,
+                                   const std::string& data) {
+    consumer->OnMergeSessionSuccess(data);
+  }
 
-  void OnMergeSessionSuccessConcrete(const std::string& uber_token) {
-    GoogleAutoLoginHelper::OnMergeSessionSuccess(uber_token);
+  void SimulateMergeSessionFailure(GaiaAuthConsumer* consumer,
+                                   const GoogleServiceAuthError& error) {
+    consumer->OnMergeSessionFailure(error);
   }
-};
 
-class GoogleAutoLoginHelperTest : public testing::Test {
- public:
-  std::vector<MockUbertokenFetcher*> mock_uber_fetchers_;
-  base::MessageLoop message_loop_;
+  const GoogleServiceAuthError& no_error() { return no_error_; }
+  const GoogleServiceAuthError& error() { return error_; }
+
+ private:
+  content::TestBrowserThreadBundle thread_bundle_;
+  TestingProfile profile_;
+  GoogleServiceAuthError no_error_;
+  GoogleServiceAuthError error_;
 };
 
 } // namespace
 
-using ::testing::Invoke;
 using ::testing::_;
 
-TEST_F(GoogleAutoLoginHelperTest, AllRequestsInOneGo) {
-  total = 0;
-
-  InstrumentedGoogleAutoLoginHelper* helper =
-      new InstrumentedGoogleAutoLoginHelper(&mock_uber_fetchers_);
-  EXPECT_CALL(*helper, OnMergeSessionSuccess("mock token: acc1@gmail.com"));
-  EXPECT_CALL(*helper, OnMergeSessionSuccess("mock token: acc2@gmail.com"));
-  EXPECT_CALL(*helper, OnMergeSessionSuccess("mock token: acc3@gmail.com"));
-  EXPECT_CALL(*helper, OnMergeSessionSuccess("mock token: acc4@gmail.com"));
-  EXPECT_CALL(*helper, OnMergeSessionSuccess("mock token: acc5@gmail.com"));
-
-  // Don't completely mock out OnMergeSessionSuccess, let GoogleAutoLoginHelper
-  // actually call OnMergeSessionSuccess to clean up it's work queue because
-  // it'll delete itself once the work queue becomes empty.
-  ON_CALL(*helper, OnMergeSessionSuccess(_)).WillByDefault(Invoke(helper,
-          &InstrumentedGoogleAutoLoginHelper::OnMergeSessionSuccessConcrete));
-  helper->LogIn("acc1@gmail.com");
-  helper->LogIn("acc2@gmail.com");
-  helper->LogIn("acc3@gmail.com");
-  helper->LogIn("acc4@gmail.com");
-  helper->LogIn("acc5@gmail.com");
-  helper->completePendingFetchers();
-
-  // Make sure GoogleAutoLoginHelper cleans itself up after everything is done.
-  message_loop_.RunUntilIdle();
-  EXPECT_EQ(0, total);
+TEST_F(GoogleAutoLoginHelperTest, Success) {
+  InstrumentedGoogleAutoLoginHelper helper(profile());
+  MockObserver observer(&helper);
+
+  EXPECT_CALL(helper, StartFetching());
+  EXPECT_CALL(observer, MergeSessionCompleted("acc1@gmail.com", no_error()));
+
+  helper.LogIn("acc1@gmail.com");
+  SimulateMergeSessionSuccess(&helper, "token");
+}
+
+TEST_F(GoogleAutoLoginHelperTest, FailedMergeSession) {
+  InstrumentedGoogleAutoLoginHelper helper(profile());
+  MockObserver observer(&helper);
+
+  EXPECT_CALL(helper, StartFetching());
+  EXPECT_CALL(observer, MergeSessionCompleted("acc1@gmail.com", error()));
+
+  helper.LogIn("acc1@gmail.com");
+  SimulateMergeSessionFailure(&helper, error());
+}
+
+TEST_F(GoogleAutoLoginHelperTest, FailedUbertoken) {
+  InstrumentedGoogleAutoLoginHelper helper(profile());
+  MockObserver observer(&helper);
+
+  EXPECT_CALL(helper, StartFetching());
+  EXPECT_CALL(observer, MergeSessionCompleted("acc1@gmail.com", error()));
+
+  helper.LogIn("acc1@gmail.com");
+  SimulateUbertokenFailure(&helper, error());
+}
+
+TEST_F(GoogleAutoLoginHelperTest, ContinueAfterSuccess) {
+  InstrumentedGoogleAutoLoginHelper helper(profile());
+  MockObserver observer(&helper);
+
+  EXPECT_CALL(helper, StartFetching()).Times(2);
+  EXPECT_CALL(observer, MergeSessionCompleted("acc1@gmail.com", no_error()));
+  EXPECT_CALL(observer, MergeSessionCompleted("acc2@gmail.com", no_error()));
+
+  helper.LogIn("acc1@gmail.com");
+  helper.LogIn("acc2@gmail.com");
+  SimulateMergeSessionSuccess(&helper, "token1");
+  SimulateMergeSessionSuccess(&helper, "token2");
+}
+
+TEST_F(GoogleAutoLoginHelperTest, ContinueAfterFailure1) {
+  InstrumentedGoogleAutoLoginHelper helper(profile());
+  MockObserver observer(&helper);
+
+  EXPECT_CALL(helper, StartFetching()).Times(2);
+  EXPECT_CALL(observer, MergeSessionCompleted("acc1@gmail.com", error()));
+  EXPECT_CALL(observer, MergeSessionCompleted("acc2@gmail.com", no_error()));
+
+  helper.LogIn("acc1@gmail.com");
+  helper.LogIn("acc2@gmail.com");
+  SimulateMergeSessionFailure(&helper, error());
+  SimulateMergeSessionSuccess(&helper, "token2");
+}
+
+TEST_F(GoogleAutoLoginHelperTest, ContinueAfterFailure2) {
+  InstrumentedGoogleAutoLoginHelper helper(profile());
+  MockObserver observer(&helper);
+
+  EXPECT_CALL(helper, StartFetching()).Times(2);
+  EXPECT_CALL(observer, MergeSessionCompleted("acc1@gmail.com", error()));
+  EXPECT_CALL(observer, MergeSessionCompleted("acc2@gmail.com", no_error()));
+
+  helper.LogIn("acc1@gmail.com");
+  helper.LogIn("acc2@gmail.com");
+  SimulateUbertokenFailure(&helper, error());
+  SimulateMergeSessionSuccess(&helper, "token2");
+}
+
+TEST_F(GoogleAutoLoginHelperTest, AllRequestsInMultipleGoes) {
+  InstrumentedGoogleAutoLoginHelper helper(profile());
+  MockObserver observer(&helper);
+
+  EXPECT_CALL(helper, StartFetching()).Times(4);
+  EXPECT_CALL(observer, MergeSessionCompleted(_, no_error())).Times(4);
+
+  helper.LogIn("acc1@gmail.com");
+  helper.LogIn("acc2@gmail.com");
+
+  SimulateMergeSessionSuccess(&helper, "token1");
+
+  helper.LogIn("acc3@gmail.com");
+
+  SimulateMergeSessionSuccess(&helper, "token2");
+  SimulateMergeSessionSuccess(&helper, "token3");
+
+  helper.LogIn("acc4@gmail.com");
+
+  SimulateMergeSessionSuccess(&helper, "token4");
 }

chrome/browser/signin/signin_manager.cc

@@ -117,6 +117,18 @@ bool SigninManager::HasSigninProcess() const {
   return signin_host_id_ != ChildProcessHost::kInvalidUniqueID;
 }
 
+void SigninManager::AddMergeSessionObserver(
+    GoogleAutoLoginHelper::Observer* observer) {
+  if (merge_session_helper_)
+    merge_session_helper_->AddObserver(observer);
+}
+
+void SigninManager::RemoveMergeSessionObserver(
+    GoogleAutoLoginHelper::Observer* observer) {
+  if (merge_session_helper_)
+    merge_session_helper_->RemoveObserver(observer);
+}
+
 SigninManager::~SigninManager() {
 }
 
@@ -369,6 +381,9 @@ void SigninManager::Initialize(Profile* profile, PrefService* local_state) {
 }
 
 void SigninManager::Shutdown() {
+  if (merge_session_helper_)
+    merge_session_helper_->CancelAll();
+
   local_state_pref_registrar_.RemoveAll();
   account_id_helper_.reset();
   SigninManagerBase::Shutdown();
@@ -557,6 +572,15 @@ void SigninManager::CompletePendingSignin() {
   DCHECK(!possibly_invalid_username_.empty());
   OnSignedIn(possibly_invalid_username_);
 
+  // If inline sign in is enabled, but new profile management is not, perform a
+  // merge session now to push the user's credentials into the cookie jar.
+  bool do_merge_session_in_signin_manager =
+      !switches::IsEnableWebBasedSignin() &&
+      !switches::IsNewProfileManagement();
+
+  if (do_merge_session_in_signin_manager)
+    merge_session_helper_.reset(new GoogleAutoLoginHelper(profile_, NULL));
+
   DCHECK(!temp_oauth_login_tokens_.refresh_token.empty());
   DCHECK(!GetAuthenticatedUsername().empty());
   ProfileOAuth2TokenService* token_service =
@@ -564,6 +588,9 @@ void SigninManager::CompletePendingSignin() {
   token_service->UpdateCredentials(GetAuthenticatedUsername(),
                                    temp_oauth_login_tokens_.refresh_token);
   temp_oauth_login_tokens_ = ClientOAuthResult();
+
+  if (do_merge_session_in_signin_manager)
+    merge_session_helper_->LogIn();
 }
 
 void SigninManager::OnExternalSigninCompleted(const std::string& username) {

chrome/browser/signin/signin_manager.h

@@ -31,6 +31,7 @@
 #include "base/prefs/pref_change_registrar.h"
 #include "base/prefs/pref_member.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/signin/google_auto_login_helper.h"
 #include "chrome/browser/signin/signin_internals_util.h"
 #include "chrome/browser/signin/signin_manager_base.h"
 #include "components/browser_context_keyed_service/browser_context_keyed_service.h"
@@ -185,6 +186,10 @@ class SigninManager : public SigninManagerBase,
   bool IsSigninProcess(int host_id) const;
   bool HasSigninProcess() const;
 
+  // Add or remove observers for the merge session notification.
+  void AddMergeSessionObserver(GoogleAutoLoginHelper::Observer* observer);
+  void RemoveMergeSessionObserver(GoogleAutoLoginHelper::Observer* observer);
+
  protected:
   // Flag saying whether signing out is allowed.
   bool prohibit_signout_;
@@ -291,6 +296,9 @@ class SigninManager : public SigninManagerBase,
   // Helper object to listen for changes to the signin allowed preference.
   BooleanPrefMember signin_allowed_;
 
+  // Helper to merge signed in account into the content area.
+  scoped_ptr<GoogleAutoLoginHelper> merge_session_helper_;
+
   DISALLOW_COPY_AND_ASSIGN(SigninManager);
 };
 

chrome/browser/signin/signin_tracker.cc

@@ -6,14 +6,19 @@
 
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/signin/account_reconcilor_factory.h"
 #include "chrome/browser/signin/profile_oauth2_token_service.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
-#include "chrome/browser/signin/signin_manager.h"
-#include "chrome/browser/signin/signin_manager_factory.h"
+#include "chrome/common/profile_management_switches.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
 #include "google_apis/gaia/gaia_constants.h"
 
+#if !defined(OS_CHROMEOS)
+#include "chrome/browser/signin/signin_manager.h"
+#include "chrome/browser/signin/signin_manager_factory.h"
+#endif
+
 SigninTracker::SigninTracker(Profile* profile, Observer* observer)
     : profile_(profile), observer_(observer) {
   DCHECK(profile_);
@@ -23,6 +28,18 @@ SigninTracker::SigninTracker(Profile* profile, Observer* observer)
 SigninTracker::~SigninTracker() {
   ProfileOAuth2TokenServiceFactory::GetForProfile(profile_)->
       RemoveObserver(this);
+
+  if (!switches::IsEnableWebBasedSignin()) {
+    if (switches::IsNewProfileManagement()) {
+        AccountReconcilorFactory::GetForProfile(profile_)->
+            RemoveMergeSessionObserver(this);
+#if !defined(OS_CHROMEOS)
+    } else {
+        SigninManagerFactory::GetForProfile(profile_)->
+            RemoveMergeSessionObserver(this);
+#endif
+    }
+  }
 }
 
 void SigninTracker::Initialize() {
@@ -52,9 +69,27 @@ void SigninTracker::Observe(int type,
 void SigninTracker::OnRefreshTokenAvailable(const std::string& account_id) {
   // TODO: when OAuth2TokenService handles multi-login, this should check
   // that |account_id| is the primary account before signalling success.
+  if (!switches::IsEnableWebBasedSignin()) {
+    if (switches::IsNewProfileManagement()) {
+      AccountReconcilorFactory::GetForProfile(profile_)->
+          AddMergeSessionObserver(this);
+#if !defined(OS_CHROMEOS)
+    } else {
+      SigninManagerFactory::GetForProfile(profile_)->
+          AddMergeSessionObserver(this);
+#endif
+    }
+  }
+
   observer_->SigninSuccess();
 }
 
 void SigninTracker::OnRefreshTokenRevoked(const std::string& account_id) {
   NOTREACHED();
 }
+
+void SigninTracker::MergeSessionCompleted(
+    const std::string& account_id,
+    const GoogleServiceAuthError& error) {
+  observer_->MergeSessionComplete(error);
+}

chrome/browser/signin/signin_tracker.h

@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_SIGNIN_SIGNIN_TRACKER_H_
 #define CHROME_BROWSER_SIGNIN_SIGNIN_TRACKER_H_
 
+#include "base/memory/scoped_ptr.h"
+#include "chrome/browser/signin/google_auto_login_helper.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_types.h"
@@ -47,7 +49,8 @@ class Profile;
 // sync, and provides an Observer interface to notify the UI layer of changes
 // in sync state so they can be reflected in the UI.
 class SigninTracker : public content::NotificationObserver,
-                      public OAuth2TokenService::Observer {
+                      public OAuth2TokenService::Observer,
+                      public GoogleAutoLoginHelper::Observer {
  public:
   class Observer {
    public:
@@ -56,6 +59,10 @@ class SigninTracker : public content::NotificationObserver,
 
     // The signin attempt succeeded.
     virtual void SigninSuccess() = 0;
+
+    // The signed in account has been merged into the content area cookie jar.
+    // This will be called only after a call to SigninSuccess().
+    virtual void MergeSessionComplete(const GoogleServiceAuthError& error) = 0;
   };
 
   // Creates a SigninTracker that tracks the signin status on the passed
@@ -77,6 +84,11 @@ class SigninTracker : public content::NotificationObserver,
   // Initializes this by adding notifications and observers.
   void Initialize();
 
+  // GoogleAutoLoginHelper::Observer implementation.
+  virtual void MergeSessionCompleted(
+      const std::string& account_id,
+      const GoogleServiceAuthError& error) OVERRIDE;
+
   // The profile whose signin status we are tracking.
   Profile* profile_;
 

chrome/browser/signin/signin_tracker_unittest.cc

@@ -42,12 +42,19 @@ class MockObserver : public SigninTracker::Observer {
 
   MOCK_METHOD1(SigninFailed, void(const GoogleServiceAuthError&));
   MOCK_METHOD0(SigninSuccess, void(void));
+  MOCK_METHOD1(MergeSessionComplete, void(const GoogleServiceAuthError&));
 };
 
 }  // namespace
 
 class SigninTrackerTest : public testing::Test {
  public:
+#if defined(OS_CHROMEOS)
+   typedef FakeSigninManagerBase FakeSigninManagerForTesting;
+#else
+   typedef FakeSigninManager FakeSigninManagerForTesting;
+#endif
+
   SigninTrackerTest() {}
   virtual void SetUp() OVERRIDE {
     TestingProfile::Builder builder;
@@ -60,9 +67,9 @@ class SigninTrackerTest : public testing::Test {
         static_cast<FakeProfileOAuth2TokenService*>(
             ProfileOAuth2TokenServiceFactory::GetForProfile(profile_.get()));
 
-    mock_signin_manager_ = static_cast<FakeSigninManagerBase*>(
+    mock_signin_manager_ = static_cast<FakeSigninManagerForTesting*>(
         SigninManagerFactory::GetInstance()->SetTestingFactoryAndUse(
-            profile_.get(), FakeSigninManagerBase::Build));
+            profile_.get(), FakeSigninManagerForTesting::Build));
     mock_signin_manager_->Initialize(profile_.get(), NULL);
 
     tracker_.reset(new SigninTracker(profile_.get(), &observer_));
@@ -75,7 +82,7 @@ class SigninTrackerTest : public testing::Test {
   content::TestBrowserThreadBundle thread_bundle_;
   scoped_ptr<SigninTracker> tracker_;
   scoped_ptr<TestingProfile> profile_;
-  FakeSigninManagerBase* mock_signin_manager_;
+  FakeSigninManagerForTesting* mock_signin_manager_;
   FakeProfileOAuth2TokenService* fake_oauth2_token_service_;
   MockObserver observer_;
 };

chrome/browser/ui/sync/one_click_signin_sync_starter.cc

@@ -35,6 +35,7 @@
 #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/signin/profile_signin_confirmation_dialog.h"
+#include "chrome/common/profile_management_switches.h"
 #include "chrome/common/url_constants.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
@@ -366,6 +367,15 @@ void OneClickSigninSyncStarter::SigninFailed(
 }
 
 void OneClickSigninSyncStarter::SigninSuccess() {
+  if (switches::IsEnableWebBasedSignin())
+    MergeSessionComplete(GoogleServiceAuthError(GoogleServiceAuthError::NONE));
+}
+
+void OneClickSigninSyncStarter::MergeSessionComplete(
+    const GoogleServiceAuthError& error) {
+  // Regardless of whether the merge session completed sucessfully or not,
+  // continue with sync starting.
+
   if (!sync_setup_completed_callback_.is_null())
     sync_setup_completed_callback_.Run(SYNC_SETUP_SUCCESS);
 

chrome/browser/ui/sync/one_click_signin_sync_starter.h

@@ -112,6 +112,8 @@ class OneClickSigninSyncStarter : public SigninTracker::Observer,
   // SigninTracker::Observer override.
   virtual void SigninFailed(const GoogleServiceAuthError& error) OVERRIDE;
   virtual void SigninSuccess() OVERRIDE;
+  virtual void MergeSessionComplete(
+      const GoogleServiceAuthError& error) OVERRIDE;
 
 #if defined(ENABLE_CONFIGURATION_POLICY)
   // User input handler for the signin confirmation dialog.