From 20281770dded5b0fbce9474da212dc84b8c41756 Mon Sep 17 00:00:00 2001
From: Yi Gu <[email protected]>
Date: Wed, 12 Aug 2026 09:17:33 -0700
Subject: [PATCH] [FedCM] Observe dialog widget visibility to update input protector

When the FedCM dialog is managed by TabDialogManager, re-showing it
(e.g., switching back to the tab or resizing the window) skips
ShowDialogWidget() and does not re-arm the InputEventActivationProtector.
This could allow unintended rapid clicks on the re-shown UI.

This CL makes FedCmAccountSelectionView observe the dialog widget's
visibility via views::WidgetObserver, ensuring
InputEventActivationProtector::VisibilityChanged() is invoked on every
visibility transition.

Fixed: 514041087
Change-Id: I1411c8ea360be0bd24cdfe451a5e43a87b3b7a8e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8234102
Reviewed-by: Nicolás Peña <[email protected]>
Commit-Queue: Yi Gu <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1678098}
---

diff --git a/chrome/browser/ui/views/webid/fedcm_account_selection_view_desktop.cc b/chrome/browser/ui/views/webid/fedcm_account_selection_view_desktop.cc
index 6e0066d3..f287a104 100644
--- a/chrome/browser/ui/views/webid/fedcm_account_selection_view_desktop.cc
+++ b/chrome/browser/ui/views/webid/fedcm_account_selection_view_desktop.cc
@@ -196,7 +196,6 @@
     return;
   }
 
-  input_protector_->VisibilityChanged(true);
   auto params = std::make_unique<tabs::TabDialogManager::Params>();
   params->close_on_navigate = true;
   params->close_on_detach = true;
@@ -657,6 +656,7 @@
                          rp_mode, &dialog_type_);
   }
   dialog_widget_ = CreateDialogWidget();
+  dialog_widget_observation_.Observe(dialog_widget_.get());
   dialog_widget_->MakeCloseSynchronous(base::BindOnce(
       &FedCmAccountSelectionView::OnUserClosedDialog, base::Unretained(this)));
 }
@@ -1259,11 +1259,6 @@
   UpdateDialogVisibility(false);
   scoped_ignore_input_events_.reset();
   tab_accept_mouse_events_.reset();
-  // TODO(crbug.com/331166928): This is only null in one test. Fix the test to
-  // match production.
-  if (input_protector_) {
-    input_protector_->VisibilityChanged(false);
-  }
 }
 
 base::WeakPtr<FedCmAccountSelectionView>
@@ -1277,6 +1272,18 @@
   }
 }
 
+void FedCmAccountSelectionView::OnWidgetVisibilityChanged(views::Widget* widget,
+                                                          bool visible) {
+  if (widget != dialog_widget_.get()) {
+    return;
+  }
+  // TODO(crbug.com/331166928): This is only null in one test. Fix the test to
+  // match production.
+  if (input_protector_) {
+    input_protector_->VisibilityChanged(visible);
+  }
+}
+
 void FedCmAccountSelectionView::ShowMultiAccountPicker(
     const std::vector<IdentityRequestAccountPtr>& accounts,
     const std::vector<IdentityProviderDataPtr>& idp_list,
@@ -1388,6 +1395,7 @@
     parked_dialog_view_.reset();
   }
   scoped_ignore_input_events_.reset();
+  dialog_widget_observation_.Reset();
   dialog_widget_.reset();
   widget_delegate_.reset();
 
@@ -1420,6 +1428,7 @@
   if (should_show_dialog) {
     if (!dialog_widget_) {
       dialog_widget_ = CreateDialogWidget();
+      dialog_widget_observation_.Observe(dialog_widget_.get());
       dialog_widget_->MakeCloseSynchronous(
           base::BindOnce(&FedCmAccountSelectionView::OnUserClosedDialog,
                          base::Unretained(this)));
diff --git a/chrome/browser/ui/views/webid/fedcm_account_selection_view_desktop.h b/chrome/browser/ui/views/webid/fedcm_account_selection_view_desktop.h
index 8f1cdbf..feca0a4e 100644
--- a/chrome/browser/ui/views/webid/fedcm_account_selection_view_desktop.h
+++ b/chrome/browser/ui/views/webid/fedcm_account_selection_view_desktop.h
@@ -10,6 +10,7 @@
 #include "base/gtest_prod_util.h"
 #include "base/memory/raw_ptr.h"
 #include "base/memory/weak_ptr.h"
+#include "base/scoped_observation.h"
 #include "chrome/browser/picture_in_picture/picture_in_picture_occlusion_observer.h"
 #include "chrome/browser/picture_in_picture/scoped_picture_in_picture_occlusion_observation.h"
 #include "chrome/browser/ui/page_action/page_action_observer.h"
@@ -24,6 +25,7 @@
 #include "content/public/browser/web_contents_observer.h"
 #include "ui/views/input_event_activation_protector.h"
 #include "ui/views/widget/widget_delegate.h"
+#include "ui/views/widget/widget_observer.h"
 #include "ui/views/window/dialog_delegate.h"
 
 namespace tabs {
@@ -69,7 +71,8 @@
                                   public FedCmModalDialogView::Observer,
                                   public content::WebContentsObserver,
                                   public PictureInPictureOcclusionObserver,
-                                  public page_actions::PageActionObserver {
+                                  public page_actions::PageActionObserver,
+                                  public views::WidgetObserver {
  public:
   enum class DialogType {
     // FedCM dialog inherits a bubble dialog, which is typically shown on the
@@ -161,6 +164,9 @@
   void CloseModalDialog() override;
   void PrimaryMainFrameWasResized(bool width_changed) override;
 
+  // views::WidgetObserver:
+  void OnWidgetVisibilityChanged(views::Widget* widget, bool visible) override;
+
   base::WeakPtr<FedCmAccountSelectionView> GetWeakPtr();
 
   // Called when the associated tab enters the foreground.
@@ -634,6 +640,9 @@
   // Widget that owns the view.
   std::unique_ptr<views::Widget> dialog_widget_;
 
+  base::ScopedObservation<views::Widget, views::WidgetObserver>
+      dialog_widget_observation_{this};
+
   // This controls the contents of the dialog_widget_. Conceptually there
   // is a view if and only if there is a widget. The two are constructed
   // together and destroyed together. `dialog_widget_` owns
diff --git a/chrome/browser/ui/views/webid/fedcm_account_selection_view_desktop_unittest.cc b/chrome/browser/ui/views/webid/fedcm_account_selection_view_desktop_unittest.cc
index e88157f..d7bea92 100644
--- a/chrome/browser/ui/views/webid/fedcm_account_selection_view_desktop_unittest.cc
+++ b/chrome/browser/ui/views/webid/fedcm_account_selection_view_desktop_unittest.cc
@@ -2695,4 +2695,94 @@
   EXPECT_TRUE(controller->IsDialogWidgetVisible());
 }
 
+class FedCmAccountSelectionViewDesktopClickProtectionTest
+    : public FedCmAccountSelectionViewDesktopTest,
+      public testing::WithParamInterface<blink::mojom::RpMode> {};
+
+INSTANTIATE_TEST_SUITE_P(All,
+                         FedCmAccountSelectionViewDesktopClickProtectionTest,
+                         testing::Values(blink::mojom::RpMode::kPassive,
+                                         blink::mojom::RpMode::kActive));
+
+// Tests that switching to another tab and back re-arms the input event
+// activation protector for both bubble (passive mode) and modal (active mode)
+// dialogs, so that immediate clicks on the re-shown dialog are rejected while
+// delayed clicks after the cooldown period are accepted.
+TEST_P(FedCmAccountSelectionViewDesktopClickProtectionTest,
+       ClickProtectionAfterTabSwitch) {
+  std::unique_ptr<TestFedCmAccountSelectionView> controller =
+      CreateAndShow(accounts_, GetParam());
+
+  // Use a real InputEventActivationProtector.
+  controller->SetInputEventActivationProtectorForTesting(
+      std::make_unique<views::InputEventActivationProtector>());
+
+  // Advance time past the initial 500ms protection window.
+  task_environment()->FastForwardBy(base::Milliseconds(600));
+
+  // Simulate tab entering background and returning to foreground.
+  TabWillEnterBackground(controller.get());
+  TabForegrounded(controller.get());
+
+  ui::MouseEvent immediate_event(ui::EventType::kMousePressed, gfx::Point(),
+                                 gfx::Point(), base::TimeTicks::Now(),
+                                 ui::EF_LEFT_MOUSE_BUTTON, 0);
+
+  // Immediate click after re-show should be rejected by the protector.
+  EXPECT_FALSE(controller->OnAccountSelected(accounts_[0], immediate_event));
+
+  // Advance past the 500ms protection window.
+  task_environment()->FastForwardBy(base::Milliseconds(600));
+
+  ui::MouseEvent delayed_event(ui::EventType::kMousePressed, gfx::Point(),
+                               gfx::Point(), base::TimeTicks::Now(),
+                               ui::EF_LEFT_MOUSE_BUTTON, 0);
+
+  // Click after delay should be accepted.
+  EXPECT_TRUE(controller->OnAccountSelected(accounts_[0], delayed_event));
+}
+
+// Tests that when a bubble dialog is hidden because it cannot fit in the web
+// contents during a window resize, re-showing it upon resizing back re-arms the
+// input event activation protector. (Note: Modal dialogs are tab-modal and
+// centered, so they are not hidden when resized).
+TEST_F(FedCmAccountSelectionViewDesktopTest, ClickProtectionAfterResize) {

diff --git a/chrome/browser/ui/views/webid/fedcm_account_selection_view_desktop_unittest.cc b/chrome/browser/ui/views/webid/fedcm_account_selection_view_desktop_unittest.cc
index e88157f..d7bea92 100644
--- a/chrome/browser/ui/views/webid/fedcm_account_selection_view_desktop_unittest.cc
+++ b/chrome/browser/ui/views/webid/fedcm_account_selection_view_desktop_unittest.cc
@@ -2695,4 +2695,94 @@
   EXPECT_TRUE(controller->IsDialogWidgetVisible());
 }
 
+class FedCmAccountSelectionViewDesktopClickProtectionTest
+    : public FedCmAccountSelectionViewDesktopTest,
+      public testing::WithParamInterface<blink::mojom::RpMode> {};
+
+INSTANTIATE_TEST_SUITE_P(All,
+                         FedCmAccountSelectionViewDesktopClickProtectionTest,
+                         testing::Values(blink::mojom::RpMode::kPassive,
+                                         blink::mojom::RpMode::kActive));
+
+// Tests that switching to another tab and back re-arms the input event
+// activation protector for both bubble (passive mode) and modal (active mode)
+// dialogs, so that immediate clicks on the re-shown dialog are rejected while
+// delayed clicks after the cooldown period are accepted.
+TEST_P(FedCmAccountSelectionViewDesktopClickProtectionTest,
+       ClickProtectionAfterTabSwitch) {
+  std::unique_ptr<TestFedCmAccountSelectionView> controller =
+      CreateAndShow(accounts_, GetParam());
+
+  // Use a real InputEventActivationProtector.
+  controller->SetInputEventActivationProtectorForTesting(
+      std::make_unique<views::InputEventActivationProtector>());
+
+  // Advance time past the initial 500ms protection window.
+  task_environment()->FastForwardBy(base::Milliseconds(600));
+
+  // Simulate tab entering background and returning to foreground.
+  TabWillEnterBackground(controller.get());
+  TabForegrounded(controller.get());
+
+  ui::MouseEvent immediate_event(ui::EventType::kMousePressed, gfx::Point(),
+                                 gfx::Point(), base::TimeTicks::Now(),
+                                 ui::EF_LEFT_MOUSE_BUTTON, 0);
+
+  // Immediate click after re-show should be rejected by the protector.
+  EXPECT_FALSE(controller->OnAccountSelected(accounts_[0], immediate_event));
+
+  // Advance past the 500ms protection window.
+  task_environment()->FastForwardBy(base::Milliseconds(600));
+
+  ui::MouseEvent delayed_event(ui::EventType::kMousePressed, gfx::Point(),
+                               gfx::Point(), base::TimeTicks::Now(),
+                               ui::EF_LEFT_MOUSE_BUTTON, 0);
+
+  // Click after delay should be accepted.
+  EXPECT_TRUE(controller->OnAccountSelected(accounts_[0], delayed_event));
+}
+
+// Tests that when a bubble dialog is hidden because it cannot fit in the web
+// contents during a window resize, re-showing it upon resizing back re-arms the
+// input event activation protector. (Note: Modal dialogs are tab-modal and
+// centered, so they are not hidden when resized).
+TEST_F(FedCmAccountSelectionViewDesktopTest, ClickProtectionAfterResize) {
+  std::unique_ptr<TestFedCmAccountSelectionView> controller =
+      CreateAndShow(accounts_);
+
+  // Use a real InputEventActivationProtector.
+  controller->SetInputEventActivationProtectorForTesting(
+      std::make_unique<views::InputEventActivationProtector>());
+
+  // Advance time past the initial 500ms protection window.
+  task_environment()->FastForwardBy(base::Milliseconds(600));
+
+  // Resize so bubble dialog cannot fit, which hides it.
+  controller->can_fit_in_web_contents_ = false;
+  controller->PrimaryMainFrameWasResized(/*width_changed=*/true);
+  EXPECT_FALSE(controller->IsDialogWidgetVisible());
+
+  // Resize back so dialog can fit again, which re-shows it.
+  controller->can_fit_in_web_contents_ = true;
+  controller->PrimaryMainFrameWasResized(/*width_changed=*/true);
+  EXPECT_TRUE(controller->IsDialogWidgetVisible());
+
+  ui::MouseEvent immediate_event(ui::EventType::kMousePressed, gfx::Point(),
+                                 gfx::Point(), base::TimeTicks::Now(),
+                                 ui::EF_LEFT_MOUSE_BUTTON, 0);
+
+  // Immediate click after re-show should be rejected.
+  EXPECT_FALSE(controller->OnAccountSelected(accounts_[0], immediate_event));
+
+  // Advance past the 500ms protection window.
+  task_environment()->FastForwardBy(base::Milliseconds(600));
+
+  ui::MouseEvent delayed_event(ui::EventType::kMousePressed, gfx::Point(),
+                               gfx::Point(), base::TimeTicks::Now(),
+                               ui::EF_LEFT_MOUSE_BUTTON, 0);
+
+  // Click after delay should be accepted.
+  EXPECT_TRUE(controller->OnAccountSelected(accounts_[0], delayed_event));
+}
+
 }  // namespace webid