fix: remove menu observer before destroying menu_controller_ (#49657)

* fix: remove menu observer before destroying menu_controller_

Co-authored-by: Keeley Hammond <khammond@slack-corp.com>

* fix: resolves private inheritance conflict

Co-authored-by: Keeley Hammond <khammond@slack-corp.com>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Keeley Hammond <khammond@slack-corp.com>

shell/browser/api/electron_api_menu.cc

@@ -68,6 +68,10 @@ Menu::Menu(gin::Arguments* args)
 }
 
 Menu::~Menu() {
+  RemoveModelObserver();
+}
+
+void Menu::RemoveModelObserver() {
   if (model_) {
     model_->RemoveObserver(this);
   }

shell/browser/api/electron_api_menu.h

@@ -61,6 +61,10 @@ class Menu : public gin::Wrappable<Menu>,
   ElectronMenuModel* model() const { return model_.get(); }
 
  protected:
+  // Remove this instance as an observer from the model. Called by derived
+  // class destructors to ensure observer is removed before platform-specific
+  // cleanup that may trigger model callbacks.
+  void RemoveModelObserver();
   // Returns a new callback which keeps references of the JS wrapper until the
   // passed |callback| is called.
   base::OnceClosure BindSelfToClosure(base::OnceClosure callback);

shell/browser/api/electron_api_menu_mac.mm

@@ -51,7 +51,11 @@ namespace electron::api {
 
 MenuMac::MenuMac(gin::Arguments* args) : Menu{args} {}
 
-MenuMac::~MenuMac() = default;
+MenuMac::~MenuMac() {
+  // Must remove observer before destroying menu_controller_, which holds
+  // a weak reference to model_
+  RemoveModelObserver();
+}
 
 void MenuMac::PopupAt(BaseWindow* window,
                       std::optional<WebFrameMain*> frame,