[FedCM] Show account chooser for non-returning user on approved client

Currently on button mode, for this particular case of a logged out
user signing in as a non-returning user on an RP which is in the list
of approved clients, we would show disclosure UI without the
disclosure text. This patch makes it such that we show the account
chooser instead because the disclosure UI without the disclosure text
is unnecessary new UI surface which doesn't convey any additional
information compared to the account chooser.

Note that we show the multi account picker instead of the single account
picker. This is because when we were showing disclosure UI, the user was
able to go back and retrieve the original list of accounts by clicking
the "Back" button. However, the "back" button is not available on the
account pickers. We prefer a multi account picker compared to having an
additional UI surface. The most recently signed in accounts are shown
at the top of the multi account picker.

This patch also refactors the state machine. We used to set the state_
to State::REQUEST_PERMISSION for single account pickers on the bubble
but that is confusing. This patch changes it to have state_ be
State::SINGLE_ACCOUNT_PICKER.

Bug: 342557704
Change-Id: I9435ee5a34cf47567c6a4a16bbffd99ba8eee798
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5578254
Reviewed-by: Yi Gu <yigu@chromium.org>
Commit-Queue: Zachary Tan <tanzachary@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1313829}

chrome/browser/ui/views/webid/account_selection_view_base.h

@@ -254,8 +254,9 @@ class AccountSelectionViewBase {
       const IdentityProviderDisplayData& idp_data,
       const std::u16string& title) = 0;
 
-  // Updates to show single account plus a confirm dialog. Used when showing the
+  // Updates to show a single account. On widget mode, used when showing the
   // account confirmation dialog after the user picks one of multiple accounts.
+  // On button mode, used for the user to pick the single account.
   virtual void ShowSingleAccountConfirmDialog(
       const std::u16string& top_frame_for_display,
       const std::optional<std::u16string>& iframe_for_display,

chrome/browser/ui/views/webid/fedcm_account_selection_view_desktop.cc

@@ -14,6 +14,7 @@
 #include "chrome/browser/ui/views/webid/account_selection_modal_view.h"
 #include "chrome/browser/ui/views/webid/account_selection_view_base.h"
 #include "chrome/browser/ui/views/webid/fedcm_modal_dialog_view.h"
+#include "chrome/browser/ui/views/webid/identity_provider_display_data.h"
 #include "chrome/browser/ui/webid/account_selection_view.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/constrained_window/constrained_window_views.h"
@@ -171,6 +172,8 @@ bool FedCmAccountSelectionView::Show(
   }
 
   if (sign_in_mode == Account::SignInMode::kAuto) {
+    // Auto re-authn is currently only supported on widget flows.
+    CHECK(GetDialogType() == DialogType::BUBBLE);
     state_ = State::AUTO_REAUTHN;
 
     // When auto re-authn flow is triggered, the parameter
@@ -185,41 +188,61 @@ bool FedCmAccountSelectionView::Show(
       return false;
     }
   } else if (new_account_idp) {
-    // When we just logged in to an account, show that account right away.
+    // When we just logged in to an account that is not a single returning
+    // account: on the modal, we'd show all the accounts and on the bubble, we'd
+    // show only that account.
     new_account_idp_display_data_ = IdentityProviderDisplayData(
         base::UTF8ToUTF16(new_account_idp->idp_for_display),
         new_account_idp->idp_metadata, new_account_idp->client_metadata,
         new_account_idp->accounts, new_account_idp->request_permission,
         new_account_idp->has_login_status_mismatch);
 
-    // We use the browser trusted login state because this boolean controls
-    // whether we'd skip the permission modal entirely whereas "login_state"
-    // only controls whether to show the disclosure text.
-    bool is_returning_account_on_modal =
-        GetDialogType() == DialogType::MODAL &&
-        new_account_idp_display_data_->accounts[0]
-                .browser_trusted_login_state != Account::LoginState::kSignUp;
+    if (GetDialogType() == DialogType::MODAL) {
+      // TODO(crbug.com/342194490): Consider case when there's more than one
+      // newly signed in account.
 
-    if (is_returning_account_on_modal) {
-      state_ = State::VERIFYING;
-      // ShowVerifyingSheet will call delegate_->OnAccountSelected to proceed.
-      if (!ShowVerifyingSheet(new_account_idp_display_data_->accounts[0],
-                              *new_account_idp_display_data_)) {
-        return false;
-      }
-    } else {
-      state_ = State::REQUEST_PERMISSION;
-      if (GetDialogType() == DialogType::MODAL) {
+      // The browser trusted login state controls whether we'd skip the next
+      // dialog.
+      bool should_skip_dialog =
+          new_account_idp_display_data_->accounts[0]
+              .browser_trusted_login_state == Account::LoginState::kSignIn;
+      // The IDP claimed login state controls whether we show disclosure text,
+      // if we do not skip the next dialog.
+      bool should_hide_disclosure_text =
+          new_account_idp_display_data_->accounts[0].login_state ==
+          Account::LoginState::kSignIn;
+
+      if (should_skip_dialog) {
+        state_ = State::VERIFYING;
+        // ShowVerifyingSheet will call delegate_->OnAccountSelected to proceed.
+        if (!ShowVerifyingSheet(new_account_idp_display_data_->accounts[0],
+                                *new_account_idp_display_data_)) {
+          return false;
+        }
+      } else if (should_hide_disclosure_text) {
+        // Normally we'd show the request permission dialog but without the
+        // disclosure text, there is no material difference between the account
+        // picker and the request permission dialog. We show the account picker
+        // with most recently signed in accounts at the top to reduce the
+        // exposure of extra UI surfaces and to work around the account picker
+        // not having a back button.
+        state_ = State::MULTI_ACCOUNT_PICKER;
+        account_selection_view_->ShowMultiAccountPicker(
+            idp_display_data_list_,
+            /*show_back_button=*/false);
+      } else {
+        state_ = State::REQUEST_PERMISSION;
         account_selection_view_->ShowRequestPermissionDialog(
             top_frame_for_display_, new_account_idp_display_data_->accounts[0],
             *new_account_idp_display_data_);
-      } else {
-        account_selection_view_->ShowSingleAccountConfirmDialog(
-            top_frame_for_display_, iframe_for_display_,
-            new_account_idp_display_data_->accounts[0],
-            *new_account_idp_display_data_,
-            /*show_back_button=*/accounts_size > 1u);
       }
+    } else {
+      state_ = State::SINGLE_ACCOUNT_PICKER;
+      account_selection_view_->ShowSingleAccountConfirmDialog(
+          top_frame_for_display_, iframe_for_display_,
+          new_account_idp_display_data_->accounts[0],
+          *new_account_idp_display_data_,
+          /*show_back_button=*/accounts_size > 1u);
     }
   } else if (idp_display_data_list_.size() == 1u && accounts_size == 1u) {
     if (GetDialogType() == DialogType::MODAL) {
@@ -235,7 +258,7 @@ bool FedCmAccountSelectionView::Show(
       account_selection_view_->ShowMultiAccountPicker(
           idp_display_data_list_, /*show_back_button=*/false);
     } else {
-      state_ = State::REQUEST_PERMISSION;
+      state_ = State::SINGLE_ACCOUNT_PICKER;
       account_selection_view_->ShowSingleAccountConfirmDialog(
           top_frame_for_display_, iframe_for_display_,
           idp_display_data_list_[0].accounts[0], idp_display_data_list_[0],
@@ -664,6 +687,8 @@ void FedCmAccountSelectionView::OnAccountSelected(
   // verifying sheet.
   if (account.login_state != Account::LoginState::kSignUp ||
       state_ == State::REQUEST_PERMISSION ||
+      (state_ == State::SINGLE_ACCOUNT_PICKER &&
+       GetDialogType() == DialogType::BUBBLE) ||
       !idp_display_data.request_permission) {
     state_ = State::VERIFYING;
     ShowVerifyingSheet(account, idp_display_data);
@@ -682,7 +707,7 @@ void FedCmAccountSelectionView::OnAccountSelected(
   // At this point, the account is a non-returning user, the dialog is a bubble
   // and it is a multi account picker, there is no disclosure text on the dialog
   // so we'd request permission through a single account dialog.
-  state_ = State::REQUEST_PERMISSION;
+  state_ = State::SINGLE_ACCOUNT_PICKER;
   account_selection_view_->ShowSingleAccountConfirmDialog(
       top_frame_for_display_, iframe_for_display_, account, idp_display_data,
       /*show_back_button=*/true);

chrome/browser/ui/views/webid/fedcm_account_selection_view_desktop.h

@@ -174,15 +174,17 @@ class FedCmAccountSelectionView : public AccountSelectionView,
     IDP_SIGNIN_STATUS_MISMATCH,
 
     // User is shown a single account they have with IDP and is prompted to
-    // continue with the account.
+    // select or continue with the account. On a widget flow bubble, this may
+    // contain disclosure text which prompts the user to grant permission for
+    // this account they have with IDP to communicate with RP.
     SINGLE_ACCOUNT_PICKER,
 
     // User is shown list of accounts they have with IDP and is prompted to
     // select an account.
     MULTI_ACCOUNT_PICKER,
 
-    // User is prompted to grant permission for specific account they have with
-    // IDP to communicate with RP.
+    // User is prompted to grant permission for a specific account they have
+    // with IDP to communicate with RP on the button flow modal.
     REQUEST_PERMISSION,
 
     // Shown after the user has granted permission while the id token is being
@@ -201,7 +203,8 @@ class FedCmAccountSelectionView : public AccountSelectionView,
     // are being fetched.
     LOADING,
 
-    // Shown when we wish to display only a single returning account.
+    // Shown when we wish to display only a single returning account. Used when
+    // there are multiple IDPs and exactly one returning account.
     SINGLE_RETURNING_ACCOUNT_PICKER
   };
 

chrome/browser/ui/views/webid/fedcm_account_selection_view_desktop_unittest.cc

@@ -316,13 +316,30 @@ class FedCmAccountSelectionViewDesktopTest : public ChromeViewsTestBase {
   }
 
   std::vector<content::IdentityRequestAccount> CreateAccount(
-      LoginState login_state,
+      LoginState idp_claimed_login_state,
+      LoginState browser_trusted_login_state,
       std::string account_id = kAccountId1) {
     return {{account_id, "", "", "", GURL(),
              /*login_hints=*/std::vector<std::string>(),
              /*domain_hints=*/std::vector<std::string>(),
-             /*labels=*/std::vector<std::string>(), /*login_state=*/login_state,
-             /*browser_trusted_login_state=*/login_state}};
+             /*labels=*/std::vector<std::string>(),
+             /*login_state=*/idp_claimed_login_state,
+             /*browser_trusted_login_state=*/browser_trusted_login_state}};
+  }
+
+  std::vector<content::IdentityRequestAccount> CreateAccounts(
+      const std::vector<std::pair<std::string, LoginState>>& account_infos) {
+    std::vector<content::IdentityRequestAccount> accounts;
+    for (const auto& account_info : account_infos) {
+      accounts.emplace_back(
+          account_info.first, "", "", "", GURL(),
+          /*login_hints=*/std::vector<std::string>(),
+          /*domain_hints=*/std::vector<std::string>(),
+          /*labels=*/std::vector<std::string>(),
+          /*login_state=*/account_info.second,
+          /*browser_trusted_login_state=*/account_info.second);
+    }
+    return accounts;
   }
 
   content::IdentityProviderData CreateIdentityProviderData(
@@ -441,6 +458,78 @@ class FedCmAccountSelectionViewDesktopTest : public ChromeViewsTestBase {
     return controller;
   }
 
+  std::unique_ptr<TestFedCmAccountSelectionView>
+  CreateAndShowAccountsModalThroughPopupWindow(
+      std::vector<content::IdentityRequestAccount> new_accounts,
+      content::IdentityProviderData new_idp_data) {
+    std::unique_ptr<TestFedCmAccountSelectionView> controller =
+        CreateAndShowLoadingDialog();
+    AccountSelectionViewBase::Observer* observer =
+        static_cast<AccountSelectionViewBase::Observer*>(controller.get());
+
+    // Emulate the login to IdP flow.
+    observer->OnLoginToIdP(GURL(kConfigUrl), GURL(kLoginUrl),
+                           CreateMouseEvent());
+    CreateAndShowPopupWindow(*controller);
+
+    // Emulate user completing the sign-in flow and IdP prompts closing the
+    // pop-up window and sending new accounts.
+    controller->CloseModalDialog();
+
+    Show(*controller, new_accounts, SignInMode::kExplicit,
+         blink::mojom::RpMode::kButton, new_idp_data);
+
+    return controller;
+  }
+
+  std::unique_ptr<TestFedCmAccountSelectionView>
+  CreateAndShowAccountsThroughUseAnotherAccount(
+      const std::vector<std::pair<std::string, LoginState>>& old_account_infos,
+      const std::vector<std::pair<std::string, LoginState>>& new_account_infos,
+      blink::mojom::RpMode rp_mode = blink::mojom::RpMode::kWidget) {
+    IdentityProviderDisplayData idp_data =
+        CreateIdentityProviderDisplayData(old_account_infos);
+    const std::vector<Account>& accounts = idp_data.accounts;
+    std::unique_ptr<TestFedCmAccountSelectionView> controller =
+        CreateAndShow(accounts, SignInMode::kExplicit, rp_mode);
+    AccountSelectionViewBase::Observer* observer =
+        static_cast<AccountSelectionViewBase::Observer*>(controller.get());
+
+    // Emulate the user clicking "use another account button".
+    observer->OnLoginToIdP(GURL(kConfigUrl), GURL(kLoginUrl),
+                           CreateMouseEvent());
+    CreateAndShowPopupWindow(*controller);
+
+    // Emulate user completing the sign-in flow and IdP prompts closing the
+    // pop-up window and sending new accounts.
+    controller->CloseModalDialog();
+
+    std::vector<content::IdentityRequestAccount> new_accounts =
+        CreateAccounts(new_account_infos);
+    content::IdentityProviderData new_idp_data =
+        CreateIdentityProviderData(new_accounts);
+
+    std::vector<std::pair<std::string, LoginState>> combined_account_infos =
+        old_account_infos;
+    for (const auto& account_info : new_account_infos) {
+      if (std::find(combined_account_infos.begin(),
+                    combined_account_infos.end(),
+                    account_info) != combined_account_infos.end()) {
+        continue;
+      }
+
+      combined_account_infos.emplace_back(account_info);
+    }
+
+    IdentityProviderDisplayData combined_idp_data =
+        CreateIdentityProviderDisplayData(combined_account_infos);
+
+    Show(*controller, combined_idp_data.accounts, SignInMode::kExplicit,
+         rp_mode, new_idp_data);
+
+    return controller;
+  }
+
   ui::MouseEvent CreateMouseEvent() {
     return ui::MouseEvent(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
                           base::TimeTicks(), ui::EF_LEFT_MOUSE_BUTTON, 0);
@@ -1240,7 +1329,7 @@ TEST_F(FedCmAccountSelectionViewDesktopTest,
   IdentityProviderDisplayData idp_data = CreateIdentityProviderDisplayData(
       {{kAccountId1, LoginState::kSignUp}, {kAccountId2, LoginState::kSignUp}});
   std::vector<content::IdentityRequestAccount> new_accounts =
-      CreateAccount(LoginState::kSignUp);
+      CreateAccount(LoginState::kSignUp, LoginState::kSignUp);
   content::IdentityProviderData new_idp_data =
       CreateIdentityProviderData(new_accounts);
 
@@ -1263,40 +1352,13 @@ TEST_F(FedCmAccountSelectionViewDesktopTest,
 // Test the use another account flow, resulting in the new account being shown
 // after logging in.
 TEST_F(FedCmAccountSelectionViewDesktopTest, UseAnotherAccount) {
-  IdentityProviderDisplayData idp_data =
-      CreateIdentityProviderDisplayData({{kAccountId1, LoginState::kSignUp}});
-  const std::vector<Account>& accounts = idp_data.accounts;
   std::unique_ptr<TestFedCmAccountSelectionView> controller =
-      CreateAndShow(accounts, SignInMode::kExplicit);
+      CreateAndShowAccountsThroughUseAnotherAccount(
+          /*old_account_infos=*/{{kAccountId1, LoginState::kSignUp}},
+          /*new_account_infos=*/{{kAccountId2, LoginState::kSignUp}});
   AccountSelectionViewBase::Observer* observer =
       static_cast<AccountSelectionViewBase::Observer*>(controller.get());
 
-  EXPECT_FALSE(account_selection_view_->show_back_button_);
-  EXPECT_THAT(account_selection_view_->account_ids_,
-              testing::ElementsAre(kAccountId1));
-
-  // Emulate the user clicking "use another account button".
-  observer->OnLoginToIdP(GURL(kConfigUrl), GURL(kLoginUrl), CreateMouseEvent());
-  CreateAndShowPopupWindow(*controller);
-
-  // Bubble does not remain visible.
-  EXPECT_FALSE(dialog_widget_->IsVisible());
-
-  // Emulate user completing the sign-in flow and IdP prompts closing the
-  // pop-up window and sending new accounts.
-  controller->CloseModalDialog();
-
-  IdentityProviderDisplayData idp_data2 = CreateIdentityProviderDisplayData(
-      {{kAccountId1, LoginState::kSignUp}, {kAccountId2, LoginState::kSignUp}});
-  // The new account would be kAccountId2.
-  std::vector<content::IdentityRequestAccount> new_accounts =
-      CreateAccount(LoginState::kSignUp, kAccountId2);
-  content::IdentityProviderData new_idp_data =
-      CreateIdentityProviderData(new_accounts);
-
-  Show(*controller, idp_data2.accounts, SignInMode::kExplicit,
-       blink::mojom::RpMode::kWidget, new_idp_data);
-
   // Only the newly logged in account is shown in the latest dialog.
   EXPECT_EQ(TestAccountSelectionView::SheetType::kConfirmAccount,
             account_selection_view_->sheet_type_);
@@ -1315,155 +1377,152 @@ TEST_F(FedCmAccountSelectionViewDesktopTest, UseAnotherAccount) {
               testing::ElementsAre(kAccountId1, kAccountId2));
 }
 
-// Test the use another account flow in a modal, resulting in the new account
-// being shown after logging in.
-TEST_F(FedCmAccountSelectionViewDesktopTest, UseAnotherAccountModal) {
-  IdentityProviderDisplayData idp_data =
-      CreateIdentityProviderDisplayData({{kAccountId1, LoginState::kSignUp}});
-  const std::vector<Account>& accounts = idp_data.accounts;
-  std::unique_ptr<TestFedCmAccountSelectionView> controller = CreateAndShow(
-      accounts, SignInMode::kExplicit, blink::mojom::RpMode::kButton);
-  AccountSelectionViewBase::Observer* observer =
-      static_cast<AccountSelectionViewBase::Observer*>(controller.get());
+// Test the use another account flow when signing into the same account that the
+// user started with.
+TEST_F(FedCmAccountSelectionViewDesktopTest, UseAnotherAccountForSameAccount) {
+  CreateAndShowAccountsThroughUseAnotherAccount(
+      /*old_account_infos=*/{{kAccountId1, LoginState::kSignUp}},
+      /*new_account_infos=*/{{kAccountId1, LoginState::kSignUp}});
 
-  EXPECT_FALSE(account_selection_view_->show_back_button_);
+  // Only the newly logged in account is shown in the latest dialog.
+  EXPECT_EQ(TestAccountSelectionView::SheetType::kConfirmAccount,
+            account_selection_view_->sheet_type_);
   EXPECT_THAT(account_selection_view_->account_ids_,
               testing::ElementsAre(kAccountId1));
 
-  // Emulate the user clicking "use another account button".
-  observer->OnLoginToIdP(GURL(kConfigUrl), GURL(kLoginUrl), CreateMouseEvent());
-  CreateAndShowPopupWindow(*controller);
+  // Back button must NOT be showing since there is only one account.
+  EXPECT_FALSE(account_selection_view_->show_back_button_);
+}
 
-  // Modal remains visible.
-  EXPECT_TRUE(dialog_widget_->IsVisible());
+// Test the use another account flow, resulting in account chooser UI if it's a
+// returning account.
+TEST_F(FedCmAccountSelectionViewDesktopTest,
+       UseAnotherAccountForReturningAccount) {
+  CreateAndShowAccountsThroughUseAnotherAccount(
+      /*old_account_infos=*/{{kAccountId1, LoginState::kSignUp}},
+      /*new_account_infos=*/{{kAccountId2, LoginState::kSignIn}});
 
-  // Emulate user completing the sign-in flow and IdP prompts closing the
-  // pop-up window and sending new accounts.
-  controller->CloseModalDialog();
-
-  IdentityProviderDisplayData idp_data2 = CreateIdentityProviderDisplayData(
-      {{kAccountId1, LoginState::kSignUp}, {kAccountId2, LoginState::kSignUp}});
-  // The new account would be kAccountId2.
-  std::vector<content::IdentityRequestAccount> new_accounts =
-      CreateAccount(LoginState::kSignUp, kAccountId2);
-  content::IdentityProviderData new_idp_data =
-      CreateIdentityProviderData(new_accounts);
-
-  Show(*controller, idp_data2.accounts, SignInMode::kExplicit,
-       blink::mojom::RpMode::kButton, new_idp_data);
-
-  // Only the newly logged in account is shown in the latest dialog.
-  EXPECT_EQ(TestAccountSelectionView::SheetType::kRequestPermission,
+  // The account chooser UI is NOT skipped.
+  EXPECT_EQ(TestAccountSelectionView::SheetType::kConfirmAccount,
             account_selection_view_->sheet_type_);
   EXPECT_THAT(account_selection_view_->account_ids_,
               testing::ElementsAre(kAccountId2));
-
-  // Back button must be showing since we are on the request permission dialog.
-  EXPECT_TRUE(account_selection_view_->show_back_button_);
-
-  // Clicking the back button shows all the accounts.
-  observer->OnBackButtonClicked();
-  EXPECT_FALSE(account_selection_view_->show_back_button_);
-  EXPECT_EQ(TestAccountSelectionView::SheetType::kAccountPicker,
-            account_selection_view_->sheet_type_);
-  EXPECT_THAT(account_selection_view_->account_ids_,
-              testing::ElementsAre(kAccountId1, kAccountId2));
 }
 
-// Test the use another account flow in a modal, resulting in no permission UI
-// if it's a returning account.
-TEST_F(FedCmAccountSelectionViewDesktopTest,
-       UseAnotherAccountModalForReturningAccount) {
-  IdentityProviderDisplayData idp_data =
-      CreateIdentityProviderDisplayData({{kAccountId1, LoginState::kSignUp}});
-  const std::vector<Account>& accounts = idp_data.accounts;
-  std::unique_ptr<TestFedCmAccountSelectionView> controller = CreateAndShow(
-      accounts, SignInMode::kExplicit, blink::mojom::RpMode::kButton);
-  AccountSelectionViewBase::Observer* observer =
-      static_cast<AccountSelectionViewBase::Observer*>(controller.get());
-
-  // Emulate the user clicking "use another account button".
-  observer->OnLoginToIdP(GURL(kConfigUrl), GURL(kLoginUrl), CreateMouseEvent());
-  CreateAndShowPopupWindow(*controller);
-
-  // Emulate user completing the sign-in flow and IdP prompts closing the
-  // pop-up window and sending new accounts.
-  controller->CloseModalDialog();
-
-  IdentityProviderDisplayData idp_data2 = CreateIdentityProviderDisplayData(
-      {{kAccountId1, LoginState::kSignUp}, {kAccountId2, LoginState::kSignUp}});
-  // The new account would be kAccountId2 whose login state is kSignIn.
-  std::vector<content::IdentityRequestAccount> new_accounts =
-      CreateAccount(LoginState::kSignIn, kAccountId2);
-  content::IdentityProviderData new_idp_data =
-      CreateIdentityProviderData(new_accounts);
-
-  Show(*controller, idp_data2.accounts, SignInMode::kExplicit,
-       blink::mojom::RpMode::kButton, new_idp_data);
-
-  // The permission UI is skipped.
-  EXPECT_EQ(TestAccountSelectionView::SheetType::kVerifying,
-            account_selection_view_->sheet_type_);
-}
-
-// Test the logged-out LoginStatus flow in a modal, resulting in no permission
-// UI if it's a returning account.
-TEST_F(FedCmAccountSelectionViewDesktopTest,
-       LoginStatusLoggedOutModalForReturningAccount) {
-  std::unique_ptr<TestFedCmAccountSelectionView> controller =
-      CreateAndShowLoadingDialog();
-  AccountSelectionViewBase::Observer* observer =
-      static_cast<AccountSelectionViewBase::Observer*>(controller.get());
-
-  // Emulate the login to IdP flow.
-  observer->OnLoginToIdP(GURL(kConfigUrl), GURL(kLoginUrl), CreateMouseEvent());
-  CreateAndShowPopupWindow(*controller);
-
-  // Emulate user completing the sign-in flow and IdP prompts closing the
-  // pop-up window and sending new accounts.
-  controller->CloseModalDialog();
-
-  std::vector<content::IdentityRequestAccount> new_accounts =
-      CreateAccount(LoginState::kSignIn);
-  content::IdentityProviderData new_idp_data =
-      CreateIdentityProviderData(new_accounts);
-
-  Show(*controller, new_accounts, SignInMode::kExplicit,
-       blink::mojom::RpMode::kButton, new_idp_data);
-
-  // The permission UI is skipped.
-  EXPECT_EQ(TestAccountSelectionView::SheetType::kVerifying,
-            account_selection_view_->sheet_type_);
-}
-
-// Test the browser trusted login state controls whether to skip the permissions
-// UI when in conflict with login state.
-TEST_F(FedCmAccountSelectionViewDesktopTest,
-       BrowserTrustedLoginStateTakesPrecedenceOverLoginState) {
-  std::unique_ptr<TestFedCmAccountSelectionView> controller =
-      CreateAndShowLoadingDialog();
-  AccountSelectionViewBase::Observer* observer =
-      static_cast<AccountSelectionViewBase::Observer*>(controller.get());
-
-  // Emulate the login to IdP flow.
-  observer->OnLoginToIdP(GURL(kConfigUrl), GURL(kLoginUrl), CreateMouseEvent());
-  CreateAndShowPopupWindow(*controller);
-
-  // Emulate user completing the sign-in flow and IdP prompts closing the
-  // pop-up window and sending new accounts.
-  controller->CloseModalDialog();
-
-  std::vector<content::IdentityRequestAccount> new_accounts =
-      CreateAccount(LoginState::kSignUp);
-  content::IdentityProviderData new_idp_data =
-      CreateIdentityProviderData(new_accounts);
-
-  Show(*controller, new_accounts, SignInMode::kExplicit,
-       blink::mojom::RpMode::kButton, new_idp_data);
+// Test the use another account flow in a modal, resulting in the new account
+// being shown after logging in.
+TEST_F(FedCmAccountSelectionViewDesktopTest, UseAnotherAccountModal) {
+  CreateAndShowAccountsThroughUseAnotherAccount(
+      /*old_account_infos=*/{{kAccountId1, LoginState::kSignUp}},
+      /*new_account_infos=*/{{kAccountId2, LoginState::kSignUp}},
+      /*rp_mode=*/blink::mojom::RpMode::kButton);
 
   // The permission UI is NOT skipped.
   EXPECT_EQ(TestAccountSelectionView::SheetType::kRequestPermission,
             account_selection_view_->sheet_type_);
+  EXPECT_THAT(account_selection_view_->account_ids_,
+              testing::ElementsAre(kAccountId2));
+}
+
+// Test the use another account flow in a modal when signing into the same
+// account that the user started with.
+TEST_F(FedCmAccountSelectionViewDesktopTest,
+       UseAnotherAccountModalForSameAccount) {
+  CreateAndShowAccountsThroughUseAnotherAccount(
+      /*old_account_infos=*/{{kAccountId1, LoginState::kSignUp}},
+      /*new_account_infos=*/{{kAccountId1, LoginState::kSignUp}},
+      /*rp_mode=*/blink::mojom::RpMode::kButton);
+
+  // The permission UI is NOT skipped.
+  EXPECT_EQ(TestAccountSelectionView::SheetType::kRequestPermission,
+            account_selection_view_->sheet_type_);
+  EXPECT_THAT(account_selection_view_->account_ids_,
+              testing::ElementsAre(kAccountId1));
+}
+
+// Test the use another account flow in a modal, resulting in no account chooser
+// UI if it's a returning account.
+TEST_F(FedCmAccountSelectionViewDesktopTest,
+       UseAnotherAccountModalForReturningAccount) {
+  CreateAndShowAccountsThroughUseAnotherAccount(
+      /*old_account_infos=*/{{kAccountId1, LoginState::kSignUp}},
+      /*new_account_infos=*/{{kAccountId2, LoginState::kSignIn}},
+      /*rp_mode=*/blink::mojom::RpMode::kButton);
+
+  // The account chooser UI is skipped.
+  EXPECT_EQ(TestAccountSelectionView::SheetType::kVerifying,
+            account_selection_view_->sheet_type_);
+  EXPECT_THAT(account_selection_view_->account_ids_,
+              testing::ElementsAre(kAccountId2));
+}
+
+// Test the logged-out LoginStatus flow in a modal, resulting in no account
+// chooser UI if it's a returning account.
+TEST_F(FedCmAccountSelectionViewDesktopTest,
+       LoginStatusLoggedOutModalForReturningAccount) {
+  IdentityProviderDisplayData idp_data = CreateIdentityProviderDisplayData(
+      {{kAccountId1, LoginState::kSignIn}},
+      /*has_login_status_mismatch=*/false, /*request_permission=*/false);
+  std::vector<content::IdentityRequestAccount> new_accounts =
+      CreateAccount(LoginState::kSignIn, LoginState::kSignIn);
+  content::IdentityProviderData new_idp_data =
+      CreateIdentityProviderData(new_accounts);
+
+  std::unique_ptr<TestFedCmAccountSelectionView> controller =
+      CreateAndShowAccountsModalThroughPopupWindow(new_accounts, new_idp_data);
+
+  // The account chooser UI is skipped.
+  EXPECT_EQ(TestAccountSelectionView::SheetType::kVerifying,
+            account_selection_view_->sheet_type_);
+  EXPECT_THAT(account_selection_view_->account_ids_,
+              testing::ElementsAre(kAccountId1));
+}
+
+// Test the logged-out LoginStatus flow in a modal, resulting in showing account
+// chooser UI if it's a non-returning account.
+TEST_F(FedCmAccountSelectionViewDesktopTest,
+       LoginStatusLoggedOutModalForNonReturningAccount) {
+  IdentityProviderDisplayData idp_data = CreateIdentityProviderDisplayData(
+      {{kAccountId1, LoginState::kSignUp}},
+      /*has_login_status_mismatch=*/false, /*request_permission=*/false);
+  std::vector<content::IdentityRequestAccount> new_accounts =
+      CreateAccount(LoginState::kSignUp, LoginState::kSignUp);
+  content::IdentityProviderData new_idp_data =
+      CreateIdentityProviderData(new_accounts);
+
+  std::unique_ptr<TestFedCmAccountSelectionView> controller =
+      CreateAndShowAccountsModalThroughPopupWindow(new_accounts, new_idp_data);
+
+  // The permission UI is NOT skipped.
+  EXPECT_EQ(TestAccountSelectionView::SheetType::kRequestPermission,
+            account_selection_view_->sheet_type_);
+  EXPECT_THAT(account_selection_view_->account_ids_,
+              testing::ElementsAre(kAccountId1));
+}
+
+// Test the browser trusted login state controls whether to skip the account
+// chooser UI when in conflict with login state.
+TEST_F(FedCmAccountSelectionViewDesktopTest,
+       BrowserTrustedLoginStateTakesPrecedenceOverLoginState) {
+  IdentityProviderDisplayData idp_data = CreateIdentityProviderDisplayData(
+      {{kAccountId1, LoginState::kSignUp}},
+      /*has_login_status_mismatch=*/false, /*request_permission=*/false);
+  std::vector<content::IdentityRequestAccount> new_accounts =
+      CreateAccount(/*idp_claimed_login_state=*/LoginState::kSignIn,
+                    /*browser_trusted_login_state=*/LoginState::kSignUp);
+  content::IdentityProviderData new_idp_data =
+      CreateIdentityProviderData(new_accounts);
+
+  std::unique_ptr<TestFedCmAccountSelectionView> controller =
+      CreateAndShowAccountsModalThroughPopupWindow(new_accounts, new_idp_data);
+
+  // The account chooser UI is NOT skipped. Normally, this is permission UI but
+  // because we do not want to show disclosure UI without disclosure text, we
+  // show the account chooser UI instead.
+  EXPECT_EQ(TestAccountSelectionView::SheetType::kAccountPicker,
+            account_selection_view_->sheet_type_);
+  EXPECT_THAT(account_selection_view_->account_ids_,
+              testing::ElementsAre(kAccountId1));
 }
 
 // Test user triggering the use another account flow twice in a modal, without
@@ -1744,7 +1803,7 @@ TEST_F(FedCmAccountSelectionViewDesktopTest, MultiIdpMismatchAndShow) {
   controller->CloseModalDialog();
 
   std::vector<content::IdentityRequestAccount> new_accounts =
-      CreateAccount(LoginState::kSignUp);
+      CreateAccount(LoginState::kSignUp, LoginState::kSignUp);
   content::IdentityProviderData new_idp_data =
       CreateIdentityProviderData(new_accounts);
 
@@ -2199,39 +2258,16 @@ TEST_F(FedCmAccountSelectionViewDesktopTest, AccountChooserResultMetric) {
   CheckForSampleAndReset(
       FedCmAccountSelectionView::AccountChooserResult::kTabClosed);
 
-  {
-    // Widget flow should not record a sample.
-    std::unique_ptr<TestFedCmAccountSelectionView> controller =
-        CreateAndShow(idp_data.accounts, SignInMode::kExplicit,
-                      blink::mojom::RpMode::kWidget);
-  }
-  histogram_tester_->ExpectTotalCount("Blink.FedCm.Button.AccountChooserResult",
-                                      0);
-
   {
     // Non-returning user signing in via IDP sign-in pop-up should not record a
     // sample.
-    std::unique_ptr<TestFedCmAccountSelectionView> controller =
-        CreateAndShowLoadingDialog();
-    AccountSelectionViewBase::Observer* observer =
-        static_cast<AccountSelectionViewBase::Observer*>(controller.get());
-
-    // Emulate the login to IdP flow.
-    observer->OnLoginToIdP(GURL(kConfigUrl), GURL(kLoginUrl),
-                           CreateMouseEvent());
-    CreateAndShowPopupWindow(*controller);
-
-    // Emulate user completing the sign-in flow and IdP prompts closing the
-    // pop-up window and sending new accounts.
-    controller->CloseModalDialog();
-
     std::vector<content::IdentityRequestAccount> new_accounts =
-        CreateAccount(LoginState::kSignUp);
+        CreateAccount(LoginState::kSignUp, LoginState::kSignUp);
     content::IdentityProviderData new_idp_data =
         CreateIdentityProviderData(new_accounts);
-
-    Show(*controller, new_accounts, SignInMode::kExplicit,
-         blink::mojom::RpMode::kButton, new_idp_data);
+    std::unique_ptr<TestFedCmAccountSelectionView> controller =
+        CreateAndShowAccountsModalThroughPopupWindow(new_accounts,
+                                                     new_idp_data);
 
     // User is shown the request permission dialog, skipping the account
     // chooser.
@@ -2244,27 +2280,13 @@ TEST_F(FedCmAccountSelectionViewDesktopTest, AccountChooserResultMetric) {
   {
     // Returning user signing in via IDP sign-in pop-up should not record a
     // sample.
-    std::unique_ptr<TestFedCmAccountSelectionView> controller =
-        CreateAndShowLoadingDialog();
-    AccountSelectionViewBase::Observer* observer =
-        static_cast<AccountSelectionViewBase::Observer*>(controller.get());
-
-    // Emulate the login to IdP flow.
-    observer->OnLoginToIdP(GURL(kConfigUrl), GURL(kLoginUrl),
-                           CreateMouseEvent());
-    CreateAndShowPopupWindow(*controller);
-
-    // Emulate user completing the sign-in flow and IdP prompts closing the
-    // pop-up window and sending new accounts.
-    controller->CloseModalDialog();
-
     std::vector<content::IdentityRequestAccount> new_accounts =
-        CreateAccount(LoginState::kSignIn);
+        CreateAccount(LoginState::kSignIn, LoginState::kSignIn);
     content::IdentityProviderData new_idp_data =
         CreateIdentityProviderData(new_accounts);
-
-    Show(*controller, new_accounts, SignInMode::kExplicit,
-         blink::mojom::RpMode::kButton, new_idp_data);
+    std::unique_ptr<TestFedCmAccountSelectionView> controller =
+        CreateAndShowAccountsModalThroughPopupWindow(new_accounts,
+                                                     new_idp_data);
 
     // User is shown the verifying dialog, skipping both the account chooser and
     // request permission dialog.
@@ -2273,6 +2295,15 @@ TEST_F(FedCmAccountSelectionViewDesktopTest, AccountChooserResultMetric) {
   }
   histogram_tester_->ExpectTotalCount("Blink.FedCm.Button.AccountChooserResult",
                                       0);
+
+  {
+    // Widget flow should not record a sample.
+    std::unique_ptr<TestFedCmAccountSelectionView> controller =
+        CreateAndShow(idp_data.accounts, SignInMode::kExplicit,
+                      blink::mojom::RpMode::kWidget);
+  }
+  histogram_tester_->ExpectTotalCount("Blink.FedCm.Button.AccountChooserResult",
+                                      0);
 }
 
 // Tests that for button flows, going from an accounts dialog to an error dialog

content/browser/webid/federated_auth_request_impl.cc

@@ -35,6 +35,7 @@
 #include "content/public/browser/federated_identity_api_permission_context_delegate.h"
 #include "content/public/browser/federated_identity_auto_reauthn_permission_context_delegate.h"
 #include "content/public/browser/federated_identity_permission_context_delegate.h"
+#include "content/public/browser/identity_request_account.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_client.h"
@@ -1565,25 +1566,33 @@ void FederatedAuthRequestImpl::MaybeShowAccountsDialog() {
   std::optional<IdentityProviderData> new_account_idp;
   for (const auto& idp : idp_order_) {
     auto idp_info_it = idp_infos_.find(idp);
-    if (idp_info_it != idp_infos_.end() && idp_info_it->second->data) {
-      idp_data_for_display_.push_back(*idp_info_it->second->data);
-    }
     if (idp_infos_.size() > 1u ||
         IsFedCmUseOtherAccountEnabled(rp_mode_ == RpMode::kButton)) {
       if (!login_url_.is_empty() &&
           login_url_ == idp_info_it->second->metadata.idp_login_url) {
+        std::vector<IdentityRequestAccount> reordered_accounts_list;
         for (const auto& account : idp_info_it->second->data->accounts) {
           if (!account_ids_before_login_.contains(account.id)) {
             // Even though it is theoretically possible for more than one
             // account to be new, just show the first one we encounter.
+            // TODO(crbug.com/342194490): Consider case when there's more than
+            // one newly signed in account.
             new_account_idp = idp_info_it->second->data;
             new_account_idp->accounts = {account};
-            break;
+            reordered_accounts_list.emplace(reordered_accounts_list.begin(),
+                                            account);
+          } else {
+            reordered_accounts_list.emplace_back(account);
           }
         }
         account_ids_before_login_.clear();
+        idp_info_it->second->data->accounts =
+            std::move(reordered_accounts_list);
       }
     }
+    if (idp_info_it != idp_infos_.end() && idp_info_it->second->data) {
+      idp_data_for_display_.push_back(*idp_info_it->second->data);
+    }
   }
 
   // We want to show IDPs in the following order in the UI:

content/browser/webid/federated_auth_request_impl_unittest.cc

@@ -157,6 +157,30 @@ static const std::vector<IdentityRequestAccount> kSingleAccountWithDomainHint{{
     std::vector<std::string>()   // labels
 }};
 
+static const std::vector<IdentityRequestAccount> kTwoAccounts{
+    {
+        kAccountIdNicolas,           // id
+        kAccountEmailNicolas,        // email
+        "Nicolas P",                 // name
+        "Nicolas",                   // given_name
+        GURL(),                      // picture
+        std::vector<std::string>(),  // login_hints
+        std::vector<std::string>(),  // domain_hints
+        std::vector<std::string>(),  // labels
+        LoginState::kSignUp          // login_state
+    },
+    {
+        kAccountIdZach,              // id
+        "zach@email.com",            // email
+        "Zachary T",                 // name
+        "Zach",                      // given_name
+        GURL(),                      // picture
+        std::vector<std::string>(),  // login_hints
+        std::vector<std::string>(),  // domain_hints
+        std::vector<std::string>(),  // labels
+        LoginState::kSignUp          // login_state
+    }};
+
 static const std::vector<IdentityRequestAccount> kMultipleAccounts{
     {
         kAccountIdNicolas,           // id
@@ -546,7 +570,8 @@ class TestIdpNetworkRequestManager : public MockIdpNetworkRequestManager {
 
     base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
         FROM_HERE, base::BindOnce(std::move(callback), info.accounts_response,
-                                  info.accounts));
+                                  accounts_list_.empty() ? info.accounts
+                                                         : accounts_list_));
   }
 
   void SendTokenRequest(
@@ -618,6 +643,10 @@ class TestIdpNetworkRequestManager : public MockIdpNetworkRequestManager {
 
   std::map<FetchedEndpoint, size_t> num_fetched_;
 
+  // If non-empty, the accounts endpoint will return this accounts list instead
+  // of the accounts list in the `config_`.
+  AccountList accounts_list_;
+
  protected:
   MockConfiguration config_{kConfigurationValid};
   std::vector<base::OnceClosure> delayed_callbacks_;
@@ -779,6 +808,9 @@ class TestDialogController
                 std::move(on_add_account),
                 identity_provider_data.data()->idp_metadata.config_url,
                 identity_provider_data.data()->idp_metadata.idp_login_url));
+        // Set `accounts_dialog_action_` such that subsequent calls will select
+        // the first account.
+        accounts_dialog_action_ = AccountsDialogAction::kSelectFirstAccount;
         break;
       case AccountsDialogAction::kNone:
         break;
@@ -7427,4 +7459,43 @@ TEST_F(FederatedAuthRequestImplTest, FailureDialogImmediateDismiss) {
   RunAuthTest(kDefaultRequestParameters, expectations, configuration);
 }
 
+TEST_F(FederatedAuthRequestImplTest, UseOtherAccountAccountOrder) {
+  base::test::ScopedFeatureList list;
+  list.InitAndEnableFeature(features::kFedCmUseOtherAccount);
+
+  MockConfiguration configuration = kConfigurationValid;
+  configuration.accounts_dialog_action = AccountsDialogAction::kAddAccount;
+
+  // User has accounts, kAccountIdNicolas and kAccountIdZach, in that order.
+  configuration.idp_info[kProviderUrlFull].accounts = kTwoAccounts;
+
+  auto dialog_controller =
+      std::make_unique<TestDialogController>(configuration);
+  base::WeakPtr<TestDialogController> weak_dialog_controller =
+      dialog_controller->AsWeakPtr();
+  SetDialogController(std::move(dialog_controller));
+
+  std::unique_ptr<WebContents> modal(CreateTestWebContents());
+  EXPECT_CALL(*weak_dialog_controller, ShowModalDialog(_, _))
+      .WillOnce(::testing::WithArg<0>([&modal, this](const GURL& url) {
+        // The user signs in with account, kAccountIdPeter. User now has
+        // accounts, kAccountIdNicolas, kAccountIdPeter and kAccountIdZach,
+        // in that order.
+        test_network_request_manager_->accounts_list_ = kMultipleAccounts;
+        federated_auth_request_impl_->OnIdpSigninStatusReceived(
+            OriginFromString(kProviderUrlFull), true);
+        return modal.get();
+      }));
+
+  RunAuthDontWaitForCallback(kDefaultRequestParameters, configuration);
+
+  ASSERT_EQ(displayed_accounts().size(), 3u);
+
+  // Accounts are reordered to have the most recently signed in account,
+  // kAccountIdPeter, displayed first.
+  EXPECT_EQ(displayed_accounts()[0].id, kAccountIdPeter);
+  EXPECT_EQ(displayed_accounts()[1].id, kAccountIdNicolas);
+  EXPECT_EQ(displayed_accounts()[2].id, kAccountIdZach);
+}
+
 }  // namespace content

third_party/blink/web_tests/external/wpt/credential-management/fedcm-button-and-other-account/fedcm-use-other-account-button-flow.tentative.https.html

@@ -46,7 +46,8 @@ fedcm_test(async t => {
   type = await fedcm_get_dialog_type_promise(t);
   assert_equals(type, "AccountChooser");
 
-  await window.test_driver.select_fedcm_account(1);
+  // Select the first account, which is the most recently signed in account.
+  await window.test_driver.select_fedcm_account(0);
   const cred = await cred_promise;
   assert_equals(cred.token, "account_id=jane_doe");
 }, 'Test that the "Use Other Account" button works correctly.');

third_party/blink/web_tests/external/wpt/credential-management/fedcm-button-and-other-account/fedcm-use-other-account.tentative.https.html

@@ -41,7 +41,8 @@ fedcm_test(async t => {
   type = await fedcm_get_dialog_type_promise(t);
   assert_equals(type, "AccountChooser");
 
-  await window.test_driver.select_fedcm_account(1);
+  // Select the first account, which is the most recently signed in account.
+  await window.test_driver.select_fedcm_account(0);
   const cred = await cred_promise;
   assert_equals(cred.token, "account_id=jane_doe");
 }, 'Test that the "Use Other Account" button works correctly.');