Mac async popup

DEPS

@@ -4,7 +4,7 @@ use_relative_paths = True
 
 deps = {
   "vendor/node": "https://github.com/brave/node.git@f51b9ab8ff446ca7b13be0de1bc12b854b23938d",
-  "vendor/native_mate": "https://github.com/zcbenz/native-mate.git@b5e5de626c6a57e44c7e6448d8bbaaac475d493c",
+  "vendor/native_mate": "https://github.com/zcbenz/native-mate.git@ad0fd825663932ee3fa29ff935dfec99933bdd8c",
   "vendor/requests": "https://github.com/kennethreitz/requests@e4d59bedfd3c7f4f254f4f5d036587bcd8152458",
   "vendor/boto": "https://github.com/boto/boto@f7574aa6cc2c819430c1f05e9a1a1a666ef8169b",
   "vendor/python-patch": "https://github.com/svn2github/python-patch@a336a458016ced89aba90dfc3f4c8222ae3b1403",

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) = 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,24 @@ 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) override;
+  void PopupOnUI(const base::WeakPtr<NativeWindow>& native_window,
+                 int32_t window_id, int x, int y, int positioning_item);
+  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,55 @@
 
 #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) {
   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);
+
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, popup);
+}
+
+void MenuMac::PopupOnUI(const base::WeakPtr<NativeWindow>& native_window,
+                        int32_t window_id, int x, int y, int positioning_item) {
+  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,13 +89,28 @@
   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());
+
+  // 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;
-
-  // Show the menu.
   [menu popUpMenuPositioningItem:item atLocation:position inView:view];
 }
 
+void MenuMac::ClosePopupAt(int32_t window_id) {
+  popup_controllers_.erase(window_id);
+  Destroy();
+}
+
 // static
 void Menu::SetApplicationMenu(Menu* base_menu) {
   MenuMac* menu = static_cast<MenuMac*>(base_menu);

atom/browser/api/atom_api_menu_views.cc

@@ -2,23 +2,28 @@
 // Use of this source code is governed by the MIT license that can be
 // found in the LICENSE file.
 
+#include <memory>
+
 #include "atom/browser/api/atom_api_menu_views.h"
 
 #include "atom/browser/native_window_views.h"
 #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) {
   NativeWindow* native_window = static_cast<NativeWindow*>(window->window());
   if (!native_window)
     return;
@@ -38,21 +43,32 @@ 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 |
+              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);
+  Destroy();
+}
+
 // 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,13 @@
 #ifndef ATOM_BROWSER_API_ATOM_API_MENU_VIEWS_H_
 #define ATOM_BROWSER_API_ATOM_API_MENU_VIEWS_H_
 
+#include <map>
+#include <memory>
+
 #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 +22,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) 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/api/trackable_object.h

@@ -16,6 +16,13 @@ namespace base {
 class SupportsUserData;
 }
 
+namespace atom {
+namespace api {
+class MenuMac;
+class MenuViews;
+}  // namespace api
+}  // namespace atom
+
 namespace mate {
 
 // Users should use TrackableObject instead.
@@ -45,6 +52,10 @@ class TrackableObjectBase {
   int32_t weak_map_id_;
 
  private:
+  // async menus will destroy themselves when closed
+  friend class atom::api::MenuMac;
+  friend class atom::api::MenuViews;
+
   void Destroy();
 
   base::Closure cleanup_;

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

@@ -12,9 +12,12 @@
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/base/accelerators/platform_accelerator_cocoa.h"
 #include "ui/base/l10n/l10n_util_mac.h"
+#include "content/public/browser/browser_thread.h"
 #include "ui/events/cocoa/cocoa_event_utils.h"
 #include "ui/gfx/image/image.h"
 
+using content::BrowserThread;
+
 namespace {
 
 struct Role {
@@ -71,6 +74,10 @@ - (void)dealloc {
   [super dealloc];
 }
 
+- (void)setCloseCallback:(const base::Callback<void()>&)callback {
+  closeCallback = callback;
+}
+
 - (void)populateWithModel:(atom::AtomMenuModel*)model {
   if (!menu_)
     return;
@@ -265,8 +272,13 @@ - (void)menuWillOpen:(NSMenu*)menu {
 
 - (void)menuDidClose:(NSMenu*)menu {
   if (isMenuOpen_) {
-    model_->MenuWillClose();
     isMenuOpen_ = NO;
+    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())
+      BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, closeCallback);
   }
 }
 

brave/common/extensions/url_bindings.cc

@@ -330,7 +330,8 @@ void URLBindings::Parse(
   GURL gurl(url_string);
   gin::Dictionary dict = gin::Dictionary::CreateEmpty(isolate);
   if (gurl.has_username())
-    dict.Set("auth", gurl.username() + (gurl.has_password() ? ":" + gurl.password() : ""));
+    dict.Set("auth", gurl.username() + (gurl.has_password()
+      ? ":" + gurl.password() : ""));
   dict.Set("hash", gurl.ref());
   dict.Set("hostname", gurl.host());
   dict.Set("host", gurl.host() + ":" + gurl.port());