fix: potential crash calling `tray.popUpContextMenu()` (#39231)

fix: potential crash calling tray.popUpContextMenu

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() {

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<ElectronMenuModel> menu_model) {}
+                                base::WeakPtr<ElectronMenuModel> menu_model) {}
 
 void TrayIcon::CloseContextMenu() {}
 

shell/browser/ui/tray_icon.h

@@ -89,7 +89,7 @@ class TrayIcon {
 
   // Popups the menu.
   virtual void PopUpContextMenu(const gfx::Point& pos,
-                                raw_ptr<ElectronMenuModel> menu_model);
+                                base::WeakPtr<ElectronMenuModel> menu_model);
 
   virtual void CloseContextMenu();
 

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<ElectronMenuModel> menu_model);
   void PopUpContextMenu(const gfx::Point& pos,
-                        raw_ptr<ElectronMenuModel>) override;
+                        base::WeakPtr<ElectronMenuModel> menu_model) override;
   void CloseContextMenu() override;
-  void SetContextMenu(raw_ptr<ElectronMenuModel>) override;
+  void SetContextMenu(raw_ptr<ElectronMenuModel> menu_model) override;
   gfx::Rect GetBounds() override;
 
  private:

shell/browser/ui/tray_icon_cocoa.mm

@@ -83,6 +83,11 @@
     [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 @@
   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 TrayIconCocoa::GetIgnoreDoubleClickEvents() {
   return [status_item_view_ getIgnoreDoubleClickEvents];
 }
 
-void TrayIconCocoa::PopUpOnUI(ElectronMenuModel* menu_model) {
-  [status_item_view_ popUpContextMenu:menu_model];
+void TrayIconCocoa::PopUpOnUI(base::WeakPtr<ElectronMenuModel> menu_model) {
+  [status_item_view_ popUpContextMenu:menu_model.get()];
 }
 
-void TrayIconCocoa::PopUpContextMenu(const gfx::Point& pos,
-                                     raw_ptr<ElectronMenuModel> menu_model) {
+void TrayIconCocoa::PopUpContextMenu(
+    const gfx::Point& pos,
+    base::WeakPtr<ElectronMenuModel> 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() {

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<ElectronMenuModel> menu_model) {
+                                  base::WeakPtr<ElectronMenuModel> menu_model) {
   // Returns if context menu isn't set.
   if (menu_model == nullptr && menu_model_ == nullptr)
     return;
@@ -209,9 +209,13 @@ void NotifyIcon::PopUpContextMenu(const gfx::Point& pos,
   if (pos.IsOrigin())
     rect.set_origin(display::Screen::GetScreen()->GetCursorScreenPoint());
 
-  menu_runner_ = std::make_unique<views::MenuRunner>(
-      menu_model != nullptr ? menu_model : menu_model_,
-      views::MenuRunner::HAS_MNEMONICS);
+  if (menu_model) {
+    menu_runner_ = std::make_unique<views::MenuRunner>(
+        menu_model.get(), views::MenuRunner::HAS_MNEMONICS);
+  } else {
+    menu_runner_ = std::make_unique<views::MenuRunner>(
+        menu_model_, views::MenuRunner::HAS_MNEMONICS);
+  }
   menu_runner_->RunMenuAt(nullptr, nullptr, rect,
                           views::MenuAnchorPosition::kTopLeft,
                           ui::MENU_SOURCE_MOUSE);

shell/browser/ui/win/notify_icon.h

@@ -13,6 +13,7 @@
 #include <string>
 
 #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<ElectronMenuModel> menu_model) override;
+                        base::WeakPtr<ElectronMenuModel> menu_model) override;
   void CloseContextMenu() override;
   void SetContextMenu(raw_ptr<ElectronMenuModel> menu_model) override;
   gfx::Rect GetBounds() override;