Bug 1694853 - Remove nsMenuObjectX::Content() and put it on the individual subclasses instead. r=harry

This removes some polymorphism and makes it easier to understand what's actually going on.
The explicit if checks added in this patch will go away once nsMenuX and nsMenuItemX are unified.

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

widget/cocoa/nsMenuBarX.mm

@@ -6,6 +6,7 @@
 #include <objc/objc-runtime.h>
 
 #include "nsMenuBarX.h"
+#include "nsMenuBaseX.h"
 #include "nsMenuX.h"
 #include "nsMenuItemX.h"
 #include "nsMenuUtilsX.h"
@@ -343,7 +344,12 @@ void nsMenuBarX::ForceUpdateNativeMenuAt(const nsAString& indexString) {
       if (!targetMenu) {
         return;
       }
-      if (!nsMenuUtilsX::NodeIsHiddenOrCollapsed(targetMenu->Content())) {
+      MOZ_RELEASE_ASSERT(targetMenu->MenuObjectType() == eSubmenuObjectType ||
+                         targetMenu->MenuObjectType() == eMenuItemObjectType);
+      RefPtr<nsIContent> content = targetMenu->MenuObjectType() == eSubmenuObjectType
+                                       ? static_cast<nsMenuX*>(targetMenu)->Content()
+                                       : static_cast<nsMenuItemX*>(targetMenu)->Content();
+      if (!nsMenuUtilsX::NodeIsHiddenOrCollapsed(content)) {
         visible++;
         if (targetMenu->MenuObjectType() == eSubmenuObjectType && visible == (targetIndex + 1)) {
           currentMenu = static_cast<nsMenuX*>(targetMenu);

widget/cocoa/nsMenuBaseX.h

@@ -28,7 +28,6 @@ class nsMenuObjectX {
   virtual ~nsMenuObjectX() {}
   virtual nsMenuObjectTypeX MenuObjectType() = 0;
   virtual void* NativeData() = 0;
-  nsIContent* Content() { return mContent; }
 
   /**
    * Called when an icon of a menu item somewhere in this menu has updated.
@@ -36,9 +35,6 @@ class nsMenuObjectX {
    * parent.
    */
   virtual void IconUpdated() {}
-
- protected:
-  nsCOMPtr<nsIContent> mContent;
 };
 
 //

widget/cocoa/nsMenuGroupOwnerX.h

@@ -40,6 +40,8 @@ class nsMenuGroupOwnerX : public nsMenuObjectX, public nsIMutationObserver {
 
   nsChangeObserver* LookupContentChangeObserver(nsIContent* aContent);
 
+  RefPtr<nsIContent> mContent;
+
   uint32_t mCurrentCommandID;  // unique command id (per menu-bar) to
                                // give to next item that asks
 

widget/cocoa/nsMenuItemX.h

@@ -60,11 +60,14 @@ class nsMenuItemX final : public nsMenuObjectX, public nsChangeObserver {
   nsresult DispatchDOMEvent(const nsString& eventName,
                             bool* preventDefaultCalled);
   void SetupIcon();
+  nsIContent* Content() { return mContent; }
 
  protected:
   void UncheckRadioSiblings(nsIContent* aCheckedElement);
   void SetKeyEquiv();
 
+  nsCOMPtr<nsIContent> mContent;  // XUL <menuitem> or <menuseparator>
+
   EMenuItemType mType;
 
   // nsMenuItemX objects should always have a valid native menu item.

widget/cocoa/nsMenuItemX.mm

@@ -28,12 +28,11 @@ using mozilla::dom::CallerType;
 
 nsMenuItemX::nsMenuItemX(nsMenuX* aParent, const nsString& aLabel, EMenuItemType aItemType,
                          nsMenuGroupOwnerX* aMenuGroupOwner, nsIContent* aNode)
-    : mType(aItemType), mMenuParent(aParent), mMenuGroupOwner(aMenuGroupOwner) {
+    : mContent(aNode), mType(aItemType), mMenuParent(aParent), mMenuGroupOwner(aMenuGroupOwner) {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
 
   MOZ_COUNT_CTOR(nsMenuItemX);
 
-  mContent = aNode;
   NS_ASSERTION(mMenuGroupOwner, "No menu owner given, must have one!");
 
   mMenuGroupOwner->RegisterForContentChanges(mContent, this);

widget/cocoa/nsMenuX.h

@@ -51,15 +51,21 @@ class nsMenuX final : public nsMenuObjectX, public nsChangeObserver {
 
   // nsMenuX
   nsresult Create(nsMenuObjectX* aParent, nsMenuGroupOwnerX* aMenuGroupOwner, nsIContent* aNode);
-  uint32_t GetItemCount();
+
+  // The returned object is an nsMenuX or an nsMenuItemX object
   nsMenuObjectX* GetItemAt(uint32_t aPos);
-  nsresult GetVisibleItemCount(uint32_t& aCount);
+  uint32_t GetItemCount();
+
+  // The returned object is an nsMenuX or an nsMenuItemX object
   nsMenuObjectX* GetVisibleItemAt(uint32_t aPos);
+  nsresult GetVisibleItemCount(uint32_t& aCount);
+
   nsEventStatus MenuOpened();
   void MenuClosed();
   void SetRebuild(bool aMenuEvent);
   NSMenuItem* NativeMenuItem();
   nsresult SetupIcon();
+  nsIContent* Content() { return mContent; }
 
   static bool IsXULHelpMenu(nsIContent* aMenuContent);
 
@@ -77,7 +83,11 @@ class nsMenuX final : public nsMenuObjectX, public nsChangeObserver {
   void LoadSubMenu(nsIContent* aMenuContent);
   GeckoNSMenu* CreateMenuWithGeckoString(nsString& aMenuTitle);
 
+  nsCOMPtr<nsIContent> mContent;  // XUL <menu> or <menupopup>
+
+  // Contains nsMenuX and nsMenuItemX objects
   nsTArray<mozilla::UniquePtr<nsMenuObjectX>> mMenuObjectsArray;
+
   nsString mLabel;
   uint32_t mVisibleItemsCount = 0;               // cache
   nsMenuObjectX* mParent = nullptr;              // [weak]

widget/cocoa/nsMenuX.mm

@@ -104,7 +104,7 @@ static void SwizzleDynamicIndexingMethods() {
 //
 
 nsMenuX::nsMenuX(nsMenuObjectX* aParent, nsMenuGroupOwnerX* aMenuGroupOwner, nsIContent* aContent)
-    : mParent(aParent), mMenuGroupOwner(aMenuGroupOwner) {
+    : mContent(aContent), mParent(aParent), mMenuGroupOwner(aMenuGroupOwner) {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
 
   MOZ_COUNT_CTOR(nsMenuX);
@@ -117,7 +117,6 @@ nsMenuX::nsMenuX(nsMenuObjectX* aParent, nsMenuGroupOwnerX* aMenuGroupOwner, nsI
     nsMenuBarX::sNativeEventTarget = [[NativeMenuItemTarget alloc] init];
   }
 
-  mContent = aContent;
   if (mContent->IsElement()) {
     mContent->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::label, mLabel);
   }
@@ -277,11 +276,15 @@ nsMenuObjectX* nsMenuX::GetVisibleItemAt(uint32_t aPos) {
   }
 
   // Otherwise, traverse the array until we find the the item we're looking for.
-  nsMenuObjectX* item;
   uint32_t visibleNodeIndex = 0;
   for (uint32_t i = 0; i < count; i++) {
-    item = mMenuObjectsArray[i].get();
-    if (!nsMenuUtilsX::NodeIsHiddenOrCollapsed(item->Content())) {
+    nsMenuObjectX* item = mMenuObjectsArray[i].get();
+    MOZ_RELEASE_ASSERT(item->MenuObjectType() == eSubmenuObjectType ||
+                       item->MenuObjectType() == eMenuItemObjectType);
+    RefPtr<nsIContent> content = item->MenuObjectType() == eSubmenuObjectType
+                                     ? static_cast<nsMenuX*>(item)->Content()
+                                     : static_cast<nsMenuItemX*>(item)->Content();
+    if (!nsMenuUtilsX::NodeIsHiddenOrCollapsed(content)) {
       if (aPos == visibleNodeIndex) {
         // we found the visible node we're looking for, return it
         return item;

widget/cocoa/nsStandaloneNativeMenu.mm

@@ -6,6 +6,7 @@
 #import <Cocoa/Cocoa.h>
 
 #include "nsStandaloneNativeMenu.h"
+#include "nsMenuItemX.h"
 #include "nsMenuUtilsX.h"
 #include "nsIMutationObserver.h"
 #include "nsGkAtoms.h"
@@ -144,7 +145,12 @@ nsStandaloneNativeMenu::ForceUpdateNativeMenuAt(const nsAString& indexString) {
       if (!targetMenu) {
         return NS_OK;
       }
-      if (!nsMenuUtilsX::NodeIsHiddenOrCollapsed(targetMenu->Content())) {
+      MOZ_RELEASE_ASSERT(targetMenu->MenuObjectType() == eSubmenuObjectType ||
+                         targetMenu->MenuObjectType() == eMenuItemObjectType);
+      RefPtr<nsIContent> content = targetMenu->MenuObjectType() == eSubmenuObjectType
+                                       ? static_cast<nsMenuX*>(targetMenu)->Content()
+                                       : static_cast<nsMenuItemX*>(targetMenu)->Content();
+      if (!nsMenuUtilsX::NodeIsHiddenOrCollapsed(content)) {
         visible++;
         if (targetMenu->MenuObjectType() == eSubmenuObjectType && visible == (targetIndex + 1)) {
           currentMenu = static_cast<nsMenuX*>(targetMenu);