From cc4a1f34f2305fb96d7eaec3a026f7dd8a837dbe Mon Sep 17 00:00:00 2001
From: Maggie Chen <[email protected]>
Date: Fri, 17 Jul 2026 10:43:39 -0700
Subject: [PATCH] [agy][content] Dismiss popup on permission prompt

Mac-specific popup menus (like <select> dropdowns) run a native NSMenu
nested run loop. During this loop, application tasks are still pumped,
allowing a permission prompt to appear under the menu. This can lead to
clickjacking.

This CL starts a periodic timer in PopupMenuHelper to check if the menu
anchor element intersects with any permission prompt. If it does, the
menu is dismissed.

Fixed: 514069975
Test: content_browsertests
Change-Id: I374f651435a3ab7e53b6cc80752c8c8ca6442500
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8092080
Reviewed-by: Avi Drissman <[email protected]>
Commit-Queue: Maggie Chen <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1664018}
---

diff --git a/content/browser/renderer_host/popup_menu_helper_mac.h b/content/browser/renderer_host/popup_menu_helper_mac.h
index 4a2a4eb..0f08652 100644
--- a/content/browser/renderer_host/popup_menu_helper_mac.h
+++ b/content/browser/renderer_host/popup_menu_helper_mac.h
@@ -11,6 +11,7 @@
 #include "base/memory/raw_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/scoped_observation.h"
+#include "base/timer/timer.h"
 #include "content/common/content_export.h"
 #include "content/common/render_widget_host_ns_view.mojom.h"
 #include "content/public/browser/render_widget_host.h"
@@ -66,6 +67,14 @@
 
   RenderWidgetHostViewMac* GetRootRenderWidgetHostView() const;
 
+  // Returns true if the menu, as positioned at |bounds_in_screen|, would
+  // overlap a permission prompt that is currently showing.
+  bool IntersectsPermissionPrompt(const gfx::Rect& bounds_in_screen) const;
+
+  // Periodically called while the menu is open to ensure it does not occlude a
+  // permission prompt that appeared after the menu was opened.
+  void CheckPermissionPromptOcclusion();
+
   raw_ptr<Delegate> delegate_;  // Weak. Owns |this|.
 
   base::ScopedObservation<RenderWidgetHost, RenderWidgetHostObserver>
@@ -75,6 +84,10 @@
 
   bool popup_was_hidden_ = false;
 
+  // Screen bounds of the anchor element while the menu is open.
+  gfx::Rect anchor_bounds_in_screen;
+  base::RepeatingTimer occlusion_check_timer_;
+
   mojo::Remote<remote_cocoa::mojom::PopupMenuRunner> remote_runner_;
 
   base::WeakPtrFactory<PopupMenuHelper> weak_ptr_factory_{this};
diff --git a/content/browser/renderer_host/popup_menu_helper_mac.mm b/content/browser/renderer_host/popup_menu_helper_mac.mm
index 3b76623..cf7b9977 100644
--- a/content/browser/renderer_host/popup_menu_helper_mac.mm
+++ b/content/browser/renderer_host/popup_menu_helper_mac.mm
@@ -22,6 +22,10 @@
 
 bool g_allow_showing_popup_menus = true;
 
+// Interval at which to re-evaluate whether the open menu overlaps a
+// permission prompt that may have appeared after the menu was opened.
+constexpr base::TimeDelta kOcclusionCheckInterval = base::Milliseconds(100);
+
 }  // namespace
 
 PopupMenuHelper::PopupMenuHelper(
@@ -61,22 +65,26 @@
 
   // Convert element_bounds to be in screen.
   gfx::Rect client_area = web_contents->GetContainerBounds();
-  gfx::Rect bounds_in_screen = bounds + client_area.OffsetFromOrigin();
+  anchor_bounds_in_screen = bounds + client_area.OffsetFromOrigin();
 
   // The new popup menu would overlap the permission prompt, which could lead to
   // users making decisions based on incorrect information. We should close the
   // popup if it intersects with the permission prompt.
-  auto permission_exclusion_area_bounds =
-      PermissionControllerImpl::FromBrowserContext(
-          web_contents->GetBrowserContext())
-          ->GetExclusionAreaBoundsInScreen(web_contents);
-  if (permission_exclusion_area_bounds &&
-      permission_exclusion_area_bounds->Intersects(bounds_in_screen)) {
+  if (IntersectsPermissionPrompt(anchor_bounds_in_screen)) {
     popup_client_->DidCancel();
     delegate_->OnMenuClosed();  // May delete |this|.
     return;
   }
 
+  // The native menu runs a nested run loop in which application tasks are
+  // pumped, so a permission prompt may appear after the menu has opened.
+  // Re-evaluate periodically and dismiss the menu if it would overlap. The
+  // timer must be started before DisplayPopupMenu(), which may run the nested
+  // loop synchronously. See https://crbug.com/514069975
+  occlusion_check_timer_.Start(
+      FROM_HERE, kOcclusionCheckInterval, this,
+      &PopupMenuHelper::CheckPermissionPromptOcclusion);
+
   remote_runner_.reset();
   rwhvm->GetNSView()->DisplayPopupMenu(
       remote_cocoa::mojom::PopupMenu::New(
@@ -87,6 +95,7 @@
 }
 
 void PopupMenuHelper::Hide() {
+  occlusion_check_timer_.Stop();
   if (remote_runner_) {
     remote_runner_->Hide();
   }
@@ -94,6 +103,35 @@
   popup_client_.reset();
 }
 
+bool PopupMenuHelper::IntersectsPermissionPrompt(
+    const gfx::Rect& bounds_in_screen) const {
+  if (!render_frame_host_) {
+    return false;
+  }
+  RenderWidgetHostViewMac* rwhvm = GetRootRenderWidgetHostView();
+  if (!rwhvm) {
+    return false;
+  }
+  auto* web_contents = rwhvm->GetWebContents();
+  auto permission_exclusion_area_bounds =
+      PermissionControllerImpl::FromBrowserContext(
+          web_contents->GetBrowserContext())
+          ->GetExclusionAreaBoundsInScreen(web_contents);
+  return permission_exclusion_area_bounds &&
+         permission_exclusion_area_bounds->Intersects(bounds_in_screen);
+}
+
+void PopupMenuHelper::CheckPermissionPromptOcclusion() {
+  if (popup_was_hidden_ ||
+      !IntersectsPermissionPrompt(anchor_bounds_in_screen)) {
+    return;
+  }
+  if (popup_client_) {
+    popup_client_->DidCancel();
+  }
+  Hide();
+}
+
 RenderWidgetHostViewMac* PopupMenuHelper::GetRootRenderWidgetHostView() const {
   RenderWidgetHostViewBase* root_view =
       render_frame_host_->GetView()->GetRootView();
@@ -114,6 +152,8 @@
 }
 
 void PopupMenuHelper::PopupMenuClosed(std::optional<uint32_t> selected_item) {
+  occlusion_check_timer_.Stop();
+
   // The RenderFrameHost may be deleted while running the menu, or it may have
   // requested the close. Don't notify in these cases.
   if (popup_client_ && !popup_was_hidden_) {
diff --git a/content/browser/renderer_host/render_widget_host_browsertest.cc b/content/browser/renderer_host/render_widget_host_browsertest.cc
index 8182ccf..254c10b6 100644
--- a/content/browser/renderer_host/render_widget_host_browsertest.cc
+++ b/content/browser/renderer_host/render_widget_host_browsertest.cc
@@ -772,8 +772,10 @@
       public blink::mojom::PopupMenuClient {
  public:
   explicit ShowPopupMenuInterceptor(RenderFrameHostImpl* render_frame_host,
-                                    const gfx::Rect& overriden_bounds)
+                                    const gfx::Rect& overriden_bounds,
+                                    base::OnceClosure task_in_nested_loop = {})
       : overriden_bounds_(overriden_bounds),
+        task_in_nested_loop_(std::move(task_in_nested_loop)),
         swapped_impl_(
             render_frame_host->local_frame_host_receiver_for_testing(),
             this) {}
@@ -795,10 +797,18 @@
       bool right_aligned,
       bool allow_multiple_selection) override {
     CHECK(GetForwardingInterface());
+    // If supplied, post a task that will run inside the menu's nested event
+    // loop after the menu is opened by the synchronous call below.
+    if (task_in_nested_loop_) {
+      base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
+          FROM_HERE, std::move(task_in_nested_loop_));
+    }
     GetForwardingInterface()->ShowPopupMenu(
         receiver_.BindNewPipeAndPassRemote(), overriden_bounds_, font_size,
         selected_item, std::move(menu_items), right_aligned,
         allow_multiple_selection);
+    base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
+        FROM_HERE, run_loop_.QuitClosure());
   }
 
   void DidAcceptIndices(const std::vector<int32_t>& indices) override {
@@ -808,7 +818,6 @@
   void DidCancel() override {

diff --git a/content/browser/renderer_host/render_widget_host_browsertest.cc b/content/browser/renderer_host/render_widget_host_browsertest.cc
index 8182ccf..254c10b6 100644
--- a/content/browser/renderer_host/render_widget_host_browsertest.cc
+++ b/content/browser/renderer_host/render_widget_host_browsertest.cc
@@ -772,8 +772,10 @@
       public blink::mojom::PopupMenuClient {
  public:
   explicit ShowPopupMenuInterceptor(RenderFrameHostImpl* render_frame_host,
-                                    const gfx::Rect& overriden_bounds)
+                                    const gfx::Rect& overriden_bounds,
+                                    base::OnceClosure task_in_nested_loop = {})
       : overriden_bounds_(overriden_bounds),
+        task_in_nested_loop_(std::move(task_in_nested_loop)),
         swapped_impl_(
             render_frame_host->local_frame_host_receiver_for_testing(),
             this) {}
@@ -795,10 +797,18 @@
       bool right_aligned,
       bool allow_multiple_selection) override {
     CHECK(GetForwardingInterface());
+    // If supplied, post a task that will run inside the menu's nested event
+    // loop after the menu is opened by the synchronous call below.
+    if (task_in_nested_loop_) {
+      base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
+          FROM_HERE, std::move(task_in_nested_loop_));
+    }
     GetForwardingInterface()->ShowPopupMenu(
         receiver_.BindNewPipeAndPassRemote(), overriden_bounds_, font_size,
         selected_item, std::move(menu_items), right_aligned,
         allow_multiple_selection);
+    base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
+        FROM_HERE, run_loop_.QuitClosure());
   }
 
   void DidAcceptIndices(const std::vector<int32_t>& indices) override {
@@ -808,7 +818,6 @@
   void DidCancel() override {
     is_cancelled_ = true;
     receiver_.reset();
-    run_loop_.Quit();
   }
 
   bool is_cancelled() const { return is_cancelled_; }
@@ -817,6 +826,7 @@
   base::RunLoop run_loop_;
   bool is_cancelled_{false};
   gfx::Rect overriden_bounds_;
+  base::OnceClosure task_in_nested_loop_;
   mojo::test::ScopedSwapImplForTesting<blink::mojom::LocalFrameHost>
       swapped_impl_;
   mojo::Receiver<blink::mojom::PopupMenuClient> receiver_{this};
@@ -871,6 +881,50 @@
 #endif  // BUILDFLAG(IS_MAC)
 }
 
+#if BUILDFLAG(IS_MAC)
+// Variant of the above where no permission prompt is showing when the popup
+// opens, but one appears while the native menu's nested run loop is active.
+// The browser should detect the new prompt and dismiss the open popup.
+IN_PROC_BROWSER_TEST_F(RenderWidgetHostSitePerProcessTest,
+                       BrowserClosesOpenPopupWhenPermissionPromptAppears) {
+  GURL main_url(embedded_test_server()->GetURL(
+      "a.com", "/site_isolation/page-with-select.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), main_url));
+  SimulateEndOfPaintHoldingOnPrimaryMainFrame(shell()->web_contents());
+
+  auto* contents = static_cast<WebContentsImpl*>(web_contents());
+  FrameTreeNode* root = contents->GetPrimaryFrameTree().root();
+  RenderFrameHostImpl* root_frame_host = root->current_frame_host();
+
+  auto* permission_controller = static_cast<PermissionControllerImpl*>(
+      root_frame_host->GetBrowserContext()->GetPermissionController());
+
+  gfx::Rect permission_exclusion_area_bounds(100, 100, 100, 100);
+  // Simulate a permission prompt being shown only after the popup has opened
+  // and entered its nested run loop.
+  ShowPopupMenuInterceptor show_popup_menu_interceptor(
+      root_frame_host,
+      permission_exclusion_area_bounds -
+          contents->GetContainerBounds().OffsetFromOrigin(),
+      base::BindLambdaForTesting([&]() {
+        permission_controller->set_exclusion_area_bounds_for_tests(
+            permission_exclusion_area_bounds);
+      }));
+
+  input::NativeWebKeyboardEvent event(
+      blink::WebKeyboardEvent::Type::kChar, blink::WebInputEvent::kNoModifiers,
+      blink::WebInputEvent::GetStaticTimeStampForTests());
+  event.text[0] = ' ';
+  EXPECT_TRUE(ExecJs(root_frame_host, "focusSelectMenu();"));
+  root_frame_host->GetRenderWidgetHost()->ForwardKeyboardEvent(event);
+
+  show_popup_menu_interceptor.Wait();
+  EXPECT_TRUE(show_popup_menu_interceptor.is_cancelled());
+
+  permission_controller->set_exclusion_area_bounds_for_tests(std::nullopt);
+}
+#endif  // BUILDFLAG(IS_MAC)
+
 #endif  // !BUILDFLAG(IS_ANDROID) && !BUILDFLAG(IS_IOS)
 
 // Tests that `window.screen` dimensions match the display, not the viewport,