diff --git a/shell/browser/api/electron_api_menu_mac.mm b/shell/browser/api/electron_api_menu_mac.mm index a70725d983..2be1bbd7fd 100644 --- a/shell/browser/api/electron_api_menu_mac.mm +++ b/shell/browser/api/electron_api_menu_mac.mm @@ -155,7 +155,8 @@ void MenuMac::PopupOnUI(const base::WeakPtr& native_window, if (rightmostMenuPoint > screenRight) position.x = position.x - [menu size].width; - [popup_controllers_[window_id] setCloseCallback:std::move(close_callback)]; + [popup_controllers_[window_id] + setPopupCloseCallback:std::move(close_callback)]; if (frame && frame->render_frame_host()) { auto* rfh = frame->render_frame_host()->GetOutermostMainFrameOrEmbedder(); diff --git a/shell/browser/ui/cocoa/electron_menu_controller.h b/shell/browser/ui/cocoa/electron_menu_controller.h index c6153ce0bc..489f42ad77 100644 --- a/shell/browser/ui/cocoa/electron_menu_controller.h +++ b/shell/browser/ui/cocoa/electron_menu_controller.h @@ -28,7 +28,7 @@ class ElectronMenuModel; NSMenu* __strong menu_; BOOL isMenuOpen_; BOOL useDefaultAccelerator_; - base::OnceClosure closeCallback; + base::OnceClosure popupCloseCallback; } // Builds a NSMenu from the pre-built model (must not be nil). Changes made @@ -36,7 +36,7 @@ class ElectronMenuModel; - (id)initWithModel:(electron::ElectronMenuModel*)model useDefaultAccelerator:(BOOL)use; -- (void)setCloseCallback:(base::OnceClosure)callback; +- (void)setPopupCloseCallback:(base::OnceClosure)callback; // Populate current NSMenu with |model|. - (void)populateWithModel:(electron::ElectronMenuModel*)model; diff --git a/shell/browser/ui/cocoa/electron_menu_controller.mm b/shell/browser/ui/cocoa/electron_menu_controller.mm index 77e22fa49a..844ca12c31 100644 --- a/shell/browser/ui/cocoa/electron_menu_controller.mm +++ b/shell/browser/ui/cocoa/electron_menu_controller.mm @@ -186,8 +186,8 @@ NSArray* ConvertSharingItemToNS(const SharingItem& item) { model_ = nullptr; } -- (void)setCloseCallback:(base::OnceClosure)callback { - closeCallback = std::move(callback); +- (void)setPopupCloseCallback:(base::OnceClosure)callback { + popupCloseCallback = std::move(callback); } - (void)populateWithModel:(electron::ElectronMenuModel*)model { @@ -221,9 +221,9 @@ NSArray* ConvertSharingItemToNS(const SharingItem& item) { isMenuOpen_ = NO; if (model_) model_->MenuWillClose(); - if (!closeCallback.is_null()) { - content::GetUIThreadTaskRunner({})->PostTask(FROM_HERE, - std::move(closeCallback)); + if (!popupCloseCallback.is_null()) { + content::GetUIThreadTaskRunner({})->PostTask( + FROM_HERE, std::move(popupCloseCallback)); } } } @@ -567,18 +567,25 @@ NSArray* ConvertSharingItemToNS(const SharingItem& item) { if (!isMenuOpen_) return; - // We should only respond to the top-level menu's close event. - if (menu != menu_) - return; + bool has_close_cb = !popupCloseCallback.is_null(); + + // There are two scenarios where we should emit menu-did-close: + // 1. It's a popup and the top level menu is closed. + // 2. It's an application menu, and the current menu's supermenu + // is the top-level menu. + if (menu != menu_) { + if (has_close_cb || menu.supermenu != menu_) + return; + } isMenuOpen_ = NO; if (model_) model_->MenuWillClose(); // Post async task so that itemSelected runs before the close callback - // deletes the controller from the map which deallocates it - if (!closeCallback.is_null()) { + // deletes the controller from the map which deallocates it. + if (has_close_cb) { content::GetUIThreadTaskRunner({})->PostTask(FROM_HERE, - std::move(closeCallback)); + std::move(popupCloseCallback)); } }