1 files changed, 86 insertions, 42 deletions

diff --git a/chromium/components/signin/core/browser/account_reconcilor.cc b/chromium/components/signin/core/browser/account_reconcilor.cc
index 46f6ae05c21..8d62b9f3c17 100644
--- a/chromium/components/signin/core/browser/account_reconcilor.cc
+++ b/chromium/components/signin/core/browser/account_reconcilor.cc
@@ -31,9 +31,6 @@ using signin::AccountReconcilorDelegate;
 
 namespace {
 
-// String used for source parameter in GAIA cookie manager calls.
-const char kSource[] = "ChromiumAccountReconcilor";
-
 class AccountEqualToFunc {
  public:
   explicit AccountEqualToFunc(const gaia::ListedAccount& account)
@@ -95,7 +92,7 @@ AccountReconcilor::AccountReconcilor(
       registered_with_content_settings_(false),
       is_reconcile_started_(false),
       first_execution_(true),
-      error_during_last_reconcile_(false),
+      error_during_last_reconcile_(GoogleServiceAuthError::AuthErrorNone()),
       reconcile_is_noop_(true),
       chrome_accounts_changed_(false),
       account_reconcilor_lock_count_(0),
@@ -213,7 +210,8 @@ void AccountReconcilor::UnregisterWithCookieManagerService() {
 
 signin_metrics::AccountReconcilorState AccountReconcilor::GetState() {
   if (!is_reconcile_started_) {
-    return error_during_last_reconcile_
+    return (error_during_last_reconcile_.state() !=
+            GoogleServiceAuthError::State::NONE)
                ? signin_metrics::ACCOUNT_RECONCILOR_ERROR
                : signin_metrics::ACCOUNT_RECONCILOR_OK;
   }
@@ -273,7 +271,7 @@ void AccountReconcilor::OnAuthErrorChanged(
   // This should cover well the Mirror and Desktop Identity Consistency cases as
   // the cookies are always bound to the refresh tokens in these cases.
   if (error != GoogleServiceAuthError::AuthErrorNone())
-    cookie_manager_service_->TriggerListAccounts(kSource);
+    cookie_manager_service_->TriggerListAccounts(delegate_->GetGaiaApiSource());
 }
 
 void AccountReconcilor::PerformMergeAction(const std::string& account_id) {
@@ -283,7 +281,8 @@ void AccountReconcilor::PerformMergeAction(const std::string& account_id) {
     return;
   }
   VLOG(1) << "AccountReconcilor::PerformMergeAction: " << account_id;
-  cookie_manager_service_->AddAccountToCookie(account_id, kSource);
+  cookie_manager_service_->AddAccountToCookie(account_id,
+                                              delegate_->GetGaiaApiSource());
 }
 
 void AccountReconcilor::PerformLogoutAllAccountsAction() {
@@ -291,7 +290,7 @@ void AccountReconcilor::PerformLogoutAllAccountsAction() {
   if (!delegate_->IsAccountConsistencyEnforced())
     return;
   VLOG(1) << "AccountReconcilor::PerformLogoutAllAccountsAction";
-  cookie_manager_service_->LogOutAllAccounts(kSource);
+  cookie_manager_service_->LogOutAllAccounts(delegate_->GetGaiaApiSource());
 }
 
 void AccountReconcilor::StartReconcile() {
@@ -318,6 +317,15 @@ void AccountReconcilor::StartReconcile() {
     return;
   }
 
+  // Begin reconciliation. Reset initial states.
+  for (auto& observer : observer_list_)
+    observer.OnStartReconcile();
+  add_to_cookie_.clear();
+  reconcile_start_time_ = base::Time::Now();
+  is_reconcile_started_ = true;
+  error_during_last_reconcile_ = GoogleServiceAuthError::AuthErrorNone();
+  reconcile_is_noop_ = true;
+
   if (!timeout_.is_max()) {
     // This is NOT a repeating callback but to test it, we need a |MockTimer|,
     // which mocks |Timer| and not |OneShotTimer|. |Timer| currently does not
@@ -328,26 +336,20 @@ void AccountReconcilor::StartReconcile() {
                             base::Unretained(this)));
   }
 
-  if (token_service_->RefreshTokenHasError(
-          signin_manager_->GetAuthenticatedAccountId()) &&
+  const std::string& account_id = signin_manager_->GetAuthenticatedAccountId();
+  if (token_service_->RefreshTokenHasError(account_id) &&
       delegate_->ShouldAbortReconcileIfPrimaryHasError()) {
     VLOG(1) << "AccountReconcilor::StartReconcile: primary has error, abort.";
+    error_during_last_reconcile_ = token_service_->GetAuthError(account_id);
+    AbortReconcile();
     return;
   }
 
-  for (auto& observer : observer_list_)
-    observer.OnStartReconcile();
-  add_to_cookie_.clear();
-  reconcile_start_time_ = base::Time::Now();
-  is_reconcile_started_ = true;
-  error_during_last_reconcile_ = false;
-  reconcile_is_noop_ = true;
-
   // Rely on the GCMS to manage calls to and responses from ListAccounts.
   std::vector<gaia::ListedAccount> accounts;
   std::vector<gaia::ListedAccount> signed_out_accounts;
   if (cookie_manager_service_->ListAccounts(&accounts, &signed_out_accounts,
-                                            kSource)) {
+                                            delegate_->GetGaiaApiSource())) {
     OnGaiaAccountsInCookieUpdated(
         accounts, signed_out_accounts,
         GoogleServiceAuthError(GoogleServiceAuthError::NONE));
@@ -405,15 +407,21 @@ void AccountReconcilor::OnGaiaAccountsInCookieUpdated(
     if (delegate_->ShouldAbortReconcileIfPrimaryHasError() &&
         token_service_->RefreshTokenHasError(primary_account)) {
       VLOG(1) << "Primary account has error, abort.";
-      is_reconcile_started_ = false;
+      DCHECK(is_reconcile_started_);
+      AbortReconcile();
       return;
     }
 
     FinishReconcile(primary_account, LoadValidAccountsFromTokenService(),
                     std::move(verified_gaia_accounts));
   } else {
-    if (is_reconcile_started_)
-      error_during_last_reconcile_ = true;
+    // We may have seen a series of errors during reconciliation. Delegates may
+    // rely on the severity of the last seen error (see |OnReconcileError|) and
+    // hence do not override a persistent error, if we have seen one.
+    if (is_reconcile_started_ &&
+        !error_during_last_reconcile_.IsPersistentError()) {
+      error_during_last_reconcile_ = error;
+    }
     AbortReconcile();
   }
 }
@@ -446,7 +454,7 @@ std::vector<std::string> AccountReconcilor::LoadValidAccountsFromTokenService()
 void AccountReconcilor::OnReceivedManageAccountsResponse(
     signin::GAIAServiceType service_type) {
   if (service_type == signin::GAIA_SERVICE_TYPE_ADDSESSION) {
-    cookie_manager_service_->TriggerListAccounts(kSource);
+    cookie_manager_service_->TriggerListAccounts(delegate_->GetGaiaApiSource());
   }
 }
 
@@ -457,16 +465,7 @@ void AccountReconcilor::FinishReconcile(
   VLOG(1) << "AccountReconcilor::FinishReconcile";
   DCHECK(add_to_cookie_.empty());
 
-  std::string first_account = delegate_->GetFirstGaiaAccountForReconcile(
-      chrome_accounts, gaia_accounts, primary_account, first_execution_);
-  // |first_account| must be in |chrome_accounts|.
-  DCHECK(first_account.empty() ||
-         (std::find(chrome_accounts.begin(), chrome_accounts.end(),
-                    first_account) != chrome_accounts.end()));
   size_t number_gaia_accounts = gaia_accounts.size();
-  bool first_account_mismatch =
-      (number_gaia_accounts > 0) && (first_account != gaia_accounts[0].id);
-
   // If there are any accounts in the gaia cookie but not in chrome, then
   // those accounts need to be removed from the cookie.  This means we need
   // to blow the cookie away.
@@ -480,6 +479,12 @@ void AccountReconcilor::FinishReconcile(
     }
   }
 
+  std::string first_account = delegate_->GetFirstGaiaAccountForReconcile(
+      chrome_accounts, gaia_accounts, primary_account, first_execution_,
+      removed_from_cookie > 0);
+  bool first_account_mismatch =
+      (number_gaia_accounts > 0) && (first_account != gaia_accounts[0].id);
+
   bool rebuild_cookie = first_account_mismatch || (removed_from_cookie > 0);
   std::vector<gaia::ListedAccount> original_gaia_accounts = gaia_accounts;
   if (rebuild_cookie) {
@@ -493,13 +498,18 @@ void AccountReconcilor::FinishReconcile(
 
   if (first_account.empty()) {
     DCHECK(!delegate_->ShouldAbortReconcileIfPrimaryHasError());
-    // Gaia cookie has been cleared or was already empty.
-    DCHECK((first_account_mismatch && rebuild_cookie) ||
-           (number_gaia_accounts == 0));
     RevokeAllSecondaryTokens(primary_account, chrome_accounts);
   } else {
     // Create a list of accounts that need to be added to the Gaia cookie.
-    add_to_cookie_.push_back(first_account);
+    if (base::ContainsValue(chrome_accounts, first_account)) {
+      add_to_cookie_.push_back(first_account);
+    } else {
+      // If the first account is not empty and not in chrome_accounts, it is
+      // impossible to rebuild it. It must be already the current default
+      // account, and no logout can happen.
+      DCHECK_EQ(gaia_accounts[0].gaia_id, first_account);
+      DCHECK(!rebuild_cookie);
+    }
     for (size_t i = 0; i < chrome_accounts.size(); ++i) {
       if (chrome_accounts[i] != first_account)
         add_to_cookie_.push_back(chrome_accounts[i]);
@@ -544,15 +554,34 @@ void AccountReconcilor::AbortReconcile() {
   VLOG(1) << "AccountReconcilor::AbortReconcile: try again later";
   add_to_cookie_.clear();
   CalculateIfReconcileIsDone();
+
+  DCHECK(!is_reconcile_started_);
+  DCHECK(!timer_->IsRunning());
 }
 
 void AccountReconcilor::CalculateIfReconcileIsDone() {
   base::TimeDelta duration = base::Time::Now() - reconcile_start_time_;
   // Record the duration if reconciliation was underway and now it is over.
   if (is_reconcile_started_ && add_to_cookie_.empty()) {
-    signin_metrics::LogSigninAccountReconciliationDuration(duration,
-        !error_during_last_reconcile_);
+    bool was_last_reconcile_successful =
+        (error_during_last_reconcile_.state() ==
+         GoogleServiceAuthError::State::NONE);
+    signin_metrics::LogSigninAccountReconciliationDuration(
+        duration, was_last_reconcile_successful);
+
+    // Reconciliation has actually finished (and hence stop the timer), but it
+    // may have ended in some failures. Pass this information to the
+    // |delegate_|.
     timer_->Stop();
+    if (!was_last_reconcile_successful) {
+      // Note: This is the only call to |OnReconcileError| in this file. We MUST
+      // make sure that we do not call |OnReconcileError| multiple times in the
+      // same reconciliation batch.
+      // The enclosing if-condition |is_reconcile_started_ &&
+      // add_to_cookie_.empty()| represents the halting condition for one batch
+      // of reconciliation.
+      delegate_->OnReconcileError(error_during_last_reconcile_);
+    }
   }
 
   is_reconcile_started_ = !add_to_cookie_.empty();
@@ -623,8 +652,13 @@ void AccountReconcilor::OnAddAccountToCookieCompleted(
           << "Error was " << error.ToString();
   // Always listens to GaiaCookieManagerService. Only proceed if reconciling.
   if (is_reconcile_started_ && MarkAccountAsAddedToCookie(account_id)) {
-    if (error.state() != GoogleServiceAuthError::State::NONE)
-      error_during_last_reconcile_ = true;
+    // We may have seen a series of errors during reconciliation. Delegates may
+    // rely on the severity of the last seen error (see |OnReconcileError|) and
+    // hence do not override a persistent error, if we have seen one.
+    if (error.state() != GoogleServiceAuthError::State::NONE &&
+        !error_during_last_reconcile_.IsPersistentError()) {
+      error_during_last_reconcile_ = error;
+    }
     CalculateIfReconcileIsDone();
     ScheduleStartReconcileIfChromeAccountsChanged();
   }
@@ -677,7 +711,17 @@ void AccountReconcilor::set_timer_for_testing(
 }
 
 void AccountReconcilor::HandleReconcileTimeout() {
+  // A reconciliation was still succesfully in progress but could not complete
+  // in the given time. For a delegate, this is equivalent to a
+  // |GoogleServiceAuthError::State::CONNECTION_FAILED|.
+  if (error_during_last_reconcile_.state() ==
+      GoogleServiceAuthError::State::NONE) {
+    error_during_last_reconcile_ = GoogleServiceAuthError(
+        GoogleServiceAuthError::State::CONNECTION_FAILED);
+  }
+
+  // Will stop reconciliation and inform |delegate_| about
+  // |error_during_last_reconcile_|, through |CalculateIfReconcileIsDone|.
   AbortReconcile();
-  delegate_->OnReconcileError(
-      GoogleServiceAuthError(GoogleServiceAuthError::CONNECTION_FAILED));
+  DCHECK(!timer_->IsRunning());
 }