Revert "Improve the no-op reconcilor so that it doesn't "hang" after the first...

Revert "Improve the no-op reconcilor so that it doesn't "hang" after the first execution, but instead successfully completes the reconciliation." (https://codereview.chromium.org/309843002/)

The original CL causes crashes about 20 secs after launching the linux build of chromeos.

This reverts commit 660a4d00.

Conflicts:
	chrome/browser/signin/account_reconcilor_unittest.cc

Original issue's description:
> Improve the no-op reconcilor so that it doesn't "hang" after the first execution, but instead successfully completes the reconciliation.
> Improve the UMAHistogramHelper so that it can nicely take snapshots between tests, allowing the test writer to easily test ONLY the changes logged in "this" test.
> Write a new unit test that executes the reconcilor twice, and make all reconcilor unit tests execute with and without the flag.
>
> BUG=357693
> TBR=phajdan.jr@chromium.org
>
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=277605

BUG=357693,386045
TEST=None
TBR=mlerman@chromium.org

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

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

base/test/statistics_delta_reader.h

@@ -17,10 +17,6 @@ class HistogramSamples;
 
 // This class acts as a differential reader for histogram samples, enabling
 // tests to check that metrics were recorded as they should be.
-//
-// This class is DEPRECATED.
-// TODO(mlerman): Remove all references to this class with UMAHistogramHelper
-// references. crbug.com/384011
 class StatisticsDeltaReader {
  public:
   StatisticsDeltaReader();

chrome/browser/signin/account_reconcilor_unittest.cc

@@ -20,11 +20,11 @@
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chrome/test/base/testing_profile_manager.h"
-#include "chrome/test/base/uma_histogram_helper.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/profile_management_switches.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"
@@ -34,13 +34,6 @@
 namespace {
 
 const char kTestEmail[] = "user@gmail.com";
-const char* const kHistogramsToSnapshot[] = {
-    "Signin.Reconciler.DifferentPrimaryAccounts.FirstRun",
-    "Signin.Reconciler.AddedToCookieJar.FirstRun",
-    "Signin.Reconciler.AddedToChrome.FirstRun",
-    "Signin.Reconciler.DifferentPrimaryAccounts.SubsequentRun",
-    "Signin.Reconciler.AddedToCookieJar.SubsequentRun",
-    "Signin.Reconciler.AddedToChrome.SubsequentRun"};
 
 class MockAccountReconcilor : public testing::StrictMock<AccountReconcilor> {
  public:
@@ -84,7 +77,7 @@ MockAccountReconcilor::MockAccountReconcilor(
 
 }  // namespace
 
-class AccountReconcilorTest : public ::testing::TestWithParam<bool> {
+class AccountReconcilorTest : public testing::Test {
  public:
   AccountReconcilorTest();
   virtual void SetUp() OVERRIDE;
@@ -92,7 +85,6 @@ class AccountReconcilorTest : public ::testing::TestWithParam<bool> {
   TestingProfile* profile() { return profile_; }
   FakeSigninManagerForTesting* signin_manager() { return signin_manager_; }
   FakeProfileOAuth2TokenService* token_service() { return token_service_; }
-  UMAHistogramHelper* histogram_helper() { return &histogram_helper_; }
 
   void SetFakeResponse(const std::string& url,
                        const std::string& data,
@@ -121,9 +113,6 @@ class AccountReconcilorTest : public ::testing::TestWithParam<bool> {
   MockAccountReconcilor* mock_reconcilor_;
   net::FakeURLFetcherFactory url_fetcher_factory_;
   scoped_ptr<TestingProfileManager> testing_profile_manager_;
-  UMAHistogramHelper histogram_helper_;
-
-  DISALLOW_COPY_AND_ASSIGN(AccountReconcilorTest);
 };
 
 AccountReconcilorTest::AccountReconcilorTest()
@@ -133,12 +122,8 @@ AccountReconcilorTest::AccountReconcilorTest()
       url_fetcher_factory_(NULL) {}
 
 void AccountReconcilorTest::SetUp() {
-  // If it's a non-parameterized test, or we have a parameter of true, set flag.
-  if (!::testing::UnitTest::GetInstance()->current_test_info()->value_param() ||
-      GetParam()) {
-    switches::EnableAccountConsistencyForTesting(
-        CommandLine::ForCurrentProcess());
-  }
+  CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kEnableNewProfileManagement);
 
   testing_profile_manager_.reset(
       new TestingProfileManager(TestingBrowserProcess::GetGlobal()));
@@ -167,10 +152,6 @@ void AccountReconcilorTest::SetUp() {
   token_service_ =
       static_cast<FakeProfileOAuth2TokenService*>(
           ProfileOAuth2TokenServiceFactory::GetForProfile(profile()));
-
-  // Take a new snapshot of the concerned histograms before each test
-  histogram_helper_.PrepareSnapshot(kHistogramsToSnapshot,
-                                    arraysize(kHistogramsToSnapshot));
 }
 
 MockAccountReconcilor* AccountReconcilorTest::GetMockReconcilor() {
@@ -358,7 +339,7 @@ TEST_F(AccountReconcilorTest, ValidateAccountsFromTokensFailedTokenRequest) {
   ASSERT_EQ(1u, reconcilor->GetInvalidChromeAccountsForTesting().size());
 }
 
-TEST_P(AccountReconcilorTest, StartReconcileNoop) {
+TEST_F(AccountReconcilorTest, StartReconcileNoop) {
   signin_manager()->SetAuthenticatedUsername(kTestEmail);
   token_service()->UpdateCredentials(kTestEmail, "refresh_token");
 
@@ -387,12 +368,6 @@ TEST_P(AccountReconcilorTest, StartReconcileNoop) {
   base::RunLoop().RunUntilIdle();
   ASSERT_TRUE(reconcilor->AreAllRefreshTokensChecked());
   ASSERT_FALSE(reconcilor->is_reconcile_started_);
-
-  histogram_helper()->Fetch();
-  histogram_helper()->ExpectTotalCount(
-      "Signin.Reconciler.DifferentPrimaryAccounts.FirstRun", 1);
-  histogram_helper()->ExpectUniqueSample(
-      "Signin.Reconciler.DifferentPrimaryAccounts.FirstRun", 0, 1);
 }
 
 // This is test is needed until chrome changes to use gaia obfuscated id.
@@ -403,7 +378,7 @@ TEST_P(AccountReconcilorTest, StartReconcileNoop) {
 // tests makes sure that an email like "Dot.S@hmail.com", as seen by the
 // token service, will be considered the same as "dots@gmail.com" as returned
 // by gaia::ParseListAccountsData().
-TEST_P(AccountReconcilorTest, StartReconcileNoopWithDots) {
+TEST_F(AccountReconcilorTest, StartReconcileNoopWithDots) {
   signin_manager()->SetAuthenticatedUsername("Dot.S@gmail.com");
   token_service()->UpdateCredentials("Dot.S@gmail.com", "refresh_token");
 
@@ -434,13 +409,9 @@ TEST_P(AccountReconcilorTest, StartReconcileNoopWithDots) {
   base::RunLoop().RunUntilIdle();
   ASSERT_TRUE(reconcilor->AreAllRefreshTokensChecked());
   ASSERT_FALSE(reconcilor->is_reconcile_started_);
-
-  histogram_helper()->Fetch();
-  histogram_helper()->ExpectUniqueSample(
-      "Signin.Reconciler.DifferentPrimaryAccounts.FirstRun", 0, 1);
 }
 
-TEST_P(AccountReconcilorTest, StartReconcileNoopMultiple) {
+TEST_F(AccountReconcilorTest, StartReconcileNoopMultiple) {
   signin_manager()->SetAuthenticatedUsername("user@gmail.com");
   token_service()->UpdateCredentials("user@gmail.com", "refresh_token");
   token_service()->UpdateCredentials("other@gmail.com", "refresh_token");
@@ -477,15 +448,9 @@ TEST_P(AccountReconcilorTest, StartReconcileNoopMultiple) {
   base::RunLoop().RunUntilIdle();
   ASSERT_TRUE(reconcilor->AreAllRefreshTokensChecked());
   ASSERT_FALSE(reconcilor->is_reconcile_started_);
-
-  histogram_helper()->Fetch();
-  histogram_helper()->ExpectTotalCount(
-      "Signin.Reconciler.DifferentPrimaryAccounts.FirstRun", 1);
-  histogram_helper()->ExpectUniqueSample(
-      "Signin.Reconciler.DifferentPrimaryAccounts.FirstRun", 0, 1);
 }
 
-TEST_P(AccountReconcilorTest, StartReconcileAddToCookie) {
+TEST_F(AccountReconcilorTest, StartReconcileAddToCookie) {
   signin_manager()->SetAuthenticatedUsername("user@gmail.com");
   token_service()->UpdateCredentials("user@gmail.com", "refresh_token");
   token_service()->UpdateCredentials("other@gmail.com", "refresh_token");
@@ -510,106 +475,9 @@ TEST_P(AccountReconcilorTest, StartReconcileAddToCookie) {
   SimulateMergeSessionCompleted(reconcilor, "other@gmail.com",
                                 GoogleServiceAuthError::AuthErrorNone());
   ASSERT_FALSE(reconcilor->is_reconcile_started_);
-
-  histogram_helper()->Fetch();
-  histogram_helper()->ExpectUniqueSample(
-      "Signin.Reconciler.DifferentPrimaryAccounts.FirstRun", 0, 1);
-  histogram_helper()->ExpectUniqueSample(
-      "Signin.Reconciler.AddedToCookieJar.FirstRun", 1, 1);
-  histogram_helper()->ExpectUniqueSample(
-      "Signin.Reconciler.AddedToChrome.FirstRun", 0, 1);
 }
 
-TEST_P(AccountReconcilorTest, StartReconcileAddToCookieTwice) {
-  signin_manager()->SetAuthenticatedUsername("user@gmail.com");
-  token_service()->UpdateCredentials("user@gmail.com", "refresh_token");
-  token_service()->UpdateCredentials("other@gmail.com", "refresh_token");
-
-  EXPECT_CALL(*GetMockReconcilor(), PerformMergeAction("other@gmail.com"));
-  EXPECT_CALL(*GetMockReconcilor(), PerformMergeAction("third@gmail.com"));
-
-  SetFakeResponse(
-      GaiaUrls::GetInstance()->list_accounts_url().spec(),
-      "[\"f\", [[\"b\", 0, \"n\", \"user@gmail.com\", \"p\", 0, 0, 0, 0, 1]]]",
-      net::HTTP_OK,
-      net::URLRequestStatus::SUCCESS);
-  SetFakeResponse(GaiaUrls::GetInstance()->people_get_url().spec(),
-                  "{\"id\":\"foo\"}",
-                  net::HTTP_OK,
-                  net::URLRequestStatus::SUCCESS);
-
-  AccountReconcilor* reconcilor = GetMockReconcilor();
-  reconcilor->StartReconcile();
-  token_service()->IssueAllTokensForAccount(
-      "other@gmail.com",
-      "access_token",
-      base::Time::Now() + base::TimeDelta::FromHours(1));
-  token_service()->IssueAllTokensForAccount(
-      "user@gmail.com",
-      "access_token",
-      base::Time::Now() + base::TimeDelta::FromHours(1));
-
-  base::RunLoop().RunUntilIdle();
-  ASSERT_TRUE(reconcilor->is_reconcile_started_);
-  SimulateMergeSessionCompleted(
-      reconcilor, "other@gmail.com", GoogleServiceAuthError::AuthErrorNone());
-  ASSERT_FALSE(reconcilor->is_reconcile_started_);
-
-  histogram_helper()->Fetch();
-  histogram_helper()->ExpectUniqueSample(
-      "Signin.Reconciler.DifferentPrimaryAccounts.FirstRun", 0, 1);
-  histogram_helper()->ExpectUniqueSample(
-      "Signin.Reconciler.AddedToCookieJar.FirstRun", 1, 1);
-  histogram_helper()->ExpectUniqueSample(
-      "Signin.Reconciler.AddedToChrome.FirstRun", 0, 1);
-
-  // Do another pass after I've added a third account to the token service
-
-  SetFakeResponse(
-      GaiaUrls::GetInstance()->list_accounts_url().spec(),
-      "[\"f\", [[\"b\", 0, \"n\", \"user@gmail.com\", \"p\", 0, 0, 0, 0, 1], "
-      "[\"b\", 0, \"n\", \"other@gmail.com\", \"p\", 0, 0, 0, 0, 1]]]",
-      net::HTTP_OK,
-      net::URLRequestStatus::SUCCESS);
-  // This will cause the reconcilor to fire.
-  token_service()->UpdateCredentials("third@gmail.com", "refresh_token");
-
-  token_service()->IssueAllTokensForAccount(
-      "other@gmail.com",
-      "access_token",
-      base::Time::Now() + base::TimeDelta::FromHours(1));
-  token_service()->IssueAllTokensForAccount(
-      "user@gmail.com",
-      "access_token",
-      base::Time::Now() + base::TimeDelta::FromHours(1));
-  token_service()->IssueAllTokensForAccount(
-      "third@gmail.com",
-      "access_token",
-      base::Time::Now() + base::TimeDelta::FromHours(1));
-
-  base::RunLoop().RunUntilIdle();
-
-  ASSERT_TRUE(reconcilor->is_reconcile_started_);
-  SimulateMergeSessionCompleted(
-      reconcilor, "third@gmail.com", GoogleServiceAuthError::AuthErrorNone());
-  ASSERT_FALSE(reconcilor->is_reconcile_started_);
-
-  histogram_helper()->Fetch();
-  histogram_helper()->ExpectUniqueSample(
-      "Signin.Reconciler.DifferentPrimaryAccounts.FirstRun", 0, 1);
-  histogram_helper()->ExpectUniqueSample(
-      "Signin.Reconciler.AddedToCookieJar.FirstRun", 1, 1);
-  histogram_helper()->ExpectUniqueSample(
-      "Signin.Reconciler.AddedToChrome.FirstRun", 0, 1);
-  histogram_helper()->ExpectUniqueSample(
-      "Signin.Reconciler.DifferentPrimaryAccounts.SubsequentRun", 0, 1);
-  histogram_helper()->ExpectUniqueSample(
-      "Signin.Reconciler.AddedToCookieJar.SubsequentRun", 1, 1);
-  histogram_helper()->ExpectUniqueSample(
-      "Signin.Reconciler.AddedToChrome.SubsequentRun", 0, 1);
-}
-
-TEST_P(AccountReconcilorTest, StartReconcileAddToChrome) {
+TEST_F(AccountReconcilorTest, StartReconcileAddToChrome) {
   signin_manager()->SetAuthenticatedUsername("user@gmail.com");
   token_service()->UpdateCredentials("user@gmail.com", "refresh_token");
 
@@ -632,17 +500,9 @@ TEST_P(AccountReconcilorTest, StartReconcileAddToChrome) {
   ASSERT_TRUE(reconcilor->is_reconcile_started_);
   SimulateRefreshTokenFetched(reconcilor, "other@gmail.com", "");
   ASSERT_FALSE(reconcilor->is_reconcile_started_);
-
-  histogram_helper()->Fetch();
-  histogram_helper()->ExpectUniqueSample(
-      "Signin.Reconciler.DifferentPrimaryAccounts.FirstRun", 0, 1);
-  histogram_helper()->ExpectUniqueSample(
-      "Signin.Reconciler.AddedToCookieJar.FirstRun", 0, 1);
-  histogram_helper()->ExpectUniqueSample(
-      "Signin.Reconciler.AddedToChrome.FirstRun", 1, 1);
 }
 
-TEST_P(AccountReconcilorTest, StartReconcileBadPrimary) {
+TEST_F(AccountReconcilorTest, StartReconcileBadPrimary) {
   signin_manager()->SetAuthenticatedUsername("user@gmail.com");
   token_service()->UpdateCredentials("user@gmail.com", "refresh_token");
   token_service()->UpdateCredentials("other@gmail.com", "refresh_token");
@@ -673,17 +533,9 @@ TEST_P(AccountReconcilorTest, StartReconcileBadPrimary) {
   SimulateMergeSessionCompleted(reconcilor, "user@gmail.com",
                                 GoogleServiceAuthError::AuthErrorNone());
   ASSERT_FALSE(reconcilor->is_reconcile_started_);
-
-  histogram_helper()->Fetch();
-  histogram_helper()->ExpectUniqueSample(
-      "Signin.Reconciler.DifferentPrimaryAccounts.FirstRun", 1, 1);
-  histogram_helper()->ExpectUniqueSample(
-      "Signin.Reconciler.AddedToCookieJar.FirstRun", 2, 1);
-  histogram_helper()->ExpectUniqueSample(
-      "Signin.Reconciler.AddedToChrome.FirstRun", 0, 1);
 }
 
-TEST_P(AccountReconcilorTest, StartReconcileOnlyOnce) {
+TEST_F(AccountReconcilorTest, StartReconcileOnlyOnce) {
   signin_manager()->SetAuthenticatedUsername(kTestEmail);
   token_service()->UpdateCredentials(kTestEmail, "refresh_token");
 
@@ -708,7 +560,7 @@ TEST_P(AccountReconcilorTest, StartReconcileOnlyOnce) {
   ASSERT_FALSE(reconcilor->is_reconcile_started_);
 }
 
-TEST_P(AccountReconcilorTest, StartReconcileWithSessionInfoExpiredDefault) {
+TEST_F(AccountReconcilorTest, StartReconcileWithSessionInfoExpiredDefault) {
   signin_manager()->SetAuthenticatedUsername("user@gmail.com");
   token_service()->UpdateCredentials("user@gmail.com", "refresh_token");
   token_service()->UpdateCredentials("other@gmail.com", "refresh_token");
@@ -740,7 +592,3 @@ TEST_P(AccountReconcilorTest, StartReconcileWithSessionInfoExpiredDefault) {
                                 GoogleServiceAuthError::AuthErrorNone());
   ASSERT_FALSE(reconcilor->is_reconcile_started_);
 }
-
-INSTANTIATE_TEST_CASE_P(AccountReconcilorMaybeEnabled,
-                        AccountReconcilorTest,
-                        testing::Bool());

chrome/test/base/uma_histogram_helper.cc

@@ -11,27 +11,6 @@
 #include "content/public/browser/histogram_fetcher.h"
 
 UMAHistogramHelper::UMAHistogramHelper() {
-  base::StatisticsRecorder::Initialize();
-}
-
-UMAHistogramHelper::~UMAHistogramHelper() {
-}
-
-void UMAHistogramHelper::PrepareSnapshot(const char* const histogram_names[],
-                                         size_t num_histograms) {
-  for (size_t i = 0; i < num_histograms; ++i) {
-    std::string histogram_name = histogram_names[i];
-
-    base::HistogramBase* histogram =
-        base::StatisticsRecorder::FindHistogram(histogram_name);
-    // If there is no histogram present, then don't record a snapshot. The logic
-    // in the Expect* methods will act to treat no histogram equivalent to
-    // samples with zeros.
-    if (histogram) {
-      histogram_snapshots[histogram_name] =
-          make_linked_ptr(histogram->SnapshotSamples().release());
-    }
-  }
 }
 
 void UMAHistogramHelper::Fetch() {
@@ -103,25 +82,18 @@ void UMAHistogramHelper::CheckBucketCount(
     base::HistogramBase::Sample sample,
     base::HistogramBase::Count expected_count,
     base::HistogramSamples& samples) {
-  int actual_count = samples.GetCount(sample);
-  if (histogram_snapshots.count(name))
-    actual_count -= histogram_snapshots[name]->GetCount(sample);
-  EXPECT_EQ(expected_count, actual_count)
+  EXPECT_EQ(expected_count, samples.GetCount(sample))
       << "Histogram \"" << name
       << "\" does not have the right number of samples (" << expected_count
-      << ") in the expected bucket (" << sample << "). It has (" << actual_count
-      << ").";
+      << ") in the expected bucket (" << sample << ").";
 }
 
 void UMAHistogramHelper::CheckTotalCount(
     const std::string& name,
     base::HistogramBase::Count expected_count,
     base::HistogramSamples& samples) {
-  int actual_count = samples.TotalCount();
-  if (histogram_snapshots.count(name))
-    actual_count -= histogram_snapshots[name]->TotalCount();
-  EXPECT_EQ(expected_count, actual_count)
+  EXPECT_EQ(expected_count, samples.TotalCount())
       << "Histogram \"" << name
       << "\" does not have the right total number of samples ("
-      << expected_count << "). It has (" << actual_count << ").";
+      << expected_count << ").";
 }

chrome/test/base/uma_histogram_helper.h

@@ -5,7 +5,6 @@
 #ifndef CHROME_TEST_BASE_UMA_HISTOGRAM_HELPER_H_
 #define CHROME_TEST_BASE_UMA_HISTOGRAM_HELPER_H_
 
-#include "base/memory/linked_ptr.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/histogram_base.h"
 #include "base/metrics/histogram_samples.h"
@@ -15,40 +14,25 @@
 // intended.
 class UMAHistogramHelper {
  public:
-  // UMAHistogramHelper should be created before the execution of the test case.
   UMAHistogramHelper();
 
-  ~UMAHistogramHelper();
-
-  // Parameters should be string literals of all histograms to snapshot.
-  // Call this before executing the test code. This method can be called
-  // multiple times. The existing snapshots are preserved, except when one of
-  // the |histogram_names| was previously passed as a parameter, then a new
-  // snapshot will replace the existing one.
-  void PrepareSnapshot(const char* const histogram_names[],
-                       size_t num_histograms);
-
   // Each child process may have its own histogram data, make sure this data
   // gets accumulated into the browser process before we examine the histograms.
   void Fetch();
 
   // We know the exact number of samples in a bucket, and that no other bucket
-  // should have samples. If |PrepareSnapshot| was called for the histogram
-  // named |name| then the |expected_count| is the diff from the snapshot.
+  // should have samples.
   void ExpectUniqueSample(const std::string& name,
                           base::HistogramBase::Sample sample,
                           base::HistogramBase::Count expected_count);
 
   // We know the exact number of samples in a bucket, but other buckets may
-  // have samples as well. If |PrepareSnapshot| was called for histogram named
-  // |name| then the |expected_count| is the diff from the snapshot.
+  // have samples as well.
   void ExpectBucketCount(const std::string& name,
                           base::HistogramBase::Sample sample,
                           base::HistogramBase::Count expected_count);
 
-  // We don't know the values of the samples, but we know how many there are. If
-  // |PrepareSnapshot| was called for |name| histogram, then the
-  // |count| is the diff from the snapshot.
+  // We don't know the values of the samples, but we know how many there are.
   void ExpectTotalCount(const std::string& name,
                         base::HistogramBase::Count count);
 
@@ -65,10 +49,6 @@ class UMAHistogramHelper {
                        base::HistogramSamples& samples);
 
   DISALLOW_COPY_AND_ASSIGN(UMAHistogramHelper);
-
-  // The map from histogram names to their snapshots
-  std::map<std::string, linked_ptr<base::HistogramSamples> >
-      histogram_snapshots;
 };
 
 #endif  // CHROME_TEST_BASE_UMA_HISTOGRAM_HELPER_H_

components/signin/core/browser/account_reconcilor.cc

@@ -359,10 +359,8 @@ void AccountReconcilor::GoogleSignedOut(const std::string& username) {
 }
 
 void AccountReconcilor::PerformMergeAction(const std::string& account_id) {
-  if (!switches::IsNewProfileManagement()) {
-    MarkAccountAsAddedToCookie(account_id);
+  if (!switches::IsNewProfileManagement())
     return;
-  }
   VLOG(1) << "AccountReconcilor::PerformMergeAction: " << account_id;
   merge_session_helper_.LogIn(account_id);
 }
@@ -400,10 +398,8 @@ void AccountReconcilor::PerformFinishRemoveAction(
 
 void AccountReconcilor::PerformAddToChromeAction(const std::string& account_id,
                                                  int session_index) {
-  if (!switches::IsNewProfileManagement()) {
-    MarkAccountAsAddedToChrome(account_id);
+  if (!switches::IsNewProfileManagement())
     return;
-  }
   VLOG(1) << "AccountReconcilor::PerformAddToChromeAction:"
           << " account=" << account_id << " session_index=" << session_index;
 
@@ -710,9 +706,13 @@ void AccountReconcilor::ScheduleStartReconcileIfChromeAccountsChanged() {
   }
 }
 
-// Remove the account from the list that is being merged.
-void AccountReconcilor::MarkAccountAsAddedToCookie(
-    const std::string& account_id) {
+void AccountReconcilor::MergeSessionCompleted(
+    const std::string& account_id,
+    const GoogleServiceAuthError& error) {
+  VLOG(1) << "AccountReconcilor::MergeSessionCompleted: account_id="
+          << account_id;
+
+  // Remove the account from the list that is being merged.
   for (std::vector<std::string>::iterator i = add_to_cookie_.begin();
        i != add_to_cookie_.end();
        ++i) {
@@ -721,15 +721,7 @@ void AccountReconcilor::MarkAccountAsAddedToCookie(
       break;
     }
   }
-}
-
-void AccountReconcilor::MergeSessionCompleted(
-    const std::string& account_id,
-    const GoogleServiceAuthError& error) {
-  VLOG(1) << "AccountReconcilor::MergeSessionCompleted: account_id="
-          << account_id;
 
-  MarkAccountAsAddedToCookie(account_id);
   CalculateIfReconcileIsDone();
   ScheduleStartReconcileIfChromeAccountsChanged();
 }
@@ -756,9 +748,13 @@ void AccountReconcilor::PerformAddAccountToTokenService(
   token_service_->UpdateCredentials(account_id, refresh_token);
 }
 
-// Remove the account from the list that is being updated.
-void AccountReconcilor::MarkAccountAsAddedToChrome(
-    const std::string& account_id) {
+void AccountReconcilor::HandleRefreshTokenFetched(
+    const std::string& account_id,
+    const std::string& refresh_token) {
+  if (!refresh_token.empty()) {
+    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();
        i != add_to_chrome_.end();
@@ -768,14 +764,6 @@ void AccountReconcilor::MarkAccountAsAddedToChrome(
       break;
     }
   }
-}
-
-void AccountReconcilor::HandleRefreshTokenFetched(
-    const std::string& account_id,
-    const std::string& refresh_token) {
-  if (!refresh_token.empty())
-    PerformAddAccountToTokenService(account_id, refresh_token);
 
-  MarkAccountAsAddedToChrome(account_id);
   CalculateIfReconcileIsDone();
 }

components/signin/core/browser/account_reconcilor.h

@@ -121,8 +121,6 @@ class AccountReconcilor : public KeyedService,
   FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest, StartReconcileNoopWithDots);
   FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest, StartReconcileNoopMultiple);
   FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest, StartReconcileAddToCookie);
-  FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest,
-                           StartReconcileAddToCookieTwice);
   FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest, StartReconcileAddToChrome);
   FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest, StartReconcileBadPrimary);
   FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest, StartReconcileOnlyOnce);
@@ -174,10 +172,6 @@ class AccountReconcilor : public KeyedService,
       const GoogleServiceAuthError& error,
       const std::vector<std::pair<std::string, bool> >& accounts);
   void ValidateAccountsFromTokenService();
-  // Note internally that this |account_id| is added to the cookie jar.
-  void MarkAccountAsAddedToCookie(const std::string& account_id);
-  // Note internally that this |account_id| is added to the token service.
-  void MarkAccountAsAddedToChrome(const std::string& account_id);
 
   void OnCookieChanged(const net::CanonicalCookie* cookie);