Bug 1694853 - Move nsMenuItemX::Create into the constructor. r=harry

Differential Revision: https://phabricator.services.mozilla.com/D106368

widget/cocoa/nsMenuItemX.h

@@ -41,9 +41,10 @@ enum EMenuItemType {
 // Once instantiated, this object lives until its DOM node or its parent window
 // is destroyed. Do not hold references to this, they can become invalid any
 // time the DOM node can be destroyed.
-class nsMenuItemX : public nsMenuObjectX, public nsChangeObserver {
+class nsMenuItemX final : public nsMenuObjectX, public nsChangeObserver {
  public:
-  nsMenuItemX();
+  nsMenuItemX(nsMenuX* aParent, const nsString& aLabel, EMenuItemType aItemType,
+              nsMenuGroupOwnerX* aMenuGroupOwner, nsIContent* aNode);
   virtual ~nsMenuItemX();
 
   NS_DECL_CHANGEOBSERVER
@@ -53,9 +54,6 @@ class nsMenuItemX : public nsMenuObjectX, public nsChangeObserver {
   nsMenuObjectTypeX MenuObjectType() override { return eMenuItemObjectType; }
 
   // nsMenuItemX
-  nsresult Create(nsMenuX* aParent, const nsString& aLabel,
-                  EMenuItemType aItemType, nsMenuGroupOwnerX* aMenuGroupOwner,
-                  nsIContent* aNode);
   nsresult SetChecked(bool aIsChecked);
   EMenuItemType GetMenuItemType();
   void DoCommand();
@@ -70,12 +68,12 @@ class nsMenuItemX : public nsMenuObjectX, public nsChangeObserver {
   EMenuItemType mType;
 
   // nsMenuItemX objects should always have a valid native menu item.
-  NSMenuItem* mNativeMenuItem;         // [strong]
-  nsMenuX* mMenuParent;                // [weak]
-  nsMenuGroupOwnerX* mMenuGroupOwner;  // [weak]
+  NSMenuItem* mNativeMenuItem = nil;             // [strong]
+  nsMenuX* mMenuParent = nullptr;                // [weak]
+  nsMenuGroupOwnerX* mMenuGroupOwner = nullptr;  // [weak]
   RefPtr<mozilla::dom::Element> mCommandElement;
   mozilla::UniquePtr<nsMenuItemIconX> mIcon;  // always non-null
-  bool mIsChecked;
+  bool mIsChecked = false;
 };
 
 #endif  // nsMenuItemX_h_

widget/cocoa/nsMenuItemX.mm

@@ -26,44 +26,14 @@ using namespace mozilla;
 using mozilla::dom::Event;
 using mozilla::dom::CallerType;
 
-nsMenuItemX::nsMenuItemX() {
-  mType = eRegularMenuItemType;
-  mNativeMenuItem = nil;
-  mMenuParent = nullptr;
-  mMenuGroupOwner = nullptr;
-  mIsChecked = false;
+nsMenuItemX::nsMenuItemX(nsMenuX* aParent, const nsString& aLabel, EMenuItemType aItemType,
+                         nsMenuGroupOwnerX* aMenuGroupOwner, nsIContent* aNode)
+    : mType(aItemType), mMenuParent(aParent), mMenuGroupOwner(aMenuGroupOwner) {
+  NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
 
   MOZ_COUNT_CTOR(nsMenuItemX);
-}
 
-nsMenuItemX::~nsMenuItemX() {
-  NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
-
-  // autorelease the native menu item so that anything else happening to this
-  // object happens before the native menu item actually dies
-  [mNativeMenuItem autorelease];
-
-  if (mContent) {
-    mMenuGroupOwner->UnregisterForContentChanges(mContent);
-  }
-  if (mCommandElement) {
-    mMenuGroupOwner->UnregisterForContentChanges(mCommandElement);
-  }
-
-  MOZ_COUNT_DTOR(nsMenuItemX);
-
-  NS_OBJC_END_TRY_ABORT_BLOCK;
-}
-
-nsresult nsMenuItemX::Create(nsMenuX* aParent, const nsString& aLabel, EMenuItemType aItemType,
-                             nsMenuGroupOwnerX* aMenuGroupOwner, nsIContent* aNode) {
-  NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
-
-  mType = aItemType;
-  mMenuParent = aParent;
   mContent = aNode;
-
-  mMenuGroupOwner = aMenuGroupOwner;
   NS_ASSERTION(mMenuGroupOwner, "No menu owner given, must have one!");
 
   mMenuGroupOwner->RegisterForContentChanges(mContent, this);
@@ -120,7 +90,24 @@ nsresult nsMenuItemX::Create(nsMenuX* aParent, const nsString& aLabel, EMenuItem
 
   mIcon = MakeUnique<nsMenuItemIconX>(this, mContent, mNativeMenuItem);
 
-  return NS_OK;
+  NS_OBJC_END_TRY_ABORT_BLOCK;
+}
+
+nsMenuItemX::~nsMenuItemX() {
+  NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
+
+  // autorelease the native menu item so that anything else happening to this
+  // object happens before the native menu item actually dies
+  [mNativeMenuItem autorelease];
+
+  if (mContent) {
+    mMenuGroupOwner->UnregisterForContentChanges(mContent);
+  }
+  if (mCommandElement) {
+    mMenuGroupOwner->UnregisterForContentChanges(mCommandElement);
+  }
+
+  MOZ_COUNT_DTOR(nsMenuItemX);
 
   NS_OBJC_END_TRY_ABORT_BLOCK;
 }
@@ -369,7 +356,4 @@ void nsMenuItemX::ObserveContentInserted(dom::Document* aDocument, nsIContent* a
   }
 }
 
-void nsMenuItemX::SetupIcon() {
-  MOZ_RELEASE_ASSERT(mIcon, "should have been created by nsMenuItemX::Create");
-  mIcon->SetupIcon();
-}
+void nsMenuItemX::SetupIcon() { mIcon->SetupIcon(); }

widget/cocoa/nsMenuX.mm

@@ -500,17 +500,8 @@ void nsMenuX::LoadMenuItem(nsIContent* inMenuItemContent) {
   }
 
   // Create the item.
-  nsMenuItemX* menuItem = new nsMenuItemX();
-  if (!menuItem) {
-    return;
-  }
-
-  nsresult rv = menuItem->Create(this, menuitemName, itemType, mMenuGroupOwner, inMenuItemContent);
-  if (NS_FAILED(rv)) {
-    delete menuItem;
-    return;
-  }
-
+  nsMenuItemX* menuItem =
+      new nsMenuItemX(this, menuitemName, itemType, mMenuGroupOwner, inMenuItemContent);
   AddMenuItem(menuItem);
 
   // This needs to happen after the nsIMenuItem object is inserted into