non-new-profile-management creates a "no-op" style account_reconcilor,

useful for tracking stats but won't have any real effects.

Modify the AccountReconcilor_unittest to execute with the new_profile_management flag on.

BUG=357693
TEST=Account Reconciler should function normally when
new_profile_management flag is on. Should not have effects when the
flag is off, but UMA stats (histograms) and logging (for
--vmodule=account_reconcilor=1) should still trace the execution path.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=272131

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

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

chrome/browser/profiles/profile_manager.cc

@@ -982,8 +982,7 @@ void ProfileManager::DoFinalInitForServices(Profile* profile,
   StartupTaskRunnerServiceFactory::GetForProfile(profile)->
       StartDeferredTaskRunners();
 
-  if (switches::IsNewProfileManagement())
-    AccountReconcilorFactory::GetForProfile(profile);
+  AccountReconcilorFactory::GetForProfile(profile);
 }
 
 void ProfileManager::DoFinalInitLogging(Profile* profile) {

chrome/browser/signin/account_reconcilor_unittest.cc

@@ -2,9 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/command_line.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/run_loop.h"
+#include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
+#include "chrome/browser/prefs/pref_service_syncable.h"
 #include "chrome/browser/signin/account_reconcilor_factory.h"
 #include "chrome/browser/signin/chrome_signin_client_factory.h"
 #include "chrome/browser/signin/fake_profile_oauth2_token_service.h"
@@ -12,10 +15,13 @@
 #include "chrome/browser/signin/fake_signin_manager.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
+#include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
+#include "chrome/test/base/testing_profile_manager.h"
 #include "components/signin/core/browser/account_reconcilor.h"
 #include "components/signin/core/browser/profile_oauth2_token_service.h"
 #include "components/signin/core/browser/signin_manager.h"
+#include "components/signin/core/common/signin_switches.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "google_apis/gaia/gaia_urls.h"
 #include "net/url_request/test_url_fetcher_factory.h"
@@ -36,9 +42,9 @@ class MockAccountReconcilor : public testing::StrictMock<AccountReconcilor> {
   virtual ~MockAccountReconcilor() {}
 
   MOCK_METHOD1(PerformMergeAction, void(const std::string& account_id));
-  MOCK_METHOD1(StartRemoveAction, void(const std::string& account_id));
+  MOCK_METHOD1(PerformStartRemoveAction, void(const std::string& account_id));
   MOCK_METHOD3(
-      FinishRemoveAction,
+      PerformFinishRemoveAction,
       void(const std::string& account_id,
            const GoogleServiceAuthError& error,
            const std::vector<std::pair<std::string, bool> >& accounts));
@@ -74,7 +80,7 @@ class AccountReconcilorTest : public testing::Test {
   virtual void SetUp() OVERRIDE;
   virtual void TearDown() OVERRIDE;
 
-  TestingProfile* profile() { return profile_.get(); }
+  TestingProfile* profile() { return profile_; }
   FakeSigninManagerForTesting* signin_manager() { return signin_manager_; }
   FakeProfileOAuth2TokenService* token_service() { return token_service_; }
 
@@ -99,11 +105,12 @@ class AccountReconcilorTest : public testing::Test {
 
  private:
   content::TestBrowserThreadBundle bundle_;
-  scoped_ptr<TestingProfile> profile_;
+  TestingProfile* profile_;
   FakeSigninManagerForTesting* signin_manager_;
   FakeProfileOAuth2TokenService* token_service_;
   MockAccountReconcilor* mock_reconcilor_;
   net::FakeURLFetcherFactory url_fetcher_factory_;
+  TestingProfileManager* testing_profile_manager_;
 };
 
 AccountReconcilorTest::AccountReconcilorTest()
@@ -113,14 +120,26 @@ AccountReconcilorTest::AccountReconcilorTest()
       url_fetcher_factory_(NULL) {}
 
 void AccountReconcilorTest::SetUp() {
-  TestingProfile::Builder builder;
-  builder.AddTestingFactory(ProfileOAuth2TokenServiceFactory::GetInstance(),
-                            BuildFakeProfileOAuth2TokenService);
-  builder.AddTestingFactory(SigninManagerFactory::GetInstance(),
-                            FakeSigninManagerBase::Build);
-  builder.AddTestingFactory(AccountReconcilorFactory::GetInstance(),
-                            MockAccountReconcilor::Build);
-  profile_ = builder.Build();
+  CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kNewProfileManagement);
+
+  testing_profile_manager_ =
+      new TestingProfileManager(TestingBrowserProcess::GetGlobal());
+  ASSERT_TRUE(testing_profile_manager_->SetUp());
+
+  TestingProfile::TestingFactories factories;
+  factories.push_back(std::make_pair(
+      ProfileOAuth2TokenServiceFactory::GetInstance(),
+      BuildFakeProfileOAuth2TokenService));
+  factories.push_back(std::make_pair(SigninManagerFactory::GetInstance(),
+      FakeSigninManagerBase::Build));
+  factories.push_back(std::make_pair(AccountReconcilorFactory::GetInstance(),
+      MockAccountReconcilor::Build));
+
+  profile_ = testing_profile_manager_->CreateTestingProfile("name",
+                              scoped_ptr<PrefServiceSyncable>(),
+                              base::UTF8ToUTF16("name"), 0, std::string(),
+                              factories);
 
   signin_manager_ =
       static_cast<FakeSigninManagerForTesting*>(
@@ -132,8 +151,8 @@ void AccountReconcilorTest::SetUp() {
 }
 
 void AccountReconcilorTest::TearDown() {
-  // Destroy the profile before all threads are torn down.
-  profile_.reset();
+  // The |testing_profile_manager_| will handle destroying the profile.
+  delete testing_profile_manager_;
 }
 
 MockAccountReconcilor* AccountReconcilorTest::GetMockReconcilor() {
@@ -175,6 +194,7 @@ TEST_F(AccountReconcilorTest, SigninManagerRegistration) {
   ASSERT_TRUE(reconcilor);
   ASSERT_FALSE(reconcilor->IsRegisteredWithTokenService());
 
+  signin_manager()->set_password("password");
   signin_manager()->OnExternalSigninCompleted(kTestEmail);
   ASSERT_TRUE(reconcilor->IsRegisteredWithTokenService());
 
@@ -186,6 +206,7 @@ TEST_F(AccountReconcilorTest, SigninManagerRegistration) {
 
 TEST_F(AccountReconcilorTest, Reauth) {
   signin_manager()->SetAuthenticatedUsername(kTestEmail);
+  signin_manager()->set_password("password");
 
   AccountReconcilor* reconcilor =
       AccountReconcilorFactory::GetForProfile(profile());
@@ -422,6 +443,7 @@ TEST_F(AccountReconcilorTest, StartReconcileNoopMultiple) {
   ASSERT_FALSE(reconcilor->is_reconcile_started_);
 }
 
+
 TEST_F(AccountReconcilorTest, StartReconcileAddToCookie) {
   signin_manager()->SetAuthenticatedUsername("user@gmail.com");
   token_service()->UpdateCredentials("user@gmail.com", "refresh_token");

components/signin/core/browser/account_reconcilor.cc

@@ -17,6 +17,7 @@
 #include "components/signin/core/browser/signin_client.h"
 #include "components/signin/core/browser/signin_metrics.h"
 #include "components/signin/core/browser/signin_oauth_helper.h"
+#include "components/signin/core/common/profile_management_switches.h"
 #include "google_apis/gaia/gaia_auth_fetcher.h"
 #include "google_apis/gaia/gaia_auth_util.h"
 #include "google_apis/gaia/gaia_constants.h"
@@ -335,7 +336,7 @@ void AccountReconcilor::OnRefreshTokenAvailable(const std::string& account_id) {
 
 void AccountReconcilor::OnRefreshTokenRevoked(const std::string& account_id) {
   VLOG(1) << "AccountReconcilor::OnRefreshTokenRevoked: " << account_id;
-  StartRemoveAction(account_id);
+  PerformStartRemoveAction(account_id);
 }
 
 void AccountReconcilor::OnRefreshTokensLoaded() {}
@@ -358,22 +359,28 @@ void AccountReconcilor::GoogleSignedOut(const std::string& username) {
 }
 
 void AccountReconcilor::PerformMergeAction(const std::string& account_id) {
+  if (!switches::IsNewProfileManagement())
+    return;
   VLOG(1) << "AccountReconcilor::PerformMergeAction: " << account_id;
   merge_session_helper_.LogIn(account_id);
 }
 
-void AccountReconcilor::StartRemoveAction(const std::string& account_id) {
-  VLOG(1) << "AccountReconcilor::StartRemoveAction: " << account_id;
-  GetAccountsFromCookie(base::Bind(&AccountReconcilor::FinishRemoveAction,
-                                   base::Unretained(this),
-                                   account_id));
+void AccountReconcilor::PerformStartRemoveAction(
+    const std::string& account_id) {
+  VLOG(1) << "AccountReconcilor::PerformStartRemoveAction: " << account_id;
+  GetAccountsFromCookie(base::Bind(
+      &AccountReconcilor::PerformFinishRemoveAction,
+      base::Unretained(this),
+      account_id));
 }
 
-void AccountReconcilor::FinishRemoveAction(
+void AccountReconcilor::PerformFinishRemoveAction(
     const std::string& account_id,
     const GoogleServiceAuthError& error,
     const std::vector<std::pair<std::string, bool> >& accounts) {
-  VLOG(1) << "AccountReconcilor::FinishRemoveAction:"
+  if (!switches::IsNewProfileManagement())
+    return;
+  VLOG(1) << "AccountReconcilor::PerformFinishRemoveAction:"
           << " account=" << account_id << " error=" << error.ToString();
   if (error.state() == GoogleServiceAuthError::NONE) {
     AbortReconcile();
@@ -391,6 +398,8 @@ void AccountReconcilor::FinishRemoveAction(
 
 void AccountReconcilor::PerformAddToChromeAction(const std::string& account_id,
                                                  int session_index) {
+  if (!switches::IsNewProfileManagement())
+    return;
   VLOG(1) << "AccountReconcilor::PerformAddToChromeAction:"
           << " account=" << account_id << " session_index=" << session_index;
 
@@ -401,6 +410,8 @@ void AccountReconcilor::PerformAddToChromeAction(const std::string& account_id,
 }
 
 void AccountReconcilor::PerformLogoutAllAccountsAction() {
+  if (!switches::IsNewProfileManagement())
+    return;
   VLOG(1) << "AccountReconcilor::PerformLogoutAllAccountsAction";
   merge_session_helper_.LogOutAllAccounts();
 }
@@ -714,13 +725,22 @@ void AccountReconcilor::HandleFailedAccountIdCheck(
   FinishReconcile();
 }
 
+void AccountReconcilor::PerformAddAccountToTokenService(
+    const std::string& account_id,
+    const std::string& refresh_token) {
+  // The flow should never get to this method if new_profile_management is
+  // false, but better safe than sorry.
+  if (!switches::IsNewProfileManagement())
+    return;
+  token_service_->UpdateCredentials(account_id, refresh_token);
+}
+
 void AccountReconcilor::HandleRefreshTokenFetched(
     const std::string& account_id,
     const std::string& refresh_token) {
   if (!refresh_token.empty()) {
-    token_service_->UpdateCredentials(account_id, refresh_token);
+    PerformAddAccountToTokenService(account_id, refresh_token);
   }
-
   // Remove the account from the list that is being updated.
   for (std::vector<std::pair<std::string, int> >::iterator i =
            add_to_chrome_.begin();

components/signin/core/browser/account_reconcilor.h

@@ -140,10 +140,13 @@ class AccountReconcilor : public KeyedService,
   virtual void PerformAddToChromeAction(const std::string& account_id,
                                         int session_index);
   virtual void PerformLogoutAllAccountsAction();
+  virtual void PerformAddAccountToTokenService(
+      const std::string& account_id,
+      const std::string& refresh_token);
 
   // Used to remove an account from chrome and the cookie jar.
-  virtual void StartRemoveAction(const std::string& account_id);
-  virtual void FinishRemoveAction(
+  virtual void PerformStartRemoveAction(const std::string& account_id);
+  virtual void PerformFinishRemoveAction(
       const std::string& account_id,
       const GoogleServiceAuthError& error,
       const std::vector<std::pair<std::string, bool> >& accounts);

google_apis/gaia/fake_gaia.cc

@@ -50,6 +50,7 @@ const char kAuthHeaderOAuth[] = "OAuth ";
 
 const char kListAccountsResponseFormat[] =
     "[\"gaia.l.a.r\",[[\"gaia.l.a\",1,\"\",\"%s\",\"\",1,1,0]]]";
+const char kPeopleGetResponseFormat[] = "{\"id\":\"%s\"}";
 
 typedef std::map<std::string, std::string> CookieMap;
 
@@ -176,6 +177,10 @@ void FakeGaia::Initialize() {
   // Handles /ListAccounts GAIA call.
   REGISTER_RESPONSE_HANDLER(
       gaia_urls->list_accounts_url(), HandleListAccounts);
+
+  // Handles /plus/v1/people/me
+  REGISTER_RESPONSE_HANDLER(
+      gaia_urls->people_get_url(), HandlePeopleGet);
 }
 
 scoped_ptr<HttpResponse> FakeGaia::HandleRequest(const HttpRequest& request) {
@@ -529,3 +534,11 @@ void FakeGaia::HandleListAccounts(const HttpRequest& request,
       kListAccountsResponseFormat, merge_session_params_.email.c_str()));
   http_response->set_code(net::HTTP_OK);
 }
+
+
+void FakeGaia::HandlePeopleGet(const HttpRequest& request,
+                                 BasicHttpResponse* http_response) {
+  http_response->set_content(base::StringPrintf(
+      kPeopleGetResponseFormat, "name"));
+  http_response->set_code(net::HTTP_OK);
+}

google_apis/gaia/fake_gaia.h

@@ -150,6 +150,8 @@ class FakeGaia {
                         net::test_server::BasicHttpResponse* http_response);
   void HandleListAccounts(const net::test_server::HttpRequest& request,
                           net::test_server::BasicHttpResponse* http_response);
+  void HandlePeopleGet(const net::test_server::HttpRequest& request,
+                       net::test_server::BasicHttpResponse* http_response);
 
   // Returns the access token associated with |auth_token| that matches the
   // given |client_id| and |scope_string|. If |scope_string| is empty, the first