diff --git a/docs/api/menu.md b/docs/api/menu.md index b9a8a7b09e07..949720dc0e8c 100644 --- a/docs/api/menu.md +++ b/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. diff --git a/lib/browser/api/menu.ts b/lib/browser/api/menu.ts index c0244cc3c1bf..8e0a63f76465 100644 --- a/lib/browser/api/menu.ts +++ b/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 }; }; diff --git a/patches/chromium/.patches b/patches/chromium/.patches index c6e8b731e2be..f48fa2a884c0 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -136,6 +136,7 @@ refactor_unfilter_unresponsive_events.patch build_disable_thin_lto_mac.patch feat_corner_smoothing_css_rule_and_blink_painting.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 diff --git a/patches/chromium/fix_multiple_scopedpumpmessagesinprivatemodes_instances.patch b/patches/chromium/fix_multiple_scopedpumpmessagesinprivatemodes_instances.patch new file mode 100644 index 000000000000..6605f24d7b15 --- /dev/null +++ b/patches/chromium/fix_multiple_scopedpumpmessagesinprivatemodes_instances.patch @@ -0,0 +1,51 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Calvin Watford +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() { diff --git a/shell/browser/api/electron_api_menu.cc b/shell/browser/api/electron_api_menu.cc index daef73495bf7..595a3513c499 100644 --- a/shell/browser/api/electron_api_menu.cc +++ b/shell/browser/api/electron_api_menu.cc @@ -7,6 +7,7 @@ #include #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" diff --git a/shell/browser/api/electron_api_menu.h b/shell/browser/api/electron_api_menu.h index c861bd97a3e8..6b5ee0635da3 100644 --- a/shell/browser/api/electron_api_menu.h +++ b/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, public gin_helper::EventEmitterMixin, @@ -81,6 +82,7 @@ class Menu : public gin::Wrappable, void OnMenuWillShow(ui::SimpleMenuModel* source) override; virtual void PopupAt(BaseWindow* window, + std::optional frame, int x, int y, int positioning_item, diff --git a/shell/browser/api/electron_api_menu_mac.h b/shell/browser/api/electron_api_menu_mac.h index 679bff878456..f4d92f084069 100644 --- a/shell/browser/api/electron_api_menu_mac.h +++ b/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 frame, int x, int y, int positioning_item, ui::mojom::MenuSourceType source_type, base::OnceClosure callback) override; void PopupOnUI(const base::WeakPtr& native_window, + const base::WeakPtr& frame, int32_t window_id, int x, int y, diff --git a/shell/browser/api/electron_api_menu_mac.mm b/shell/browser/api/electron_api_menu_mac.mm index 7ea23ffd9126..f5dffd4492e2 100644 --- a/shell/browser/api/electron_api_menu_mac.mm +++ b/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 frame, int x, int y, int positioning_item, @@ -57,14 +62,19 @@ void MenuMac::PopupAt(BaseWindow* window, if (!native_window) return; + base::WeakPtr 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 Menu::GetUserAcceleratorAt(int command_id) const { } void MenuMac::PopupOnUI(const base::WeakPtr& native_window, + const base::WeakPtr& frame, int32_t window_id, int x, int y, @@ -145,18 +156,27 @@ void MenuMac::PopupOnUI(const base::WeakPtr& 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(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) { diff --git a/shell/browser/api/electron_api_menu_views.cc b/shell/browser/api/electron_api_menu_views.cc index dd5a543c965c..67aa94e77032 100644 --- a/shell/browser/api/electron_api_menu_views.cc +++ b/shell/browser/api/electron_api_menu_views.cc @@ -8,6 +8,7 @@ #include #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 frame, int x, int y, int positioning_item, diff --git a/shell/browser/api/electron_api_menu_views.h b/shell/browser/api/electron_api_menu_views.h index 7759f07a898d..490d4229050f 100644 --- a/shell/browser/api/electron_api_menu_views.h +++ b/shell/browser/api/electron_api_menu_views.h @@ -22,6 +22,7 @@ class MenuViews : public Menu { protected: // Menu void PopupAt(BaseWindow* window, + std::optional frame, int x, int y, int positioning_item, diff --git a/shell/browser/api/electron_api_web_frame_main.h b/shell/browser/api/electron_api_web_frame_main.h index ca5cc07e7d1f..f62d75e0a8f6 100644 --- a/shell/browser/api/electron_api_web_frame_main.h +++ b/shell/browser/api/electron_api_web_frame_main.h @@ -73,6 +73,10 @@ class WebFrameMain final : public gin::Wrappable, content::RenderFrameHost* render_frame_host() const; + base::WeakPtr GetWeakPtr() { + return weak_factory_.GetWeakPtr(); + } + // disable copy WebFrameMain(const WebFrameMain&) = delete; WebFrameMain& operator=(const WebFrameMain&) = delete; diff --git a/typings/internal-electron.d.ts b/typings/internal-electron.d.ts index f7338daa599e..5d281261bb10 100644 --- a/typings/internal-electron.d.ts +++ b/typings/internal-electron.d.ts @@ -166,7 +166,7 @@ declare namespace Electron { commandsMap: Record; groupsMap: Record; getItemCount(): number; - popupAt(window: BaseWindow, x: number, y: number, positioning: number, sourceType: Required['sourceType'], callback: () => void): void; + popupAt(window: BaseWindow, frame: WebFrameMain | undefined, x: number, y: number, positioning: number, sourceType: Required['sourceType'], callback: () => void): void; closePopupAt(id: number): void; setSublabel(index: number, label: string): void; setToolTip(index: number, tooltip: string): void;