[FedCM] Add input event protection on desktop

This CL leverages InputEventActivationProtector to add protection
against events that happen too quickly after the FedCM bubble becomes
visible to the user.

Bug: 1406900
Change-Id: I77f65573280915fb98947a4f6d8e09bedb76fee8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4184294
Commit-Queue: Nicolás Peña <npm@chromium.org>
Reviewed-by: Yi Gu <yigu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1096393}

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

@@ -49,16 +49,19 @@ class AccountSelectionBubbleView : public views::BubbleDialogDelegateView,
     virtual void OnAccountSelected(
         const content::IdentityRequestAccount& account,
         const IdentityProviderDisplayData& idp_data,
-        bool auto_signin) = 0;
+        bool auto_signin,
+        const ui::Event& event) = 0;
 
     // Called when the user clicks "privacy policy" or "terms of service" link.
-    virtual void OnLinkClicked(LinkType link_type, const GURL& url) = 0;
+    virtual void OnLinkClicked(LinkType link_type,
+                               const GURL& url,
+                               const ui::Event& event) = 0;
 
     // Called when the user clicks "back" button.
     virtual void OnBackButtonClicked() = 0;
 
     // Called when the user clicks "close" button.
-    virtual void OnCloseButtonClicked() = 0;
+    virtual void OnCloseButtonClicked(const ui::Event& event) = 0;
   };
 
   METADATA_HEADER(AccountSelectionBubbleView);

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

@@ -88,26 +88,39 @@ void FedCmAccountSelectionView::Show(
   rp_for_display_ = base::UTF8ToUTF16(rp_etld_plus_one);
   bubble_widget_ = CreateBubble(browser, rp_for_display_, idp_title, rp_context)
                        ->GetWeakPtr();
-  if (sign_in_mode == Account::SignInMode::kExplicit) {
-    GetBubbleView()->ShowAccountPicker(idp_data_list_,
-                                       /*show_back_button=*/false);
-  } else {
-    DCHECK_EQ(sign_in_mode, Account::SignInMode::kAuto);
-
+  if (sign_in_mode == Account::SignInMode::kAuto) {
     for (const auto& idp_data : idp_data_list_) {
       for (const auto& account : idp_data.accounts_) {
         if (account.login_state != Account::LoginState::kSignIn) {
           continue;
         }
         // When auto sign-in UX flow is triggered, there will be one and only
-        // only account that's returning with LoginStatus::kSignIn.
-        OnAccountSelected(account, idp_data, /*auto_signin=*/true);
+        // only account that's returning with LoginStatus::kSignIn. This method
+        // is generally meant to be called with an associated event, so pass a
+        // dummy one, which will be ignored.
+        OnAccountSelected(
+            account, idp_data, /*auto_signin=*/true,
+            ui::MouseEvent(ui::ET_UNKNOWN, gfx::Point(), gfx::Point(),
+                           base::TimeTicks(), 0, 0));
+        // Initialize InputEventActivationProtector to handle potentially
+        // unintended input events that could close the auto signin dialog.
+        input_protector_ =
+            std::make_unique<views::InputEventActivationProtector>();
+        input_protector_->VisibilityChanged(true);
         bubble_widget_->Show();
         bubble_widget_->AddObserver(this);
         return;
       }
     }
+    // Should return in the for loop above.
+    DCHECK(false);
   }
+  GetBubbleView()->ShowAccountPicker(idp_data_list_,
+                                     /*show_back_button=*/false);
+  // Initialize InputEventActivationProtector to handle potentially unintended
+  // input events.
+  input_protector_ = std::make_unique<views::InputEventActivationProtector>();
+  input_protector_->VisibilityChanged(true);
   bubble_widget_->Show();
   bubble_widget_->AddObserver(this);
 }
@@ -138,6 +151,10 @@ void FedCmAccountSelectionView::ShowFailureDialog(
                        ->GetWeakPtr();
   GetBubbleView()->ShowFailureDialog(base::UTF8ToUTF16(rp_etld_plus_one),
                                      base::UTF8ToUTF16(idp_etld_plus_one));
+  // Initialize InputEventActivationProtector to handle potentially unintended
+  // input events.
+  input_protector_ = std::make_unique<views::InputEventActivationProtector>();
+  input_protector_->VisibilityChanged(true);
   bubble_widget_->Show();
   bubble_widget_->AddObserver(this);
 }
@@ -150,13 +167,17 @@ void FedCmAccountSelectionView::OnVisibilityChanged(
   if (visibility == content::Visibility::VISIBLE) {
     bubble_widget_->widget_delegate()->SetCanActivate(true);
     bubble_widget_->Show();
+    // This will protect against potentially unintentional inputs that happen
+    // right after the dialog becomes visible again.
+    input_protector_->VisibilityChanged(true);
   } else {
     // On Mac, NativeWidgetMac::Activate() ignores the views::Widget visibility.
     // Make the views::Widget non-activatable while it is hidden to prevent the
     // views::Widget from being shown during focus traversal.
     // TODO(crbug.com/1367309): fix the issue on Mac.
     bubble_widget_->widget_delegate()->SetCanActivate(false);
     bubble_widget_->Hide();
+    input_protector_->VisibilityChanged(false);
   }
 }
 
@@ -181,6 +202,11 @@ void FedCmAccountSelectionView::OnTabStripModelChanged(
   }
 }
 
+void FedCmAccountSelectionView::SetInputEventActivationProtectorForTesting(
+    std::unique_ptr<views::InputEventActivationProtector> input_protector) {
+  input_protector_ = std::move(input_protector);
+}
+
 views::Widget* FedCmAccountSelectionView::CreateBubble(
     Browser* browser,
     const std::u16string& rp_etld_plus_one,
@@ -215,7 +241,12 @@ void FedCmAccountSelectionView::OnWidgetDestroying(views::Widget* widget) {
 void FedCmAccountSelectionView::OnAccountSelected(
     const Account& account,
     const IdentityProviderDisplayData& idp_data,
-    bool auto_signin) {
+    bool auto_signin,
+    const ui::Event& event) {
+  if (!auto_signin &&
+      input_protector_->IsPossiblyUnintendedInteraction(event)) {
+    return;
+  }
   state_ = (state_ == State::ACCOUNT_PICKER &&
             account.login_state == Account::LoginState::kSignUp)
                ? State::PERMISSION
@@ -244,7 +275,11 @@ void FedCmAccountSelectionView::OnAccountSelected(
 }
 
 void FedCmAccountSelectionView::OnLinkClicked(LinkType link_type,
-                                              const GURL& url) {
+                                              const GURL& url,
+                                              const ui::Event& event) {
+  if (input_protector_->IsPossiblyUnintendedInteraction(event)) {
+    return;
+  }
   Browser* browser =
       chrome::FindBrowserWithWebContents(delegate_->GetWebContents());
   TabStripModel* tab_strip_model = browser->tab_strip_model();
@@ -265,12 +300,17 @@ void FedCmAccountSelectionView::OnLinkClicked(LinkType link_type,
 }
 
 void FedCmAccountSelectionView::OnBackButtonClicked() {
+  // No need to protect input here since back cannot be the first event.
   state_ = State::ACCOUNT_PICKER;
   GetBubbleView()->ShowAccountPicker(idp_data_list_,
                                      /*show_back_button=*/false);
 }
 
-void FedCmAccountSelectionView::OnCloseButtonClicked() {
+void FedCmAccountSelectionView::OnCloseButtonClicked(const ui::Event& event) {
+  if (input_protector_->IsPossiblyUnintendedInteraction(event)) {
+    return;
+  }
+
   UMA_HISTOGRAM_BOOLEAN("Blink.FedCm.CloseVerifySheet.Desktop",
                         state_ == State::VERIFYING);
   bubble_widget_->CloseWithReason(
@@ -291,6 +331,7 @@ void FedCmAccountSelectionView::OnDismiss(DismissReason dismiss_reason) {
 
   bubble_widget_->RemoveObserver(this);
   bubble_widget_.reset();
+  input_protector_.reset();
 
   if (notify_delegate_of_dismiss_)
     delegate_->OnDismiss(dismiss_reason);

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

@@ -12,6 +12,7 @@
 #include "chrome/browser/ui/views/webid/account_selection_bubble_view.h"
 #include "chrome/browser/ui/views/webid/identity_provider_display_data.h"
 #include "content/public/browser/web_contents_observer.h"
+#include "ui/views/input_event_activation_protector.h"
 #include "ui/views/widget/widget_observer.h"
 
 class AccountSelectionBubbleViewInterface;
@@ -51,6 +52,9 @@ class FedCmAccountSelectionView : public AccountSelectionView,
       const TabStripModelChange& change,
       const TabStripSelectionChange& selection) override;
 
+  void SetInputEventActivationProtectorForTesting(
+      std::unique_ptr<views::InputEventActivationProtector>);
+
  protected:
   friend class FedCmAccountSelectionViewBrowserTest;
 
@@ -85,10 +89,13 @@ class FedCmAccountSelectionView : public AccountSelectionView,
   // AccountSelectionBubbleView::Observer:
   void OnAccountSelected(const Account& account,
                          const IdentityProviderDisplayData& idp_data,
-                         bool auto_signin) override;
-  void OnLinkClicked(LinkType link_type, const GURL& url) override;
+                         bool auto_signin,
+                         const ui::Event& event) override;
+  void OnLinkClicked(LinkType link_type,
+                     const GURL& url,
+                     const ui::Event& event) override;
   void OnBackButtonClicked() override;
-  void OnCloseButtonClicked() override;
+  void OnCloseButtonClicked(const ui::Event& event) override;
 
   // Called when the user selected an account AND granted consent.
   void OnAccountSelected(const content::IdentityRequestAccount& account);
@@ -112,6 +119,8 @@ class FedCmAccountSelectionView : public AccountSelectionView,
 
   base::WeakPtr<views::Widget> bubble_widget_;
 
+  std::unique_ptr<views::InputEventActivationProtector> input_protector_;
+
   base::WeakPtrFactory<FedCmAccountSelectionView> weak_ptr_factory_{this};
 };
 

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

@@ -15,6 +15,7 @@
 #include "content/public/test/web_contents_tester.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/views/test/mock_input_event_activation_protector.h"
 #include "url/gurl.h"
 
 using LoginState = content::IdentityRequestAccount::LoginState;
@@ -157,15 +158,30 @@ class FedCmAccountSelectionViewDesktopTest : public ChromeViewsTestBase {
       SignInMode mode) {
     auto controller = std::make_unique<TestFedCmAccountSelectionView>(
         delegate_.get(), widget_.get(), bubble_view_.get());
+
     controller->Show(
         kRpEtldPlusOne,
         {{kIdpEtldPlusOne, accounts, content::IdentityProviderMetadata(),
           content::ClientMetadata(GURL(), GURL()),
           blink::mojom::RpContext::kSignIn}},
         mode);
+
+    // In tests, use a MockInputEventActivationProtector that will not block any
+    // input. This can be overridden by a specific test.
+    auto input_protector =
+        std::make_unique<views::MockInputEventActivationProtector>();
+    ON_CALL(*input_protector, IsPossiblyUnintendedInteraction)
+        .WillByDefault(testing::Return(false));
+    controller->SetInputEventActivationProtectorForTesting(
+        std::move(input_protector));
     return controller;
   }
 
+  ui::MouseEvent CreateMouseEvent() {
+    return ui::MouseEvent(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
+                          base::TimeTicks(), ui::EF_LEFT_MOUSE_BUTTON, 0);
+  }
+
  protected:
   TestingProfile profile_;
 
@@ -192,7 +208,8 @@ TEST_F(FedCmAccountSelectionViewDesktopTest, SingleAccountFlow) {
   EXPECT_FALSE(bubble_view_->show_verifying_sheet_);
   EXPECT_THAT(bubble_view_->account_ids_, testing::ElementsAre(kAccountId));
 
-  observer->OnAccountSelected(accounts[0], idp_data, /*auto_signin=*/false);
+  observer->OnAccountSelected(accounts[0], idp_data, /*auto_signin=*/false,
+                              CreateMouseEvent());
   EXPECT_TRUE(bubble_view_->show_verifying_sheet_);
   EXPECT_THAT(bubble_view_->account_ids_, testing::ElementsAre(kAccountId));
 }
@@ -213,7 +230,8 @@ TEST_F(FedCmAccountSelectionViewDesktopTest, MultipleAccountFlowReturning) {
   EXPECT_THAT(bubble_view_->account_ids_,
               testing::ElementsAre(kAccountId1, kAccountId2));
 
-  observer->OnAccountSelected(accounts[0], idp_data, /*auto_signin=*/false);
+  observer->OnAccountSelected(accounts[0], idp_data, /*auto_signin=*/false,
+                              CreateMouseEvent());
   EXPECT_TRUE(bubble_view_->show_verifying_sheet_);
   EXPECT_THAT(bubble_view_->account_ids_, testing::ElementsAre(kAccountId1));
 }
@@ -236,7 +254,8 @@ TEST_F(FedCmAccountSelectionViewDesktopTest, MultipleAccountFlowBack) {
   EXPECT_THAT(bubble_view_->account_ids_,
               testing::ElementsAre(kAccountId1, kAccountId2));
 
-  observer->OnAccountSelected(accounts[0], idp_data, /*auto_signin=*/false);
+  observer->OnAccountSelected(accounts[0], idp_data, /*auto_signin=*/false,
+                              CreateMouseEvent());
   EXPECT_TRUE(bubble_view_->show_back_button_);
   EXPECT_FALSE(bubble_view_->show_verifying_sheet_);
   EXPECT_THAT(bubble_view_->account_ids_, testing::ElementsAre(kAccountId1));
@@ -247,12 +266,14 @@ TEST_F(FedCmAccountSelectionViewDesktopTest, MultipleAccountFlowBack) {
   EXPECT_THAT(bubble_view_->account_ids_,
               testing::ElementsAre(kAccountId1, kAccountId2));
 
-  observer->OnAccountSelected(accounts[1], idp_data, /*auto_signin=*/false);
+  observer->OnAccountSelected(accounts[1], idp_data, /*auto_signin=*/false,
+                              CreateMouseEvent());
   EXPECT_TRUE(bubble_view_->show_back_button_);
   EXPECT_FALSE(bubble_view_->show_verifying_sheet_);
   EXPECT_THAT(bubble_view_->account_ids_, testing::ElementsAre(kAccountId2));
 
-  observer->OnAccountSelected(accounts[1], idp_data, /*auto_signin=*/false);
+  observer->OnAccountSelected(accounts[1], idp_data, /*auto_signin=*/false,
+                              CreateMouseEvent());
   EXPECT_TRUE(bubble_view_->show_verifying_sheet_);
   EXPECT_THAT(bubble_view_->account_ids_, testing::ElementsAre(kAccountId2));
 }
@@ -339,5 +360,40 @@ TEST_F(FedCmAccountSelectionViewDesktopTest, AccountSelectedDeletesView) {
   }
 
   // Destroys FedCmAccountSelectionView. Should not cause crash.
-  observer->OnAccountSelected(accounts[0], idp_data, /*auto_signin=*/false);
+  observer->OnAccountSelected(accounts[0], idp_data, /*auto_signin=*/false,
+                              CreateMouseEvent());
+}
+
+TEST_F(FedCmAccountSelectionViewDesktopTest, ClickProtection) {
+  const char kAccountId[] = "account_id";
+  IdentityProviderDisplayData idp_data =
+      CreateIdentityProviderDisplayData({{kAccountId, LoginState::kSignUp}});
+  const std::vector<Account>& accounts = idp_data.accounts_;
+  std::unique_ptr<TestFedCmAccountSelectionView> controller =
+      CreateAndShow(accounts, SignInMode::kExplicit);
+  AccountSelectionBubbleView::Observer* observer =
+      static_cast<AccountSelectionBubbleView::Observer*>(controller.get());
+
+  // Use a mock input protector to more easily test. The protector rejects the
+  // first input and accepts any subsequent input.
+  auto input_protector =
+      std::make_unique<views::MockInputEventActivationProtector>();
+  EXPECT_CALL(*input_protector, IsPossiblyUnintendedInteraction)
+      .WillOnce(testing::Return(true))
+      .WillRepeatedly(testing::Return(false));
+  controller->SetInputEventActivationProtectorForTesting(
+      std::move(input_protector));
+
+  observer->OnAccountSelected(accounts[0], idp_data, /*auto_signin=*/false,
+                              CreateMouseEvent());
+  // Nothing should change after first account selected.
+  EXPECT_FALSE(bubble_view_->show_back_button_);
+  EXPECT_FALSE(bubble_view_->show_verifying_sheet_);
+  EXPECT_THAT(bubble_view_->account_ids_, testing::ElementsAre(kAccountId));
+
+  observer->OnAccountSelected(accounts[0], idp_data, /*auto_signin=*/false,
+                              CreateMouseEvent());
+  // Should show verifying sheet after first account selected.
+  EXPECT_TRUE(bubble_view_->show_verifying_sheet_);
+  EXPECT_THAT(bubble_view_->account_ids_, testing::ElementsAre(kAccountId));
 }