Move cookie logic out of reconcilor/signin manager and into a new PKS.

This is necessary for the Cross-Device promo which needs to work with
the GAIA cookies. Note: The Reconcilor will still call ExternalCCConnections on every reconcile; teasing apart the reconcilor's flow will wait until /ListAccounts is refactored and the GCMS has indepdent calling of ExternalCCConnections.

Still to do, in future CLs:
- Make the GaiaCookieManagerService independently check for freshness of ExternalCCConnections before executing any MergeSession (every 3 hours?)
- Move all /ListAccounts calls into the GaiaCookieManagerService, and have it publish to all observers when that fails/succeeds
- Move all logic that listens to cookie changes into GCMS, publish a notification when change happens
- Rename the MergeSessionComplete method of the signin_tracker
- Add retry (and backoff) logic for AddAccount calls
- Remove CrOS's direct calls to gaia_auth_fetcher->StartMergeSession

BUG=463611, 466799
TBR=zea@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#322774}

chrome/browser/android/signin/signin_manager_android.cc

@@ -260,13 +260,6 @@ void SigninManagerAndroid::ClearLastSignedInUser() {
   profile_->GetPrefs()->ClearPref(prefs::kGoogleServicesLastUsername);
 }
 
-void SigninManagerAndroid::MergeSessionCompleted(
-    const std::string& account_id,
-    const GoogleServiceAuthError& error) {
-  merge_session_helper_->RemoveObserver(this);
-  merge_session_helper_.reset();
-}
-
 void SigninManagerAndroid::LogInSignedInUser(JNIEnv* env, jobject obj) {
   SigninManagerBase* signin_manager =
       SigninManagerFactory::GetForProfile(profile_);

chrome/browser/android/signin/signin_manager_android.h

@@ -14,9 +14,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/prefs/pref_change_registrar.h"
-#include "google_apis/gaia/merge_session_helper.h"
 
-class GoogleServiceAuthError;
 class Profile;
 
 namespace policy {
@@ -31,7 +29,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 : public MergeSessionHelper::Observer {
+class SigninManagerAndroid {
  public:
   SigninManagerAndroid(JNIEnv* env, jobject obj);
 
@@ -67,7 +65,7 @@ class SigninManagerAndroid : public MergeSessionHelper::Observer {
   jboolean IsSignedInOnNative(JNIEnv* env, jobject obj);
 
  private:
-  ~SigninManagerAndroid() override;
+  ~SigninManagerAndroid();
 
 #if defined(ENABLE_CONFIGURATION_POLICY)
   void OnPolicyRegisterDone(const std::string& dm_token,
@@ -81,11 +79,6 @@ class SigninManagerAndroid : public MergeSessionHelper::Observer {
 
   void OnSigninAllowedPrefChanged();
 
-  // MergeSessionHelper::Observer implementation.
-  void MergeSessionCompleted(
-      const std::string& account_id,
-      const GoogleServiceAuthError& error) override;
-
   Profile* profile_;
 
   // Java-side SigninManager object.
@@ -102,9 +95,6 @@ class SigninManagerAndroid : public MergeSessionHelper::Observer {
   std::string username_;
 #endif
 
-  // Helper to merge the signed into account into the cookie jar session.
-  scoped_ptr<MergeSessionHelper> merge_session_helper_;
-
   PrefChangeRegistrar pref_change_registrar_;
 
   base::WeakPtrFactory<SigninManagerAndroid> weak_factory_;

chrome/browser/services/gcm/fake_signin_manager.cc

@@ -9,6 +9,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/account_tracker_service_factory.h"
 #include "chrome/browser/signin/chrome_signin_client_factory.h"
+#include "chrome/browser/signin/gaia_cookie_manager_service_factory.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/signin/core/common/signin_pref_names.h"
 #include "content/public/browser/browser_context.h"
@@ -27,7 +28,8 @@ FakeSigninManager::FakeSigninManager(Profile* profile)
     : SigninManager(
         ChromeSigninClientFactory::GetInstance()->GetForProfile(profile),
         ProfileOAuth2TokenServiceFactory::GetForProfile(profile),
-        AccountTrackerServiceFactory::GetForProfile(profile)),
+        AccountTrackerServiceFactory::GetForProfile(profile),
+        GaiaCookieManagerServiceFactory::GetForProfile(profile)),
 #endif
       profile_(profile) {
   Initialize(NULL);

chrome/browser/signin/account_reconcilor_factory.cc

@@ -6,6 +6,7 @@
 
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/chrome_signin_client_factory.h"
+#include "chrome/browser/signin/gaia_cookie_manager_service_factory.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
@@ -15,6 +16,7 @@ AccountReconcilorFactory::AccountReconcilorFactory()
         "AccountReconcilor",
         BrowserContextDependencyManager::GetInstance()) {
   DependsOn(ChromeSigninClientFactory::GetInstance());
+  DependsOn(GaiaCookieManagerServiceFactory::GetInstance());
   DependsOn(ProfileOAuth2TokenServiceFactory::GetInstance());
   DependsOn(SigninManagerFactory::GetInstance());
 }
@@ -39,7 +41,8 @@ KeyedService* AccountReconcilorFactory::BuildServiceInstanceFor(
   AccountReconcilor* reconcilor = new AccountReconcilor(
       ProfileOAuth2TokenServiceFactory::GetForProfile(profile),
       SigninManagerFactory::GetForProfile(profile),
-      ChromeSigninClientFactory::GetForProfile(profile));
+      ChromeSigninClientFactory::GetForProfile(profile),
+      GaiaCookieManagerServiceFactory::GetForProfile(profile));
   reconcilor->Initialize(true /* start_reconcile_if_tokens_available */);
   return reconcilor;
 }

chrome/browser/signin/account_reconcilor_factory.h

@@ -9,7 +9,6 @@
 #include "components/keyed_service/content/browser_context_keyed_service_factory.h"
 #include "components/signin/core/browser/account_reconcilor.h"
 
-class AccountReconcilor;
 class Profile;
 
 // Singleton that owns all AccountReconcilors and associates them with

chrome/browser/signin/account_reconcilor_unittest.cc

@@ -15,6 +15,7 @@
 #include "chrome/browser/signin/fake_profile_oauth2_token_service.h"
 #include "chrome/browser/signin/fake_profile_oauth2_token_service_builder.h"
 #include "chrome/browser/signin/fake_signin_manager.h"
+#include "chrome/browser/signin/gaia_cookie_manager_service_factory.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/signin/test_signin_client_builder.h"
@@ -45,7 +46,8 @@ class MockAccountReconcilor : public testing::StrictMock<AccountReconcilor> {
 
   MockAccountReconcilor(ProfileOAuth2TokenService* token_service,
                         SigninManagerBase* signin_manager,
-                        SigninClient* client);
+                        SigninClient* client,
+                        GaiaCookieManagerService* cookie_manager_service);
   ~MockAccountReconcilor() override {}
 
   MOCK_METHOD1(PerformMergeAction, void(const std::string& account_id));
@@ -58,18 +60,21 @@ KeyedService* MockAccountReconcilor::Build(content::BrowserContext* context) {
   AccountReconcilor* reconcilor = new MockAccountReconcilor(
       ProfileOAuth2TokenServiceFactory::GetForProfile(profile),
       SigninManagerFactory::GetForProfile(profile),
-      ChromeSigninClientFactory::GetForProfile(profile));
+      ChromeSigninClientFactory::GetForProfile(profile),
+      GaiaCookieManagerServiceFactory::GetForProfile(profile));
   reconcilor->Initialize(false /* start_reconcile_if_tokens_available */);
   return reconcilor;
 }
 
 MockAccountReconcilor::MockAccountReconcilor(
     ProfileOAuth2TokenService* token_service,
     SigninManagerBase* signin_manager,
-    SigninClient* client)
+    SigninClient* client,
+    GaiaCookieManagerService* cookie_manager_service)
     : testing::StrictMock<AccountReconcilor>(token_service,
                                              signin_manager,
-                                             client) {}
+                                             client,
+                                             cookie_manager_service) {}
 
 }  // namespace
 
@@ -93,8 +98,8 @@ class AccountReconcilorTest : public ::testing::TestWithParam<bool> {
 
   MockAccountReconcilor* GetMockReconcilor();
 
-  void SimulateMergeSessionCompleted(
-      MergeSessionHelper::Observer* observer,
+  void SimulateAddAccountToCookieCompleted(
+      GaiaCookieManagerService::Observer* observer,
       const std::string& account_id,
       const GoogleServiceAuthError& error);
 
@@ -142,7 +147,7 @@ void AccountReconcilorTest::SetUp() {
       GaiaConstants::kReconcilorSource);
   get_check_connection_info_url_ =
       GaiaUrls::GetInstance()->GetCheckConnectionInfoURLWithSource(
-          GaiaConstants::kReconcilorSource);
+          GaiaConstants::kChromeSource);
 
   SetFakeResponse(get_check_connection_info_url().spec(), "[]",
       net::HTTP_OK, net::URLRequestStatus::SUCCESS);
@@ -190,11 +195,11 @@ MockAccountReconcilor* AccountReconcilorTest::GetMockReconcilor() {
   return mock_reconcilor_;
 }
 
-void AccountReconcilorTest::SimulateMergeSessionCompleted(
-    MergeSessionHelper::Observer* observer,
+void AccountReconcilorTest::SimulateAddAccountToCookieCompleted(
+    GaiaCookieManagerService::Observer* observer,
     const std::string& account_id,
     const GoogleServiceAuthError& error) {
-  observer->MergeSessionCompleted(account_id, error);
+  observer->OnAddAccountToCookieCompleted(account_id, error);
 }
 
 void AccountReconcilorTest::SimulateCookieContentSettingsChanged(
@@ -211,7 +216,6 @@ TEST_F(AccountReconcilorTest, Basic) {
   AccountReconcilor* reconcilor =
       AccountReconcilorFactory::GetForProfile(profile());
   ASSERT_TRUE(reconcilor);
-  ASSERT_EQ(token_service(), reconcilor->token_service());
 }
 
 #if !defined(OS_CHROMEOS)
@@ -487,8 +491,8 @@ TEST_P(AccountReconcilorTest, StartReconcileAddToCookie) {
 
   base::RunLoop().RunUntilIdle();
   ASSERT_TRUE(reconcilor->is_reconcile_started_);
-  SimulateMergeSessionCompleted(reconcilor, "other@gmail.com",
-                                GoogleServiceAuthError::AuthErrorNone());
+  SimulateAddAccountToCookieCompleted(reconcilor, "other@gmail.com",
+                                      GoogleServiceAuthError::AuthErrorNone());
   ASSERT_FALSE(reconcilor->is_reconcile_started_);
 
   histogram_tester()->ExpectUniqueSample(
@@ -518,8 +522,8 @@ TEST_P(AccountReconcilorTest, StartReconcileRemoveFromCookie) {
   ASSERT_TRUE(reconcilor->is_reconcile_started_);
 
   base::RunLoop().RunUntilIdle();
-  SimulateMergeSessionCompleted(reconcilor, "user@gmail.com",
-                                GoogleServiceAuthError::AuthErrorNone());
+  SimulateAddAccountToCookieCompleted(reconcilor, "user@gmail.com",
+                                      GoogleServiceAuthError::AuthErrorNone());
   ASSERT_FALSE(reconcilor->is_reconcile_started_);
 
   histogram_tester()->ExpectUniqueSample(
@@ -551,7 +555,7 @@ TEST_P(AccountReconcilorTest, StartReconcileAddToCookieTwice) {
 
   base::RunLoop().RunUntilIdle();
   ASSERT_TRUE(reconcilor->is_reconcile_started_);
-  SimulateMergeSessionCompleted(
+  SimulateAddAccountToCookieCompleted(
       reconcilor, "other@gmail.com", GoogleServiceAuthError::AuthErrorNone());
   ASSERT_FALSE(reconcilor->is_reconcile_started_);
 
@@ -578,7 +582,7 @@ TEST_P(AccountReconcilorTest, StartReconcileAddToCookieTwice) {
   base::RunLoop().RunUntilIdle();
 
   ASSERT_TRUE(reconcilor->is_reconcile_started_);
-  SimulateMergeSessionCompleted(
+  SimulateAddAccountToCookieCompleted(
       reconcilor, "third@gmail.com", GoogleServiceAuthError::AuthErrorNone());
   ASSERT_FALSE(reconcilor->is_reconcile_started_);
 
@@ -619,11 +623,11 @@ TEST_P(AccountReconcilorTest, StartReconcileBadPrimary) {
 
   base::RunLoop().RunUntilIdle();
   ASSERT_TRUE(reconcilor->is_reconcile_started_);
-  SimulateMergeSessionCompleted(reconcilor, "other@gmail.com",
-                                GoogleServiceAuthError::AuthErrorNone());
+  SimulateAddAccountToCookieCompleted(reconcilor, "other@gmail.com",
+                                      GoogleServiceAuthError::AuthErrorNone());
   ASSERT_TRUE(reconcilor->is_reconcile_started_);
-  SimulateMergeSessionCompleted(reconcilor, "user@gmail.com",
-                                GoogleServiceAuthError::AuthErrorNone());
+  SimulateAddAccountToCookieCompleted(reconcilor, "user@gmail.com",
+                                      GoogleServiceAuthError::AuthErrorNone());
   ASSERT_FALSE(reconcilor->is_reconcile_started_);
 
   histogram_tester()->ExpectUniqueSample(
@@ -677,12 +681,12 @@ TEST_P(AccountReconcilorTest, StartReconcileWithSessionInfoExpiredDefault) {
   ASSERT_TRUE(reconcilor->is_reconcile_started_);
 
   base::RunLoop().RunUntilIdle();
-  SimulateMergeSessionCompleted(reconcilor, "user@gmail.com",
-                                GoogleServiceAuthError::AuthErrorNone());
+  SimulateAddAccountToCookieCompleted(reconcilor, "user@gmail.com",
+                                      GoogleServiceAuthError::AuthErrorNone());
   ASSERT_FALSE(reconcilor->is_reconcile_started_);
 }
 
-TEST_F(AccountReconcilorTest, MergeSessionCompletedWithBogusAccount) {
+TEST_F(AccountReconcilorTest, AddAccountToCookieCompletedWithBogusAccount) {
   signin_manager()->SetAuthenticatedUsername("user@gmail.com");
   token_service()->UpdateCredentials("user@gmail.com", "refresh_token");
 
@@ -703,12 +707,12 @@ TEST_F(AccountReconcilorTest, MergeSessionCompletedWithBogusAccount) {
   base::RunLoop().RunUntilIdle();
 
   // If an unknown account id is sent, it should not upset the state.
-  SimulateMergeSessionCompleted(reconcilor, "bogus@gmail.com",
-                                GoogleServiceAuthError::AuthErrorNone());
+  SimulateAddAccountToCookieCompleted(reconcilor, "bogus@gmail.com",
+                                      GoogleServiceAuthError::AuthErrorNone());
   ASSERT_TRUE(reconcilor->is_reconcile_started_);
 
-  SimulateMergeSessionCompleted(reconcilor, "user@gmail.com",
-                                GoogleServiceAuthError::AuthErrorNone());
+  SimulateAddAccountToCookieCompleted(reconcilor, "user@gmail.com",
+                                      GoogleServiceAuthError::AuthErrorNone());
   ASSERT_FALSE(reconcilor->is_reconcile_started_);
 }
 

chrome/browser/signin/fake_account_reconcilor.cc

@@ -6,14 +6,17 @@
 
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/chrome_signin_client_factory.h"
+#include "chrome/browser/signin/gaia_cookie_manager_service_factory.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 
 FakeAccountReconcilor::FakeAccountReconcilor(
     ProfileOAuth2TokenService* token_service,
     SigninManagerBase* signin_manager,
-    SigninClient* client) :
-  AccountReconcilor(token_service, signin_manager, client) {}
+    SigninClient* client,
+    GaiaCookieManagerService* cookie_manager_service) :
+  AccountReconcilor(
+      token_service, signin_manager, client, cookie_manager_service) {}
 
 
 // static
@@ -22,7 +25,8 @@ KeyedService* FakeAccountReconcilor::Build(content::BrowserContext* context) {
   AccountReconcilor* reconcilor = new FakeAccountReconcilor(
       ProfileOAuth2TokenServiceFactory::GetForProfile(profile),
       SigninManagerFactory::GetForProfile(profile),
-      ChromeSigninClientFactory::GetForProfile(profile));
+      ChromeSigninClientFactory::GetForProfile(profile),
+      GaiaCookieManagerServiceFactory::GetForProfile(profile));
   reconcilor->Initialize(true /* start_reconcile_if_tokens_available */);
   return reconcilor;
 }

chrome/browser/signin/fake_account_reconcilor.h

@@ -14,7 +14,8 @@ class FakeAccountReconcilor : public AccountReconcilor {
  public:
   FakeAccountReconcilor(ProfileOAuth2TokenService* token_service,
                         SigninManagerBase* signin_manager,
-                        SigninClient* client);
+                        SigninClient* client,
+                        GaiaCookieManagerService* cookie_manager_service);
 
   // Helper function to be used with KeyedService::SetTestingFactory().
   static KeyedService* Build(content::BrowserContext* context);

chrome/browser/signin/fake_signin_manager.cc

@@ -9,6 +9,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/account_tracker_service_factory.h"
 #include "chrome/browser/signin/chrome_signin_client_factory.h"
+#include "chrome/browser/signin/gaia_cookie_manager_service_factory.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/ui/global_error/global_error_service.h"
@@ -42,7 +43,8 @@ FakeSigninManager::FakeSigninManager(Profile* profile)
     : SigninManager(
           ChromeSigninClientFactory::GetInstance()->GetForProfile(profile),
           ProfileOAuth2TokenServiceFactory::GetForProfile(profile),
-          AccountTrackerServiceFactory::GetForProfile(profile)) {}
+          AccountTrackerServiceFactory::GetForProfile(profile),
+          GaiaCookieManagerServiceFactory::GetForProfile(profile)) {}
 
 FakeSigninManager::~FakeSigninManager() {
 }

chrome/browser/signin/gaia_cookie_manager_service_factory.cc

@@ -0,0 +1,50 @@
+// Copyright 2013 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/signin/gaia_cookie_manager_service_factory.h"
+
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/signin/chrome_signin_client_factory.h"
+#include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
+#include "components/keyed_service/content/browser_context_dependency_manager.h"
+#include "components/signin/core/browser/gaia_cookie_manager_service.h"
+#include "components/signin/core/browser/profile_oauth2_token_service.h"
+#include "google_apis/gaia/gaia_constants.h"
+
+GaiaCookieManagerServiceFactory::GaiaCookieManagerServiceFactory()
+    : BrowserContextKeyedServiceFactory(
+        "GaiaCookieManagerService",
+        BrowserContextDependencyManager::GetInstance()) {
+  DependsOn(ChromeSigninClientFactory::GetInstance());
+  DependsOn(ProfileOAuth2TokenServiceFactory::GetInstance());
+}
+
+GaiaCookieManagerServiceFactory::~GaiaCookieManagerServiceFactory() {}
+
+// static
+GaiaCookieManagerService* GaiaCookieManagerServiceFactory::GetForProfile(
+    Profile* profile) {
+  return static_cast<GaiaCookieManagerService*>(
+      GetInstance()->GetServiceForBrowserContext(profile, true));
+}
+
+// static
+GaiaCookieManagerServiceFactory*
+GaiaCookieManagerServiceFactory::GetInstance() {
+  return Singleton<GaiaCookieManagerServiceFactory>::get();
+}
+
+KeyedService* GaiaCookieManagerServiceFactory::BuildServiceInstanceFor(
+    content::BrowserContext* context) const {
+  Profile* profile = Profile::FromBrowserContext(context);
+  GaiaCookieManagerService* cookie_service = new GaiaCookieManagerService(
+      ProfileOAuth2TokenServiceFactory::GetForProfile(profile),
+      GaiaConstants::kChromeSource,
+      ChromeSigninClientFactory::GetForProfile(profile));
+  return cookie_service;
+}
+
+void GaiaCookieManagerServiceFactory::RegisterProfilePrefs(
+    user_prefs::PrefRegistrySyncable* registry) {
+}