Handle account removal correctly on all platforms.

components/signin/core/browser/account_reconcilor.cc

Index: components/signin/core/browser/account_reconcilor.cc
diff --git a/components/signin/core/browser/account_reconcilor.cc b/components/signin/core/browser/account_reconcilor.cc
index b84beaa181b3b2c7adce73eebf2b7b7b858f33d4..57c5e0764b921c458364fd0dcfe0f8adef3d24b9 100644
--- a/components/signin/core/browser/account_reconcilor.cc
+++ b/components/signin/core/browser/account_reconcilor.cc
@@ -16,7 +16,6 @@
 #include "components/signin/core/browser/profile_oauth2_token_service.h"
 #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"
@@ -40,162 +39,25 @@ bool EmailEqualToFunc::operator()(
   return p1.second == p2.second && gaia::AreEmailsSame(p1.first, p2.first);
 }
 
-}  // namespace
-
-
-// Fetches a refresh token from the given session in the GAIA cookie.  This is
-// a best effort only.  If it should fail, another reconcile action will occur
-// shortly anyway.
-class AccountReconcilor::RefreshTokenFetcher
-    : public SigninOAuthHelper,
-      public SigninOAuthHelper::Consumer {
- public:
-  RefreshTokenFetcher(AccountReconcilor* reconcilor,
-                      const std::string& account_id,
-                      int session_index,
-                      const std::string& signin_scoped_device_id);
-  virtual ~RefreshTokenFetcher() {}
-
- private:
-  // Overridden from GaiaAuthConsumer:
-  virtual void OnSigninOAuthInformationAvailable(
-      const std::string& email,
-      const std::string& display_email,
-      const std::string& refresh_token) OVERRIDE;
-
-  // Called when an error occurs while getting the information.
-  virtual void OnSigninOAuthInformationFailure(
-      const GoogleServiceAuthError& error) OVERRIDE;
-
-  AccountReconcilor* reconcilor_;
-  const std::string account_id_;
-  int session_index_;
-
-  DISALLOW_COPY_AND_ASSIGN(RefreshTokenFetcher);
-};
-
-AccountReconcilor::RefreshTokenFetcher::RefreshTokenFetcher(
-    AccountReconcilor* reconcilor,
-    const std::string& account_id,
-    int session_index,
-    const std::string& signin_scoped_device_id)
-    : SigninOAuthHelper(reconcilor->client()->GetURLRequestContext(),
-                        base::IntToString(session_index),
-                        signin_scoped_device_id,
-                        this),
-      reconcilor_(reconcilor),
-      account_id_(account_id),
-      session_index_(session_index) {
-  DCHECK(reconcilor_);
-  DCHECK(!account_id.empty());
-}
-
-void AccountReconcilor::RefreshTokenFetcher::OnSigninOAuthInformationAvailable(
-    const std::string& email,
-    const std::string& display_email,
-    const std::string& refresh_token) {
-  VLOG(1) << "RefreshTokenFetcher::OnSigninOAuthInformationAvailable:"
-          << " account=" << account_id_ << " email=" << email
-          << " displayEmail=" << display_email;
-
-  // TODO(rogerta): because of the problem with email vs displayEmail and
-  // emails that have been canonicalized, the argument |email| is used here
-  // to make sure the correct string is used when calling the token service.
-  // This will be cleaned up when chrome moves to using gaia obfuscated id.
-  reconcilor_->HandleRefreshTokenFetched(email, refresh_token);
-}
-
-void AccountReconcilor::RefreshTokenFetcher::OnSigninOAuthInformationFailure(
-    const GoogleServiceAuthError& error) {
-  VLOG(1) << "RefreshTokenFetcher::OnSigninOAuthInformationFailure:"
-          << " account=" << account_id_ << " session_index=" << session_index_;
-  reconcilor_->HandleRefreshTokenFetched(account_id_, std::string());
-}
-
-bool AccountReconcilor::EmailLessFunc::operator()(const std::string& s1,
-                                                  const std::string& s2) const {
-  return gaia::CanonicalizeEmail(s1) < gaia::CanonicalizeEmail(s2);
-}
-
-class AccountReconcilor::UserIdFetcher
-    : public gaia::GaiaOAuthClient::Delegate {
+class AreEmailsSameFunc : public std::equal_to<std::string> {
  public:
-  UserIdFetcher(AccountReconcilor* reconcilor,
-                const std::string& access_token,
-                const std::string& account_id);
-
-  // Returns the scopes needed by the UserIdFetcher.
-  static OAuth2TokenService::ScopeSet GetScopes();
-
- private:
-  // Overriden from gaia::GaiaOAuthClient::Delegate.
-  virtual void OnGetUserIdResponse(const std::string& user_id) OVERRIDE;
-  virtual void OnOAuthError() OVERRIDE;
-  virtual void OnNetworkError(int response_code) OVERRIDE;
-
-  AccountReconcilor* const reconcilor_;
-  const std::string account_id_;
-  const std::string access_token_;
-  gaia::GaiaOAuthClient gaia_auth_client_;
-
-  DISALLOW_COPY_AND_ASSIGN(UserIdFetcher);
+  bool operator()(const std::string& p1,
+                  const std::string& p2) const;
 };
 
-AccountReconcilor::UserIdFetcher::UserIdFetcher(AccountReconcilor* reconcilor,
-                                                const std::string& access_token,
-                                                const std::string& account_id)
-    : reconcilor_(reconcilor),
-      account_id_(account_id),
-      access_token_(access_token),
-      gaia_auth_client_(reconcilor_->client()->GetURLRequestContext()) {
-  DCHECK(reconcilor_);
-  DCHECK(!account_id_.empty());
-
-  const int kMaxRetries = 5;
-  gaia_auth_client_.GetUserId(access_token_, kMaxRetries, this);
-}
-
-// static
-OAuth2TokenService::ScopeSet AccountReconcilor::UserIdFetcher::GetScopes() {
-  OAuth2TokenService::ScopeSet scopes;
-  scopes.insert("https://www.googleapis.com/auth/userinfo.profile");
-  return scopes;
-}
-
-void AccountReconcilor::UserIdFetcher::OnGetUserIdResponse(
-    const std::string& user_id) {
-  VLOG(1) << "AccountReconcilor::OnGetUserIdResponse: " << account_id_;
-
-  // HandleSuccessfulAccountIdCheck() may delete |this|, so call it last.
-  reconcilor_->HandleSuccessfulAccountIdCheck(account_id_);
+bool AreEmailsSameFunc::operator()(
+    const std::string& p1,
+    const std::string& p2) const {
+  return gaia::AreEmailsSame(p1, p2);
 }
 
-void AccountReconcilor::UserIdFetcher::OnOAuthError() {
-  VLOG(1) << "AccountReconcilor::OnOAuthError: " << account_id_;
-
-  // Invalidate the access token to force a refetch next time.
-  reconcilor_->token_service()->InvalidateToken(
-      account_id_, GetScopes(), access_token_);
-
-  // HandleFailedAccountIdCheck() may delete |this|, so call it last.
-  reconcilor_->HandleFailedAccountIdCheck(account_id_);
-}
-
-void AccountReconcilor::UserIdFetcher::OnNetworkError(int response_code) {
-  VLOG(1) << "AccountReconcilor::OnNetworkError: " << account_id_
-          << " response_code=" << response_code;
+}  // namespace
 
-  // TODO(rogerta): some response error should not be treated like
-  // permanent errors.  Figure out appropriate ones.
-  // HandleFailedAccountIdCheck() may delete |this|, so call it last.
-  reconcilor_->HandleFailedAccountIdCheck(account_id_);
-}
 
 AccountReconcilor::AccountReconcilor(ProfileOAuth2TokenService* token_service,
                                      SigninManagerBase* signin_manager,
                                      SigninClient* client)
-    : OAuth2TokenService::Consumer("account_reconcilor"),
-      token_service_(token_service),
+    : token_service_(token_service),
       signin_manager_(signin_manager),
       client_(client),
       merge_session_helper_(token_service_,
@@ -204,8 +66,7 @@ AccountReconcilor::AccountReconcilor(ProfileOAuth2TokenService* token_service,
       registered_with_token_service_(false),
       is_reconcile_started_(false),
       first_execution_(true),
-      are_gaia_accounts_set_(false),
-      requests_(NULL) {
+      are_gaia_accounts_set_(false) {
   VLOG(1) << "AccountReconcilor::AccountReconcilor";
 }
 
@@ -213,9 +74,6 @@ AccountReconcilor::~AccountReconcilor() {
   VLOG(1) << "AccountReconcilor::~AccountReconcilor";
   // Make sure shutdown was called first.
   DCHECK(!registered_with_token_service_);
-  DCHECK(!requests_);
-  DCHECK_EQ(0u, user_id_fetchers_.size());
-  DCHECK_EQ(0u, refresh_token_fetchers_.size());
 }
 
 void AccountReconcilor::Initialize(bool start_reconcile_if_tokens_available) {
@@ -242,7 +100,6 @@ void AccountReconcilor::Shutdown() {
   merge_session_helper_.RemoveObserver(this);
   gaia_fetcher_.reset();
   get_gaia_accounts_callbacks_.clear();
-  DeleteFetchers();
   UnregisterWithSigninManager();
   UnregisterWithTokenService();
   UnregisterForCookieChanges();
@@ -258,19 +115,6 @@ void AccountReconcilor::RemoveMergeSessionObserver(
   merge_session_helper_.RemoveObserver(observer);
 }
 
-void AccountReconcilor::DeleteFetchers() {
-  delete[] requests_;
-  requests_ = NULL;
-
-  user_id_fetchers_.clear();
-  refresh_token_fetchers_.clear();
-}
-
-bool AccountReconcilor::AreAllRefreshTokensChecked() const {
-  return chrome_accounts_.size() ==
-         (valid_chrome_accounts_.size() + invalid_chrome_accounts_.size());
-}
-
 void AccountReconcilor::RegisterForCookieChanges() {
   // First clear any existing registration to avoid DCHECKs that can otherwise
   // go off in some embedders on reauth (e.g., ChromeSigninClient).
@@ -332,11 +176,6 @@ void AccountReconcilor::OnCookieChanged(const net::CanonicalCookie* cookie) {
   }
 }
 
-void AccountReconcilor::OnRefreshTokenRevoked(const std::string& account_id) {
-  VLOG(1) << "AccountReconcilor::OnRefreshTokenRevoked: " << account_id;
-  PerformStartRemoveAction(account_id);
-}
-
 void AccountReconcilor::OnEndBatchChanges() {
   VLOG(1) << "AccountReconcilor::OnEndBatchChanges";
   StartReconcile();
@@ -370,54 +209,6 @@ void AccountReconcilor::PerformMergeAction(const std::string& 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),
-      account_id));
-}
-
-void AccountReconcilor::PerformFinishRemoveAction(
-    const std::string& account_id,
-    const GoogleServiceAuthError& error,
-    const std::vector<std::pair<std::string, bool> >& accounts) {
-  if (!switches::IsEnableAccountConsistency())
-    return;
-  VLOG(1) << "AccountReconcilor::PerformFinishRemoveAction:"
-          << " account=" << account_id << " error=" << error.ToString();
-  if (error.state() == GoogleServiceAuthError::NONE) {
-    AbortReconcile();
-    std::vector<std::string> accounts_only;
-    for (std::vector<std::pair<std::string, bool> >::const_iterator i =
-             accounts.begin();
-         i != accounts.end();
-         ++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,
-    const std::string& signin_scoped_device_id) {
-  if (!switches::IsEnableAccountConsistency()) {
-    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, signin_scoped_device_id));
-#endif
-}
-
 void AccountReconcilor::PerformLogoutAllAccountsAction() {
   if (!switches::IsEnableAccountConsistency())
     return;
@@ -445,11 +236,7 @@ void AccountReconcilor::StartReconcile() {
   // Reset state for validating oauth2 tokens.
   primary_account_.clear();
   chrome_accounts_.clear();
-  DeleteFetchers();
-  valid_chrome_accounts_.clear();
-  invalid_chrome_accounts_.clear();
   add_to_cookie_.clear();
-  add_to_chrome_.clear();
   ValidateAccountsFromTokenService();
 }
 
@@ -541,54 +328,6 @@ void AccountReconcilor::ValidateAccountsFromTokenService() {
   VLOG(1) << "AccountReconcilor::ValidateAccountsFromTokenService: "
           << "Chrome " << chrome_accounts_.size() << " accounts, "
           << "Primary is '" << primary_account_ << "'";
-
-  DCHECK(!requests_);
-  requests_ =
-      new scoped_ptr<OAuth2TokenService::Request>[chrome_accounts_.size()];
-  const OAuth2TokenService::ScopeSet scopes =
-      AccountReconcilor::UserIdFetcher::GetScopes();
-  for (size_t i = 0; i < chrome_accounts_.size(); ++i) {
-    requests_[i] =
-        token_service_->StartRequest(chrome_accounts_[i], scopes, this);
-  }
-
-  DCHECK_EQ(0u, user_id_fetchers_.size());
-  user_id_fetchers_.resize(chrome_accounts_.size());
-}
-
-void AccountReconcilor::OnGetTokenSuccess(
-    const OAuth2TokenService::Request* request,
-    const std::string& access_token,
-    const base::Time& expiration_time) {
-  size_t index;
-  for (index = 0; index < chrome_accounts_.size(); ++index) {
-    if (request == requests_[index].get())
-      break;
-  }
-  DCHECK(index < chrome_accounts_.size());
-
-  const std::string& account_id = chrome_accounts_[index];
-
-  VLOG(1) << "AccountReconcilor::OnGetTokenSuccess: valid " << account_id;
-
-  DCHECK(!user_id_fetchers_[index]);
-  user_id_fetchers_[index] = new UserIdFetcher(this, access_token, account_id);
-}
-
-void AccountReconcilor::OnGetTokenFailure(
-    const OAuth2TokenService::Request* request,
-    const GoogleServiceAuthError& error) {
-  size_t index;
-  for (index = 0; index < chrome_accounts_.size(); ++index) {
-    if (request == requests_[index].get())
-      break;
-  }
-  DCHECK(index < chrome_accounts_.size());
-
-  const std::string& account_id = chrome_accounts_[index];
-
-  VLOG(1) << "AccountReconcilor::OnGetTokenFailure: invalid " << account_id;
-  HandleFailedAccountIdCheck(account_id);
 }
 
 void AccountReconcilor::OnNewProfileManagementFlagChanged(
@@ -605,34 +344,32 @@ void AccountReconcilor::OnNewProfileManagementFlagChanged(
 }
 
 void AccountReconcilor::FinishReconcile() {
-  // Make sure that the process of validating the gaia cookie and the oauth2
-  // tokens individually is done before proceeding with reconciliation.
-  if (!are_gaia_accounts_set_ || !AreAllRefreshTokensChecked())
-    return;
-
   VLOG(1) << "AccountReconcilor::FinishReconcile";
-
-  DeleteFetchers();
-
+  DCHECK(are_gaia_accounts_set_);
   DCHECK(add_to_cookie_.empty());
-  DCHECK(add_to_chrome_.empty());
   int number_gaia_accounts = gaia_accounts_.size();
-  bool are_primaries_equal =
-      number_gaia_accounts > 0 &&
+  bool are_primaries_equal = number_gaia_accounts > 0 &&
       gaia::AreEmailsSame(primary_account_, gaia_accounts_[0].first);
 
-
-  if (are_primaries_equal) {
-    // Determine if we need to merge accounts from gaia cookie to chrome.
-    for (size_t i = 0; i < gaia_accounts_.size(); ++i) {
-      const std::string& gaia_account = gaia_accounts_[i].first;
-      if (gaia_accounts_[i].second &&
-          valid_chrome_accounts_.find(gaia_account) ==
-              valid_chrome_accounts_.end()) {
-        add_to_chrome_.push_back(std::make_pair(gaia_account, i));
-      }
+  // 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.
+  int removed_from_cookie = 0;
+  for (size_t i = 0; i < gaia_accounts_.size(); ++i) {
+    const std::string& gaia_account = gaia_accounts_[i].first;
+    if (gaia_accounts_[i].second &&
+        chrome_accounts_.end() ==
+            std::find_if(chrome_accounts_.begin(),
+                         chrome_accounts_.end(),
+                         std::bind1st(AreEmailsSameFunc(), gaia_account))) {
+      ++removed_from_cookie;
     }
-  } else {
+  }
+
+  bool rebuild_cookie = !are_primaries_equal || removed_from_cookie > 0;
+  std::vector<std::pair<std::string, bool> > original_gaia_accounts =
+      gaia_accounts_;
+  if (rebuild_cookie) {
     VLOG(1) << "AccountReconcilor::FinishReconcile: rebuild cookie";
     // Really messed up state.  Blow away the gaia cookie completely and
     // rebuild it, making sure the primary account as specified by the
@@ -645,11 +382,9 @@ void AccountReconcilor::FinishReconcile() {
   // The primary account must be first to make sure it becomes the default
   // account in the case where chrome is completely rebuilding the cookie.
   add_to_cookie_.push_back(primary_account_);
-  for (EmailSet::const_iterator i = valid_chrome_accounts_.begin();
-        i != valid_chrome_accounts_.end();
-        ++i) {
-    if (*i != primary_account_)
-      add_to_cookie_.push_back(*i);
+  for (size_t i = 0; i < chrome_accounts_.size(); ++i) {
+    if (chrome_accounts_[i] != primary_account_)
+      add_to_cookie_.push_back(chrome_accounts_[i]);
   }
 
   // For each account known to chrome, PerformMergeAction() if the account is
@@ -672,30 +407,25 @@ void AccountReconcilor::FinishReconcile() {
           GoogleServiceAuthError::AuthErrorNone());
     } else {
       PerformMergeAction(add_to_cookie_copy[i]);
-      added_to_cookie++;
+      if (original_gaia_accounts.end() ==
+              std::find_if(original_gaia_accounts.begin(),
+                           original_gaia_accounts.end(),
+                           std::bind1st(EmailEqualToFunc(),
+                                        std::make_pair(add_to_cookie_copy[i],
+                                                       true)))) {
+        added_to_cookie++;
+      }
     }
   }
 
   // Log whether the external connection checks were completed when we tried
   // to add the accounts to the cookie.
-  if (added_to_cookie > 0)
+  if (rebuild_cookie || added_to_cookie > 0)
     signin_metrics::LogExternalCcResultFetches(external_cc_result_completed);
 
-  std::string signin_scoped_device_id = client_->GetSigninScopedDeviceId();
-  // For each account in the gaia cookie not known to chrome,
-  // PerformAddToChromeAction. Make a copy of |add_to_chrome| since calls to
-  // PerformAddToChromeAction() may modify this array.
-  std::vector<std::pair<std::string, int> > add_to_chrome_copy = add_to_chrome_;
-  for (std::vector<std::pair<std::string, int> >::const_iterator i =
-           add_to_chrome_copy.begin();
-       i != add_to_chrome_copy.end();
-       ++i) {
-    PerformAddToChromeAction(i->first, i->second, signin_scoped_device_id);
-  }
-
-  signin_metrics::LogSigninAccountReconciliation(valid_chrome_accounts_.size(),
+  signin_metrics::LogSigninAccountReconciliation(chrome_accounts_.size(),
                                                  added_to_cookie,
-                                                 add_to_chrome_.size(),
+                                                 removed_from_cookie,
                                                  are_primaries_equal,
                                                  first_execution_,
                                                  number_gaia_accounts);
@@ -706,14 +436,12 @@ void AccountReconcilor::FinishReconcile() {
 
 void AccountReconcilor::AbortReconcile() {
   VLOG(1) << "AccountReconcilor::AbortReconcile: we'll try again later";
-  DeleteFetchers();
   add_to_cookie_.clear();
-  add_to_chrome_.clear();
   CalculateIfReconcileIsDone();
 }
 
 void AccountReconcilor::CalculateIfReconcileIsDone() {
-  is_reconcile_started_ = !add_to_cookie_.empty() || !add_to_chrome_.empty();
+  is_reconcile_started_ = !add_to_cookie_.empty();
   if (!is_reconcile_started_)
     VLOG(1) << "AccountReconcilor::CalculateIfReconcileIsDone: done";
 }
@@ -753,56 +481,10 @@ void AccountReconcilor::MergeSessionCompleted(
     const std::string& account_id,
     const GoogleServiceAuthError& error) {
   VLOG(1) << "AccountReconcilor::MergeSessionCompleted: account_id="
-          << account_id;
+          << account_id << " error=" << error.ToString();
 
   if (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::IsEnableAccountConsistency())
-    return;
-  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) {
-  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())
-    PerformAddAccountToTokenService(account_id, refresh_token);
-
-  MarkAccountAsAddedToChrome(account_id);
-  CalculateIfReconcileIsDone();
-}