Make LegacySetPrimaryAccount() go through SignIn()
This CL changes LegacySetPrimaryAccount() to call
PrimaAccountManager::SignIn() rather than SetAuthenticatedAccountInfo().
This is part of a series of CLs to port ChromeOS to use
PrimaryAccountMutator, to unify its behaviour with the other platforms,
as described in
https://docs.google.com/document/d/15y-Db27BV08vrIyelHB-3CwiAfDYh-FigNKGXrqSWto
Bug: 814787, 944012
Change-Id: Idf67c8238dfef3991e6245018c0bc1f2b244f6cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1651732
Commit-Queue: Henrique Ferreiro <hferreiro@igalia.com>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#669201}
diff --git a/components/signin/core/browser/primary_account_manager.cc b/components/signin/core/browser/primary_account_manager.cc
index d7f70f4d..5f0af06 100644
--- a/components/signin/core/browser/primary_account_manager.cc
+++ b/components/signin/core/browser/primary_account_manager.cc
@@ -256,7 +256,6 @@
observer_ = nullptr;
}
-#if !defined(OS_CHROMEOS)
void PrimaryAccountManager::SignIn(const std::string& username) {
AccountInfo info = account_tracker_service_->FindAccountInfoByEmail(username);
DCHECK(!info.gaia.empty());
@@ -277,6 +276,7 @@
client_->GetInstallDate());
}
+#if !defined(OS_CHROMEOS)
void PrimaryAccountManager::SignOut(
signin_metrics::ProfileSignout signout_source_metric,
signin_metrics::SignoutDelete signout_delete_metric) {
diff --git a/components/signin/core/browser/primary_account_manager.h b/components/signin/core/browser/primary_account_manager.h
index f7f8c20..6599ac6 100644
--- a/components/signin/core/browser/primary_account_manager.h
+++ b/components/signin/core/browser/primary_account_manager.h
@@ -124,14 +124,9 @@
void SetObserver(Observer* observer);
void ClearObserver();
- // Signin API surfaces. Not used on ChromeOS.
- // TODO(https://crbug.com/814787): Go through this API to set the primary
- // account on ChromeOS.
-#if !defined(OS_CHROMEOS)
- // Signs a user in. PrimaryAccountPolicyManager assumes that |username| can be
- // used to look up the corresponding account_id and gaia_id for this email.
+ // Signs a user in. PrimaryAccountManager assumes that |username| can be used
+ // to look up the corresponding account_id and gaia_id for this email.
void SignIn(const std::string& username);
-#endif
// Signout API surfaces (not supported on ChromeOS, where signout is not
// permitted).
diff --git a/services/identity/public/cpp/identity_manager.cc b/services/identity/public/cpp/identity_manager.cc
index 5d30a89..72db9b0 100644
--- a/services/identity/public/cpp/identity_manager.cc
+++ b/services/identity/public/cpp/identity_manager.cc
@@ -388,11 +388,13 @@
void IdentityManager::LegacySetPrimaryAccount(
const std::string& gaia_id,
const std::string& email_address) {
- primary_account_manager_->SetAuthenticatedAccountInfo(gaia_id, email_address);
-
- // TODO(https://crbug.com/944012): Unify the firing of this observer
- // notification between ChromeOS and other platforms.
- FireOnPrimaryAccountSetNotification(primary_account_.value());
+ // On ChromeOS the primary account is not guaranteed to be present in
+ // AccountTrackerService when it is set, but PrimaryAccountManager::SignIn()
+ // requires that it be so.
+ // TODO(https://crbug.com/967602): Eliminate the need to seed the account
+ // here.
+ account_tracker_service_->SeedAccountInfo(gaia_id, email_address);
+ primary_account_manager_->SignIn(email_address);
}
#endif
@@ -492,15 +494,10 @@
return account_info;
}
-void IdentityManager::FireOnPrimaryAccountSetNotification(
- const CoreAccountInfo& primary_account_info) {
- for (auto& observer : observer_list_) {
- observer.OnPrimaryAccountSet(primary_account_info);
- }
-}
-
void IdentityManager::GoogleSigninSucceeded(const AccountInfo& account_info) {
- FireOnPrimaryAccountSetNotification(account_info);
+ for (auto& observer : observer_list_) {
+ observer.OnPrimaryAccountSet(account_info);
+ }
}
void IdentityManager::GoogleSignedOut(const AccountInfo& account_info) {
diff --git a/services/identity/public/cpp/identity_manager.h b/services/identity/public/cpp/identity_manager.h
index a341ae0..be36eba 100644
--- a/services/identity/public/cpp/identity_manager.h
+++ b/services/identity/public/cpp/identity_manager.h
@@ -582,14 +582,6 @@
AccountInfo GetAccountInfoForAccountWithRefreshToken(
const CoreAccountId& account_id) const;
- // Fires the IdentityManager::Observer::OnPrimaryAccountSet() notification
- // to observers.
- // TODO(https://crbug.com/944012): Unify the firing of this observer
- // notification between ChromeOS and other platforms and eliminate the need
- // for this helper method.
- void FireOnPrimaryAccountSetNotification(
- const CoreAccountInfo& primary_account_info);
-
// PrimaryAccountManager::Observer:
void GoogleSigninSucceeded(const AccountInfo& account_info) override;
void GoogleSignedOut(const AccountInfo& account_info) override;
diff --git a/services/identity/public/cpp/identity_test_utils.cc b/services/identity/public/cpp/identity_test_utils.cc
index b5b2840..a76afa7 100644
--- a/services/identity/public/cpp/identity_test_utils.cc
+++ b/services/identity/public/cpp/identity_test_utils.cc
@@ -94,16 +94,7 @@
std::string gaia_id = account_info.gaia;
DCHECK(!gaia_id.empty());
-#if defined(OS_CHROMEOS)
- // ChromeOS has no real notion of signin, so just plumb the information
- // through. TODO(https://crbug.com/814787): What is the right long-term
- // solution here? Tests shouldn't need to call legacy APIs, so either the
- // need for this test flow should be eliminated or there should be a
- // test-only API.
- identity_manager->LegacySetPrimaryAccount(gaia_id, email);
-#else
primary_account_manager->SignIn(email);
-#endif
DCHECK(primary_account_manager->IsAuthenticated());
DCHECK(identity_manager->HasPrimaryAccount());
