From dc2cd8e7806336361ffca3f04e783861bf2ece39 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Mon, 24 Jun 2019 19:30:26 -0700 Subject: [PATCH] fix: make tray not block main process (#18880) * fix: make tray not block main process * make AtomMenuModel refcounted --- shell/browser/api/atom_api_menu.h | 2 +- shell/browser/ui/atom_menu_model.h | 7 +++++-- shell/browser/ui/tray_icon_cocoa.h | 1 + shell/browser/ui/tray_icon_cocoa.mm | 19 ++++++++++++++++++- 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/shell/browser/api/atom_api_menu.h b/shell/browser/api/atom_api_menu.h index 351ce2e18557..a473470f5613 100644 --- a/shell/browser/api/atom_api_menu.h +++ b/shell/browser/api/atom_api_menu.h @@ -63,7 +63,7 @@ class Menu : public mate::TrackableObject, const base::Closure& callback) = 0; virtual void ClosePopupAt(int32_t window_id) = 0; - std::unique_ptr model_; + scoped_refptr model_; Menu* parent_ = nullptr; // Observable: diff --git a/shell/browser/ui/atom_menu_model.h b/shell/browser/ui/atom_menu_model.h index f9341d8d9284..773bc8e679d9 100644 --- a/shell/browser/ui/atom_menu_model.h +++ b/shell/browser/ui/atom_menu_model.h @@ -13,7 +13,8 @@ namespace electron { -class AtomMenuModel : public ui::SimpleMenuModel { +class AtomMenuModel : public ui::SimpleMenuModel, + public base::RefCounted { 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() override; + Delegate* delegate_; // weak ref. std::map roles_; // command id -> role diff --git a/shell/browser/ui/tray_icon_cocoa.h b/shell/browser/ui/tray_icon_cocoa.h index e14cd2039597..8d8f82174349 100644 --- a/shell/browser/ui/tray_icon_cocoa.h +++ b/shell/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 menu_model); void PopUpContextMenu(const gfx::Point& pos, AtomMenuModel* menu_model) override; void SetContextMenu(AtomMenuModel* menu_model) override; diff --git a/shell/browser/ui/tray_icon_cocoa.mm b/shell/browser/ui/tray_icon_cocoa.mm index bf31b8d6dfb7..c5d5cafdbb8b 100644 --- a/shell/browser/ui/tray_icon_cocoa.mm +++ b/shell/browser/ui/tray_icon_cocoa.mm @@ -8,7 +8,11 @@ #include #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" @@ -333,6 +337,7 @@ const CGFloat kVerticalTitleMargin = 2; } - (void)popUpContextMenu:(electron::AtomMenuModel*)menu_model { + base::MessageLoopCurrent::ScopedNestableTaskAllower allow; // Show a custom menu. if (menu_model) { base::scoped_nsobject menuController( @@ -340,6 +345,8 @@ const CGFloat kVerticalTitleMargin = 2; 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]; @@ -349,6 +356,8 @@ const CGFloat kVerticalTitleMargin = 2; 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. @@ -489,9 +498,17 @@ bool TrayIconCocoa::GetIgnoreDoubleClickEvents() { return [status_item_view_ getIgnoreDoubleClickEvents]; } +void TrayIconCocoa::PopUpOnUI(scoped_refptr 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) {