Manual application of electron patch electron/electron#8702

(All credit goes to @kevinsawicki)

atom/browser/api/atom_api_menu.cc

@@ -190,7 +190,8 @@ void Menu::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("isItemCheckedAt", &Menu::IsItemCheckedAt)
       .SetMethod("isEnabledAt", &Menu::IsEnabledAt)
       .SetMethod("isVisibleAt", &Menu::IsVisibleAt)
-      .SetMethod("popupAt", &Menu::PopupAt);
+      .SetMethod("popupAt", &Menu::PopupAt)
+      .SetMethod("closePopupAt", &Menu::ClosePopupAt);
 }
 
 }  // namespace api

atom/browser/api/atom_api_menu.h

@@ -62,9 +62,9 @@ class Menu : public mate::TrackableObject<Menu>,
   void ExecuteCommand(int command_id, int event_flags) override;
   void MenuWillShow(ui::SimpleMenuModel* source) override;
 
-  virtual void PopupAt(Window* window,
-                       int x = -1, int y = -1,
-                       int positioning_item = 0) = 0;
+  virtual void PopupAt(
+    Window* window, int x, int y, int positioning_item, bool async) = 0;
+  virtual void ClosePopupAt(int32_t window_id) = 0;
 
   std::unique_ptr<AtomMenuModel> model_;
   Menu* parent_;

atom/browser/api/atom_api_menu_mac.h

@@ -7,10 +7,13 @@
 
 #include "atom/browser/api/atom_api_menu.h"
 
+#include <map>
 #include <string>
 
 #import "atom/browser/ui/cocoa/atom_menu_controller.h"
 
+using base::scoped_nsobject;
+
 namespace atom {
 
 namespace api {
@@ -19,15 +22,25 @@ class MenuMac : public Menu {
  protected:
   MenuMac(v8::Isolate* isolate, v8::Local<v8::Object> wrapper);
 
-  void PopupAt(Window* window, int x, int y, int positioning_item) override;
-
-  base::scoped_nsobject<AtomMenuController> menu_controller_;
+  void PopupAt(
+      Window* window, int x, int y, int positioning_item, bool async) override;
+  void PopupOnUI(const base::WeakPtr<NativeWindow>& native_window,
+                 int32_t window_id, int x, int y, int positioning_item,
+                 bool async);
+  void ClosePopupAt(int32_t window_id) override;
 
  private:
   friend class Menu;
 
   static void SendActionToFirstResponder(const std::string& action);
 
+  scoped_nsobject<AtomMenuController> menu_controller_;
+
+  // window ID -> open context menu
+  std::map<int32_t, scoped_nsobject<AtomMenuController>> popup_controllers_;
+
+  base::WeakPtrFactory<MenuMac> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(MenuMac);
 };
 

atom/browser/api/atom_api_menu_mac.mm

@@ -6,35 +6,59 @@
 
 #include "atom/browser/native_window.h"
 #include "atom/browser/unresponsive_suppressor.h"
+#include "base/mac/scoped_sending_event.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/sys_string_conversions.h"
 #include "brightray/browser/inspectable_web_contents.h"
 #include "brightray/browser/inspectable_web_contents_view.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/web_contents.h"
 
 #include "atom/common/node_includes.h"
 
+using content::BrowserThread;
+
 namespace atom {
 
 namespace api {
 
 MenuMac::MenuMac(v8::Isolate* isolate, v8::Local<v8::Object> wrapper)
-    : Menu(isolate, wrapper) {
+    : Menu(isolate, wrapper),
+      weak_factory_(this) {
 }
 
-void MenuMac::PopupAt(Window* window, int x, int y, int positioning_item) {
+void MenuMac::PopupAt(
+    Window* window, int x, int y, int positioning_item, bool async) {
   NativeWindow* native_window = window->window();
   if (!native_window)
     return;
+
+  auto popup = base::Bind(&MenuMac::PopupOnUI, weak_factory_.GetWeakPtr(),
+                          native_window->GetWeakPtr(), window->ID(), x, y,
+                          positioning_item, async);
+  if (async)
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, popup);
+  else
+    popup.Run();
+}
+
+void MenuMac::PopupOnUI(const base::WeakPtr<NativeWindow>& native_window,
+                        int32_t window_id, int x, int y, int positioning_item,
+                        bool async) {
+  if (!native_window)
+    return;
+
   brightray::InspectableWebContents* web_contents =
       native_window->inspectable_web_contents();
   if (!web_contents || !web_contents->GetWebContents())
     return;
 
-  base::scoped_nsobject<AtomMenuController> menu_controller(
-      [[AtomMenuController alloc] initWithModel:model_.get()
+  auto close_callback = base::Bind(&MenuMac::ClosePopupAt,
+                                   weak_factory_.GetWeakPtr(), window_id);
+  popup_controllers_[window_id] = base::scoped_nsobject<AtomMenuController>(
+      [[AtomMenuController alloc] initWithModel:model()
                           useDefaultAccelerator:NO]);
-  NSMenu* menu = [menu_controller menu];
+  NSMenu* menu = [popup_controllers_[window_id] menu];
   NSView* view = web_contents->GetWebContents()->GetNativeView();
 
   // Which menu item to show.
@@ -69,11 +93,32 @@
   if (rightmostMenuPoint > screenRight)
     position.x = position.x - [menu size].width;
 
-  // Don't emit unresponsive event when showing menu.
-  atom::UnresponsiveSuppressor suppressor;
+  if (async) {
+    [popup_controllers_[window_id] setCloseCallback:close_callback];
+    // Make sure events can be pumped while the menu is up.
+    base::MessageLoop::ScopedNestableTaskAllower allow(
+        base::MessageLoop::current());
+
+    // 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;
+
+    // Don't emit unresponsive event when showing menu.
+    atom::UnresponsiveSuppressor suppressor;
+    [menu popUpMenuPositioningItem:item atLocation:position inView:view];
+  } else {
+    // Don't emit unresponsive event when showing menu.
+    atom::UnresponsiveSuppressor suppressor;
+    [menu popUpMenuPositioningItem:item atLocation:position inView:view];
+    close_callback.Run();
+  }
+}
 
-  // Show the menu.
-  [menu popUpMenuPositioningItem:item atLocation:position inView:view];
+void MenuMac::ClosePopupAt(int32_t window_id) {
+  popup_controllers_.erase(window_id);
 }
 
 // static

atom/browser/api/atom_api_menu_views.cc

@@ -8,17 +8,20 @@
 #include "atom/browser/unresponsive_suppressor.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "ui/display/screen.h"
-#include "ui/views/controls/menu/menu_runner.h"
+
+using views::MenuRunner;
 
 namespace atom {
 
 namespace api {
 
 MenuViews::MenuViews(v8::Isolate* isolate, v8::Local<v8::Object> wrapper)
-    : Menu(isolate, wrapper) {
+    : Menu(isolate, wrapper),
+      weak_factory_(this) {
 }
 
-void MenuViews::PopupAt(Window* window, int x, int y, int positioning_item) {
+void MenuViews::PopupAt(
+    Window* window, int x, int y, int positioning_item, bool async) {
   NativeWindow* native_window = static_cast<NativeWindow*>(window->window());
   if (!native_window)
     return;
@@ -38,21 +41,31 @@ void MenuViews::PopupAt(Window* window, int x, int y, int positioning_item) {
     location = gfx::Point(origin.x() + x, origin.y() + y);
   }
 
+  int flags = MenuRunner::CONTEXT_MENU | MenuRunner::HAS_MNEMONICS;
+  if (async)
+    flags |= MenuRunner::ASYNC;
+
   // Don't emit unresponsive event when showing menu.
   atom::UnresponsiveSuppressor suppressor;
 
   // Show the menu.
-  views::MenuRunner menu_runner(
-      model(),
-      views::MenuRunner::CONTEXT_MENU | views::MenuRunner::HAS_MNEMONICS);
-  ignore_result(menu_runner.RunMenuAt(
+  int32_t window_id = window->ID();
+  auto close_callback = base::Bind(
+      &MenuViews::ClosePopupAt, weak_factory_.GetWeakPtr(), window_id);
+  menu_runners_[window_id] = std::unique_ptr<MenuRunner>(new MenuRunner(
+      model(), flags, close_callback));
+  ignore_result(menu_runners_[window_id]->RunMenuAt(
       static_cast<NativeWindowViews*>(window->window())->widget(),
       NULL,
       gfx::Rect(location, gfx::Size()),
       views::MENU_ANCHOR_TOPLEFT,
       ui::MENU_SOURCE_MOUSE));
 }
 
+void MenuViews::ClosePopupAt(int32_t window_id) {
+  menu_runners_.erase(window_id);
+}
+
 // static
 mate::WrappableBase* Menu::New(mate::Arguments* args) {
   return new MenuViews(args->isolate(), args->GetThis());

atom/browser/api/atom_api_menu_views.h

@@ -5,8 +5,12 @@
 #ifndef ATOM_BROWSER_API_ATOM_API_MENU_VIEWS_H_
 #define ATOM_BROWSER_API_ATOM_API_MENU_VIEWS_H_
 
+#include <map>
+
 #include "atom/browser/api/atom_api_menu.h"
+#include "base/memory/weak_ptr.h"
 #include "ui/display/screen.h"
+#include "ui/views/controls/menu/menu_runner.h"
 
 namespace atom {
 
@@ -17,9 +21,16 @@ class MenuViews : public Menu {
   MenuViews(v8::Isolate* isolate, v8::Local<v8::Object> wrapper);
 
  protected:
-  void PopupAt(Window* window, int x, int y, int positioning_item) override;
+  void PopupAt(
+      Window* window, int x, int y, int positioning_item, bool async) override;
+  void ClosePopupAt(int32_t window_id) override;
 
  private:
+  // window ID -> open context menu
+  std::map<int32_t, std::unique_ptr<views::MenuRunner>> menu_runners_;
+
+  base::WeakPtrFactory<MenuViews> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(MenuViews);
 };
 

atom/browser/api/atom_api_window.h

@@ -55,6 +55,8 @@ class Window : public mate::TrackableObject<Window>,
 
   NativeWindow* window() const { return window_.get(); }
 
+  int32_t ID() const;
+
  protected:
   Window(v8::Isolate* isolate, v8::Local<v8::Object> wrapper,
          const mate::Dictionary& options);
@@ -198,7 +200,6 @@ class Window : public mate::TrackableObject<Window>,
   void SetVisibleOnAllWorkspaces(bool visible);
   bool IsVisibleOnAllWorkspaces();
 
-  int32_t ID() const;
   v8::Local<v8::Value> WebContents(v8::Isolate* isolate);
 
   // Remove this window from parent window's |child_windows_|.

atom/browser/ui/cocoa/atom_menu_controller.h

@@ -8,6 +8,7 @@
 
 #import <Cocoa/Cocoa.h>
 
+#include "base/callback.h"
 #include "base/mac/scoped_nsobject.h"
 #include "base/strings/string16.h"
 
@@ -27,6 +28,7 @@ class AtomMenuModel;
   base::scoped_nsobject<NSMenu> menu_;
   BOOL isMenuOpen_;
   BOOL useDefaultAccelerator_;
+  base::Callback<void()> closeCallback;
 }
 
 @property(nonatomic, assign) atom::AtomMenuModel* model;
@@ -35,6 +37,8 @@ class AtomMenuModel;
 // to the contents of the model after calling this will not be noticed.
 - (id)initWithModel:(atom::AtomMenuModel*)model useDefaultAccelerator:(BOOL)use;
 
+- (void)setCloseCallback:(const base::Callback<void()>&)callback;
+
 // Populate current NSMenu with |model|.
 - (void)populateWithModel:(atom::AtomMenuModel*)model;
 

atom/browser/ui/cocoa/atom_menu_controller.mm

@@ -71,6 +71,10 @@ - (void)dealloc {
   [super dealloc];
 }
 
+- (void)setCloseCallback:(const base::Callback<void()>&)callback {
+  closeCallback = callback;
+}
+
 - (void)populateWithModel:(atom::AtomMenuModel*)model {
   if (!menu_)
     return;
@@ -265,8 +269,10 @@ - (void)menuWillOpen:(NSMenu*)menu {
 
 - (void)menuDidClose:(NSMenu*)menu {
   if (isMenuOpen_) {
-    model_->MenuWillClose();
     isMenuOpen_ = NO;
+    model_->MenuWillClose();
+    if (!closeCallback.is_null())
+      closeCallback.Run();
   }
 }
 

docs/api/menu.md

@@ -264,17 +264,28 @@ will become properties of the constructed menu items.
 
 The `menu` object has the following instance methods:
 
-#### `menu.popup([browserWindow, x, y, positioningItem])`
-
-* `browserWindow` BrowserWindow (optional) - Default is `BrowserWindow.getFocusedWindow()`.
-* `x` Number (optional) - Default is the current mouse cursor position.
-* `y` Number (**required** if `x` is used) - Default is the current mouse cursor position.
-* `positioningItem` Number (optional) _macOS_ - The index of the menu item to
-  be positioned under the mouse cursor at the specified coordinates. Default is
-  -1.
+#### `menu.popup([browserWindow, options])`
+
+* `browserWindow` BrowserWindow (optional) - Default is the focused window.
+* `options` Object (optional)
+  * `x` Number (optional) - Default is the current mouse cursor position.
+  * `y` Number (**required** if `x` is used) - Default is the current mouse
+    cursor position.
+  * `async` Boolean (optional) - Set to `true` to have this method return
+    immediately called, `false` to return after the menu has been selected
+    or closed. Defaults to `false`.
+  * `positioningItem` Number (optional) _macOS_ - The index of the menu item to
+    be positioned under the mouse cursor at the specified coordinates. Default
+    is -1.
 
 Pops up this menu as a context menu in the `browserWindow`.
 
+#### `menu.closePopup([browserWindow])`
+
+* `browserWindow` BrowserWindow (optional) - Default is the focused window.
+
+Closes the context menu in the `browserWindow`.
+
 #### `menu.append(menuItem)`
 
 * `menuItem` MenuItem

docs/tutorial/planned-breaking-changes.md

@@ -40,6 +40,15 @@ clipboard.writeHtml()
 clipboard.writeHTML()
 ```
 
+## `menu`
+
+```js
+// Deprecated
+menu.popup(browserWindow, 100, 200, 2)
+// Replace with
+menu.popup(browserWindow, {x: 100, y: 200, positioningItem: 2})
+```
+
 ## `nativeImage`
 
 ```js