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

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

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

2 files changed, 34 insertions, 16 deletions

diff --git a/components/signin/core/browser/account_reconcilor.cc b/components/signin/core/browser/account_reconcilor.cc
index 6156603..c5077bb1 100644
--- a/components/signin/core/browser/account_reconcilor.cc
+++ b/components/signin/core/browser/account_reconcilor.cc
@@ -359,8 +359,10 @@ void AccountReconcilor::GoogleSignedOut(const std::string& username) {
 }
 
 void AccountReconcilor::PerformMergeAction(const std::string& account_id) {
-  if (!switches::IsNewProfileManagement())
+  if (!switches::IsNewProfileManagement()) {
+    MarkAccountAsAddedToCookie(account_id);
     return;
+  }
   VLOG(1) << "AccountReconcilor::PerformMergeAction: " << account_id;
   merge_session_helper_.LogIn(account_id);
 }
@@ -398,8 +400,10 @@ void AccountReconcilor::PerformFinishRemoveAction(
 
 void AccountReconcilor::PerformAddToChromeAction(const std::string& account_id,
                                                  int session_index) {
-  if (!switches::IsNewProfileManagement())
+  if (!switches::IsNewProfileManagement()) {
+    MarkAccountAsAddedToChrome(account_id);
     return;
+  }
   VLOG(1) << "AccountReconcilor::PerformAddToChromeAction:"
           << " account=" << account_id << " session_index=" << session_index;
 
@@ -708,13 +712,9 @@ void AccountReconcilor::ScheduleStartReconcileIfChromeAccountsChanged() {
   }
 }
 
-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.
+// Remove the account from the list that is being merged.
+void AccountReconcilor::MarkAccountAsAddedToCookie(
+    const std::string& account_id) {
   for (std::vector<std::string>::iterator i = add_to_cookie_.begin();
        i != add_to_cookie_.end();
        ++i) {
@@ -723,7 +723,15 @@ void AccountReconcilor::MergeSessionCompleted(
       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();
 }
@@ -750,13 +758,9 @@ void AccountReconcilor::PerformAddAccountToTokenService(
   token_service_->UpdateCredentials(account_id, refresh_token);
 }
 
-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.
+// Remove the account from the list that is being updated.
+void AccountReconcilor::MarkAccountAsAddedToChrome(
+    const std::string& account_id) {
   for (std::vector<std::pair<std::string, int> >::iterator i =
            add_to_chrome_.begin();
        i != add_to_chrome_.end();
@@ -766,6 +770,14 @@ void AccountReconcilor::HandleRefreshTokenFetched(
       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();
 }
diff --git a/components/signin/core/browser/account_reconcilor.h b/components/signin/core/browser/account_reconcilor.h
index 4850c42..b5a3e37 100644
--- a/components/signin/core/browser/account_reconcilor.h
+++ b/components/signin/core/browser/account_reconcilor.h
@@ -121,6 +121,8 @@ 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);
@@ -172,6 +174,10 @@ 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);