Remove "async" option from menu.popup()

All menus are async now. See "Cleanup MenuRunner API"
https://codereview.chromium.org/2790773002

atom/browser/api/atom_api_menu.h

@@ -53,8 +53,7 @@ class Menu : public mate::TrackableObject<Menu>,
   void ExecuteCommand(int command_id, int event_flags) override;
   void MenuWillShow(ui::SimpleMenuModel* source) override;
 
-  virtual void PopupAt(
+  virtual void PopupAt(Window* window, int x, int y, int positioning_item) = 0;
-      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_;

atom/browser/api/atom_api_menu_mac.h

@@ -22,11 +22,12 @@ class MenuMac : public Menu {
  protected:
   MenuMac(v8::Isolate* isolate, v8::Local<v8::Object> wrapper);
 
-  void PopupAt(
+  void PopupAt(Window* window, int x, int y, int positioning_item) override;
-      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,
+                 int32_t window_id,
-                 bool async);
+                 int x,
+                 int y,
+                 int positioning_item);
   void ClosePopupAt(int32_t window_id) override;
 
  private:

atom/browser/api/atom_api_menu_mac.mm

@@ -27,24 +27,22 @@ MenuMac::MenuMac(v8::Isolate* isolate, v8::Local<v8::Object> wrapper)
       weak_factory_(this) {
 }
 
-void MenuMac::PopupAt(
+void MenuMac::PopupAt(Window* window, int x, int y, int positioning_item) {
-    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);
+                          positioning_item);
-  if (async)
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, popup);
-    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,
+                        int32_t window_id,
-                        bool async) {
+                        int x,
+                        int y,
+                        int positioning_item) {
   if (!native_window)
     return;
   brightray::InspectableWebContents* web_contents =
@@ -92,29 +90,21 @@ void MenuMac::PopupOnUI(const base::WeakPtr<NativeWindow>& native_window,
   if (rightmostMenuPoint > screenRight)
     position.x = position.x - [menu size].width;
 
+  [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());
 
-  if (async) {
+  // One of the events that could be pumped is |window.close()|.
-    [popup_controllers_[window_id] setCloseCallback:close_callback];
+  // User-initiated event-tracking loops protect against this by
-    // Make sure events can be pumped while the menu is up.
+  // setting flags in -[CrApplication sendEvent:], but since
-    base::MessageLoop::ScopedNestableTaskAllower allow(
+  // web-content menus are initiated by IPC message the setup has to
-        base::MessageLoop::current());
+  // be done manually.
+  base::mac::ScopedSendingEvent sendingEventScoper;
 
-    // One of the events that could be pumped is |window.close()|.
+  // Don't emit unresponsive event when showing menu.
-    // User-initiated event-tracking loops protect against this by
+  atom::UnresponsiveSuppressor suppressor;
-    // setting flags in -[CrApplication sendEvent:], but since
+  [menu popUpMenuPositioningItem:item atLocation:position inView:view];
-    // 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();
-  }
 }
 
 void MenuMac::ClosePopupAt(int32_t window_id) {

atom/browser/api/atom_api_menu_views.cc

@@ -20,8 +20,7 @@ MenuViews::MenuViews(v8::Isolate* isolate, v8::Local<v8::Object> wrapper)
       weak_factory_(this) {
 }
 
-void MenuViews::PopupAt(
+void MenuViews::PopupAt(Window* window, int x, int y, int positioning_item) {
-    Window* window, int x, int y, int positioning_item, bool async) {
   NativeWindow* native_window = static_cast<NativeWindow*>(window->window());
   if (!native_window)
     return;
@@ -42,8 +41,6 @@ void MenuViews::PopupAt(
   }
 
   int flags = MenuRunner::CONTEXT_MENU | MenuRunner::HAS_MNEMONICS;
-  if (async)
-    flags |= MenuRunner::ASYNC;
 
   // Don't emit unresponsive event when showing menu.
   atom::UnresponsiveSuppressor suppressor;

atom/browser/api/atom_api_menu_views.h

@@ -21,8 +21,7 @@ class MenuViews : public Menu {
   MenuViews(v8::Isolate* isolate, v8::Local<v8::Object> wrapper);
 
  protected:
-  void PopupAt(
+  void PopupAt(Window* window, int x, int y, int positioning_item) override;
-      Window* window, int x, int y, int positioning_item, bool async) override;
   void ClosePopupAt(int32_t window_id) override;
 
  private:

lib/browser/api/menu.js

@@ -44,7 +44,6 @@ Menu.prototype._init = function () {
 }
 
 Menu.prototype.popup = function (window, x, y, positioningItem) {
-  let asyncPopup
   let [newX, newY, newPosition, newWindow] = [x, y, positioningItem, window]
 
   // menu.popup(x, y, positioningItem)
@@ -61,7 +60,6 @@ Menu.prototype.popup = function (window, x, y, positioningItem) {
     newX = opts.x
     newY = opts.y
     newPosition = opts.positioningItem
-    asyncPopup = opts.async
   }
 
   // set defaults
@@ -69,9 +67,8 @@ Menu.prototype.popup = function (window, x, y, positioningItem) {
   if (typeof y !== 'number') newY = -1
   if (typeof positioningItem !== 'number') newPosition = -1
   if (!window) newWindow = BrowserWindow.getFocusedWindow()
-  if (typeof asyncPopup !== 'boolean') asyncPopup = false
 
-  this.popupAt(newWindow, newX, newY, newPosition, asyncPopup)
+  this.popupAt(newWindow, newX, newY, newPosition)
 }
 
 Menu.prototype.closePopup = function (window) {

spec/api-menu-spec.js

@@ -282,11 +282,9 @@ describe('Menu module', () => {
       return closeWindow(w).then(() => { w = null })
     })
 
-    describe('when called with async: true', () => {
+    it('returns immediately', () => {
-      it('returns immediately', () => {
+      menu.popup(w, {x: 100, y: 100, async: true})
-        menu.popup(w, {x: 100, y: 100, async: true})
+      menu.closePopup(w)
-        menu.closePopup(w)
-      })
     })
   })