Merge pull request #11264 from brenca/flash-menu-fix

Fix flash context menu

atom/browser/api/atom_api_menu.cc

@@ -40,6 +40,7 @@ void Menu::AfterInit(v8::Isolate* isolate) {
   delegate.Get("getAcceleratorForCommandId", &get_accelerator_);
   delegate.Get("executeCommand", &execute_command_);
   delegate.Get("menuWillShow", &menu_will_show_);
+  delegate.Get("menuClosed", &menu_closed_);
 }
 
 bool Menu::IsCommandIdChecked(int command_id) const {
@@ -75,6 +76,10 @@ void Menu::MenuWillShow(ui::SimpleMenuModel* source) {
   menu_will_show_.Run();
 }
 
+void Menu::MenuClosed(ui::SimpleMenuModel* source) {
+  menu_closed_.Run();
+}
+
 void Menu::InsertItemAt(
     int index, int command_id, const base::string16& label) {
   model_->InsertItemAt(index, command_id, label);

atom/browser/api/atom_api_menu.h

@@ -52,6 +52,7 @@ class Menu : public mate::TrackableObject<Menu>,
       ui::Accelerator* accelerator) const override;
   void ExecuteCommand(int command_id, int event_flags) override;
   void MenuWillShow(ui::SimpleMenuModel* source) override;
+  void MenuClosed(ui::SimpleMenuModel* source) override;
 
   virtual void PopupAt(Window* window, int x, int y, int positioning_item) = 0;
   virtual void ClosePopupAt(int32_t window_id) = 0;
@@ -93,6 +94,7 @@ class Menu : public mate::TrackableObject<Menu>,
   base::Callback<v8::Local<v8::Value>(int, bool)> get_accelerator_;
   base::Callback<void(v8::Local<v8::Value>, int)> execute_command_;
   base::Callback<void()> menu_will_show_;
+  base::Callback<void()> menu_closed_;
 
   DISALLOW_COPY_AND_ASSIGN(Menu);
 };

atom/browser/api/atom_api_menu_views.cc

@@ -60,7 +60,10 @@ void MenuViews::PopupAt(Window* window, int x, int y, int positioning_item) {
 }
 
 void MenuViews::ClosePopupAt(int32_t window_id) {
-  menu_runners_.erase(window_id);
+  if (menu_runners_[window_id]) {
+    menu_runners_[window_id]->Cancel();
+    menu_runners_.erase(window_id);
+  }
 }
 
 // static

atom/browser/api/atom_api_web_contents.cc

@@ -653,8 +653,10 @@ void WebContents::RendererResponsive(content::WebContents* source) {
 
 bool WebContents::HandleContextMenu(const content::ContextMenuParams& params) {
   if (params.custom_context.is_pepper_menu) {
-    Emit("pepper-context-menu", std::make_pair(params, web_contents()));
-    web_contents()->NotifyContextMenuClosed(params.custom_context);
+    Emit("pepper-context-menu",
+         std::make_pair(params, web_contents()),
+         base::Bind(&content::WebContents::NotifyContextMenuClosed,
+                    base::Unretained(web_contents()), params.custom_context));
   } else {
     Emit("context-menu", std::make_pair(params, web_contents()));
   }

docs/api/menu.md

@@ -109,6 +109,15 @@ A `MenuItem[]` array containing the menu's items.
 Each `Menu` consists of multiple [`MenuItem`](menu-item.md)s and each `MenuItem`
 can have a submenu.
 
+### Instance Events
+
+Objects created with `new Menu` or returned by `Menu.buildFromTemplate` emit
+the following events:
+
+#### Event: 'closed'
+
+Emitted when the menu is closed.
+
 ## Examples
 
 The `Menu` class is only available in the main process, but you can also use it

lib/browser/api/menu.js

@@ -39,6 +39,9 @@ Menu.prototype._init = function () {
         const found = this.groupsMap[id].find(item => item.checked) || null
         if (!found) v8Util.setHiddenValue(this.groupsMap[id][0], 'checked', true)
       }
+    },
+    menuClosed: () => {
+      this.emit('closed')
     }
   }
 }

lib/browser/api/web-contents.js

@@ -277,9 +277,10 @@ WebContents.prototype._init = function () {
   })
 
   // Handle context menu action request from pepper plugin.
-  this.on('pepper-context-menu', function (event, params) {
-    // Access Menu via electron.Menu to prevent circular require
+  this.on('pepper-context-menu', function (event, params, callback) {
+    // Access Menu via electron.Menu to prevent circular require.
     const menu = electron.Menu.buildFromTemplate(params.menu)
+    menu.once('closed', callback)
     menu.popup(event.sender.getOwnerBrowserWindow(), params.x, params.y)
   })
 

spec/api-menu-spec.js

@@ -325,6 +325,37 @@ describe('Menu module', () => {
     })
   })
 
+  describe('Menu.closePopup()', () => {
+    let w = null
+    let menu
+
+    beforeEach((done) => {
+      w = new BrowserWindow({show: false, width: 200, height: 200})
+      menu = Menu.buildFromTemplate([
+        {
+          label: '1'
+        }
+      ])
+
+      w.loadURL('data:text/html,<html>teszt</html>')
+      w.webContents.on('dom-ready', () => {
+        done()
+      })
+    })
+
+    afterEach(() => {
+      return closeWindow(w).then(() => { w = null })
+    })
+
+    it('emits closed event', (done) => {
+      menu.popup(w, {x: 100, y: 100})
+      menu.on('closed', () => {
+        done()
+      })
+      menu.closePopup(w)
+    })
+  })
+
   describe('Menu.setApplicationMenu', () => {
     it('sets a menu', () => {
       const menu = Menu.buildFromTemplate([