Improve UMA tests; improve no-op reconcilor; new reconcilor test.

components/signin/core/browser/account_reconcilor.cc

 // Copyright 2014 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 "components/signin/core/browser/account_reconcilor.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/json/json_reader.h"
@@ skipping 341 matching lines @@
   VLOG(1) << "AccountReconcilor::GoogleSignedOut: signed out";
   gaia_fetcher_.reset();
   get_gaia_accounts_callbacks_.clear();
   AbortReconcile();
   UnregisterWithTokenService();
   UnregisterForCookieChanges();
   PerformLogoutAllAccountsAction();
 }
 
 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);
 }
 
 void AccountReconcilor::PerformStartRemoveAction(
     const std::string& account_id) {
   VLOG(1) << "AccountReconcilor::PerformStartRemoveAction: " << account_id;
   GetAccountsFromCookie(base::Bind(
       &AccountReconcilor::PerformFinishRemoveAction,
       base::Unretained(this),
@@ skipping 17 matching lines @@
          ++i) {
       accounts_only.push_back(i->first);
     }
     merge_session_helper_.LogOut(account_id, accounts_only);
   }
   // Wait for the next ReconcileAction if there is an error.
 }
 
 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;
 
 #if !defined(OS_ANDROID) && !defined(OS_IOS)
   refresh_token_fetchers_.push_back(
       new RefreshTokenFetcher(this, account_id, session_index));
 #endif
 }
 
 void AccountReconcilor::PerformLogoutAllAccountsAction() {
@@ skipping 273 matching lines @@
   std::vector<std::string> new_chrome_accounts(token_service_->GetAccounts());
   std::sort(reconciled_accounts.begin(), reconciled_accounts.end());
   std::sort(new_chrome_accounts.begin(), new_chrome_accounts.end());
   if (reconciled_accounts != new_chrome_accounts) {
     base::MessageLoop::current()->PostTask(
         FROM_HERE,
         base::Bind(&AccountReconcilor::StartReconcile, base::Unretained(this)));
   }
 }
 
-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) {
     if (account_id == *i) {
       add_to_cookie_.erase(i);
       break;
     }
   }
+}
 
+void AccountReconcilor::MergeSessionCompleted(
+    const std::string& account_id,
+    const GoogleServiceAuthError& error) {
+  VLOG(1) << "AccountReconcilor::MergeSessionCompleted: account_id="

[Alexei Svitkine (slow), 2014/06/12 17:09:14]

Does this need to be VLOG(1)? The string will live in the Chrome binary and
contribute to bloat. Can it be DVLOG() instead? (VLOG's are generally
discouraged - per a chromium-dev thread).

[Mike Lerman, 2014/06/12 18:12:16]

We do use these in release builds to debug what's going on in live situations,
so I'm inclined to leave it in. I checked with rogerta@, he says these are in by
design.

+          << account_id;
+
+  MarkAccountAsAddedToCookie(account_id);
   CalculateIfReconcileIsDone();
   ScheduleStartReconcileIfChromeAccountsChanged();
 }
 
 void AccountReconcilor::HandleSuccessfulAccountIdCheck(
     const std::string& account_id) {
   valid_chrome_accounts_.insert(account_id);
   FinishReconcile();
 }
 
 void AccountReconcilor::HandleFailedAccountIdCheck(
     const std::string& account_id) {
   invalid_chrome_accounts_.insert(account_id);
   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()) {
-    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();
        ++i) {
     if (gaia::AreEmailsSame(account_id, i->first)) {
       add_to_chrome_.erase(i);
       break;
     }
   }
+}
 
+void AccountReconcilor::HandleRefreshTokenFetched(
+    const std::string& account_id,
+    const std::string& refresh_token) {
+  if (!refresh_token.empty()) {

[Alexei Svitkine (slow), 2014/06/12 17:09:14]

Nit: No {}'s

[Mike Lerman, 2014/06/12 18:12:16]

Done.

+    PerformAddAccountToTokenService(account_id, refresh_token);
+  }
+
+  MarkAccountAsAddedToChrome(account_id);
   CalculateIfReconcileIsDone();
 }