fix: make tray not block main process (#18880)

* fix: make tray not block main process * make AtomMenuModel refcounted
electron · Jun 25, 2019 · f24efde · f24efde
1 parent 212c6b1
commit f24efde
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 7 deletions.
diff --git a/atom/browser/api/atom_api_menu.h b/atom/browser/api/atom_api_menu.h
@@ -63,7 +63,7 @@ class Menu : public mate::TrackableObject<Menu>,
                        const base::Closure& callback) = 0;
   virtual void ClosePopupAt(int32_t window_id) = 0;
 
-  std::unique_ptr<AtomMenuModel> model_;
+  scoped_refptr<AtomMenuModel> model_;
   Menu* parent_ = nullptr;
 
   // Observable:

diff --git a/atom/browser/ui/atom_menu_model.h b/atom/browser/ui/atom_menu_model.h
@@ -13,7 +13,8 @@
 
 namespace atom {
 
-class AtomMenuModel : public ui::SimpleMenuModel {
+class AtomMenuModel : public ui::SimpleMenuModel,
+                      public base::RefCounted<AtomMenuModel> {
  public:
   class Delegate : public ui::SimpleMenuModel::Delegate {
    public:
@@ -48,7 +49,6 @@ class AtomMenuModel : public ui::SimpleMenuModel {
   };
 
   explicit AtomMenuModel(Delegate* delegate);
-  ~AtomMenuModel() override;
 
   void AddObserver(Observer* obs) { observers_.AddObserver(obs); }
   void RemoveObserver(Observer* obs) { observers_.RemoveObserver(obs); }
@@ -69,6 +69,9 @@ class AtomMenuModel : public ui::SimpleMenuModel {
   AtomMenuModel* GetSubmenuModelAt(int index);
 
  private:
+  friend class base::RefCounted<AtomMenuModel>;
+  ~AtomMenuModel() override;
+
   Delegate* delegate_;  // weak ref.
 
   std::map<int, base::string16> roles_;  // command id -> role

diff --git a/atom/browser/ui/tray_icon_cocoa.h b/atom/browser/ui/tray_icon_cocoa.h
@@ -30,6 +30,7 @@ class TrayIconCocoa : public TrayIcon, public AtomMenuModel::Observer {
   void SetHighlightMode(TrayIcon::HighlightMode mode) override;
   void SetIgnoreDoubleClickEvents(bool ignore) override;
   bool GetIgnoreDoubleClickEvents() override;
+  void PopUpOnUI(scoped_refptr<AtomMenuModel> menu_model);
   void PopUpContextMenu(const gfx::Point& pos,
                         AtomMenuModel* menu_model) override;
   void SetContextMenu(AtomMenuModel* menu_model) override;

diff --git a/atom/browser/ui/tray_icon_cocoa.mm b/atom/browser/ui/tray_icon_cocoa.mm
@@ -4,11 +4,15 @@
 
 #include "atom/browser/ui/tray_icon_cocoa.h"
 
-#include "atom/browser/mac/atom_application.h"
-#include "atom/browser/ui/cocoa/NSString+ANSI.h"
-#include "atom/browser/ui/cocoa/atom_menu_controller.h"
 #include "base/mac/sdk_forward_declarations.h"
+#include "base/message_loop/message_loop.h"
 #include "base/strings/sys_string_conversions.h"
+#include "base/task/post_task.h"
+#include "content/public/browser/browser_task_traits.h"
+#include "content/public/browser/browser_thread.h"
+#include "shell/browser/mac/atom_application.h"
+#include "shell/browser/ui/cocoa/NSString+ANSI.h"
+#include "shell/browser/ui/cocoa/atom_menu_controller.h"
 #include "ui/display/screen.h"
 #include "ui/events/cocoa/cocoa_event_utils.h"
 #include "ui/gfx/image/image.h"
@@ -330,13 +334,16 @@ - (void)mouseUp:(NSEvent*)event {
 }
 
 - (void)popUpContextMenu:(atom::AtomMenuModel*)menu_model {
+  base::MessageLoopCurrent::ScopedNestableTaskAllower allow;
   // Show a custom menu.
   if (menu_model) {
     base::scoped_nsobject<AtomMenuController> menuController(
         [[AtomMenuController alloc] initWithModel:menu_model
                             useDefaultAccelerator:NO]);
     forceHighlight_ = YES;  // Should highlight when showing menu.
     [self setNeedsDisplay:YES];
+
+    base::mac::ScopedSendingEvent sendingEventScoper;
     [statusItem_ popUpStatusItemMenu:[menuController menu]];
     forceHighlight_ = NO;
     [self setNeedsDisplay:YES];
@@ -346,6 +353,8 @@ - (void)popUpContextMenu:(atom::AtomMenuModel*)menu_model {
   if (menuController_ && ![menuController_ isMenuOpen]) {
     // Redraw the tray icon to show highlight if it is enabled.
     [self setNeedsDisplay:YES];
+
+    base::mac::ScopedSendingEvent sendingEventScoper;
     [statusItem_ popUpStatusItemMenu:[menuController_ menu]];
     // The popUpStatusItemMenu returns only after the showing menu is closed.
     // When it returns, we need to redraw the tray icon to not show highlight.
@@ -485,9 +494,17 @@ - (BOOL)shouldHighlight {
   return [status_item_view_ getIgnoreDoubleClickEvents];
 }
 
+void TrayIconCocoa::PopUpOnUI(scoped_refptr<AtomMenuModel> menu_model) {
+  if (auto* model = menu_model.get())
+    [status_item_view_ popUpContextMenu:model];
+}
+
 void TrayIconCocoa::PopUpContextMenu(const gfx::Point& pos,
                                      AtomMenuModel* menu_model) {
-  [status_item_view_ popUpContextMenu:menu_model];
+  base::PostTaskWithTraits(
+      FROM_HERE, {content::BrowserThread::UI},
+      base::BindOnce(&TrayIconCocoa::PopUpOnUI, base::Unretained(this),
+                     base::WrapRefCounted(menu_model)));
 }
 
 void TrayIconCocoa::SetContextMenu(AtomMenuModel* menu_model) {