[signin] Sort GetOrderedAccountsForDisplay based on cookie jar

Change GetOrderedAccountsForDisplay to put the primary Chrome account
first, then the default account, and then the other accounts. This is
done through referring to the order of the accounts in the cookie jar
rather than the vector of accounts with refresh tokens.

Bug: b/301428428

Change-Id: Ie16e03d85939edca8417d2180e0949f5f81b3482
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4942030
Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: Amelie Schneider <amelies@google.com>
Cr-Commit-Position: refs/heads/main@{#1211473}

chrome/browser/signin/signin_ui_util.cc

@@ -361,41 +361,57 @@ void EnableSyncFromMultiAccountPromo(Profile* profile,
 }
 
 std::vector<AccountInfo> GetOrderedAccountsForDisplay(
-    Profile* profile,
+    signin::IdentityManager* identity_manager,
     bool restrict_to_accounts_eligible_for_sync) {
-  // Fetch account ids for accounts that have a token.
-  signin::IdentityManager* identity_manager =
-      IdentityManagerFactory::GetForProfile(profile);
+  // Fetch account ids for accounts that have a token and are in cookie jar.
   std::vector<AccountInfo> accounts_with_tokens =
       identity_manager->GetExtendedAccountInfoForAccountsWithRefreshToken();
-
+  signin::AccountsInCookieJarInfo accounts_in_jar =
+      identity_manager->GetAccountsInCookieJar();
   // Compute the default account.
   CoreAccountId default_account_id =
       identity_manager->GetPrimaryAccountId(signin::ConsentLevel::kSignin);
 
-  // Fetch account information for each id and make sure that the first account
-  // in the list matches the unconsented primary account (if available).
   std::vector<AccountInfo> accounts;
-  for (auto& account_info : accounts_with_tokens) {
-    DCHECK(!account_info.IsEmpty());
-    if (restrict_to_accounts_eligible_for_sync &&
-        !signin::IsUsernameAllowedByPatternFromPrefs(
-            g_browser_process->local_state(), account_info.email)) {
+
+  // First, add the primary account (if available), even if it is not in the
+  // cookie jar.
+  std::vector<AccountInfo>::iterator it = base::ranges::find(
+      accounts_with_tokens, default_account_id, &AccountInfo::account_id);
+
+  if (it != accounts_with_tokens.end()) {
+    accounts.push_back(std::move(*it));
+  }
+
+  // Then, add the other accounts in the order of the accounts in the cookie
+  // jar.
+  for (auto& account_info : accounts_in_jar.signed_in_accounts) {
+    DCHECK(!account_info.id.empty());
+    if (account_info.id == default_account_id ||
+        (restrict_to_accounts_eligible_for_sync &&
+         !signin::IsUsernameAllowedByPatternFromPrefs(
+             g_browser_process->local_state(), account_info.email))) {
       continue;
     }
-    if (account_info.account_id == default_account_id)
-      accounts.insert(accounts.begin(), std::move(account_info));
-    else
-      accounts.push_back(std::move(account_info));
+
+    // Only insert the account if it has a refresh token, because we need the
+    // account info.
+    it = base::ranges::find(accounts_with_tokens, account_info.id,
+                            &AccountInfo::account_id);
+
+    if (it != accounts_with_tokens.end()) {
+      accounts.push_back(std::move(*it));
+    }
   }
   return accounts;
 }
 
 #if !BUILDFLAG(IS_CHROMEOS_ASH)
 
-AccountInfo GetSingleAccountForPromos(Profile* profile) {
+AccountInfo GetSingleAccountForPromos(
+    signin::IdentityManager* identity_manager) {
   std::vector<AccountInfo> accounts = GetOrderedAccountsForDisplay(
-      profile, /*restrict_to_accounts_eligible_for_sync=*/true);
+      identity_manager, /*restrict_to_accounts_eligible_for_sync=*/true);
   if (!accounts.empty())
     return accounts[0];
   return AccountInfo();

chrome/browser/signin/signin_ui_util.h

@@ -16,6 +16,7 @@
 #include "chrome/browser/ui/signin/signin_reauth_view_controller.h"
 #include "components/signin/public/base/signin_buildflags.h"
 #include "components/signin/public/base/signin_metrics.h"
+#include "components/signin/public/identity_manager/identity_manager.h"
 
 struct AccountInfo;
 struct CoreAccountInfo;
@@ -85,12 +86,13 @@ void EnableSyncFromMultiAccountPromo(Profile* profile,
 // |restrict_to_accounts_eligible_for_sync| is true, removes the account that
 // are not suitable for sync promos.
 std::vector<AccountInfo> GetOrderedAccountsForDisplay(
-    Profile* profile,
+    signin::IdentityManager* identity_manager,
     bool restrict_to_accounts_eligible_for_sync);
 
 #if !BUILDFLAG(IS_CHROMEOS_ASH)
 // Returns single account to use in promos.
-AccountInfo GetSingleAccountForPromos(Profile* profile);
+AccountInfo GetSingleAccountForPromos(
+    signin::IdentityManager* identity_manager);
 
 #endif
 

chrome/browser/signin/signin_ui_util_unittest.cc

@@ -404,12 +404,91 @@ TEST_F(SigninUiUtilTest, EnableSyncForNewAccountWithOneTab) {
 }
 
 TEST_F(SigninUiUtilTest, GetOrderedAccountsForDisplay) {
-  // Should start off with no accounts.
-  std::vector<AccountInfo> accounts = GetOrderedAccountsForDisplay(
-      profile(), /*restrict_to_accounts_eligible_for_sync=*/true);
-  EXPECT_TRUE(accounts.empty());
-
-  // TODO(tangltom): Flesh out this test.
+  signin::IdentityManager* identity_manager_empty =
+      IdentityManagerFactory::GetForProfile(profile());
+  std::vector<AccountInfo> accounts_empty = GetOrderedAccountsForDisplay(
+      identity_manager_empty, /*restrict_to_accounts_eligible_for_sync=*/true);
+  EXPECT_TRUE(accounts_empty.empty());
+
+  // Fill with accounts.
+  const char kTestEmail1[] = "me1@gmail.com";
+  const char kTestEmail2[] = "me2@gmail.com";
+  const char kTestEmail3[] = "me3@gmail.com";
+  const char kTestEmail4[] = "me4@gmail.com";
+
+  network::TestURLLoaderFactory url_loader_factory_ =
+      network::TestURLLoaderFactory();
+  signin::IdentityTestEnvironment identity_test_env(&url_loader_factory_);
+  signin::IdentityManager* identity_manager =
+      identity_test_env.identity_manager();
+
+  // The cookies are added separately in order to show behaviour in the case
+  // that refresh tokens and cookies are not added at the same time.
+  identity_test_env.MakeAccountAvailable(kTestEmail1);
+  identity_test_env.MakeAccountAvailable(kTestEmail2);
+  identity_test_env.MakeAccountAvailable(kTestEmail3);
+  identity_test_env.MakeAccountAvailable(kTestEmail4);
+
+  identity_test_env.SetCookieAccounts(
+      {{kTestEmail4, signin::GetTestGaiaIdForEmail(kTestEmail4)},
+       {kTestEmail3, signin::GetTestGaiaIdForEmail(kTestEmail3)},
+       {kTestEmail2, signin::GetTestGaiaIdForEmail(kTestEmail2)},
+       {kTestEmail1, signin::GetTestGaiaIdForEmail(kTestEmail1)}});
+
+  // No primary account set.
+  std::vector<AccountInfo> accounts =
+      GetOrderedAccountsForDisplay(identity_manager, false);
+
+  EXPECT_EQ(signin::GetTestGaiaIdForEmail(kTestEmail4),
+            accounts[0].account_id.ToString());
+  EXPECT_EQ(signin::GetTestGaiaIdForEmail(kTestEmail3),
+            accounts[1].account_id.ToString());
+  EXPECT_EQ(signin::GetTestGaiaIdForEmail(kTestEmail2),
+            accounts[2].account_id.ToString());
+  EXPECT_EQ(signin::GetTestGaiaIdForEmail(kTestEmail1),
+            accounts[3].account_id.ToString());
+
+  // Set a primary account.
+  identity_test_env.SetPrimaryAccount(kTestEmail3,
+                                      signin::ConsentLevel::kSignin);
+  accounts = GetOrderedAccountsForDisplay(identity_manager, false);
+
+  EXPECT_EQ(signin::GetTestGaiaIdForEmail(kTestEmail3),
+            accounts[0].account_id.ToString());
+  EXPECT_EQ(signin::GetTestGaiaIdForEmail(kTestEmail4),
+            accounts[1].account_id.ToString());
+  EXPECT_EQ(signin::GetTestGaiaIdForEmail(kTestEmail2),
+            accounts[2].account_id.ToString());
+  EXPECT_EQ(signin::GetTestGaiaIdForEmail(kTestEmail1),
+            accounts[3].account_id.ToString());
+
+  // Set a different primary account.
+  identity_test_env.SetPrimaryAccount(kTestEmail1,
+                                      signin::ConsentLevel::kSignin);
+  accounts = GetOrderedAccountsForDisplay(identity_manager, false);
+
+  EXPECT_EQ(signin::GetTestGaiaIdForEmail(kTestEmail1),
+            accounts[0].account_id.ToString());
+  EXPECT_EQ(signin::GetTestGaiaIdForEmail(kTestEmail4),
+            accounts[1].account_id.ToString());
+  EXPECT_EQ(signin::GetTestGaiaIdForEmail(kTestEmail3),
+            accounts[2].account_id.ToString());
+  EXPECT_EQ(signin::GetTestGaiaIdForEmail(kTestEmail2),
+            accounts[3].account_id.ToString());
+
+  // Primary account should still be included if not in cookies, other accounts
+  // should not.
+  identity_test_env.SetCookieAccounts(
+      {{kTestEmail4, signin::GetTestGaiaIdForEmail(kTestEmail4)},
+       {kTestEmail2, signin::GetTestGaiaIdForEmail(kTestEmail2)}});
+  accounts = GetOrderedAccountsForDisplay(identity_manager, false);
+
+  EXPECT_EQ(signin::GetTestGaiaIdForEmail(kTestEmail1),
+            accounts[0].account_id.ToString());
+  EXPECT_EQ(signin::GetTestGaiaIdForEmail(kTestEmail4),
+            accounts[1].account_id.ToString());
+  EXPECT_EQ(signin::GetTestGaiaIdForEmail(kTestEmail2),
+            accounts[2].account_id.ToString());
 }
 
 TEST_F(SigninUiUtilTest, MergeDiceSigninTab) {

chrome/browser/ui/views/profiles/profile_menu_view.cc

@@ -498,7 +498,8 @@ void ProfileMenuView::BuildIdentity() {
   } else {
     if (base::FeatureList::IsEnabled(switches::kUnoDesktop) &&
         account.IsEmpty()) {
-      account_info = signin_ui_util::GetSingleAccountForPromos(profile);
+      account_info =
+          signin_ui_util::GetSingleAccountForPromos(identity_manager);
     }
     menu_title_ = l10n_util::GetStringUTF16(IDS_PROFILES_LOCAL_PROFILE_STATE);
     // The email may be empty.
@@ -609,7 +610,7 @@ void ProfileMenuView::BuildSyncInfo() {
   CoreAccountInfo account_info =
       identity_manager->GetPrimaryAccountInfo(signin::ConsentLevel::kSignin);
   AccountInfo account_info_for_promos =
-      signin_ui_util::GetSingleAccountForPromos(profile);
+      signin_ui_util::GetSingleAccountForPromos(identity_manager);
   std::u16string description;
   std::u16string button_text;
   ActionableItem button_type = ActionableItem::kSigninAccountButton;

chrome/browser/ui/views/sync/bubble_sync_promo_view.cc

@@ -14,6 +14,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_avatar_icon_util.h"
 #include "chrome/browser/signin/account_consistency_mode_manager.h"
+#include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/browser/signin/signin_ui_util.h"
 #include "chrome/browser/ui/ui_features.h"
 #include "chrome/browser/ui/views/chrome_layout_provider.h"
@@ -37,8 +38,11 @@ BubbleSyncPromoView::BubbleSyncPromoView(
   DCHECK(!profile->IsGuestSession());
   AccountInfo account;
   // Signin promos can be shown in incognito, they use an empty account list.
-  if (!profile->IsOffTheRecord())
-    account = signin_ui_util::GetSingleAccountForPromos(profile);
+  if (!profile->IsOffTheRecord()) {
+    signin::IdentityManager* identity_manager =
+        IdentityManagerFactory::GetForProfile(profile);
+    account = signin_ui_util::GetSingleAccountForPromos(identity_manager);
+  }
 
   const views::LayoutOrientation orientation =
       account.IsEmpty() ? views::LayoutOrientation::kHorizontal

chrome/browser/ui/webui/history/history_login_handler.cc

@@ -66,13 +66,14 @@ void HistoryLoginHandler::ProfileInfoChanged() {
 void HistoryLoginHandler::HandleTurnOnSyncFlow(
     const base::Value::List& /*args*/) {
   Profile* profile = Profile::FromWebUI(web_ui());
+  signin::IdentityManager* identity_manager =
+      IdentityManagerFactory::GetForProfile(profile);
   CoreAccountInfo account_info =
-      IdentityManagerFactory::GetForProfile(profile)->GetPrimaryAccountInfo(
-          signin::ConsentLevel::kSignin);
+      identity_manager->GetPrimaryAccountInfo(signin::ConsentLevel::kSignin);
 #if !BUILDFLAG(IS_CHROMEOS_ASH)
   if (base::FeatureList::IsEnabled(switches::kUnoDesktop) &&
       account_info.IsEmpty()) {
-    account_info = signin_ui_util::GetSingleAccountForPromos(profile);
+    account_info = signin_ui_util::GetSingleAccountForPromos(identity_manager);
   }
 #endif  // !BUILDFLAG(IS_CHROMEOS_ASH)
   signin_ui_util::EnableSyncFromSingleAccountPromo(

chrome/browser/ui/webui/history/history_ui.cc

@@ -128,7 +128,8 @@ content::WebUIDataSource* CreateAndAddHistoryUIHTMLSource(Profile* profile) {
       IdentityManagerFactory::GetForProfile(profile);
   bool has_primary_account =
       identity_manager->HasPrimaryAccount(signin::ConsentLevel::kSignin);
-  AccountInfo account_info = signin_ui_util::GetSingleAccountForPromos(profile);
+  AccountInfo account_info =
+      signin_ui_util::GetSingleAccountForPromos(identity_manager);
   if (base::FeatureList::IsEnabled(switches::kUnoDesktop) &&
       !has_primary_account && !account_info.IsEmpty()) {
     source->AddString("turnOnSyncButton",

chrome/browser/ui/webui/settings/people_handler.cc

@@ -439,6 +439,9 @@ void PeopleHandler::OnExtendedAccountInfoRemoved(const AccountInfo& info) {
 base::Value::List PeopleHandler::GetStoredAccountsList() {
   base::Value::List accounts;
   bool populate_accounts_list = false;
+  signin::IdentityManager* identity_manager =
+      IdentityManagerFactory::GetForProfile(profile_);
+
 #if BUILDFLAG(ENABLE_DICE_SUPPORT)
   populate_accounts_list =
       AccountConsistencyModeManager::IsDiceEnabledForProfile(profile_);
@@ -449,7 +452,8 @@ base::Value::List PeopleHandler::GetStoredAccountsList() {
   if (populate_accounts_list) {
     // If dice is enabled, show all the accounts.
     for (const auto& account : signin_ui_util::GetOrderedAccountsForDisplay(
-             profile_, /*restrict_to_accounts_eligible_for_sync=*/true)) {
+             identity_manager,
+             /*restrict_to_accounts_eligible_for_sync=*/true)) {
       accounts.Append(GetAccountValue(account));
     }
     return accounts;
@@ -462,7 +466,6 @@ base::Value::List PeopleHandler::GetStoredAccountsList() {
   // Chrome OS) or Lacros main profile (sync with a different account than the
   // device account is not allowed), then show only the primary account,
   // whether or not that account has consented to sync.
-  auto* identity_manager = IdentityManagerFactory::GetForProfile(profile_);
   AccountInfo primary_account_info = identity_manager->FindExtendedAccountInfo(
       identity_manager->GetPrimaryAccountInfo(ConsentLevel::kSignin));
   if (!primary_account_info.IsEmpty())