feat: add support for associating a Menu with a WebFrameMain (#45138)

* feat: add support for associating a Menu with a WebFrameMain

This allows certain OS level features to activate such as Writing Tools, Autofill.. and Services.

There appears to be a bug in macOS where the responder chain isn't traversed if the menu is not popped up using an event, as such we spoof a fake mouse event at the write coordinates in the right window and use that to open the menu.

* build: fix build on non-mac

* build: oops missed a header

* fix: safely handle optional T* by checking nullptr too

* build: fix gn check and build errors

* docs: suggested changes

* feat: default `frame` to `window.webContents.mainFrame` when possible

* fix: avoid deref nullptr view

* Revert "feat: default `frame` to `window.webContents.mainFrame` when possible"

This reverts commit 2e888368199317d67f6ad931a7e9eff0295c4b1b.

* fix: lint

* Remove redundant scoped objects

This code, including the comments, matches almost exactly the behavior of this argument to the function.

* Add ScopedPumpMessagesInPrivateModes patch

* More null pointer safety

---------

Co-authored-by: clavin <clavin@electronjs.org>

docs/api/menu.md

@@ -73,6 +73,8 @@ The `menu` object has the following instance methods:
 
 * `options` Object (optional)
   * `window` [BaseWindow](base-window.md) (optional) - Default is the focused window.
+  * `frame` [WebFrameMain](web-frame-main.md) (optional) - Provide the relevant frame
+    if you want certain OS-level features such as Writing Tools on macOS to function correctly. Typically, this should be `params.frame` from the [`context-menu` event](web-contents.md#event-context-menu) on a WebContents.
   * `x` number (optional) - Default is the current mouse cursor position.
     Must be declared if `y` is declared.
   * `y` number (optional) - Default is the current mouse cursor position.

lib/browser/api/menu.ts

@@ -93,7 +93,7 @@ Menu.prototype.popup = function (options = {}) {
     }
   }
 
-  this.popupAt(window as unknown as BaseWindow, x, y, positioningItem, sourceType, callback);
+  this.popupAt(window as unknown as BaseWindow, options.frame, x, y, positioningItem, sourceType, callback);
   return { browserWindow: window, x, y, position: positioningItem };
 };
 

patches/chromium/.patches

@@ -135,6 +135,7 @@ fix_add_method_which_disables_headless_mode_on_native_widget.patch
 refactor_unfilter_unresponsive_events.patch
 build_disable_thin_lto_mac.patch
 build_add_public_config_simdutf_config.patch
+fix_multiple_scopedpumpmessagesinprivatemodes_instances.patch
 revert_code_health_clean_up_stale_macwebcontentsocclusion.patch
 ignore_parse_errors_for_resolveshortcutproperties.patch
 feat_add_signals_when_embedder_cleanup_callbacks_run_for.patch

patches/chromium/fix_multiple_scopedpumpmessagesinprivatemodes_instances.patch

@@ -0,0 +1,51 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Calvin Watford <watfordcalvin@gmail.com>
+Date: Wed, 5 Mar 2025 15:26:28 -0700
+Subject: fix: multiple ScopedPumpMessagesInPrivateModes instances
+
+Context: When swapping `Menu.popup` to use `ui::ShowContextMenu`, we
+found that its use of `ScopedPumpMessagesInPrivateModes` may potentially
+be used multiple times simultaneously. This class was designed to work
+with only one global instance.
+
+This patch adds a global reference count to keep track of
+`ScopedPumpMessagesInPrivateModes` instances and gate its
+enable/disable behavior on this reference count.
+
+diff --git a/base/message_loop/message_pump_apple.mm b/base/message_loop/message_pump_apple.mm
+index 52ed68ac3150bdeef3c5032f3f5f7df3d5aaac51..1658aece3e8fbcef89944a849e311f7949a68de9 100644
+--- a/base/message_loop/message_pump_apple.mm
++++ b/base/message_loop/message_pump_apple.mm
+@@ -760,20 +760,29 @@ explicit OptionalAutoreleasePool(MessagePumpCFRunLoopBase* pump) {
+ 
+ #else
+ 
++static int g_private_mode_ref_count = 0;
++
+ ScopedPumpMessagesInPrivateModes::ScopedPumpMessagesInPrivateModes() {
+   DCHECK(g_app_pump);
+-  DCHECK_EQ(kNSApplicationModalSafeModeMask, g_app_pump->GetModeMask());
+   // Pumping events in private runloop modes is known to interact badly with
+   // app modal windows like NSAlert.
+   if (NSApp.modalWindow) {
+     return;
+   }
+-  g_app_pump->SetModeMask(kAllModesMask);
++
++  g_private_mode_ref_count += 1;
++  if (g_private_mode_ref_count == 1) {
++    g_app_pump->SetModeMask(kAllModesMask);
++  }
+ }
+ 
+ ScopedPumpMessagesInPrivateModes::~ScopedPumpMessagesInPrivateModes() {
+   DCHECK(g_app_pump);
+-  g_app_pump->SetModeMask(kNSApplicationModalSafeModeMask);
++  DCHECK(g_private_mode_ref_count > 0);
++  g_private_mode_ref_count -= 1;
++  if (g_private_mode_ref_count == 0) {
++    g_app_pump->SetModeMask(kNSApplicationModalSafeModeMask);
++  }
+ }
+ 
+ int ScopedPumpMessagesInPrivateModes::GetModeMaskForTest() {

shell/browser/api/electron_api_menu.cc

@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "shell/browser/api/electron_api_base_window.h"
+#include "shell/browser/api/electron_api_web_frame_main.h"
 #include "shell/browser/api/ui_event.h"
 #include "shell/browser/javascript_environment.h"
 #include "shell/browser/native_window.h"
@@ -16,6 +17,7 @@
 #include "shell/common/gin_converters/file_path_converter.h"
 #include "shell/common/gin_converters/gurl_converter.h"
 #include "shell/common/gin_converters/image_converter.h"
+#include "shell/common/gin_converters/optional_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/gin_helper/object_template_builder.h"
 #include "shell/common/node_includes.h"

shell/browser/api/electron_api_menu.h

@@ -22,6 +22,7 @@ class Arguments;
 namespace electron::api {
 
 class BaseWindow;
+class WebFrameMain;
 
 class Menu : public gin::Wrappable<Menu>,
              public gin_helper::EventEmitterMixin<Menu>,
@@ -81,6 +82,7 @@ class Menu : public gin::Wrappable<Menu>,
   void OnMenuWillShow(ui::SimpleMenuModel* source) override;
 
   virtual void PopupAt(BaseWindow* window,
+                       std::optional<WebFrameMain*> frame,
                        int x,
                        int y,
                        int positioning_item,

shell/browser/api/electron_api_menu_mac.h

@@ -13,6 +13,7 @@
 
 namespace electron {
 class NativeWindow;
+class WebFrameMain;
 
 namespace api {
 
@@ -23,12 +24,14 @@ class MenuMac : public Menu {
 
   // Menu
   void PopupAt(BaseWindow* window,
+               std::optional<WebFrameMain*> frame,
                int x,
                int y,
                int positioning_item,
                ui::mojom::MenuSourceType source_type,
                base::OnceClosure callback) override;
   void PopupOnUI(const base::WeakPtr<NativeWindow>& native_window,
+                 const base::WeakPtr<WebFrameMain>& frame,
                  int32_t window_id,
                  int x,
                  int y,

shell/browser/api/electron_api_menu_mac.mm

@@ -11,11 +11,15 @@
 #include "base/strings/sys_string_conversions.h"
 #include "base/task/current_thread.h"
 #include "base/task/sequenced_task_runner.h"
+#include "content/app_shim_remote_cocoa/render_widget_host_view_cocoa.h"  // nogncheck
+#include "content/browser/renderer_host/render_widget_host_view_mac.h"  // nogncheck
 #include "content/public/browser/browser_task_traits.h"
 #include "shell/browser/api/electron_api_base_window.h"
+#include "shell/browser/api/electron_api_web_frame_main.h"
 #include "shell/browser/native_window.h"
 #include "shell/common/keyboard_util.h"
 #include "shell/common/node_includes.h"
+#include "ui/base/cocoa/menu_utils.h"
 
 namespace {
 
@@ -48,6 +52,7 @@ MenuMac::MenuMac(gin::Arguments* args) : Menu(args) {}
 MenuMac::~MenuMac() = default;
 
 void MenuMac::PopupAt(BaseWindow* window,
+                      std::optional<WebFrameMain*> frame,
                       int x,
                       int y,
                       int positioning_item,
@@ -57,14 +62,19 @@ void MenuMac::PopupAt(BaseWindow* window,
   if (!native_window)
     return;
 
+  base::WeakPtr<WebFrameMain> weak_frame;
+  if (frame && frame.value()) {
+    weak_frame = frame.value()->GetWeakPtr();
+  }
+
   // Make sure the Menu object would not be garbage-collected until the callback
   // has run.
   base::OnceClosure callback_with_ref = BindSelfToClosure(std::move(callback));
 
-  auto popup =
-      base::BindOnce(&MenuMac::PopupOnUI, weak_factory_.GetWeakPtr(),
-                     native_window->GetWeakPtr(), window->weak_map_id(), x, y,
-                     positioning_item, std::move(callback_with_ref));
+  auto popup = base::BindOnce(&MenuMac::PopupOnUI, weak_factory_.GetWeakPtr(),
+                              native_window->GetWeakPtr(), weak_frame,
+                              window->weak_map_id(), x, y, positioning_item,
+                              std::move(callback_with_ref));
   base::SequencedTaskRunner::GetCurrentDefault()->PostTask(FROM_HERE,
                                                            std::move(popup));
 }
@@ -95,6 +105,7 @@ v8::Local<v8::Value> Menu::GetUserAcceleratorAt(int command_id) const {
 }
 
 void MenuMac::PopupOnUI(const base::WeakPtr<NativeWindow>& native_window,
+                        const base::WeakPtr<WebFrameMain>& frame,
                         int32_t window_id,
                         int x,
                         int y,
@@ -145,18 +156,27 @@ void MenuMac::PopupOnUI(const base::WeakPtr<NativeWindow>& native_window,
     position.x = position.x - [menu size].width;
 
   [popup_controllers_[window_id] setCloseCallback:std::move(close_callback)];
-  // Make sure events can be pumped while the menu is up.
-  base::CurrentThread::ScopedAllowApplicationTasksInNativeNestedLoop allow;
 
-  // One of the events that could be pumped is |window.close()|.
-  // User-initiated event-tracking loops protect against this by
-  // setting flags in -[CrApplication sendEvent:], but since
-  // web-content menus are initiated by IPC message the setup has to
-  // be done manually.
-  base::mac::ScopedSendingEvent sendingEventScoper;
+  if (frame && frame->render_frame_host()) {
+    auto* rfh = frame->render_frame_host()->GetOutermostMainFrameOrEmbedder();
+    if (rfh && rfh->IsRenderFrameLive()) {
+      auto* rwhvm =
+          static_cast<content::RenderWidgetHostViewMac*>(rfh->GetView());
+      RenderWidgetHostViewCocoa* cocoa_view = rwhvm->GetInProcessNSView();
+      view = cocoa_view;
+    }
+  }
 
-  // Don't emit unresponsive event when showing menu.
-  [menu popUpMenuPositioningItem:item atLocation:position inView:view];
+  NSEvent* dummy_event = [NSEvent mouseEventWithType:NSEventTypeRightMouseDown
+                                            location:position
+                                       modifierFlags:0
+                                           timestamp:0
+                                        windowNumber:nswindow.windowNumber
+                                             context:nil
+                                         eventNumber:0
+                                          clickCount:1
+                                            pressure:0];
+  ui::ShowContextMenu(menu, dummy_event, view, true);
 }
 
 void MenuMac::ClosePopupAt(int32_t window_id) {

shell/browser/api/electron_api_menu_views.cc

@@ -8,6 +8,7 @@
 #include <utility>
 
 #include "shell/browser/api/electron_api_base_window.h"
+#include "shell/browser/api/electron_api_web_frame_main.h"
 #include "shell/browser/native_window_views.h"
 #include "ui/display/screen.h"
 
@@ -20,6 +21,7 @@ MenuViews::MenuViews(gin::Arguments* args) : Menu(args) {}
 MenuViews::~MenuViews() = default;
 
 void MenuViews::PopupAt(BaseWindow* window,
+                        std::optional<WebFrameMain*> frame,
                         int x,
                         int y,
                         int positioning_item,

shell/browser/api/electron_api_menu_views.h

@@ -22,6 +22,7 @@ class MenuViews : public Menu {
  protected:
   // Menu
   void PopupAt(BaseWindow* window,
+               std::optional<WebFrameMain*> frame,
                int x,
                int y,
                int positioning_item,

shell/browser/api/electron_api_web_frame_main.h

@@ -73,6 +73,10 @@ class WebFrameMain final : public gin::Wrappable<WebFrameMain>,
 
   content::RenderFrameHost* render_frame_host() const;
 
+  base::WeakPtr<WebFrameMain> GetWeakPtr() {
+    return weak_factory_.GetWeakPtr();
+  }
+
   // disable copy
   WebFrameMain(const WebFrameMain&) = delete;
   WebFrameMain& operator=(const WebFrameMain&) = delete;

typings/internal-electron.d.ts

@@ -166,7 +166,7 @@ declare namespace Electron {
     commandsMap: Record<string, MenuItem>;
     groupsMap: Record<string, MenuItem[]>;
     getItemCount(): number;
-    popupAt(window: BaseWindow, x: number, y: number, positioning: number, sourceType: Required<Electron.PopupOptions>['sourceType'], callback: () => void): void;
+    popupAt(window: BaseWindow, frame: WebFrameMain | undefined, x: number, y: number, positioning: number, sourceType: Required<Electron.PopupOptions>['sourceType'], callback: () => void): void;
     closePopupAt(id: number): void;
     setSublabel(index: number, label: string): void;
     setToolTip(index: number, tooltip: string): void;