[signin] SigninErrorController observes the TokenService

To simplify authentication error handling, this CL updates the
SigninErrorController to get its error state from the token service by
observing it.
This removes some coupling between the classes, and removes the
AuthStatusProvider mechanism, leading to an overall simplification of the code.

TBR=eugenebut

Bug: 836212
Change-Id: Ie9b849fcd804d4fe6b4787b277b27fb848cdf2dc
Reviewed-on: https://chromium-review.googlesource.com/c/1070154
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612610}

chrome/browser/chromeos/oauth2_token_service_delegate.cc

@@ -12,7 +12,6 @@
 #include "base/logging.h"
 #include "chrome/browser/chromeos/account_mapper_util.h"
 #include "chromeos/account_manager/account_manager.h"
-#include "components/signin/core/browser/signin_error_controller.h"
 #include "content/public/browser/network_service_instance.h"
 #include "google_apis/gaia/oauth2_access_token_fetcher_immediate_error.h"
 #include "net/base/backoff_entry.h"
@@ -41,53 +40,12 @@ const net::BackoffEntry::Policy kBackoffPolicy = {
 
 }  // namespace
 
-class ChromeOSOAuth2TokenServiceDelegate::AccountErrorStatus
-    : public SigninErrorController::AuthStatusProvider {
- public:
-  AccountErrorStatus(SigninErrorController* signin_error_controller,
-                     const std::string& account_id)
-      : signin_error_controller_(signin_error_controller),
-        account_id_(account_id) {
-    signin_error_controller_->AddProvider(this);
-  }
-
-  ~AccountErrorStatus() override {
-    signin_error_controller_->RemoveProvider(this);
-  }
-
-  // SigninErrorController::AuthStatusProvider overrides.
-  std::string GetAccountId() const override { return account_id_; }
-
-  GoogleServiceAuthError GetAuthStatus() const override {
-    return last_auth_error_;
-  }
-
-  void SetLastAuthError(const GoogleServiceAuthError& error) {
-    last_auth_error_ = error;
-    signin_error_controller_->AuthStatusChanged();
-  }
-
- private:
-  // A non-owning pointer to |SigninErrorController|.
-  SigninErrorController* const signin_error_controller_;
-
-  // The account id being tracked by |this| instance of |AccountErrorStatus|.
-  const std::string account_id_;
-
-  // The last auth error seen for |account_id_|.
-  GoogleServiceAuthError last_auth_error_;
-
-  DISALLOW_COPY_AND_ASSIGN(AccountErrorStatus);
-};
-
 ChromeOSOAuth2TokenServiceDelegate::ChromeOSOAuth2TokenServiceDelegate(
     AccountTrackerService* account_tracker_service,
-    chromeos::AccountManager* account_manager,
-    SigninErrorController* signin_error_controller)
+    chromeos::AccountManager* account_manager)
     : account_mapper_util_(
           std::make_unique<AccountMapperUtil>(account_tracker_service)),
       account_manager_(account_manager),
-      signin_error_controller_(signin_error_controller),
       backoff_entry_(&kBackoffPolicy),
       backoff_error_(GoogleServiceAuthError::NONE),
       weak_factory_(this) {
@@ -113,12 +71,12 @@ ChromeOSOAuth2TokenServiceDelegate::CreateAccessTokenFetcher(
   // We will reject the request if we are facing a persistent error for this
   // account.
   auto it = errors_.find(account_id);
-  if (it != errors_.end() && it->second->GetAuthStatus().IsPersistentError()) {
+  if (it != errors_.end() && it->second.last_auth_error.IsPersistentError()) {
     VLOG(1) << "Request for token has been rejected due to persistent error #"
-            << it->second->GetAuthStatus().state();
+            << it->second.last_auth_error.state();
     // |OAuth2TokenService| will manage the lifetime of this pointer.
     return new OAuth2AccessTokenFetcherImmediateError(
-        consumer, it->second->GetAuthStatus());
+        consumer, it->second.last_auth_error);
   }
   // Or when we need to backoff.
   if (backoff_entry_.ShouldRejectRequest()) {
@@ -160,22 +118,16 @@ void ChromeOSOAuth2TokenServiceDelegate::UpdateAuthError(
   }
 
   auto it = errors_.find(account_id);
-  if (error.state() == GoogleServiceAuthError::NONE) {
-    // If the error status is NONE, and we were not tracking any error anyways,
-    // just ignore the update.
-    // Otherwise, delete the error tracking for this account.
-    if (it != errors_.end()) {
+  if ((it != errors_.end())) {
+    // Update the existing error.
+    if (error.state() == GoogleServiceAuthError::NONE)
       errors_.erase(it);
-      FireAuthErrorChanged(account_id, error);
-    }
-  } else if ((it == errors_.end()) || (it->second->GetAuthStatus() != error)) {
-    // Error status is not NONE. We need to start tracking the account / update
-    // the last seen error, if it is different.
-    if (it == errors_.end()) {
-      errors_.emplace(account_id, std::make_unique<AccountErrorStatus>(
-                                      signin_error_controller_, account_id));
-    }
-    errors_[account_id]->SetLastAuthError(error);
+    else
+      it->second.last_auth_error = error;
+    FireAuthErrorChanged(account_id, error);
+  } else if (error.state() != GoogleServiceAuthError::NONE) {
+    // Add a new error.
+    errors_.emplace(account_id, AccountErrorStatus{error});
     FireAuthErrorChanged(account_id, error);
   }
 }
@@ -184,7 +136,7 @@ GoogleServiceAuthError ChromeOSOAuth2TokenServiceDelegate::GetAuthError(
     const std::string& account_id) const {
   auto it = errors_.find(account_id);
   if (it != errors_.end()) {
-    return it->second->GetAuthStatus();
+    return it->second.last_auth_error;
   }
 
   return GoogleServiceAuthError::AuthErrorNone();

chrome/browser/chromeos/oauth2_token_service_delegate.h

@@ -19,7 +19,6 @@
 #include "services/network/public/cpp/network_connection_tracker.h"
 
 class AccountTrackerService;
-class SigninErrorController;
 
 namespace chromeos {
 
@@ -30,15 +29,13 @@ class ChromeOSOAuth2TokenServiceDelegate
       public AccountManager::Observer,
       public network::NetworkConnectionTracker::NetworkConnectionObserver {
  public:
-  // Accepts non-owning pointers to |AccountTrackerService|, |AccountManager|
-  // and |SigninErrorController|. |AccountTrackerService| and
-  // |SigninErrorController| are |KeyedService|s. |AccountManager| transitively
-  // belongs to |g_browser_process|. They outlive (as they must) |this|
-  // delegate.
+  // Accepts non-owning pointers to |AccountTrackerService| and
+  // |AccountManager|. |AccountTrackerService| is a |KeyedService|.
+  // |AccountManager| transitively belongs to |g_browser_process|. They outlive
+  // (as they must) |this| delegate.
   ChromeOSOAuth2TokenServiceDelegate(
       AccountTrackerService* account_tracker_service,
-      AccountManager* account_manager,
-      SigninErrorController* signin_error_controller);
+      AccountManager* account_manager);
   ~ChromeOSOAuth2TokenServiceDelegate() override;
 
   // OAuth2TokenServiceDelegate overrides.
@@ -75,10 +72,10 @@ class ChromeOSOAuth2TokenServiceDelegate
                            BackOffIsResetOnNetworkChange);
 
   // A utility class to keep track of |GoogleServiceAuthError|s for an account.
-  // This is used for providing account error status reports to
-  // |SigninErrorController| and for firing
-  // |OAuth2TokenService::Observer::OnAuthErrorChanged|.
-  class AccountErrorStatus;
+  struct AccountErrorStatus {
+    // The last auth error seen.
+    GoogleServiceAuthError last_auth_error;
+  };
 
   // Callback handler for |AccountManager::GetAccounts|.
   void GetAccountsCallback(
@@ -90,14 +87,11 @@ class ChromeOSOAuth2TokenServiceDelegate
   // throughout the lifetime of a user session.
   AccountManager* account_manager_;
 
-  // A non-owning pointer to |SigninErrorController|.
-  SigninErrorController* const signin_error_controller_;
-
   // A cache of AccountKeys.
   std::set<AccountManager::AccountKey> account_keys_;
 
   // A map from account id to the last seen error for that account.
-  std::map<std::string, std::unique_ptr<AccountErrorStatus>> errors_;
+  std::map<std::string, AccountErrorStatus> errors_;
 
   // Used to rate-limit token fetch requests so as to not overload the server.
   net::BackoffEntry backoff_entry_;

chrome/browser/chromeos/oauth2_token_service_delegate_unittest.cc

@@ -17,7 +17,6 @@
 #include "chromeos/account_manager/account_manager.h"
 #include "components/signin/core/browser/account_info.h"
 #include "components/signin/core/browser/account_tracker_service.h"
-#include "components/signin/core/browser/signin_error_controller.h"
 #include "components/signin/core/browser/signin_pref_names.h"
 #include "components/signin/core/browser/test_signin_client.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
@@ -107,43 +106,13 @@ class TokenServiceObserver : public OAuth2TokenService::Observer {
   std::vector<std::vector<std::string>> batch_change_records_;
 };
 
-class SigninErrorObserver : public SigninErrorController::Observer {
- public:
-  explicit SigninErrorObserver(SigninErrorController* signin_error_controller)
-      : signin_error_controller_(signin_error_controller) {
-    signin_error_controller_->AddObserver(this);
-  }
-
-  ~SigninErrorObserver() override {
-    signin_error_controller_->RemoveObserver(this);
-  }
-
-  // |SigninErrorController::Observer| overrides.
-  void OnErrorChanged() override {
-    last_err_account_id_ = signin_error_controller_->error_account_id();
-    last_err_ = signin_error_controller_->auth_error();
-  }
-
-  std::string last_err_account_id_;
-  GoogleServiceAuthError last_err_ = GoogleServiceAuthError::AuthErrorNone();
-
- private:
-  // Non-owning pointer to |SigninErrorController|.
-  SigninErrorController* const signin_error_controller_;
-
-  DISALLOW_COPY_AND_ASSIGN(SigninErrorObserver);
-};
-
 }  // namespace
 
 class CrOSOAuthDelegateTest : public testing::Test {
  public:
   CrOSOAuthDelegateTest()
       : scoped_task_environment_(
-            base::test::ScopedTaskEnvironment::MainThreadType::UI),
-        signin_error_controller_(
-            SigninErrorController::AccountMode::ANY_ACCOUNT),
-        signin_error_observer_(&signin_error_controller_) {}
+            base::test::ScopedTaskEnvironment::MainThreadType::UI) {}
   ~CrOSOAuthDelegateTest() override = default;
 
  protected:
@@ -168,8 +137,7 @@ class CrOSOAuthDelegateTest : public testing::Test {
     gaia_account_key_ = {account_info_.gaia, ACCOUNT_TYPE_GAIA};
 
     delegate_ = std::make_unique<ChromeOSOAuth2TokenServiceDelegate>(
-        &account_tracker_service_, &account_manager_,
-        &signin_error_controller_);
+        &account_tracker_service_, &account_manager_);
     delegate_->LoadCredentials(
         account_info_.account_id /* primary_account_id */);
   }
@@ -216,8 +184,6 @@ class CrOSOAuthDelegateTest : public testing::Test {
   AccountManager::AccountKey gaia_account_key_;
   AccountTrackerService account_tracker_service_;
   AccountManager account_manager_;
-  SigninErrorController signin_error_controller_;
-  SigninErrorObserver signin_error_observer_;
   std::unique_ptr<ChromeOSOAuth2TokenServiceDelegate> delegate_;
   AccountManager::DelayNetworkCallRunner immediate_callback_runner_ =
       base::BindRepeating(
@@ -336,7 +302,7 @@ TEST_F(CrOSOAuthDelegateTest, BatchChangeObserversAreNotifiedOncePerBatch) {
 
   // Register callbacks before AccountManager has been fully initialized.
   auto delegate = std::make_unique<ChromeOSOAuth2TokenServiceDelegate>(
-      &account_tracker_service_, &account_manager, &signin_error_controller_);
+      &account_tracker_service_, &account_manager);
   delegate->LoadCredentials(account1.account_id /* primary_account_id */);
   TokenServiceObserver observer;
   delegate->AddObserver(&observer);
@@ -416,28 +382,19 @@ TEST_F(CrOSOAuthDelegateTest,
   auto error =
       GoogleServiceAuthError(GoogleServiceAuthError::State::SERVICE_ERROR);
 
-  EXPECT_EQ(GoogleServiceAuthError::AuthErrorNone(),
-            signin_error_observer_.last_err_);
   delegate_->UpdateAuthError(account_info_.account_id, error);
 
   EXPECT_EQ(error, delegate_->GetAuthError(account_info_.account_id));
-  EXPECT_EQ(account_info_.account_id,
-            signin_error_observer_.last_err_account_id_);
-  EXPECT_EQ(error, signin_error_observer_.last_err_);
 }
 
 TEST_F(CrOSOAuthDelegateTest, TransientErrorsAreNotShown) {
   auto transient_error = GoogleServiceAuthError(
       GoogleServiceAuthError::State::SERVICE_UNAVAILABLE);
-  EXPECT_EQ(GoogleServiceAuthError::AuthErrorNone(),
-            signin_error_observer_.last_err_);
   EXPECT_EQ(GoogleServiceAuthError::AuthErrorNone(),
             delegate_->GetAuthError(account_info_.account_id));
 
   delegate_->UpdateAuthError(account_info_.account_id, transient_error);
 
-  EXPECT_EQ(GoogleServiceAuthError::AuthErrorNone(),
-            signin_error_observer_.last_err_);
   EXPECT_EQ(GoogleServiceAuthError::AuthErrorNone(),
             delegate_->GetAuthError(account_info_.account_id));
 }

chrome/browser/signin/chrome_signin_client_unittest.cc

@@ -23,6 +23,7 @@
 #include "components/signin/core/browser/signin_error_controller.h"
 #include "content/public/browser/network_service_instance.h"
 #include "content/public/test/test_browser_thread_bundle.h"
+#include "google_apis/gaia/fake_oauth2_token_service.h"
 #include "services/network/test/test_network_connection_tracker.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -177,14 +178,18 @@ class ChromeSigninClientSignoutTest : public BrowserWithTestWindowTest {
   }
 
   void CreateClient(Profile* profile) {
-    client_.reset(new MockChromeSigninClient(profile));
-    SigninErrorController* controller = new SigninErrorController(
-        SigninErrorController::AccountMode::ANY_ACCOUNT);
-    fake_controller_.reset(controller);
+    if (fake_controller_)
+      fake_controller_->Shutdown();
+
+    client_ = std::make_unique<MockChromeSigninClient>(profile);
+    token_service_ = std::make_unique<FakeOAuth2TokenService>();
+    fake_controller_ = std::make_unique<SigninErrorController>(
+        SigninErrorController::AccountMode::ANY_ACCOUNT, token_service_.get());
   }
 
-  std::unique_ptr<SigninErrorController> fake_controller_;
   std::unique_ptr<MockChromeSigninClient> client_;
+  std::unique_ptr<FakeOAuth2TokenService> token_service_;
+  std::unique_ptr<SigninErrorController> fake_controller_;
   std::unique_ptr<MockSigninManager> manager_;
 };
 

chrome/browser/signin/dice_response_handler_unittest.cc

@@ -124,7 +124,8 @@ class DiceResponseHandlerTest : public testing::Test,
         token_service_(&pref_service_,
                        std::make_unique<FakeOAuth2TokenServiceDelegate>()),
         signin_error_controller_(
-            SigninErrorController::AccountMode::PRIMARY_ACCOUNT),
+            SigninErrorController::AccountMode::PRIMARY_ACCOUNT,
+            &token_service_),
         signin_manager_(&signin_client_,
                         &token_service_,
                         &account_tracker_service_,

chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.cc

@@ -324,47 +324,9 @@ void MutableProfileOAuth2TokenServiceDelegate::RevokeServerRefreshToken::
       }));
 }
 
-MutableProfileOAuth2TokenServiceDelegate::AccountStatus::AccountStatus(
-    SigninErrorController* signin_error_controller,
-    const std::string& account_id,
-    const std::string& refresh_token)
-    : signin_error_controller_(signin_error_controller),
-      account_id_(account_id),
-      refresh_token_(refresh_token),
-      last_auth_error_(GoogleServiceAuthError::NONE) {
-  DCHECK(signin_error_controller_);
-  DCHECK(!account_id_.empty());
-  DCHECK(!refresh_token.empty());
-}
-
-void MutableProfileOAuth2TokenServiceDelegate::AccountStatus::Initialize() {
-  signin_error_controller_->AddProvider(this);
-}
-
-MutableProfileOAuth2TokenServiceDelegate::AccountStatus::~AccountStatus() {
-  signin_error_controller_->RemoveProvider(this);
-}
-
-void MutableProfileOAuth2TokenServiceDelegate::AccountStatus::SetLastAuthError(
-    const GoogleServiceAuthError& error) {
-  last_auth_error_ = error;
-  signin_error_controller_->AuthStatusChanged();
-}
-
-std::string
-MutableProfileOAuth2TokenServiceDelegate::AccountStatus::GetAccountId() const {
-  return account_id_;
-}
-
-GoogleServiceAuthError
-MutableProfileOAuth2TokenServiceDelegate::AccountStatus::GetAuthStatus() const {
-  return last_auth_error_;
-}
-
 MutableProfileOAuth2TokenServiceDelegate::
     MutableProfileOAuth2TokenServiceDelegate(
         SigninClient* client,
-        SigninErrorController* signin_error_controller,
         AccountTrackerService* account_tracker_service,
         scoped_refptr<TokenWebData> token_web_data,
         signin::AccountConsistencyMethod account_consistency,
@@ -374,7 +336,6 @@ MutableProfileOAuth2TokenServiceDelegate::
       backoff_entry_(&backoff_policy_),
       backoff_error_(GoogleServiceAuthError::NONE),
       client_(client),
-      signin_error_controller_(signin_error_controller),
       account_tracker_service_(account_tracker_service),
       token_web_data_(token_web_data),
       account_consistency_(account_consistency),
@@ -382,7 +343,6 @@ MutableProfileOAuth2TokenServiceDelegate::
       can_revoke_credentials_(can_revoke_credentials) {
   VLOG(1) << "MutablePO2TS::MutablePO2TS";
   DCHECK(client);
-  DCHECK(signin_error_controller);
   DCHECK(account_tracker_service_);
   // It's okay to fill the backoff policy after being used in construction.
   backoff_policy_.num_errors_to_ignore = 0;
@@ -415,11 +375,11 @@ MutableProfileOAuth2TokenServiceDelegate::CreateAccessTokenFetcher(
     OAuth2AccessTokenConsumer* consumer) {
   ValidateAccountId(account_id);
   // check whether the account has persistent error.
-  if (refresh_tokens_[account_id]->GetAuthStatus().IsPersistentError()) {
+  if (refresh_tokens_[account_id].last_auth_error.IsPersistentError()) {
     VLOG(1) << "Request for token has been rejected due to persistent error #"
-            << refresh_tokens_[account_id]->GetAuthStatus().state();
+            << refresh_tokens_[account_id].last_auth_error.state();
     return new OAuth2AccessTokenFetcherImmediateError(
-        consumer, refresh_tokens_[account_id]->GetAuthStatus());
+        consumer, refresh_tokens_[account_id].last_auth_error);
   }
   if (backoff_entry_.ShouldRejectRequest()) {
     VLOG(1) << "Request for token has been rejected due to backoff rules from"
@@ -436,7 +396,7 @@ GoogleServiceAuthError MutableProfileOAuth2TokenServiceDelegate::GetAuthError(
     const std::string& account_id) const {
   auto it = refresh_tokens_.find(account_id);
   return (it == refresh_tokens_.end()) ? GoogleServiceAuthError::AuthErrorNone()
-                                       : it->second->GetAuthStatus();
+                                       : it->second.last_auth_error;
 }
 
 void MutableProfileOAuth2TokenServiceDelegate::UpdateAuthError(
@@ -464,9 +424,9 @@ void MutableProfileOAuth2TokenServiceDelegate::UpdateAuthError(
     return;
   }
 
-  AccountStatus* status = refresh_tokens_[account_id].get();
-  if (error != status->GetAuthStatus()) {
-    status->SetLastAuthError(error);
+  AccountStatus* status = &refresh_tokens_[account_id];
+  if (error != status->last_auth_error) {
+    status->last_auth_error = error;
     FireAuthErrorChanged(account_id, error);
   }
 }
@@ -475,11 +435,10 @@ std::string MutableProfileOAuth2TokenServiceDelegate::GetTokenForMultilogin(
     const std::string& account_id) const {
   auto iter = refresh_tokens_.find(account_id);
   if (iter == refresh_tokens_.end() ||
-      iter->second->GetAuthStatus() !=
-          GoogleServiceAuthError::AuthErrorNone()) {
+      iter->second.last_auth_error != GoogleServiceAuthError::AuthErrorNone()) {
     return std::string();
   }
-  const std::string& refresh_token = iter->second->refresh_token();
+  const std::string& refresh_token = iter->second.refresh_token;
   DCHECK(!refresh_token.empty());
   return refresh_token;
 }
@@ -494,7 +453,7 @@ std::string MutableProfileOAuth2TokenServiceDelegate::GetRefreshToken(
     const std::string& account_id) const {
   auto iter = refresh_tokens_.find(account_id);
   if (iter != refresh_tokens_.end()) {
-    const std::string refresh_token = iter->second->refresh_token();
+    const std::string refresh_token = iter->second.refresh_token;
     DCHECK(!refresh_token.empty());
     return refresh_token;
   }
@@ -806,7 +765,7 @@ void MutableProfileOAuth2TokenServiceDelegate::UpdateCredentialsInMemory(
   // If token present, and different from the new one, cancel its requests,
   // and clear the entries in cache related to that account.
   if (refresh_token_present) {
-    DCHECK_NE(refresh_token, refresh_tokens_[account_id]->refresh_token());
+    DCHECK_NE(refresh_token, refresh_tokens_[account_id].refresh_token);
     VLOG(1) << "MutablePO2TS::UpdateCredentials; Refresh Token was present. "
             << "account_id=" << account_id;
 
@@ -822,9 +781,9 @@ void MutableProfileOAuth2TokenServiceDelegate::UpdateCredentialsInMemory(
     // would also be invalidated server-side).
     // See http://crbug.com/865189 for more information about this regression.
     if (is_refresh_token_invalidated)
-      RevokeCredentialsOnServer(refresh_tokens_[account_id]->refresh_token());
+      RevokeCredentialsOnServer(refresh_tokens_[account_id].refresh_token);
 
-    refresh_tokens_[account_id]->set_refresh_token(refresh_token);
+    refresh_tokens_[account_id].refresh_token = refresh_token;
     UpdateAuthError(account_id, error);
   } else {
     VLOG(1) << "MutablePO2TS::UpdateCredentials; Refresh Token was absent. "
@@ -885,7 +844,7 @@ void MutableProfileOAuth2TokenServiceDelegate::RevokeCredentials(
   if (refresh_tokens_.count(account_id) > 0) {
     VLOG(1) << "MutablePO2TS::RevokeCredentials for account_id=" << account_id;
     ScopedBatchChange batch(this);
-    const std::string& token = refresh_tokens_[account_id]->refresh_token();
+    const std::string& token = refresh_tokens_[account_id].refresh_token;
     RecordTokenRevoked(token);
     RevokeCredentialsOnServer(token);
     refresh_tokens_.erase(account_id);
@@ -949,12 +908,8 @@ void MutableProfileOAuth2TokenServiceDelegate::AddAccountStatus(
     const std::string& refresh_token,
     const GoogleServiceAuthError& error) {
   DCHECK_EQ(0u, refresh_tokens_.count(account_id));
-  AccountStatus* status =
-      new AccountStatus(signin_error_controller_, account_id, refresh_token);
-  refresh_tokens_[account_id].reset(status);
-  status->Initialize();
-  status->SetLastAuthError(error);
-  FireAuthErrorChanged(account_id, status->GetAuthStatus());
+  refresh_tokens_[account_id] = AccountStatus{refresh_token, error};
+  FireAuthErrorChanged(account_id, error);
 }
 
 void MutableProfileOAuth2TokenServiceDelegate::FinishLoadingCredentials() {

chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.h

@@ -14,7 +14,6 @@
 #include "components/signin/core/browser/account_consistency_method.h"
 #include "components/signin/core/browser/account_tracker_service.h"
 #include "components/signin/core/browser/profile_oauth2_token_service.h"
-#include "components/signin/core/browser/signin_error_controller.h"
 #include "components/signin/core/browser/webdata/token_web_data.h"
 #include "components/webdata/common/web_data_service_base.h"
 #include "components/webdata/common/web_data_service_consumer.h"
@@ -34,7 +33,6 @@ class MutableProfileOAuth2TokenServiceDelegate
  public:
   MutableProfileOAuth2TokenServiceDelegate(
       SigninClient* client,
-      SigninErrorController* signin_error_controller,
       AccountTrackerService* account_tracker_service,
       scoped_refptr<TokenWebData> token_web_data,
       signin::AccountConsistencyMethod account_consistency,
@@ -89,32 +87,9 @@ class MutableProfileOAuth2TokenServiceDelegate
 
   class RevokeServerRefreshToken;
 
-  class AccountStatus : public SigninErrorController::AuthStatusProvider {
-   public:
-    AccountStatus(SigninErrorController* signin_error_controller,
-                  const std::string& account_id,
-                  const std::string& refresh_token);
-    ~AccountStatus() override;
-
-    // Must be called after the account has been added to the AccountStatusMap.
-    void Initialize();
-
-    const std::string& refresh_token() const { return refresh_token_; }
-    void set_refresh_token(const std::string& token) { refresh_token_ = token; }
-
-    void SetLastAuthError(const GoogleServiceAuthError& error);
-
-    // SigninErrorController::AuthStatusProvider implementation.
-    std::string GetAccountId() const override;
-    GoogleServiceAuthError GetAuthStatus() const override;
-
-   private:
-    SigninErrorController* signin_error_controller_;
-    std::string account_id_;
-    std::string refresh_token_;
-    GoogleServiceAuthError last_auth_error_;
-
-    DISALLOW_COPY_AND_ASSIGN(AccountStatus);
+  struct AccountStatus {
+    std::string refresh_token;
+    GoogleServiceAuthError last_auth_error;
   };
 
   FRIEND_TEST_ALL_PREFIXES(MutableProfileOAuth2TokenServiceDelegateTest,
@@ -203,8 +178,7 @@ class MutableProfileOAuth2TokenServiceDelegate
 
   // Maps the |account_id| of accounts known to ProfileOAuth2TokenService
   // to information about the account.
-  typedef std::map<std::string, std::unique_ptr<AccountStatus>>
-      AccountStatusMap;
+  typedef std::map<std::string, AccountStatus> AccountStatusMap;
   // In memory refresh token store mapping account_id to refresh_token.
   AccountStatusMap refresh_tokens_;
 
@@ -227,7 +201,6 @@ class MutableProfileOAuth2TokenServiceDelegate
   GoogleServiceAuthError backoff_error_;
 
   SigninClient* client_;
-  SigninErrorController* signin_error_controller_;
   AccountTrackerService* account_tracker_service_;
   scoped_refptr<TokenWebData> token_web_data_;
   signin::AccountConsistencyMethod account_consistency_;

chrome/browser/signin/profile_oauth2_token_service_factory.cc

@@ -14,7 +14,6 @@
 #include "chrome/browser/signin/account_consistency_mode_manager.h"
 #include "chrome/browser/signin/account_tracker_service_factory.h"
 #include "chrome/browser/signin/chrome_signin_client_factory.h"
-#include "chrome/browser/signin/signin_error_controller_factory.h"
 #include "chrome/browser/web_data_service_factory.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "components/pref_registry/pref_registry_syncable.h"
@@ -65,8 +64,7 @@ CreateCrOsOAuthDelegate(Profile* profile) {
 
   return std::make_unique<chromeos::ChromeOSOAuth2TokenServiceDelegate>(
       AccountTrackerServiceFactory::GetInstance()->GetForProfile(profile),
-      account_manager,
-      SigninErrorControllerFactory::GetInstance()->GetForProfile(profile));
+      account_manager);
 }
 #endif  // defined(OS_CHROMEOS)
 
@@ -102,7 +100,6 @@ CreateMutableProfileOAuthDelegate(Profile* profile) {
 
   return std::make_unique<MutableProfileOAuth2TokenServiceDelegate>(
       ChromeSigninClientFactory::GetInstance()->GetForProfile(profile),
-      SigninErrorControllerFactory::GetInstance()->GetForProfile(profile),
       AccountTrackerServiceFactory::GetInstance()->GetForProfile(profile),
       WebDataServiceFactory::GetTokenWebDataForProfile(
           profile, ServiceAccessType::EXPLICIT_ACCESS),
@@ -145,7 +142,6 @@ ProfileOAuth2TokenServiceFactory::ProfileOAuth2TokenServiceFactory()
 #endif
   DependsOn(WebDataServiceFactory::GetInstance());
   DependsOn(ChromeSigninClientFactory::GetInstance());
-  DependsOn(SigninErrorControllerFactory::GetInstance());
   DependsOn(AccountTrackerServiceFactory::GetInstance());
   DependsOn(WebDataServiceFactory::GetInstance());
 }

chrome/browser/signin/signin_error_controller_factory.cc

@@ -7,13 +7,17 @@
 #include "build/build_config.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/account_consistency_mode_manager.h"
+#include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "components/signin/core/browser/account_consistency_method.h"
+#include "components/signin/core/browser/profile_oauth2_token_service.h"
 
 SigninErrorControllerFactory::SigninErrorControllerFactory()
     : BrowserContextKeyedServiceFactory(
           "SigninErrorController",
-          BrowserContextDependencyManager::GetInstance()) {}
+          BrowserContextDependencyManager::GetInstance()) {
+  DependsOn(ProfileOAuth2TokenServiceFactory::GetInstance());
+}
 
 SigninErrorControllerFactory::~SigninErrorControllerFactory() {}
 
@@ -31,14 +35,15 @@ SigninErrorControllerFactory* SigninErrorControllerFactory::GetInstance() {
 
 KeyedService* SigninErrorControllerFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
+  Profile* profile = Profile::FromBrowserContext(context);
   SigninErrorController::AccountMode account_mode =
 #if defined(OS_CHROMEOS)
       SigninErrorController::AccountMode::ANY_ACCOUNT;
 #else
-      AccountConsistencyModeManager::IsMirrorEnabledForProfile(
-          Profile::FromBrowserContext(context))
+      AccountConsistencyModeManager::IsMirrorEnabledForProfile(profile)
           ? SigninErrorController::AccountMode::ANY_ACCOUNT
           : SigninErrorController::AccountMode::PRIMARY_ACCOUNT;
 #endif
-  return new SigninErrorController(account_mode);
+  return new SigninErrorController(
+      account_mode, ProfileOAuth2TokenServiceFactory::GetForProfile(profile));
 }

components/signin/core/browser/BUILD.gn

@@ -243,8 +243,6 @@ static_library("test_support") {
   sources = [
     "fake_account_fetcher_service.cc",
     "fake_account_fetcher_service.h",
-    "fake_auth_status_provider.cc",
-    "fake_auth_status_provider.h",
   ]
 
   deps = [

components/signin/core/browser/fake_auth_status_provider.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/fake_auth_status_provider.h"
-
-FakeAuthStatusProvider::FakeAuthStatusProvider(SigninErrorController* error)
-  : error_provider_(error),
-    auth_error_(GoogleServiceAuthError::AuthErrorNone()) {
-  error_provider_->AddProvider(this);
-}
-
-FakeAuthStatusProvider::~FakeAuthStatusProvider() {
-  error_provider_->RemoveProvider(this);
-}
-
-std::string FakeAuthStatusProvider::GetAccountId() const {
-  return account_id_;
-}
-
-GoogleServiceAuthError FakeAuthStatusProvider::GetAuthStatus() const {
-  return auth_error_;
-}
-
-void FakeAuthStatusProvider::SetAuthError(const std::string& account_id,
-                                          const GoogleServiceAuthError& error) {
-  account_id_ = account_id;
-  auth_error_ = error;
-  error_provider_->AuthStatusChanged();
-}

components/signin/core/browser/fake_auth_status_provider.h

-// 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.
-
-#ifndef COMPONENTS_SIGNIN_CORE_BROWSER_FAKE_AUTH_STATUS_PROVIDER_H_
-#define COMPONENTS_SIGNIN_CORE_BROWSER_FAKE_AUTH_STATUS_PROVIDER_H_
-
-#include "components/signin/core/browser/signin_error_controller.h"
-
-// Helper class that reports auth errors to SigninErrorController. Automatically
-// registers and de-registers itself as an AuthStatusProvider in the
-// constructor and destructor.
-class FakeAuthStatusProvider
-    : public SigninErrorController::AuthStatusProvider {
- public:
-  explicit FakeAuthStatusProvider(SigninErrorController* error);
-  ~FakeAuthStatusProvider() override;
-
-  // Sets the auth error that this provider reports to SigninErrorController.
-  // Also notifies SigninErrorController via AuthStatusChanged().
-  void SetAuthError(const std::string& account_id,
-                    const GoogleServiceAuthError& error);
-
-  void set_error_without_status_change(const GoogleServiceAuthError& error) {
-    auth_error_ = error;
-  }
-
-  // AuthStatusProvider implementation.
-  std::string GetAccountId() const override;
-  GoogleServiceAuthError GetAuthStatus() const override;
-
- private:
-  SigninErrorController* error_provider_;
-  std::string account_id_;
-  GoogleServiceAuthError auth_error_;
-};
-
-#endif  // COMPONENTS_SIGNIN_CORE_BROWSER_FAKE_AUTH_STATUS_PROVIDER_H_

components/signin/core/browser/signin_error_controller.cc

@@ -6,45 +6,24 @@
 
 #include "components/signin/core/browser/signin_metrics.h"
 
-namespace {
-
-typedef std::set<const SigninErrorController::AuthStatusProvider*>
-    AuthStatusProviderSet;
-
-}  // namespace
-
-SigninErrorController::AuthStatusProvider::AuthStatusProvider() {
-}
-
-SigninErrorController::AuthStatusProvider::~AuthStatusProvider() {
-}
-
-SigninErrorController::SigninErrorController(AccountMode mode)
+SigninErrorController::SigninErrorController(AccountMode mode,
+                                             OAuth2TokenService* token_service)
     : account_mode_(mode),
-      auth_error_(GoogleServiceAuthError::AuthErrorNone()) {}
-
-SigninErrorController::~SigninErrorController() {
-  DCHECK(provider_set_.empty())
-      << "All AuthStatusProviders should be unregistered before "
-      << "SigninErrorController is destroyed";
+      token_service_(token_service),
+      scoped_token_service_observer_(this),
+      auth_error_(GoogleServiceAuthError::AuthErrorNone()) {
+  DCHECK(token_service_);
+  scoped_token_service_observer_.Add(token_service_);
+  Update();
 }
 
-void SigninErrorController::AddProvider(const AuthStatusProvider* provider) {
-  DCHECK(provider_set_.find(provider) == provider_set_.end())
-      << "Adding same AuthStatusProvider multiple times";
-  provider_set_.insert(provider);
-  AuthStatusChanged();
-}
+SigninErrorController::~SigninErrorController() = default;
 
-void SigninErrorController::RemoveProvider(const AuthStatusProvider* provider) {
-  auto iter = provider_set_.find(provider);
-  DCHECK(iter != provider_set_.end())
-      << "Removing provider that was never added";
-  provider_set_.erase(iter);
-  AuthStatusChanged();
+void SigninErrorController::Shutdown() {
+  scoped_token_service_observer_.RemoveAll();
 }
 
-void SigninErrorController::AuthStatusChanged() {
+void SigninErrorController::Update() {
   GoogleServiceAuthError::State prev_state = auth_error_.state();
   std::string prev_account_id = error_account_id_;
   bool error_changed = false;
@@ -53,22 +32,19 @@ void SigninErrorController::AuthStatusChanged() {
   // actionable error state and some provider exposes a similar error and
   // account id, use that error. Otherwise, just take the first actionable
   // error we find.
-  for (auto it = provider_set_.begin(); it != provider_set_.end(); ++it) {
-    std::string account_id = (*it)->GetAccountId();
-
+  for (const std::string& account_id : token_service_->GetAccounts()) {
     // In PRIMARY_ACCOUNT mode, ignore all secondary accounts.
     if (account_mode_ == AccountMode::PRIMARY_ACCOUNT &&
         (account_id != primary_account_id_)) {
       continue;
     }
 
-    GoogleServiceAuthError error = (*it)->GetAuthStatus();
-
-    // Ignore the states we don't want to elevate to the user.
-    if (error.state() == GoogleServiceAuthError::NONE ||
-        error.IsTransientError()) {
+    if (!token_service_->RefreshTokenHasError(account_id))
       continue;
-    }
+
+    GoogleServiceAuthError error = token_service_->GetAuthError(account_id);
+    // TokenService only reports persistent errors.
+    DCHECK(error.IsPersistentError());
 
     // Prioritize this error if it matches the previous |auth_error_|.
     if (error.state() == prev_state && account_id == prev_account_id) {
@@ -102,14 +78,14 @@ void SigninErrorController::AuthStatusChanged() {
 }
 
 bool SigninErrorController::HasError() const {
-  return auth_error_.state() != GoogleServiceAuthError::NONE &&
-      auth_error_.state() != GoogleServiceAuthError::CONNECTION_FAILED;
+  DCHECK(!auth_error_.IsTransientError());
+  return auth_error_.state() != GoogleServiceAuthError::NONE;
 }
 
 void SigninErrorController::SetPrimaryAccountID(const std::string& account_id) {
   primary_account_id_ = account_id;
   if (account_mode_ == AccountMode::PRIMARY_ACCOUNT)
-    AuthStatusChanged();  // Recompute the error state.
+    Update();  // Recompute the error state.
 }
 
 void SigninErrorController::AddObserver(Observer* observer) {
@@ -119,3 +95,13 @@ void SigninErrorController::AddObserver(Observer* observer) {
 void SigninErrorController::RemoveObserver(Observer* observer) {
   observer_list_.RemoveObserver(observer);
 }
+
+void SigninErrorController::OnEndBatchChanges() {
+  Update();
+}
+
+void SigninErrorController::OnAuthErrorChanged(
+    const std::string& account_id,
+    const GoogleServiceAuthError& auth_error) {
+  Update();
+}

components/signin/core/browser/signin_error_controller.h

@@ -9,14 +9,17 @@
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/observer_list.h"
+#include "base/scoped_observer.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "google_apis/gaia/google_service_auth_error.h"
+#include "google_apis/gaia/oauth2_token_service.h"
 
 // Keep track of auth errors and expose them to observers in the UI. Services
 // that wish to expose auth errors to the user should register an
 // AuthStatusProvider to report their current authentication state, and should
 // invoke AuthStatusChanged() when their authentication state may have changed.
-class SigninErrorController : public KeyedService {
+class SigninErrorController : public KeyedService,
+                              public OAuth2TokenService::Observer {
  public:
   enum class AccountMode {
     // Signin error controller monitors all the accounts. When multiple accounts
@@ -28,19 +31,6 @@ class SigninErrorController : public KeyedService {
     PRIMARY_ACCOUNT
   };
 
-  class AuthStatusProvider {
-   public:
-    AuthStatusProvider();
-    virtual ~AuthStatusProvider();
-
-    // Returns the account id with the status specified by GetAuthStatus().
-    virtual std::string GetAccountId() const = 0;
-
-    // API invoked by SigninErrorController to get the current auth status of
-    // the various signed in services.
-    virtual GoogleServiceAuthError GetAuthStatus() const = 0;
-  };
-
   // The observer class for SigninErrorController lets the controller notify
   // observers when an error arises or changes.
   class Observer {
@@ -49,19 +39,12 @@ class SigninErrorController : public KeyedService {
     virtual void OnErrorChanged() = 0;
   };
 
-  explicit SigninErrorController(AccountMode mode);
+  explicit SigninErrorController(AccountMode mode,
+                                 OAuth2TokenService* token_service);
   ~SigninErrorController() override;
 
-  // Adds a provider which the SigninErrorController object will start querying
-  // for auth status.
-  void AddProvider(const AuthStatusProvider* provider);
-
-  // Removes a provider previously added by SigninErrorController (generally
-  // only called in preparation for shutdown).
-  void RemoveProvider(const AuthStatusProvider* provider);
-
-  // Invoked when the auth status of an AuthStatusProvider has changed.
-  void AuthStatusChanged();
+  // KeyedService implementation:
+  void Shutdown() override;
 
   // True if there exists an error worth elevating to the user.
   bool HasError() const;
@@ -76,8 +59,18 @@ class SigninErrorController : public KeyedService {
   const GoogleServiceAuthError& auth_error() const { return auth_error_; }
 
  private:
+  // Invoked when the auth status has changed.
+  void Update();
+
+  // OAuth2TokenService::Observer implementation:
+  void OnEndBatchChanges() override;
+  void OnAuthErrorChanged(const std::string& account_id,
+                          const GoogleServiceAuthError& auth_error) override;
+
   const AccountMode account_mode_;
-  std::set<const AuthStatusProvider*> provider_set_;
+  OAuth2TokenService* token_service_;
+  ScopedObserver<OAuth2TokenService, SigninErrorController>
+      scoped_token_service_observer_;
 
   // The primary account ID. Only used in the PRIMARY_ACCOUNT account mode.
   std::string primary_account_id_;

components/signin/ios/browser/profile_oauth2_token_service_ios_delegate.h

@@ -8,9 +8,7 @@
 
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
-#include "base/memory/linked_ptr.h"
 #include "base/threading/thread_checker.h"
-#include "components/signin/core/browser/signin_error_controller.h"
 #include "google_apis/gaia/oauth2_token_service_delegate.h"
 
 class AccountTrackerService;
@@ -22,8 +20,7 @@ class ProfileOAuth2TokenServiceIOSDelegate : public OAuth2TokenServiceDelegate {
   ProfileOAuth2TokenServiceIOSDelegate(
       SigninClient* client,
       std::unique_ptr<ProfileOAuth2TokenServiceIOSProvider> provider,
-      AccountTrackerService* account_tracker_service,
-      SigninErrorController* signin_error_controller);
+      AccountTrackerService* account_tracker_service);
   ~ProfileOAuth2TokenServiceIOSDelegate() override;
 
   OAuth2AccessTokenFetcher* CreateAccessTokenFetcher(
@@ -78,29 +75,13 @@ class ProfileOAuth2TokenServiceIOSDelegate : public OAuth2TokenServiceDelegate {
   FRIEND_TEST_ALL_PREFIXES(ProfileOAuth2TokenServiceIOSDelegateTest,
                            LoadRevokeCredentialsClearsExcludedAccounts);
 
-  class AccountStatus : public SigninErrorController::AuthStatusProvider {
-   public:
-    AccountStatus(SigninErrorController* signin_error_controller,
-                  const std::string& account_id);
-    ~AccountStatus() override;
-
-    void SetLastAuthError(const GoogleServiceAuthError& error);
-
-    // SigninErrorController::AuthStatusProvider implementation.
-    std::string GetAccountId() const override;
-    GoogleServiceAuthError GetAuthStatus() const override;
-
-   private:
-    SigninErrorController* signin_error_controller_;
-    std::string account_id_;
-    GoogleServiceAuthError last_auth_error_;
-
-    DISALLOW_COPY_AND_ASSIGN(AccountStatus);
+  struct AccountStatus {
+    GoogleServiceAuthError last_auth_error;
   };
 
   // Maps the |account_id| of accounts known to ProfileOAuth2TokenService
   // to information about the account.
-  typedef std::map<std::string, linked_ptr<AccountStatus>> AccountStatusMap;
+  typedef std::map<std::string, AccountStatus> AccountStatusMap;
 
   // Clears exclude secondary accounts preferences.
   void ClearExcludedSecondaryAccounts();
@@ -121,9 +102,6 @@ class ProfileOAuth2TokenServiceIOSDelegate : public OAuth2TokenServiceDelegate {
   std::unique_ptr<ProfileOAuth2TokenServiceIOSProvider> provider_;
   AccountTrackerService* account_tracker_service_;
 
-  // The error controller with which this instance was initialized, or NULL.
-  SigninErrorController* signin_error_controller_;
-
   DISALLOW_COPY_AND_ASSIGN(ProfileOAuth2TokenServiceIOSDelegate);
 };
 #endif  // COMPONENTS_SIGNIN_IOS_BROWSER_PROFILE_OAUTH2_TOKEN_SERVICE_IOS_DELEGATE_H_

components/signin/ios/browser/profile_oauth2_token_service_ios_delegate.mm

@@ -147,50 +147,16 @@ void SSOAccessTokenFetcher::OnAccessTokenResponse(NSString* token,
 
 }  // namespace
 
-ProfileOAuth2TokenServiceIOSDelegate::AccountStatus::AccountStatus(
-    SigninErrorController* signin_error_controller,
-    const std::string& account_id)
-    : signin_error_controller_(signin_error_controller),
-      account_id_(account_id),
-      last_auth_error_(GoogleServiceAuthError::NONE) {
-  DCHECK(signin_error_controller_);
-  DCHECK(!account_id_.empty());
-  signin_error_controller_->AddProvider(this);
-}
-
-ProfileOAuth2TokenServiceIOSDelegate::AccountStatus::~AccountStatus() {
-  signin_error_controller_->RemoveProvider(this);
-}
-
-void ProfileOAuth2TokenServiceIOSDelegate::AccountStatus::SetLastAuthError(
-    const GoogleServiceAuthError& error) {
-    last_auth_error_ = error;
-    signin_error_controller_->AuthStatusChanged();
-}
-
-std::string ProfileOAuth2TokenServiceIOSDelegate::AccountStatus::GetAccountId()
-    const {
-  return account_id_;
-}
-
-GoogleServiceAuthError
-ProfileOAuth2TokenServiceIOSDelegate::AccountStatus::GetAuthStatus() const {
-  return last_auth_error_;
-}
-
 ProfileOAuth2TokenServiceIOSDelegate::ProfileOAuth2TokenServiceIOSDelegate(
     SigninClient* client,
     std::unique_ptr<ProfileOAuth2TokenServiceIOSProvider> provider,
-    AccountTrackerService* account_tracker_service,
-    SigninErrorController* signin_error_controller)
+    AccountTrackerService* account_tracker_service)
     : client_(client),
       provider_(std::move(provider)),
-      account_tracker_service_(account_tracker_service),
-      signin_error_controller_(signin_error_controller) {
+      account_tracker_service_(account_tracker_service) {
   DCHECK(client_);
   DCHECK(provider_);
   DCHECK(account_tracker_service_);
-  DCHECK(signin_error_controller_);
 }
 
 ProfileOAuth2TokenServiceIOSDelegate::~ProfileOAuth2TokenServiceIOSDelegate() {
@@ -333,7 +299,7 @@ GoogleServiceAuthError ProfileOAuth2TokenServiceIOSDelegate::GetAuthError(
     const std::string& account_id) const {
   auto it = accounts_.find(account_id);
   return (it == accounts_.end()) ? GoogleServiceAuthError::AuthErrorNone()
-                                 : it->second->GetAuthStatus();
+                                 : it->second.last_auth_error;
 }
 
 void ProfileOAuth2TokenServiceIOSDelegate::UpdateAuthError(
@@ -352,9 +318,9 @@ void ProfileOAuth2TokenServiceIOSDelegate::UpdateAuthError(
     return;
   }
 
-  AccountStatus* status = accounts_[account_id].get();
-  if (error.state() != status->GetAuthStatus().state()) {
-    status->SetLastAuthError(error);
+  AccountStatus* status = &accounts_[account_id];
+  if (error.state() != status->last_auth_error.state()) {
+    status->last_auth_error = error;
     FireAuthErrorChanged(account_id, error);
   }
 }
@@ -370,21 +336,16 @@ void ProfileOAuth2TokenServiceIOSDelegate::AddOrUpdateAccount(
   DCHECK(!account_tracker_service_->GetAccountInfo(account_id).email.empty());
 
   bool account_present = accounts_.count(account_id) > 0;
-  if (account_present &&
-      accounts_[account_id]->GetAuthStatus().state() ==
-          GoogleServiceAuthError::NONE) {
+  if (account_present && accounts_[account_id].last_auth_error.state() ==
+                             GoogleServiceAuthError::NONE) {
     // No need to update the account if it is already a known account and if
     // there is no auth error.
     return;
   }
 
-  if (!account_present) {
-    accounts_[account_id].reset(
-        new AccountStatus(signin_error_controller_, account_id));
-    FireAuthErrorChanged(account_id, accounts_[account_id]->GetAuthStatus());
-  }
-
-  UpdateAuthError(account_id, GoogleServiceAuthError::AuthErrorNone());
+  accounts_[account_id].last_auth_error =
+      GoogleServiceAuthError::AuthErrorNone();
+  FireAuthErrorChanged(account_id, GoogleServiceAuthError::AuthErrorNone());
   FireRefreshTokenAvailable(account_id);
 }
 

components/signin/ios/browser/profile_oauth2_token_service_ios_delegate_unittest.mm

@@ -11,7 +11,6 @@
 #include "components/prefs/testing_pref_service.h"
 #include "components/signin/core/browser/account_tracker_service.h"
 #include "components/signin/core/browser/profile_oauth2_token_service.h"
-#include "components/signin/core/browser/signin_error_controller.h"
 #include "components/signin/core/browser/signin_pref_names.h"
 #include "components/signin/core/browser/test_signin_client.h"
 #include "components/signin/ios/browser/fake_profile_oauth2_token_service_ios_provider.h"
@@ -30,20 +29,16 @@ typedef ProfileOAuth2TokenServiceIOSProvider::AccountInfo ProviderAccount;
 class ProfileOAuth2TokenServiceIOSDelegateTest
     : public testing::Test,
       public OAuth2AccessTokenConsumer,
-      public OAuth2TokenService::Observer,
-      public SigninErrorController::Observer {
+      public OAuth2TokenService::Observer {
  public:
   ProfileOAuth2TokenServiceIOSDelegateTest()
       : factory_(NULL),
         client_(&prefs_),
-        signin_error_controller_(
-            SigninErrorController::AccountMode::ANY_ACCOUNT),
         token_available_count_(0),
         token_revoked_count_(0),
         tokens_loaded_count_(0),
         access_token_success_(0),
         access_token_failure_(0),
-        error_changed_count_(0),
         auth_error_changed_count_(0),
         last_access_token_error_(GoogleServiceAuthError::NONE) {}
 
@@ -64,14 +59,11 @@ class ProfileOAuth2TokenServiceIOSDelegateTest
     factory_.SetFakeResponse(GaiaUrls::GetInstance()->oauth2_revoke_url(), "",
                              net::HTTP_OK, net::URLRequestStatus::SUCCESS);
     oauth2_delegate_.reset(new ProfileOAuth2TokenServiceIOSDelegate(
-        &client_, base::WrapUnique(fake_provider_), &account_tracker_,
-        &signin_error_controller_));
+        &client_, base::WrapUnique(fake_provider_), &account_tracker_));
     oauth2_delegate_->AddObserver(this);
-    signin_error_controller_.AddObserver(this);
   }
 
   void TearDown() override {
-    signin_error_controller_.RemoveObserver(this);
     oauth2_delegate_->RemoveObserver(this);
     oauth2_delegate_->Shutdown();
   }
@@ -100,16 +92,12 @@ class ProfileOAuth2TokenServiceIOSDelegateTest
     ++auth_error_changed_count_;
   }
 
-  // SigninErrorController::Observer implementation.
-  void OnErrorChanged() override { ++error_changed_count_; }
-
   void ResetObserverCounts() {
     token_available_count_ = 0;
     token_revoked_count_ = 0;
     tokens_loaded_count_ = 0;
     token_available_count_ = 0;
     access_token_failure_ = 0;
-    error_changed_count_ = 0;
     auth_error_changed_count_ = 0;
   }
 
@@ -124,7 +112,6 @@ class ProfileOAuth2TokenServiceIOSDelegateTest
   TestingPrefServiceSimple prefs_;
   TestSigninClient client_;
   AccountTrackerService account_tracker_;
-  SigninErrorController signin_error_controller_;
   FakeProfileOAuth2TokenServiceIOSProvider* fake_provider_;
   std::unique_ptr<ProfileOAuth2TokenServiceIOSDelegate> oauth2_delegate_;
   TestingOAuth2TokenServiceConsumer consumer_;
@@ -133,7 +120,6 @@ class ProfileOAuth2TokenServiceIOSDelegateTest
   int tokens_loaded_count_;
   int access_token_success_;
   int access_token_failure_;
-  int error_changed_count_;
   int auth_error_changed_count_;
   GoogleServiceAuthError last_access_token_error_;
 };
@@ -311,13 +297,11 @@ TEST_F(ProfileOAuth2TokenServiceIOSDelegateTest,
   oauth2_delegate_->UpdateAuthError(GetAccountId(account1), error);
   EXPECT_EQ(error, oauth2_delegate_->GetAuthError("gaia_1"));
   EXPECT_EQ(1, auth_error_changed_count_);
-  EXPECT_EQ(1, error_changed_count_);
 
   oauth2_delegate_->RevokeAllCredentials();
   ResetObserverCounts();
   oauth2_delegate_->UpdateAuthError(GetAccountId(account1), error);
   EXPECT_EQ(0, auth_error_changed_count_);
-  EXPECT_EQ(0, error_changed_count_);
 }
 
 TEST_F(ProfileOAuth2TokenServiceIOSDelegateTest, GetAuthError) {

google_apis/gaia/fake_oauth2_token_service_delegate.cc

@@ -114,7 +114,7 @@ FakeOAuth2TokenServiceDelegate::GetURLLoaderFactory() const {
 void FakeOAuth2TokenServiceDelegate::UpdateAuthError(
     const std::string& account_id,
     const GoogleServiceAuthError& error) {
-  if (GetAuthError(account_id) == error)
+  if (error.IsTransientError() || GetAuthError(account_id) == error)
     return;
 
   // Drop transient errors to match OAuth2TokenService's stated contract for

ios/chrome/browser/signin/authentication_service_unittest.mm

@@ -31,7 +31,6 @@
 #include "ios/chrome/browser/signin/ios_chrome_signin_client.h"
 #include "ios/chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "ios/chrome/browser/signin/signin_client_factory.h"
-#include "ios/chrome/browser/signin/signin_error_controller_factory.h"
 #include "ios/chrome/browser/sync/ios_chrome_profile_sync_test_util.h"
 #include "ios/chrome/browser/sync/profile_sync_service_factory.h"
 #include "ios/chrome/browser/sync/sync_setup_service_factory.h"
@@ -62,7 +61,6 @@ class FakeSigninClient : public IOSChromeSigninClient {
  public:
   explicit FakeSigninClient(
       ios::ChromeBrowserState* browser_state,
-      SigninErrorController* signin_error_controller,
       scoped_refptr<content_settings::CookieSettings> cookie_settings,
       scoped_refptr<HostContentSettingsMap> host_content_settings_map)
       : IOSChromeSigninClient(browser_state,
@@ -79,8 +77,6 @@ std::unique_ptr<KeyedService> BuildFakeTestSigninClient(
       ios::ChromeBrowserState::FromBrowserState(context);
   return std::make_unique<FakeSigninClient>(
       chrome_browser_state,
-      ios::SigninErrorControllerFactory::GetForBrowserState(
-          chrome_browser_state),
       ios::CookieSettingsFactory::GetForBrowserState(chrome_browser_state),
       ios::HostContentSettingsMapFactory::GetForBrowserState(
           chrome_browser_state));

ios/chrome/browser/signin/fake_oauth2_token_service_builder.mm

@@ -10,7 +10,6 @@
 #include "ios/chrome/browser/signin/account_tracker_service_factory.h"
 #include "ios/chrome/browser/signin/profile_oauth2_token_service_ios_provider_impl.h"
 #include "ios/chrome/browser/signin/signin_client_factory.h"
-#include "ios/chrome/browser/signin/signin_error_controller_factory.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
@@ -24,8 +23,7 @@ std::unique_ptr<KeyedService> BuildFakeOAuth2TokenService(
       std::make_unique<ProfileOAuth2TokenServiceIOSDelegate>(
           SigninClientFactory::GetForBrowserState(browser_state),
           std::make_unique<ProfileOAuth2TokenServiceIOSProviderImpl>(),
-          ios::AccountTrackerServiceFactory::GetForBrowserState(browser_state),
-          ios::SigninErrorControllerFactory::GetForBrowserState(browser_state));
+          ios::AccountTrackerServiceFactory::GetForBrowserState(browser_state));
   return std::make_unique<FakeProfileOAuth2TokenService>(
       browser_state->GetPrefs(), std::move(delegate));
 }

ios/chrome/browser/signin/profile_oauth2_token_service_factory.mm

@@ -13,7 +13,6 @@
 #include "ios/chrome/browser/signin/account_tracker_service_factory.h"
 #include "ios/chrome/browser/signin/profile_oauth2_token_service_ios_provider_impl.h"
 #include "ios/chrome/browser/signin/signin_client_factory.h"
-#include "ios/chrome/browser/signin/signin_error_controller_factory.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
@@ -25,7 +24,6 @@ ProfileOAuth2TokenServiceFactory::ProfileOAuth2TokenServiceFactory()
           BrowserStateDependencyManager::GetInstance()) {
   DependsOn(ios::AccountTrackerServiceFactory::GetInstance());
   DependsOn(SigninClientFactory::GetInstance());
-  DependsOn(ios::SigninErrorControllerFactory::GetInstance());
 }
 
 ProfileOAuth2TokenServiceFactory::~ProfileOAuth2TokenServiceFactory() {}
@@ -56,8 +54,6 @@ ProfileOAuth2TokenServiceFactory::BuildServiceInstanceFor(
       SigninClientFactory::GetForBrowserState(chrome_browser_state),
       std::make_unique<ProfileOAuth2TokenServiceIOSProviderImpl>(),
       ios::AccountTrackerServiceFactory::GetForBrowserState(
-          chrome_browser_state),
-      ios::SigninErrorControllerFactory::GetForBrowserState(
           chrome_browser_state));
   return std::make_unique<ProfileOAuth2TokenService>(
       chrome_browser_state->GetPrefs(), std::move(delegate));

ios/chrome/browser/signin/signin_client_factory.h

@@ -21,7 +21,7 @@ namespace ios {
 class ChromeBrowserState;
 }
 
-// Singleton that owns all SigninErrorControllers and associates them with
+// Singleton that owns all SigninClients and associates them with
 // ios::ChromeBrowserState.
 class SigninClientFactory : public BrowserStateKeyedServiceFactory {
  public:

ios/chrome/browser/signin/signin_error_controller_factory.cc

@@ -10,8 +10,10 @@
 #include "base/memory/singleton.h"
 #include "components/keyed_service/core/service_access_type.h"
 #include "components/keyed_service/ios/browser_state_dependency_manager.h"
+#include "components/signin/core/browser/profile_oauth2_token_service.h"
 #include "components/signin/core/browser/signin_error_controller.h"
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
+#include "ios/chrome/browser/signin/profile_oauth2_token_service_factory.h"
 
 namespace ios {
 
@@ -31,6 +33,7 @@ SigninErrorControllerFactory::SigninErrorControllerFactory()
     : BrowserStateKeyedServiceFactory(
           "SigninErrorController",
           BrowserStateDependencyManager::GetInstance()) {
+  DependsOn(ProfileOAuth2TokenServiceFactory::GetInstance());
 }
 
 SigninErrorControllerFactory::~SigninErrorControllerFactory() {
@@ -39,8 +42,12 @@ SigninErrorControllerFactory::~SigninErrorControllerFactory() {
 std::unique_ptr<KeyedService>
 SigninErrorControllerFactory::BuildServiceInstanceFor(
     web::BrowserState* context) const {
+  ChromeBrowserState* chrome_browser_state =
+      ChromeBrowserState::FromBrowserState(context);
   return std::make_unique<SigninErrorController>(
-      SigninErrorController::AccountMode::ANY_ACCOUNT);
+      SigninErrorController::AccountMode::ANY_ACCOUNT,
+      ProfileOAuth2TokenServiceFactory::GetForBrowserState(
+          chrome_browser_state));
 }
 
 }  // namespace ios

ios/web_view/internal/signin/web_view_oauth2_token_service_factory.mm

@@ -13,7 +13,6 @@
 #include "ios/web_view/internal/signin/web_view_account_tracker_service_factory.h"
 #include "ios/web_view/internal/signin/web_view_profile_oauth2_token_service_ios_provider_impl.h"
 #include "ios/web_view/internal/signin/web_view_signin_client_factory.h"
-#include "ios/web_view/internal/signin/web_view_signin_error_controller_factory.h"
 #include "ios/web_view/internal/web_view_browser_state.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
@@ -28,7 +27,6 @@ WebViewOAuth2TokenServiceFactory::WebViewOAuth2TokenServiceFactory()
           BrowserStateDependencyManager::GetInstance()) {
   DependsOn(WebViewAccountTrackerServiceFactory::GetInstance());
   DependsOn(WebViewSigninClientFactory::GetInstance());
-  DependsOn(WebViewSigninErrorControllerFactory::GetInstance());
 }
 
 ProfileOAuth2TokenService* WebViewOAuth2TokenServiceFactory::GetForBrowserState(
@@ -60,8 +58,7 @@ WebViewOAuth2TokenServiceFactory::BuildServiceInstanceFor(
           signin_client);
   auto delegate = std::make_unique<ProfileOAuth2TokenServiceIOSDelegate>(
       signin_client, std::move(token_service_provider),
-      WebViewAccountTrackerServiceFactory::GetForBrowserState(browser_state),
-      WebViewSigninErrorControllerFactory::GetForBrowserState(browser_state));
+      WebViewAccountTrackerServiceFactory::GetForBrowserState(browser_state));
   return std::make_unique<ProfileOAuth2TokenService>(browser_state->GetPrefs(),
                                                      std::move(delegate));
 }

ios/web_view/internal/signin/web_view_signin_error_controller_factory.mm

@@ -9,7 +9,9 @@
 #include "base/memory/singleton.h"
 #include "components/keyed_service/core/service_access_type.h"
 #include "components/keyed_service/ios/browser_state_dependency_manager.h"
+#include "components/signin/core/browser/profile_oauth2_token_service.h"
 #include "components/signin/core/browser/signin_error_controller.h"
+#include "ios/web_view/internal/signin/web_view_oauth2_token_service_factory.h"
 #include "ios/web_view/internal/web_view_browser_state.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
@@ -34,13 +36,18 @@ WebViewSigninErrorControllerFactory::GetInstance() {
 WebViewSigninErrorControllerFactory::WebViewSigninErrorControllerFactory()
     : BrowserStateKeyedServiceFactory(
           "SigninErrorController",
-          BrowserStateDependencyManager::GetInstance()) {}
+          BrowserStateDependencyManager::GetInstance()) {
+  DependsOn(WebViewOAuth2TokenServiceFactory::GetInstance());
+}
 
 std::unique_ptr<KeyedService>
 WebViewSigninErrorControllerFactory::BuildServiceInstanceFor(
     web::BrowserState* context) const {
+  WebViewBrowserState* browser_state =
+      WebViewBrowserState::FromBrowserState(context);
   return std::make_unique<SigninErrorController>(
-      SigninErrorController::AccountMode::ANY_ACCOUNT);
+      SigninErrorController::AccountMode::ANY_ACCOUNT,
+      WebViewOAuth2TokenServiceFactory::GetForBrowserState(browser_state));
 }
 
 }  // namespace ios_web_view

ios/web_view/internal/sync/cwv_sync_controller.mm

@@ -12,6 +12,7 @@
 #include "components/signin/core/browser/account_info.h"
 #include "components/signin/core/browser/account_tracker_service.h"
 #include "components/signin/core/browser/profile_oauth2_token_service.h"
+#include "components/signin/core/browser/signin_error_controller.h"
 #include "components/signin/core/browser/signin_manager.h"
 #include "components/signin/ios/browser/profile_oauth2_token_service_ios_delegate.h"
 #include "ios/web/public/web_thread.h"

ios/web_view/internal/sync/cwv_sync_controller_unittest.mm

@@ -50,17 +50,17 @@ class CWVSyncControllerTest : public PlatformTest {
   CWVSyncControllerTest()
       : browser_state_(/*off_the_record=*/false),
         signin_client_(browser_state_.GetPrefs()),
-        signin_error_controller_(
-            SigninErrorController::AccountMode::ANY_ACCOUNT),
         token_service_delegate_(new ProfileOAuth2TokenServiceIOSDelegate(
             &signin_client_,
             std::make_unique<FakeProfileOAuth2TokenServiceIOSProvider>(),
-            &account_tracker_service_,
-            &signin_error_controller_)),
+            &account_tracker_service_)),
         token_service_(browser_state_.GetPrefs(),
                        std::unique_ptr<ProfileOAuth2TokenServiceIOSDelegate>(
                            token_service_delegate_)),
         gaia_cookie_manager_service_(&token_service_, &signin_client_),
+        signin_error_controller_(
+            SigninErrorController::AccountMode::ANY_ACCOUNT,
+            &token_service_),
         signin_manager_(&signin_client_,
                         &token_service_,
                         &account_tracker_service_,
@@ -110,13 +110,13 @@ class CWVSyncControllerTest : public PlatformTest {
   std::unique_ptr<browser_sync::ProfileSyncServiceMock> profile_sync_service_;
   AccountTrackerService account_tracker_service_;
   TestSigninClient signin_client_;
-  SigninErrorController signin_error_controller_;
 
   // Weak, owned by the token service.
   ProfileOAuth2TokenServiceIOSDelegate* token_service_delegate_;
 
   FakeProfileOAuth2TokenService token_service_;
   FakeGaiaCookieManagerService gaia_cookie_manager_service_;
+  SigninErrorController signin_error_controller_;
   FakeSigninManager signin_manager_;
   CWVSyncController* sync_controller_;
   syncer::SyncServiceObserver* sync_service_observer_;