chore: use ScopedPumpMessagesInPrivateModes in tray (#18977)

* chore: use ScopedPumpMessagesInPrivateModes in tray

* revert refcounting of AtomMenuModel

* Prefer WeakPtr for posting tasks to handle unexpected destruction
This commit is contained in:
Shelley Vohr 2019-06-26 10:18:53 -07:00 committed by GitHub
parent d643921313
commit 6243dba068
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 16 additions and 15 deletions

View file

@ -63,7 +63,7 @@ class Menu : public mate::TrackableObject<Menu>,
const base::Closure& callback) = 0; const base::Closure& callback) = 0;
virtual void ClosePopupAt(int32_t window_id) = 0; virtual void ClosePopupAt(int32_t window_id) = 0;
scoped_refptr<AtomMenuModel> model_; std::unique_ptr<AtomMenuModel> model_;
Menu* parent_ = nullptr; Menu* parent_ = nullptr;
// Observable: // Observable:

View file

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

View file

@ -30,7 +30,7 @@ class TrayIconCocoa : public TrayIcon, public AtomMenuModel::Observer {
void SetHighlightMode(TrayIcon::HighlightMode mode) override; void SetHighlightMode(TrayIcon::HighlightMode mode) override;
void SetIgnoreDoubleClickEvents(bool ignore) override; void SetIgnoreDoubleClickEvents(bool ignore) override;
bool GetIgnoreDoubleClickEvents() override; bool GetIgnoreDoubleClickEvents() override;
void PopUpOnUI(scoped_refptr<AtomMenuModel> menu_model); void PopUpOnUI(AtomMenuModel* menu_model);
void PopUpContextMenu(const gfx::Point& pos, void PopUpContextMenu(const gfx::Point& pos,
AtomMenuModel* menu_model) override; AtomMenuModel* menu_model) override;
void SetContextMenu(AtomMenuModel* menu_model) override; void SetContextMenu(AtomMenuModel* menu_model) override;
@ -50,6 +50,8 @@ class TrayIconCocoa : public TrayIcon, public AtomMenuModel::Observer {
// Used for unregistering observer. // Used for unregistering observer.
AtomMenuModel* menu_model_ = nullptr; // weak ref. AtomMenuModel* menu_model_ = nullptr; // weak ref.
base::WeakPtrFactory<TrayIconCocoa> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(TrayIconCocoa); DISALLOW_COPY_AND_ASSIGN(TrayIconCocoa);
}; };

View file

@ -337,7 +337,12 @@ const CGFloat kVerticalTitleMargin = 2;
} }
- (void)popUpContextMenu:(electron::AtomMenuModel*)menu_model { - (void)popUpContextMenu:(electron::AtomMenuModel*)menu_model {
// Make sure events can be pumped while the menu is up.
base::MessageLoopCurrent::ScopedNestableTaskAllower allow; base::MessageLoopCurrent::ScopedNestableTaskAllower allow;
// Ensure the UI can update while the menu is fading out.
base::ScopedPumpMessagesInPrivateModes pump_private;
// Show a custom menu. // Show a custom menu.
if (menu_model) { if (menu_model) {
base::scoped_nsobject<AtomMenuController> menuController( base::scoped_nsobject<AtomMenuController> menuController(
@ -346,7 +351,6 @@ const CGFloat kVerticalTitleMargin = 2;
forceHighlight_ = YES; // Should highlight when showing menu. forceHighlight_ = YES; // Should highlight when showing menu.
[self setNeedsDisplay:YES]; [self setNeedsDisplay:YES];
base::mac::ScopedSendingEvent sendingEventScoper;
[statusItem_ popUpStatusItemMenu:[menuController menu]]; [statusItem_ popUpStatusItemMenu:[menuController menu]];
forceHighlight_ = NO; forceHighlight_ = NO;
[self setNeedsDisplay:YES]; [self setNeedsDisplay:YES];
@ -357,7 +361,6 @@ const CGFloat kVerticalTitleMargin = 2;
// Redraw the tray icon to show highlight if it is enabled. // Redraw the tray icon to show highlight if it is enabled.
[self setNeedsDisplay:YES]; [self setNeedsDisplay:YES];
base::mac::ScopedSendingEvent sendingEventScoper;
[statusItem_ popUpStatusItemMenu:[menuController_ menu]]; [statusItem_ popUpStatusItemMenu:[menuController_ menu]];
// The popUpStatusItemMenu returns only after the showing menu is closed. // The popUpStatusItemMenu returns only after the showing menu is closed.
// When it returns, we need to redraw the tray icon to not show highlight. // When it returns, we need to redraw the tray icon to not show highlight.
@ -456,7 +459,7 @@ const CGFloat kVerticalTitleMargin = 2;
namespace electron { namespace electron {
TrayIconCocoa::TrayIconCocoa() { TrayIconCocoa::TrayIconCocoa() : weak_factory_(this) {
status_item_view_.reset([[StatusItemView alloc] initWithIcon:this]); status_item_view_.reset([[StatusItemView alloc] initWithIcon:this]);
} }
@ -498,17 +501,16 @@ bool TrayIconCocoa::GetIgnoreDoubleClickEvents() {
return [status_item_view_ getIgnoreDoubleClickEvents]; return [status_item_view_ getIgnoreDoubleClickEvents];
} }
void TrayIconCocoa::PopUpOnUI(scoped_refptr<AtomMenuModel> menu_model) { void TrayIconCocoa::PopUpOnUI(AtomMenuModel* menu_model) {
if (auto* model = menu_model.get()) [status_item_view_ popUpContextMenu:menu_model];
[status_item_view_ popUpContextMenu:model];
} }
void TrayIconCocoa::PopUpContextMenu(const gfx::Point& pos, void TrayIconCocoa::PopUpContextMenu(const gfx::Point& pos,
AtomMenuModel* menu_model) { AtomMenuModel* menu_model) {
base::PostTaskWithTraits( base::PostTaskWithTraits(
FROM_HERE, {content::BrowserThread::UI}, FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(&TrayIconCocoa::PopUpOnUI, base::Unretained(this), base::BindOnce(&TrayIconCocoa::PopUpOnUI, weak_factory_.GetWeakPtr(),
base::WrapRefCounted(menu_model))); base::Unretained(menu_model)));
} }
void TrayIconCocoa::SetContextMenu(AtomMenuModel* menu_model) { void TrayIconCocoa::SetContextMenu(AtomMenuModel* menu_model) {