fix: menu close event missing after opening a submenu (#49964)

* fix: menu close event missing after opening a submenu

Co-authored-by: Jarek Radosz <jarek@cvx.dev>

* add a unit-like test

Co-authored-by: Jarek Radosz <jarek@cvx.dev>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Jarek Radosz <jarek@cvx.dev>

shell/browser/api/electron_api_menu.cc

@@ -311,6 +311,8 @@ void Menu::FillObjectTemplate(v8::Isolate* isolate,
       .SetMethod("_getAcceleratorTextAt", &Menu::GetAcceleratorTextAtForTesting)
 #if BUILDFLAG(IS_MAC)
       .SetMethod("_getUserAcceleratorAt", &Menu::GetUserAcceleratorAt)
+      .SetMethod("_simulateSubmenuCloseSequenceForTesting",
+                 &Menu::SimulateSubmenuCloseSequenceForTesting)
 #endif
       .Build();
 }

shell/browser/api/electron_api_menu.h

@@ -84,6 +84,7 @@ class Menu : public gin::Wrappable<Menu>,
       int command_id,
       ElectronMenuModel::SharingItem* item) const override;
   v8::Local<v8::Value> GetUserAcceleratorAt(int command_id) const;
+  virtual void SimulateSubmenuCloseSequenceForTesting();
 #endif
   void ExecuteCommand(int command_id, int event_flags) override;
   void OnMenuWillShow(ui::SimpleMenuModel* source) override;

shell/browser/api/electron_api_menu_mac.mm

@@ -7,6 +7,7 @@
 #include <string>
 #include <utility>
 
+#include "base/functional/bind.h"
 #include "base/mac/scoped_sending_event.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/task/current_thread.h"
@@ -246,6 +247,20 @@ std::u16string MenuMac::GetAcceleratorTextAtForTesting(int index) const {
   return text;
 }
 
+void Menu::SimulateSubmenuCloseSequenceForTesting() {
+  ElectronMenuController* controller =
+      [[ElectronMenuController alloc] initWithModel:model()
+                              useDefaultAccelerator:NO];
+  NSMenu* menu = [controller menu];
+  NSMenu* submenu = menu.itemArray[0].submenu;
+
+  [controller setPopupCloseCallback:base::BindOnce([] {})];
+  [controller menuWillOpen:menu];
+  [controller menuWillOpen:submenu];
+  [controller menuDidClose:submenu];
+  [controller menuDidClose:menu];
+}
+
 void MenuMac::ClosePopupOnUI(int32_t window_id) {
   auto controller = popup_controllers_.find(window_id);
   if (controller != popup_controllers_.end()) {

shell/browser/ui/cocoa/electron_menu_controller.mm

@@ -583,18 +583,25 @@ NSArray* ConvertSharingItemToNS(const SharingItem& item) {
   if (!isMenuOpen_)
     return;
 
-  isMenuOpen_ = NO;
-
   // 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.
   bool has_close_cb = !popupCloseCallback.is_null();
+  bool should_emit_close = true;
   if (menu != menu_) {
-    if (has_close_cb || menu.supermenu != menu_)
-      return;
+    should_emit_close = !has_close_cb && menu.supermenu == menu_;
   }
 
+  [self refreshMenuTree:menu];
+
+  // Submenu's close event arrives before the top-level menu closes.
+  // Don't change isMenuOpen_ until the top-level one receives the close event.
+  if (!should_emit_close)
+    return;
+
+  isMenuOpen_ = NO;
+
   if (model_)
     model_->MenuWillClose();
   // Post async task so that itemSelected runs before the close callback

spec/api-menu-spec.ts

@@ -946,6 +946,27 @@ describe('Menu module', function () {
     }
   });
 
+  ifit(process.platform === 'darwin')(
+    'emits menu close event even if submenu closes first',
+    async () => {
+      const menu = Menu.buildFromTemplate([{
+        label: 'parent',
+        submenu: [{
+          label: 'child'
+        }]
+      }]);
+
+      const menuWillClose = once(menu, 'menu-will-close');
+      (menu as any)._simulateSubmenuCloseSequenceForTesting();
+
+      await Promise.race([
+        menuWillClose,
+        setTimeout(1000).then(() => {
+          throw new Error('menu-will-close was not emitted');
+        })
+      ]);
+    });
+
   describe('Menu.setApplicationMenu', () => {
     it('sets a menu', () => {
       const menu = Menu.buildFromTemplate([