Cache authenticated account info in PrimaryAccountManager

The PrimaryAccountManager used to store just the CoreAccountId of the
authenticated account and to fetch the extended information from the
AccountTrackerService.

This could result in missing information if the account is removed
from the AccountTrackerService while the authenticated account is
not yet cleared. This should not have been a problem as it should not
happen in production however some regression where found during the
introduction of IdentityManager.

To fix those inconsistencies, IdentityManager was changed to cache
the information about the primary account. This however meant that
two objects are now storing the same information. Also the value
passed to GoogleSignedOut() method could still be inconsistent.

This CL removes the cache from IdentityManager and instead move it
to PrimaryAccountManager. This was not possible at the time due to
complex inheritance in PrimaryAccountManager (formerly SigninManager).

After this change, the information is stored in PrimaryAccountManager
and updated by IdentityManager when information is updated in the
AccountTrackerService (AccountTrackerService does not use Observer
but instead a callback which can only have one target, this target
is IdentityManager::OnAccountUpdated).

The CL also remove PrimaryAccountManager::SetAuthenticatedInfo() as
it is equivalent to calling AccountTrackerService::SeedAccountInfo()
followed by PrimaryAccountManager::SignIn().

Bug: 862619
Change-Id: I5f0d7a13f6886c2d1f5db5b83d967f60380d206f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1782173
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694764}

components/signin/internal/identity_manager/accounts_mutator_impl.cc

@@ -80,7 +80,7 @@ void AccountsMutatorImpl::RemoveAllAccounts(
 void AccountsMutatorImpl::InvalidateRefreshTokenForPrimaryAccount(
     signin_metrics::SourceForRefreshTokenOperation source) {
   DCHECK(primary_account_manager_->IsAuthenticated());
-  AccountInfo primary_account_info =
+  CoreAccountInfo primary_account_info =
       primary_account_manager_->GetAuthenticatedAccountInfo();
   AddOrUpdateAccount(primary_account_info.gaia, primary_account_info.email,
                      GaiaConstants::kInvalidRefreshToken,

components/signin/internal/identity_manager/primary_account_manager.cc

@@ -21,7 +21,6 @@
 #include "components/signin/public/base/signin_metrics.h"
 #include "components/signin/public/base/signin_pref_names.h"
 #include "components/signin/public/base/signin_switches.h"
-#include "components/signin/public/identity_manager/account_info.h"
 
 PrimaryAccountManager::PrimaryAccountManager(
     SigninClient* client,
@@ -108,7 +107,7 @@ void PrimaryAccountManager::Initialize(PrefService* local_state) {
       // in this case the account tracker should have the gaia_id so fetch it
       // from there.
       if (pref_gaia_id.empty()) {
-        AccountInfo info = account_tracker_service_->FindAccountInfoByEmail(
+        CoreAccountInfo info = account_tracker_service_->FindAccountInfoByEmail(
             pref_account_username);
         pref_gaia_id = info.gaia;
       }
@@ -142,7 +141,7 @@ void PrimaryAccountManager::Initialize(PrefService* local_state) {
   if (!pref_account_id.empty()) {
     if (account_tracker_service_->GetMigrationState() ==
         AccountTrackerService::MIGRATION_IN_PROGRESS) {
-      AccountInfo account_info =
+      CoreAccountInfo account_info =
           account_tracker_service_->FindAccountInfoByEmail(pref_account_id);
       // |account_info.gaia| could be empty if |account_id| is already gaia id.
       if (!account_info.gaia.empty()) {
@@ -151,7 +150,8 @@ void PrimaryAccountManager::Initialize(PrefService* local_state) {
                                        pref_account_id);
       }
     }
-    SetAuthenticatedAccountId(CoreAccountId(pref_account_id));
+    SetAuthenticatedAccountInfo(account_tracker_service_->GetAccountInfo(
+        CoreAccountId(pref_account_id)));
   }
   if (policy_manager_) {
     policy_manager_->InitializePolicy(local_state, this);
@@ -166,30 +166,19 @@ bool PrimaryAccountManager::IsInitialized() const {
   return initialized_;
 }
 
-AccountInfo PrimaryAccountManager::GetAuthenticatedAccountInfo() const {
-  return account_tracker_service_->GetAccountInfo(GetAuthenticatedAccountId());
+CoreAccountInfo PrimaryAccountManager::GetAuthenticatedAccountInfo() const {
+  return authenticated_account_info_.value_or(CoreAccountInfo());
 }
 
-const CoreAccountId& PrimaryAccountManager::GetAuthenticatedAccountId() const {
-  return authenticated_account_id_;
+CoreAccountId PrimaryAccountManager::GetAuthenticatedAccountId() const {
+  return GetAuthenticatedAccountInfo().account_id;
 }
 
 void PrimaryAccountManager::SetAuthenticatedAccountInfo(
-    const std::string& gaia_id,
-    const std::string& email) {
-  DCHECK(!gaia_id.empty());
-  DCHECK(!email.empty());
-
-  CoreAccountId account_id =
-      account_tracker_service_->SeedAccountInfo(gaia_id, email);
-  SetAuthenticatedAccountId(account_id);
-}
-
-void PrimaryAccountManager::SetAuthenticatedAccountId(
-    const CoreAccountId& account_id) {
-  DCHECK(!account_id.empty());
-  if (!authenticated_account_id_.empty()) {
-    DCHECK_EQ(account_id, authenticated_account_id_)
+    const CoreAccountInfo& account_info) {
+  DCHECK(!account_info.account_id.empty());
+  if (IsAuthenticated()) {
+    DCHECK_EQ(account_info.account_id, GetAuthenticatedAccountId())
         << "Changing the authenticated account while authenticated is not "
            "allowed.";
     return;
@@ -198,49 +187,50 @@ void PrimaryAccountManager::SetAuthenticatedAccountId(
   std::string pref_account_id =
       client_->GetPrefs()->GetString(prefs::kGoogleServicesAccountId);
 
-  DCHECK(pref_account_id.empty() || pref_account_id == account_id.id)
-      << "account_id=" << account_id << " pref_account_id=" << pref_account_id;
-  authenticated_account_id_ = account_id;
-  client_->GetPrefs()->SetString(prefs::kGoogleServicesAccountId,
-                                 account_id.id);
+  DCHECK(pref_account_id.empty() ||
+         pref_account_id == account_info.account_id.id)
+      << "account_id=" << account_info.account_id
+      << " pref_account_id=" << pref_account_id;
+  authenticated_account_info_ = account_info;
 
   // This preference is set so that code on I/O thread has access to the
   // Gaia id of the signed in user.
-  AccountInfo info = account_tracker_service_->GetAccountInfo(account_id);
+  client_->GetPrefs()->SetString(prefs::kGoogleServicesAccountId,
+                                 account_info.account_id.id);
 
   // When this function is called from Initialize(), it's possible for
   // |info.gaia| to be empty when migrating from a really old profile.
-  if (!info.gaia.empty()) {
+  if (!account_info.gaia.empty()) {
     client_->GetPrefs()->SetString(prefs::kGoogleServicesUserAccountId,
-                                   info.gaia);
+                                   account_info.gaia);
   }
 
   // Go ahead and update the last signed in account info here as well. Once a
   // user is signed in the corresponding preferences should match. Doing it here
   // as opposed to on signin allows us to catch the upgrade scenario.
   client_->GetPrefs()->SetString(prefs::kGoogleServicesLastAccountId,
-                                 account_id.id);
+                                 account_info.account_id.id);
   client_->GetPrefs()->SetString(prefs::kGoogleServicesLastUsername,
-                                 info.email);
+                                 account_info.email);
 
   // Commit authenticated account info immediately so that it does not get lost
   // if Chrome crashes before the next commit interval.
   client_->GetPrefs()->CommitPendingWrite();
 
   if (on_authenticated_account_set_callback_) {
-    on_authenticated_account_set_callback_.Run(info);
+    on_authenticated_account_set_callback_.Run(account_info);
   }
 }
 
-void PrimaryAccountManager::ClearAuthenticatedAccountId() {
-  authenticated_account_id_ = CoreAccountId();
+void PrimaryAccountManager::ClearAuthenticatedAccountInfo() {
+  authenticated_account_info_ = base::nullopt;
   if (on_authenticated_account_cleared_callback_) {
     on_authenticated_account_cleared_callback_.Run();
   }
 }
 
 bool PrimaryAccountManager::IsAuthenticated() const {
-  return !authenticated_account_id_.empty();
+  return authenticated_account_info_.has_value();
 }
 
 void PrimaryAccountManager::SetGoogleSigninSucceededCallback(
@@ -274,18 +264,26 @@ void PrimaryAccountManager::SetAuthenticatedAccountClearedCallback(
 }
 
 void PrimaryAccountManager::SignIn(const std::string& username) {
-  AccountInfo info = account_tracker_service_->FindAccountInfoByEmail(username);
+  CoreAccountInfo info =
+      account_tracker_service_->FindAccountInfoByEmail(username);
   DCHECK(!info.gaia.empty());
   DCHECK(!info.email.empty());
 
   bool reauth_in_progress = IsAuthenticated();
-
-  SetAuthenticatedAccountInfo(info.gaia, info.email);
+  SetAuthenticatedAccountInfo(info);
 
   if (!reauth_in_progress && on_google_signin_succeeded_callback_)
     on_google_signin_succeeded_callback_.Run(GetAuthenticatedAccountInfo());
 }
 
+void PrimaryAccountManager::UpdateAuthenticatedAccountInfo() {
+  DCHECK(authenticated_account_info_.has_value());
+  const CoreAccountInfo info = account_tracker_service_->GetAccountInfo(
+      authenticated_account_info_->account_id);
+  DCHECK_EQ(info.account_id, authenticated_account_info_->account_id);
+  authenticated_account_info_ = info;
+}
+
 #if !defined(OS_CHROMEOS)
 void PrimaryAccountManager::SignOut(
     signin_metrics::ProfileSignout signout_source_metric,
@@ -346,10 +344,9 @@ void PrimaryAccountManager::OnSignoutDecisionReached(
     return;
   }
 
-  AccountInfo account_info = GetAuthenticatedAccountInfo();
-  const CoreAccountId account_id = GetAuthenticatedAccountId();
-  const std::string username = account_info.email;
-  ClearAuthenticatedAccountId();
+  const CoreAccountInfo account_info = GetAuthenticatedAccountInfo();
+
+  ClearAuthenticatedAccountInfo();
   client_->GetPrefs()->ClearPref(prefs::kGoogleServicesHostedDomain);
   client_->GetPrefs()->ClearPref(prefs::kGoogleServicesAccountId);
   client_->GetPrefs()->ClearPref(prefs::kGoogleServicesUserAccountId);
@@ -365,10 +362,11 @@ void PrimaryAccountManager::OnSignoutDecisionReached(
               kPrimaryAccountManager_ClearAccount);
       break;
     case RemoveAccountsOption::kRemoveAuthenticatedAccountIfInError:
-      if (token_service_->RefreshTokenHasError(account_id))
+      if (token_service_->RefreshTokenHasError(account_info.account_id))
         token_service_->RevokeCredentials(
-            account_id, signin_metrics::SourceForRefreshTokenOperation::
-                            kPrimaryAccountManager_ClearAccount);
+            account_info.account_id,
+            signin_metrics::SourceForRefreshTokenOperation::
+                kPrimaryAccountManager_ClearAccount);
       break;
     case RemoveAccountsOption::kKeepAllAccounts:
       // Do nothing.
@@ -379,7 +377,7 @@ void PrimaryAccountManager::OnSignoutDecisionReached(
 }
 
 void PrimaryAccountManager::FireGoogleSignedOut(
-    const AccountInfo& account_info) {
+    const CoreAccountInfo& account_info) {
   if (on_google_signed_out_callback_) {
     on_google_signed_out_callback_.Run(account_info);
   }
@@ -397,8 +395,9 @@ void PrimaryAccountManager::OnRefreshTokensLoaded() {
   if (token_service_->HasLoadCredentialsFinishedWithNoErrors()) {
     std::vector<AccountInfo> accounts_in_tracker_service =
         account_tracker_service_->GetAccounts();
+    const CoreAccountId authenticated_account_id = GetAuthenticatedAccountId();
     for (const auto& account : accounts_in_tracker_service) {
-      if (GetAuthenticatedAccountId() != account.account_id &&
+      if (authenticated_account_id != account.account_id &&
           !token_service_->RefreshTokenIsAvailable(account.account_id)) {
         VLOG(0) << "Removed account from account tracker service: "
                 << account.account_id;

components/signin/internal/identity_manager/primary_account_manager.h

@@ -24,11 +24,12 @@
 #include "base/callback.h"
 #include "base/callback_list.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "components/signin/internal/identity_manager/profile_oauth2_token_service_observer.h"
 #include "components/signin/public/base/account_consistency_method.h"
 #include "components/signin/public/base/signin_client.h"
+#include "components/signin/public/identity_manager/account_info.h"
 
-struct AccountInfo;
 class AccountTrackerService;
 class PrefRegistrySimple;
 class PrefService;
@@ -42,7 +43,7 @@ enum class SignoutDelete;
 
 class PrimaryAccountManager : public ProfileOAuth2TokenServiceObserver {
  public:
-  typedef base::RepeatingCallback<void(const AccountInfo&)>
+  typedef base::RepeatingCallback<void(const CoreAccountInfo&)>
       AccountSigninCallback;
   typedef base::RepeatingCallback<void()> AccountClearedCallback;
 
@@ -78,25 +79,19 @@ class PrimaryAccountManager : public ProfileOAuth2TokenServiceObserver {
 
   // If a user has previously signed in (and has not signed out), this returns
   // the know information of the account. Otherwise, it returns an empty struct.
-  AccountInfo GetAuthenticatedAccountInfo() const;
+  CoreAccountInfo GetAuthenticatedAccountInfo() const;
 
   // If a user has previously signed in (and has not signed out), this returns
-  // the account id. Otherwise, it returns an empty string.  This id is the
-  // G+/Focus obfuscated gaia id of the user. It can be used to uniquely
+  // the account id. Otherwise, it returns an empty CoreAccountId.  This id is
+  // the G+/Focus obfuscated gaia id of the user. It can be used to uniquely
   // identify an account, so for example as a key to map accounts to data. For
   // code that needs a unique id to represent the connected account, call this
   // method. Example: the AccountStatusMap type in
   // MutableProfileOAuth2TokenService. For code that needs to know the
   // normalized email address of the connected account, use
-  // GetAuthenticatedAccountInfo().email.  Example: to show the string "Signed
-  // in as XXX" in the hotdog menu.
-  const CoreAccountId& GetAuthenticatedAccountId() const;
-
-  // Sets the authenticated user's Gaia ID and display email.  Internally,
-  // this will seed the account information in AccountTrackerService and pick
-  // the right account_id for this account.
-  void SetAuthenticatedAccountInfo(const std::string& gaia_id,
-                                   const std::string& email);
+  // GetAuthenticatedAccountInfo().email.  Example: to show the string
+  // "Signed in as XXX" in the hotdog menu.
+  CoreAccountId GetAuthenticatedAccountId() const;
 
   // Returns true if there is an authenticated user.
   bool IsAuthenticated() const;
@@ -123,6 +118,9 @@ class PrimaryAccountManager : public ProfileOAuth2TokenServiceObserver {
   // to look up the corresponding account_id and gaia_id for this email.
   void SignIn(const std::string& username);
 
+  // Updates the authenticated account information from AccountTrackerService.
+  void UpdateAuthenticatedAccountInfo();
+
   // Signout API surfaces (not supported on ChromeOS, where signout is not
   // permitted).
 #if !defined(OS_CHROMEOS)
@@ -159,13 +157,13 @@ class PrimaryAccountManager : public ProfileOAuth2TokenServiceObserver {
   // with a different account (this method will DCHECK in that case).
   // |account_id| must not be empty. To log the user out, use
   // ClearAuthenticatedAccountId() instead.
-  void SetAuthenticatedAccountId(const CoreAccountId& account_id);
+  void SetAuthenticatedAccountInfo(const CoreAccountInfo& account_info);
 
   // Clears the authenticated user's account id.
   // This method is not public because PrimaryAccountManager does not allow
   // signing out by default. Subclasses implementing a sign-out functionality
   // need to call this.
-  void ClearAuthenticatedAccountId();
+  void ClearAuthenticatedAccountInfo();
 
 #if !defined(OS_CHROMEOS)
   // Starts the sign out process.
@@ -181,7 +179,7 @@ class PrimaryAccountManager : public ProfileOAuth2TokenServiceObserver {
       SigninClient::SignoutDecision signout_decision);
 
   // Send all observers |GoogleSignedOut| notifications.
-  void FireGoogleSignedOut(const AccountInfo& account_info);
+  void FireGoogleSignedOut(const CoreAccountInfo& account_info);
 
   // ProfileOAuth2TokenServiceObserver:
   void OnRefreshTokensLoaded() override;
@@ -191,14 +189,13 @@ class PrimaryAccountManager : public ProfileOAuth2TokenServiceObserver {
 
   // The ProfileOAuth2TokenService instance associated with this object. Must
   // outlive this object.
-  ProfileOAuth2TokenService* token_service_;
-
-  AccountTrackerService* account_tracker_service_;
+  ProfileOAuth2TokenService* token_service_ = nullptr;
+  AccountTrackerService* account_tracker_service_ = nullptr;
 
-  bool initialized_;
+  bool initialized_ = false;
 
   // Account id after successful authentication.
-  CoreAccountId authenticated_account_id_;
+  base::Optional<CoreAccountInfo> authenticated_account_info_;
 
   // Callbacks which will be invoked, if set, for signin-related events.
   AccountSigninCallback on_google_signin_succeeded_callback_;

components/signin/internal/identity_manager/primary_account_manager_unittest.cc

@@ -124,11 +124,11 @@ class PrimaryAccountManagerTest : public testing::Test {
     EXPECT_EQ(1, num_successful_signins_);
   }
 
-  void GoogleSigninSucceeded(const AccountInfo& account_info) {
+  void GoogleSigninSucceeded(const CoreAccountInfo& account_info) {
     num_successful_signins_++;
   }
 
-  void GoogleSignedOut(const AccountInfo& account_info) { num_signouts_++; }
+  void GoogleSignedOut(const CoreAccountInfo& account_info) { num_signouts_++; }
 
   base::test::TaskEnvironment task_environment_;
   sync_preferences::TestingPrefServiceSyncable user_prefs_;
@@ -246,7 +246,8 @@ TEST_F(PrimaryAccountManagerTest, SignOutWhileProhibited) {
   EXPECT_TRUE(manager_->GetAuthenticatedAccountInfo().email.empty());
   EXPECT_TRUE(manager_->GetAuthenticatedAccountId().empty());
 
-  manager_->SetAuthenticatedAccountInfo("gaia_id", "user@gmail.com");
+  AddToAccountTracker("gaia_id", "user@gmail.com");
+  manager_->SignIn("user@gmail.com");
   signin_client()->set_is_signout_allowed(false);
   manager_->SignOut(signin_metrics::SIGNOUT_TEST,
                     signin_metrics::SignoutDelete::IGNORE_METRIC);

components/signin/internal/identity_manager/primary_account_policy_manager_impl.cc

@@ -39,7 +39,7 @@ void PrimaryAccountPolicyManagerImpl::InitializePolicy(
       base::Bind(&PrimaryAccountPolicyManagerImpl::OnSigninAllowedPrefChanged,
                  base::Unretained(this), primary_account_manager));
 
-  AccountInfo account_info =
+  CoreAccountInfo account_info =
       primary_account_manager->GetAuthenticatedAccountInfo();
   if (!account_info.account_id.empty() &&
       (!IsAllowedUsername(account_info.email) || !IsSigninAllowed())) {

components/signin/public/identity_manager/identity_manager.cc

@@ -98,14 +98,11 @@ IdentityManager::IdentityManager(
       base::BindRepeating(&IdentityManager::OnRefreshTokenRevokedFromSource,
                           base::Unretained(this)));
 
-  // Seed the primary account with any state that |primary_account_manager_|
+  // Seed the unconsented primary account with any state that
+  // |primary_account_manager_| and |account_tracker_service_|
   // loaded from prefs.
-  if (primary_account_manager_->IsAuthenticated()) {
-    CoreAccountInfo account =
-        primary_account_manager_->GetAuthenticatedAccountInfo();
-    DCHECK(!account.account_id.empty());
-    SetPrimaryAccountInternal(std::move(account));
-  }
+  if (primary_account_manager_->IsAuthenticated())
+    UpdateUnconsentedPrimaryAccount();
 
 #if defined(OS_ANDROID)
   java_identity_manager_ = Java_IdentityManager_create(
@@ -139,21 +136,7 @@ void IdentityManager::RemoveObserver(Observer* observer) {
 
 // TODO(862619) change return type to base::Optional<CoreAccountInfo>
 CoreAccountInfo IdentityManager::GetPrimaryAccountInfo() const {
-  DCHECK_EQ(primary_account_.has_value(),
-            primary_account_manager_->IsAuthenticated());
-  auto result = primary_account_.value_or(CoreAccountInfo());
-  DCHECK_EQ(result.account_id,
-            primary_account_manager_->GetAuthenticatedAccountId());
-#if DCHECK_IS_ON()
-  CoreAccountInfo primary_account_manager_account =
-      primary_account_manager_->GetAuthenticatedAccountInfo();
-  if (!primary_account_manager_account.account_id.empty()) {
-    DCHECK_EQ(primary_account_manager_account, result)
-        << "If primary_account_manager_'s account is set (account has a "
-           "refresh token), primary_account_ must have the same value.";
-  }
-#endif
-  return result;
+  return primary_account_manager_->GetAuthenticatedAccountInfo();
 }
 
 CoreAccountId IdentityManager::GetPrimaryAccountId() const {
@@ -161,9 +144,7 @@ CoreAccountId IdentityManager::GetPrimaryAccountId() const {
 }
 
 bool IdentityManager::HasPrimaryAccount() const {
-  DCHECK_EQ(primary_account_.has_value(),
-            primary_account_manager_->IsAuthenticated());
-  return primary_account_.has_value();
+  return primary_account_manager_->IsAuthenticated();
 }
 
 CoreAccountId IdentityManager::GetUnconsentedPrimaryAccountId() const {
@@ -397,10 +378,7 @@ IdentityManager::GetAccountIdMigrationState() const {
 CoreAccountId IdentityManager::PickAccountIdForAccount(
     const std::string& gaia,
     const std::string& email) const {
-  // TODO(triploblastic@): Remove explicit conversion once
-  // primary_account_manager has been fixed to use CoreAccountId.
-  return CoreAccountId(
-      account_tracker_service_->PickAccountIdForAccount(gaia, email));
+  return account_tracker_service_->PickAccountIdForAccount(gaia, email);
 }
 
 // static
@@ -502,12 +480,6 @@ AccountInfo IdentityManager::GetAccountInfoForAccountWithRefreshToken(
   return account_info;
 }
 
-void IdentityManager::SetPrimaryAccountInternal(
-    base::Optional<CoreAccountInfo> account_info) {
-  primary_account_ = std::move(account_info);
-  UpdateUnconsentedPrimaryAccount();
-}
-
 void IdentityManager::UpdateUnconsentedPrimaryAccount() {
   base::Optional<CoreAccountInfo> new_unconsented_primary_account =
       ComputeUnconsentedPrimaryAccountInfo();
@@ -544,7 +516,8 @@ IdentityManager::ComputeUnconsentedPrimaryAccountInfo() const {
 #endif
 }
 
-void IdentityManager::GoogleSigninSucceeded(const AccountInfo& account_info) {
+void IdentityManager::GoogleSigninSucceeded(
+    const CoreAccountInfo& account_info) {
   for (auto& observer : observer_list_) {
     observer.OnPrimaryAccountSet(account_info);
   }
@@ -556,8 +529,9 @@ void IdentityManager::GoogleSigninSucceeded(const AccountInfo& account_info) {
 #endif
 }
 
-void IdentityManager::GoogleSignedOut(const AccountInfo& account_info) {
+void IdentityManager::GoogleSignedOut(const CoreAccountInfo& account_info) {
   DCHECK(!HasPrimaryAccount());
+  DCHECK(!account_info.IsEmpty());
   for (auto& observer : observer_list_) {
     observer.OnPrimaryAccountCleared(account_info);
   }
@@ -568,13 +542,16 @@ void IdentityManager::GoogleSignedOut(const AccountInfo& account_info) {
         ConvertToJavaCoreAccountInfo(account_info));
 #endif
 }
-void IdentityManager::AuthenticatedAccountSet(const AccountInfo& account_info) {
+
+void IdentityManager::AuthenticatedAccountSet(
+    const CoreAccountInfo& account_info) {
   DCHECK(primary_account_manager_->IsAuthenticated());
-  SetPrimaryAccountInternal(account_info);
+  UpdateUnconsentedPrimaryAccount();
 }
+
 void IdentityManager::AuthenticatedAccountCleared() {
   DCHECK(!primary_account_manager_->IsAuthenticated());
-  SetPrimaryAccountInternal(base::nullopt);
+  UpdateUnconsentedPrimaryAccount();
 }
 
 void IdentityManager::OnRefreshTokenAvailable(const CoreAccountId& account_id) {
@@ -679,9 +656,14 @@ void IdentityManager::OnRefreshTokenRevokedFromSource(
 }
 
 void IdentityManager::OnAccountUpdated(const AccountInfo& info) {
-  if (primary_account_ && primary_account_->account_id == info.account_id) {
-    SetPrimaryAccountInternal(info);
+  if (HasPrimaryAccount()) {
+    const CoreAccountId primary_account_id = GetPrimaryAccountId();
+    if (primary_account_id == info.account_id) {
+      primary_account_manager_->UpdateAuthenticatedAccountInfo();
+      UpdateUnconsentedPrimaryAccount();
+    }
   }
+
   for (auto& observer : observer_list_) {
     observer.OnExtendedAccountInfoUpdated(info);
   }

components/signin/public/identity_manager/identity_manager.h

@@ -611,9 +611,9 @@ class IdentityManager : public KeyedService,
   base::Optional<CoreAccountInfo> ComputeUnconsentedPrimaryAccountInfo() const;
 
   // PrimaryAccountManager callbacks:
-  void GoogleSigninSucceeded(const AccountInfo& account_info);
-  void GoogleSignedOut(const AccountInfo& account_info);
-  void AuthenticatedAccountSet(const AccountInfo& account_info);
+  void GoogleSigninSucceeded(const CoreAccountInfo& account_info);
+  void GoogleSignedOut(const CoreAccountInfo& account_info);
+  void AuthenticatedAccountSet(const CoreAccountInfo& account_info);
   void AuthenticatedAccountCleared();
 
   // ProfileOAuth2TokenServiceObserver:
@@ -685,8 +685,8 @@ class IdentityManager : public KeyedService,
   base::ObserverList<DiagnosticsObserver, true>::Unchecked
       diagnostics_observer_list_;
 
-  // If |primary_account_| is set, it must equal |unconsented_primary_account_|.
-  base::Optional<CoreAccountInfo> primary_account_;
+  // If HasPrimaryAccount() is true, then |unconsented_primary_account_|
+  // must be equal to the value returned by GetPrimaryAccountInfo().
   base::Optional<CoreAccountInfo> unconsented_primary_account_;
 
 #if defined(OS_ANDROID)

components/signin/public/identity_manager/identity_manager_unittest.cc

@@ -347,8 +347,8 @@ class IdentityManagerTest : public testing::Test {
 
     if (primary_account_manager_setup ==
         PrimaryAccountManagerSetup::kWithAuthenticatedAccout) {
-      primary_account_manager->SetAuthenticatedAccountInfo(kTestGaiaId,
-                                                           kTestEmail);
+      account_tracker_service->SeedAccountInfo(kTestGaiaId, kTestEmail);
+      primary_account_manager->SignIn(kTestEmail);
     }
 
     auto accounts_cookie_mutator = std::make_unique<AccountsCookieMutatorImpl>(
@@ -549,6 +549,9 @@ TEST_F(IdentityManagerTest, HasPrimaryAccount) {
   ClearPrimaryAccount(identity_manager(), ClearPrimaryAccountPolicy::DEFAULT);
   EXPECT_FALSE(identity_manager()->HasPrimaryAccount());
   EXPECT_FALSE(identity_manager()->HasUnconsentedPrimaryAccount());
+  EXPECT_FALSE(identity_manager_observer()
+                   ->PrimaryAccountFromClearedCallback()
+                   .IsEmpty());
 #endif
 }
 
@@ -1080,8 +1083,9 @@ TEST_F(IdentityManagerTest, RemoveAccessTokenFromCache) {
   std::set<std::string> scopes{"scope"};
   std::string access_token = "access_token";
 
-  identity_manager()->GetPrimaryAccountManager()->SetAuthenticatedAccountInfo(
-      kTestGaiaId, kTestEmail);
+  identity_manager()->GetAccountTrackerService()->SeedAccountInfo(kTestGaiaId,
+                                                                  kTestEmail);
+  identity_manager()->GetPrimaryAccountManager()->SignIn(kTestEmail);
   token_service()->UpdateCredentials(primary_account_id(), "refresh_token");
 
   base::RunLoop run_loop;
@@ -1119,8 +1123,9 @@ TEST_F(IdentityManagerTest,
   identity_manager_diagnostics_observer()
       ->set_on_access_token_requested_callback(run_loop.QuitClosure());
 
-  identity_manager()->GetPrimaryAccountManager()->SetAuthenticatedAccountInfo(
-      kTestGaiaId, kTestEmail);
+  identity_manager()->GetAccountTrackerService()->SeedAccountInfo(kTestGaiaId,
+                                                                  kTestEmail);
+  identity_manager()->GetPrimaryAccountManager()->SignIn(kTestEmail);
   token_service()->UpdateCredentials(primary_account_id(), "refresh_token");
 
   std::set<std::string> scopes{"scope"};
@@ -1207,8 +1212,9 @@ TEST_F(IdentityManagerTest, ObserveAccessTokenFetch) {
   identity_manager_diagnostics_observer()
       ->set_on_access_token_requested_callback(run_loop.QuitClosure());
 
-  identity_manager()->GetPrimaryAccountManager()->SetAuthenticatedAccountInfo(
-      kTestGaiaId, kTestEmail);
+  identity_manager()->GetAccountTrackerService()->SeedAccountInfo(kTestGaiaId,
+                                                                  kTestEmail);
+  identity_manager()->GetPrimaryAccountManager()->SignIn(kTestEmail);
   token_service()->UpdateCredentials(primary_account_id(), "refresh_token");
 
   std::set<std::string> scopes{"scope"};
@@ -1260,8 +1266,9 @@ TEST_F(IdentityManagerTest,
   identity_manager_diagnostics_observer()
       ->set_on_access_token_request_completed_callback(run_loop.QuitClosure());
 
-  identity_manager()->GetPrimaryAccountManager()->SetAuthenticatedAccountInfo(
-      kTestGaiaId, kTestEmail);
+  identity_manager()->GetAccountTrackerService()->SeedAccountInfo(kTestGaiaId,
+                                                                  kTestEmail);
+  identity_manager()->GetPrimaryAccountManager()->SignIn(kTestEmail);
   token_service()->UpdateCredentials(primary_account_id(), "refresh_token");
   token_service()->set_auto_post_fetch_response_on_message_loop(true);
 
@@ -2022,8 +2029,9 @@ TEST_F(IdentityManagerTest, OnNetworkInitialized) {
 
 TEST_F(IdentityManagerTest,
        BatchChangeObserversAreNotifiedOnCredentialsUpdate) {
-  identity_manager()->GetPrimaryAccountManager()->SetAuthenticatedAccountInfo(
-      kTestGaiaId, kTestEmail);
+  identity_manager()->GetAccountTrackerService()->SeedAccountInfo(kTestGaiaId,
+                                                                  kTestEmail);
+  identity_manager()->GetPrimaryAccountManager()->SignIn(kTestEmail);
   token_service()->UpdateCredentials(primary_account_id(), "refresh_token");
 
   EXPECT_EQ(1ul, identity_manager_observer()->BatchChangeRecords().size());