correctly swap shared Application menu when painting menu bars. also remove nsSupportsArray usage from nsMenuBarX. b=316076 sr=pinkerton

widget/src/cocoa/nsMenuBarX.h

@@ -45,7 +45,7 @@
 #include "nsIChangeManager.h"
 #include "nsIMenuCommandDispatcher.h"
 #include "nsPresContext.h"
-#include "nsSupportsArray.h"
+#include "nsCOMArray.h"
 #include "nsHashtable.h"
 #include "nsWeakReference.h"
 #include "nsIContent.h"
@@ -137,7 +137,7 @@ protected:
     nsHashtable             mObserverTable;       // stores observers for content change notification
 
     PRUint32                mNumMenus;
-    nsSupportsArray         mMenusArray;          // holds refs
+    nsCOMArray<nsIMenu>     mMenusArray;          // holds refs
     nsCOMPtr<nsIContent>    mMenuBarContent;      // menubar content node, strong ref
     nsCOMPtr<nsIContent>    mPrefItemContent;     // on X, holds the content node for the prefs item that has
                                                   // been removed from the menubar

widget/src/cocoa/nsMenuBarX.mm

@@ -105,12 +105,11 @@ nsMenuBarX::MenuItemSelected(const nsMenuEvent &aMenuEvent)
   // Dispatch menu event
   nsEventStatus eventStatus = nsEventStatus_eIgnore;
   
-  PRUint32 numItems;
-  mMenusArray.Count(&numItems);
+  PRUint32 numItems = mMenusArray.Count();
   
   for (PRUint32 i = numItems; i > 0; i--) {
-    nsCOMPtr<nsISupports>     menuSupports = getter_AddRefs(mMenusArray.ElementAt(i - 1));
-    nsCOMPtr<nsIMenuListener> menuListener = do_QueryInterface(menuSupports);
+    nsCOMPtr<nsIMenu> menu                 = mMenusArray.ObjectAt(i - 1);
+    nsCOMPtr<nsIMenuListener> menuListener = do_QueryInterface(menu);
     if (menuListener) {
       eventStatus = menuListener->MenuItemSelected(aMenuEvent);
       if (nsEventStatus_eIgnore != eventStatus)
@@ -136,11 +135,10 @@ nsMenuBarX::MenuSelected(const nsMenuEvent &aMenuEvent)
       return eventStatus;
   }
   else {
-    PRUint32  numItems;
-    mMenusArray.Count(&numItems);
+    PRUint32 numItems = mMenusArray.Count();
     for (PRUint32 i = numItems; i > 0; i--) {
-      nsCOMPtr<nsISupports>     menuSupports = getter_AddRefs(mMenusArray.ElementAt(i - 1));
-      nsCOMPtr<nsIMenuListener> thisListener = do_QueryInterface(menuSupports);
+      nsCOMPtr<nsIMenu> menu                 = mMenusArray.ObjectAt(i - 1);
+      nsCOMPtr<nsIMenuListener> thisListener = do_QueryInterface(menu);
       if (thisListener) {
         //TODO: MenuSelected is the right thing to call...
         //eventStatus = menuListener->MenuSelected(aMenuEvent);
@@ -493,27 +491,22 @@ NS_IMETHODIMP nsMenuBarX::SetParent(nsIWidget *aParent)
 
 NS_IMETHODIMP nsMenuBarX::AddMenu(nsIMenu * aMenu)
 {
-  // keep track of all added menus.
-  mMenusArray.AppendElement(aMenu); // owner
+  // keep track of all added menus
+  mMenusArray.AppendObject(aMenu); // owner
   
-  if (mNumMenus == 0) {
-    // if application menu hasn't been created, create it.
+  // if no menus have been added yet, add a menu item as a placeholder for
+  // the application menu (the NSMenu of which gets swapped in on Paint())
+  if (mNumMenus == 0) {    
+    [mRootMenu insertItem:[[[NSMenuItem alloc] initWithTitle:@"AppMenu" action:NULL keyEquivalent:@""] autorelease] atIndex:mNumMenus];
+    mNumMenus++;
+    
+    // if we haven't generated an application menu yet, then we can use this
+    // nsIMenu to create one
     if (!sApplicationMenu) {
       nsresult rv = NS_OK; // avoid warning about rv being unused
       rv = CreateApplicationMenu(aMenu);
       NS_ASSERTION(NS_SUCCEEDED(rv), "Can't create Application menu");
     }
-    
-    // add shared Application menu to our menubar
-    if (sApplicationMenu) {
-      [mRootMenu insertItem:[[[NSMenuItem alloc] initWithTitle:@"AppMenu" action:NULL keyEquivalent:@""] autorelease] atIndex:mNumMenus];
-      // an NSMenu can't have multiple supermenus, so we clone the shared menu and insert the clone.
-      // this is actually a bad solution since we want any mods to the application menu to affect all
-      // application menus. Really we should unhook the application menu when we switch menu bars.
-      [[mRootMenu itemAtIndex:0] setSubmenu:[sApplicationMenu copy]]; //XXXJOSH memory leak?
-      // |mNumMenus| is incremented so the following menu won't overwrite the application menu by reusing the ID.
-      mNumMenus++;
-    }
   }
 
   PRBool helpMenu;
@@ -593,10 +586,11 @@ NS_IMETHODIMP nsMenuBarX::GetMenuCount(PRUint32 &aCount)
 NS_IMETHODIMP nsMenuBarX::GetMenuAt(const PRUint32 aCount, nsIMenu *& aMenu)
 { 
   aMenu = NULL;
-  nsCOMPtr<nsISupports> supports = getter_AddRefs(mMenusArray.ElementAt(aCount));
-  if (!supports) return NS_OK;
+  nsCOMPtr<nsIMenu> menu = mMenusArray.ObjectAt(aCount);
+  if (!menu)
+    return NS_OK;
   
-  return CallQueryInterface(supports, &aMenu); // addref
+  return CallQueryInterface(menu, &aMenu); // addref
 }
 
 //-------------------------------------------------------------------------
@@ -608,7 +602,7 @@ NS_IMETHODIMP nsMenuBarX::InsertMenuAt(const PRUint32 aCount, nsIMenu *& aMenu)
 //-------------------------------------------------------------------------
 NS_IMETHODIMP nsMenuBarX::RemoveMenu(const PRUint32 aCount)
 {
-  mMenusArray.RemoveElementAt(aCount);
+  mMenusArray.RemoveObjectAt(aCount);
   [mRootMenu removeItemAtIndex:aCount];
   return NS_OK;
 }
@@ -634,7 +628,23 @@ NS_IMETHODIMP nsMenuBarX::SetNativeData(void* aData)
 
 //-------------------------------------------------------------------------
 NS_IMETHODIMP nsMenuBarX::Paint()
-{  
+{
+  // swap in the shared Application menu
+  // if application menu hasn't been created, create it.
+  if (sApplicationMenu) {
+    // an NSMenu can't have multiple supermenus, so when we paint a menu bar we unhook the
+    // application menu from its current supermenu and hook it up to this menu bar's
+    // application menu item. This way all changes to the application menu perist across
+    // all instances of nsMenuBarX. We could assume 0 for indexOfItemWithSubmenu, but lets
+    // be safe... If the algorithm for that starts looking at 0 it will still be fast.
+    NSMenu* supermenu = [sApplicationMenu supermenu];
+    if (supermenu) {
+      int supermenuItemIndex = [supermenu indexOfItemWithSubmenu:sApplicationMenu];
+      [[supermenu itemAtIndex:supermenuItemIndex] setSubmenu:nil];
+    }
+    [[mRootMenu itemAtIndex:0] setSubmenu:sApplicationMenu];
+  }
+  
   [NSApp setMainMenu:mRootMenu];  
   return NS_OK;
 }