Bug 1694853 - Stop giving the nsMenuBarX a reference to the widget. r…

…=harry Differential Revision: https://phabricator.services.mozilla.com/D106379
dothq · Mar 2, 2021 · 168f75a · 168f75a
1 parent 419b917
commit 168f75a
Show file tree

Hide file tree

Showing 5 changed files with 12 additions and 24 deletions.
diff --git a/widget/cocoa/NativeMenuSupport.mm b/widget/cocoa/NativeMenuSupport.mm
@@ -6,6 +6,7 @@
 #include "mozilla/widget/NativeMenuSupport.h"
 
 #include "MainThreadUtils.h"
+#include "nsCocoaWindow.h"
 #include "nsMenuBarX.h"
 
 namespace mozilla::widget {
@@ -15,8 +16,11 @@
 
   RefPtr<nsMenuBarX> mb = new nsMenuBarX();
 
-  nsresult rv = mb->Create(aParent, aMenuBarElement);
+  nsresult rv = mb->Create(aMenuBarElement);
   MOZ_RELEASE_ASSERT(NS_SUCCEEDED(rv));
+
+  // Give the menubar to the parent window. The parent takes ownership.
+  static_cast<nsCocoaWindow*>(aParent)->SetMenuBar(std::move(mb));
 }
 
 }  // namespace mozilla::widget
diff --git a/widget/cocoa/nsCocoaWindow.h b/widget/cocoa/nsCocoaWindow.h
@@ -327,7 +327,7 @@ class nsCocoaWindow final : public nsBaseWidget, public nsPIWidgetCocoa {
   bool HasModalDescendents() { return mNumModalDescendents > 0; }
   NSWindow* GetCocoaWindow() { return mWindow; }
 
-  void SetMenuBar(nsMenuBarX* aMenuBar);
+  void SetMenuBar(RefPtr<nsMenuBarX>&& aMenuBar);
   nsMenuBarX* GetMenuBar();
 
   virtual void SetInputContext(const InputContext& aContext,

diff --git a/widget/cocoa/nsCocoaWindow.mm b/widget/cocoa/nsCocoaWindow.mm
@@ -2130,13 +2130,12 @@ static nsSizeMode GetWindowSizeMode(NSWindow* aWindow, bool aFullScreen) {
   Unused << bc->SetExplicitActive(ExplicitActiveStatus::Active);
 }
 
-void nsCocoaWindow::SetMenuBar(nsMenuBarX* aMenuBar) {
-  if (mMenuBar) mMenuBar->SetParent(nullptr);
+void nsCocoaWindow::SetMenuBar(RefPtr<nsMenuBarX>&& aMenuBar) {
   if (!mWindow) {
     mMenuBar = nullptr;
     return;
   }
-  mMenuBar = aMenuBar;
+  mMenuBar = std::move(aMenuBar);
 
   // Only paint for active windows, or paint the hidden window menu bar if no
   // other menu bar has been painted yet so that some reasonable menu bar is

diff --git a/widget/cocoa/nsMenuBarX.h b/widget/cocoa/nsMenuBarX.h
@@ -97,8 +97,7 @@ class nsMenuBarX : public nsMenuGroupOwnerX, public nsChangeObserver {
   nsMenuObjectTypeX MenuObjectType() override { return eMenuBarObjectType; }
 
   // nsMenuBarX
-  nsresult Create(nsIWidget* aParent, mozilla::dom::Element* aElement);
-  void SetParent(nsIWidget* aParent);
+  nsresult Create(mozilla::dom::Element* aElement);
   uint32_t GetMenuCount();
   bool MenuContainsAppMenu();
   nsMenuX* GetMenuAt(uint32_t aIndex);
@@ -125,7 +124,6 @@ class nsMenuBarX : public nsMenuGroupOwnerX, public nsChangeObserver {
   void CreateApplicationMenu(nsMenuX* aMenu);
 
   nsTArray<mozilla::UniquePtr<nsMenuX>> mMenuArray;
-  nsIWidget* mParentWindow;  // [weak]
   GeckoNSMenu* mNativeMenu;  // root menu, representing entire menu bar
   bool mNeedsRebuild;
   ApplicationMenuDelegate* mApplicationMenuDelegate;

diff --git a/widget/cocoa/nsMenuBarX.mm b/widget/cocoa/nsMenuBarX.mm
@@ -11,7 +11,6 @@
 #include "nsMenuItemX.h"
 #include "nsMenuUtilsX.h"
 #include "nsCocoaUtils.h"
-#include "nsCocoaWindow.h"
 #include "nsChildView.h"
 
 #include "nsCOMPtr.h"
@@ -32,6 +31,7 @@
 #include "mozilla/dom/Element.h"
 
 using namespace mozilla;
+using mozilla::dom::Element;
 
 NativeMenuItemTarget* nsMenuBarX::sNativeEventTarget = nil;
 nsMenuBarX* nsMenuBarX::sLastGeckoMenuBarPainted = nullptr;
@@ -73,10 +73,7 @@ - (void)menuDidClose:(NSMenu*)menu {
 @end
 
 nsMenuBarX::nsMenuBarX()
-    : nsMenuGroupOwnerX(),
-      mParentWindow(nullptr),
-      mNeedsRebuild(false),
-      mApplicationMenuDelegate(nil) {
+    : nsMenuGroupOwnerX(), mNeedsRebuild(false), mApplicationMenuDelegate(nil) {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
 
   mNativeMenu = [[GeckoNSMenu alloc] initWithTitle:@"MainMenuBar"];
@@ -123,14 +120,9 @@ - (void)menuDidClose:(NSMenu*)menu {
   NS_OBJC_END_TRY_ABORT_BLOCK;
 }
 
-nsresult nsMenuBarX::Create(nsIWidget* aParent, Element* aContent) {
+nsresult nsMenuBarX::Create(Element* aContent) {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
 
-  if (!aParent) {
-    return NS_ERROR_INVALID_ARG;
-  }
-
-  mParentWindow = aParent;
   mContent = aContent;
 
   if (mContent) {
@@ -147,9 +139,6 @@ - (void)menuDidClose:(NSMenu*)menu {
     ConstructFallbackNativeMenus();
   }
 
-  // Give this to the parent window. The parent takes ownership.
-  static_cast<nsCocoaWindow*>(mParentWindow)->SetMenuBar(this);
-
   return NS_OK;
 
   NS_OBJC_END_TRY_ABORT_BLOCK;
@@ -793,8 +782,6 @@ developers need only add each node with a label and a key equivalent (if they
   NS_OBJC_END_TRY_ABORT_BLOCK;
 }
 
-void nsMenuBarX::SetParent(nsIWidget* aParent) { mParentWindow = aParent; }
-
 //
 // Objective-C class used to allow us to have keyboard commands
 // look like they are doing something but actually do nothing.