From 770bdff7021c936a242c313d0de623006c8a2f5f Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Tue, 25 Jul 2023 21:07:19 +0200 Subject: [PATCH] fix: potential crash calling tray.popUpContextMenu --- shell/browser/api/electron_api_tray.cc | 4 +++- shell/browser/ui/tray_icon.cc | 2 +- shell/browser/ui/tray_icon.h | 2 +- shell/browser/ui/tray_icon_cocoa.h | 6 +++--- shell/browser/ui/tray_icon_cocoa.mm | 20 ++++++++++++-------- shell/browser/ui/win/notify_icon.cc | 4 ++-- shell/browser/ui/win/notify_icon.h | 3 ++- 7 files changed, 24 insertions(+), 17 deletions(-) diff --git a/shell/browser/api/electron_api_tray.cc b/shell/browser/api/electron_api_tray.cc index e1bcab8ffcc7f..8b729fb00e974 100644 --- a/shell/browser/api/electron_api_tray.cc +++ b/shell/browser/api/electron_api_tray.cc @@ -342,7 +342,9 @@ void Tray::PopUpContextMenu(gin::Arguments* args) { } } } - tray_icon_->PopUpContextMenu(pos, menu.IsEmpty() ? nullptr : menu->model()); + + tray_icon_->PopUpContextMenu( + pos, menu.IsEmpty() ? nullptr : menu->model()->GetWeakPtr()); } void Tray::CloseContextMenu() { diff --git a/shell/browser/ui/tray_icon.cc b/shell/browser/ui/tray_icon.cc index 07b801583fdeb..7f22f3b6da555 100644 --- a/shell/browser/ui/tray_icon.cc +++ b/shell/browser/ui/tray_icon.cc @@ -21,7 +21,7 @@ void TrayIcon::RemoveBalloon() {} void TrayIcon::Focus() {} void TrayIcon::PopUpContextMenu(const gfx::Point& pos, - raw_ptr menu_model) {} + base::WeakPtr menu_model) {} void TrayIcon::CloseContextMenu() {} diff --git a/shell/browser/ui/tray_icon.h b/shell/browser/ui/tray_icon.h index a8b4581a43c43..85a66163b622f 100644 --- a/shell/browser/ui/tray_icon.h +++ b/shell/browser/ui/tray_icon.h @@ -89,7 +89,7 @@ class TrayIcon { // Popups the menu. virtual void PopUpContextMenu(const gfx::Point& pos, - raw_ptr menu_model); + base::WeakPtr menu_model); virtual void CloseContextMenu(); diff --git a/shell/browser/ui/tray_icon_cocoa.h b/shell/browser/ui/tray_icon_cocoa.h index 6f6241df68590..d7a7904571ac2 100644 --- a/shell/browser/ui/tray_icon_cocoa.h +++ b/shell/browser/ui/tray_icon_cocoa.h @@ -32,11 +32,11 @@ class TrayIconCocoa : public TrayIcon { std::string GetTitle() override; void SetIgnoreDoubleClickEvents(bool ignore) override; bool GetIgnoreDoubleClickEvents() override; - void PopUpOnUI(ElectronMenuModel* menu_model); + void PopUpOnUI(base::WeakPtr menu_model); void PopUpContextMenu(const gfx::Point& pos, - raw_ptr) override; + base::WeakPtr menu_model) override; void CloseContextMenu() override; - void SetContextMenu(raw_ptr) override; + void SetContextMenu(raw_ptr menu_model) override; gfx::Rect GetBounds() override; private: diff --git a/shell/browser/ui/tray_icon_cocoa.mm b/shell/browser/ui/tray_icon_cocoa.mm index e0d5f5c6a4a22..9d6d6d4bd0f21 100644 --- a/shell/browser/ui/tray_icon_cocoa.mm +++ b/shell/browser/ui/tray_icon_cocoa.mm @@ -83,6 +83,11 @@ - (void)removeItem { [self removeTrackingArea:trackingArea_]; trackingArea_ = nil; } + + // Ensure any open menu is closed. + if ([statusItem_ menu]) + [[statusItem_ menu] cancelTracking]; + [[NSStatusBar systemStatusBar] removeStatusItem:statusItem_]; [self removeFromSuperview]; statusItem_ = nil; @@ -228,7 +233,6 @@ - (void)popUpContextMenu:(electron::ElectronMenuModel*)menu_model { if (menuController_ && ![menuController_ isMenuOpen]) { // Ensure the UI can update while the menu is fading out. base::ScopedPumpMessagesInPrivateModes pump_private; - [[statusItem_ button] performClick:self]; } } @@ -354,16 +358,16 @@ - (BOOL)performDragOperation:(id)sender { return [status_item_view_ getIgnoreDoubleClickEvents]; } -void TrayIconCocoa::PopUpOnUI(ElectronMenuModel* menu_model) { - [status_item_view_ popUpContextMenu:menu_model]; +void TrayIconCocoa::PopUpOnUI(base::WeakPtr menu_model) { + [status_item_view_ popUpContextMenu:menu_model.get()]; } -void TrayIconCocoa::PopUpContextMenu(const gfx::Point& pos, - raw_ptr menu_model) { +void TrayIconCocoa::PopUpContextMenu( + const gfx::Point& pos, + base::WeakPtr menu_model) { content::GetUIThreadTaskRunner({})->PostTask( - FROM_HERE, - base::BindOnce(&TrayIconCocoa::PopUpOnUI, weak_factory_.GetWeakPtr(), - base::Unretained(menu_model))); + FROM_HERE, base::BindOnce(&TrayIconCocoa::PopUpOnUI, + weak_factory_.GetWeakPtr(), menu_model)); } void TrayIconCocoa::CloseContextMenu() { diff --git a/shell/browser/ui/win/notify_icon.cc b/shell/browser/ui/win/notify_icon.cc index 4a5171328dc8e..2cf6df60546f5 100644 --- a/shell/browser/ui/win/notify_icon.cc +++ b/shell/browser/ui/win/notify_icon.cc @@ -86,7 +86,7 @@ void NotifyIcon::HandleClickEvent(int modifiers, return; } else if (!double_button_click) { // single right click if (menu_model_) - PopUpContextMenu(gfx::Point(), menu_model_); + PopUpContextMenu(gfx::Point(), menu_model_->GetWeakPtr()); else NotifyRightClicked(bounds, modifiers); } @@ -191,7 +191,7 @@ void NotifyIcon::Focus() { } void NotifyIcon::PopUpContextMenu(const gfx::Point& pos, - raw_ptr menu_model) { + base::WeakPtr menu_model) { // Returns if context menu isn't set. if (menu_model == nullptr && menu_model_ == nullptr) return; diff --git a/shell/browser/ui/win/notify_icon.h b/shell/browser/ui/win/notify_icon.h index b0d0b78eb0aa5..ee45864292d6a 100644 --- a/shell/browser/ui/win/notify_icon.h +++ b/shell/browser/ui/win/notify_icon.h @@ -13,6 +13,7 @@ #include #include "base/compiler_specific.h" +#include "base/memory/weak_ptr.h" #include "base/win/scoped_gdi_object.h" #include "shell/browser/ui/tray_icon.h" #include "shell/browser/ui/win/notify_icon_host.h" @@ -65,7 +66,7 @@ class NotifyIcon : public TrayIcon { void RemoveBalloon() override; void Focus() override; void PopUpContextMenu(const gfx::Point& pos, - raw_ptr menu_model) override; + base::WeakPtr menu_model) override; void CloseContextMenu() override; void SetContextMenu(raw_ptr menu_model) override; gfx::Rect GetBounds() override;