Fix leak regression from bug 368773 (Add a bunch of DOM objects to cycle collection). r/sr=bz.

embedding/components/commandhandler/src/nsCommandManager.cpp

@@ -53,7 +53,7 @@
 #include "nsIDOMWindowInternal.h"
 #include "nsIFocusController.h"
 
-#include "nsSupportsArray.h"
+#include "nsCOMArray.h"
 
 #include "nsCommandManager.h"
 
@@ -70,30 +70,27 @@ nsCommandManager::~nsCommandManager()
 }
 
 
-PR_STATIC_CALLBACK(PRBool)
+PR_STATIC_CALLBACK(PLDHashOperator)
-TraverseCommandObservers(nsHashKey *aKey, void *aData, void* aClosure)
+TraverseCommandObservers(const char* aKey, nsCOMArray<nsIObserver>* aObservers,
+                         void* aClosure)
 {
-  nsISupportsArray *observers = NS_STATIC_CAST(nsISupportsArray*, aData);
   nsCycleCollectionTraversalCallback *cb = 
     NS_STATIC_CAST(nsCycleCollectionTraversalCallback*, aClosure);
 
-  PRUint32 i, numItems;
+  PRInt32 i, numItems = aObservers->Count();
-  nsresult rv = observers->Count(&numItems);
-  NS_ENSURE_SUCCESS(rv, kHashEnumerateStop);
-  
   for (i = 0; i < numItems; ++i) {
-    cb->NoteXPCOMChild(observers->ElementAt(i));
+    cb->NoteXPCOMChild(aObservers->ObjectAt(i));
   }
 
-  return kHashEnumerateNext;
+  return PL_DHASH_NEXT;
 }
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(nsCommandManager)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsCommandManager)
-  tmp->mCommandObserversTable.Reset();
+  tmp->mObserversTable.Clear();
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsCommandManager)
-  tmp->mCommandObserversTable.Enumerate(TraverseCommandObservers, &cb);
+  tmp->mObserversTable.EnumerateRead(TraverseCommandObservers, &cb);
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF_AMBIGUOUS(nsCommandManager, nsICommandManager)
@@ -126,31 +123,22 @@ nsCommandManager::Init(nsIDOMWindow *aWindow)
 NS_IMETHODIMP
 nsCommandManager::CommandStatusChanged(const char * aCommandName)
 {
-	nsCStringKey hashKey(aCommandName);
+  nsCOMArray<nsIObserver>* commandObservers;
+  mObserversTable.Get(aCommandName, &commandObservers);
 
-  
+  if (commandObservers)
-  nsresult rv = NS_OK;
+  {
-	nsCOMPtr<nsISupports>       commandSupports  = getter_AddRefs(mCommandObserversTable.Get(&hashKey));
+    // XXX Should we worry about observers removing themselves from Observe()?
-	nsCOMPtr<nsISupportsArray>  commandObservers = do_QueryInterface(commandSupports);
+    PRInt32 i, numItems = commandObservers->Count();
-	if (commandObservers)
+    for (i = 0; i < numItems;  ++i)
-	{
+    {
-	  PRUint32  numItems;
+      nsCOMPtr<nsIObserver> observer = commandObservers->ObjectAt(i);
-	  rv = commandObservers->Count(&numItems);
+      // should we get the command state to pass here? This might be expensive.
-	  if (NS_FAILED(rv)) return rv;
+      observer->Observe(NS_ISUPPORTS_CAST(nsICommandManager*, this),
-	  
+                        aCommandName,
-	  for (PRUint32 i = 0; i < numItems; i ++)
+                        NS_LITERAL_STRING("command_status_changed").get());
-	  {
+    }
-	    nsCOMPtr<nsISupports>   itemSupports;
+  }
-	    rv = commandObservers->GetElementAt(i, getter_AddRefs(itemSupports));
-	    if (NS_FAILED(rv)) break;
-      nsCOMPtr<nsIObserver>   itemObserver = do_QueryInterface(itemSupports);
-      if (itemObserver)
-      {
-      	// should we get the command state to pass here? This might be expensive.
-        itemObserver->Observe((nsICommandManager *)this, aCommandName,NS_LITERAL_STRING("command_status_changed").get());
-      }
-	  }
-	}
 
   return NS_OK;
 }
@@ -163,32 +151,27 @@ nsCommandManager::CommandStatusChanged(const char * aCommandName)
 NS_IMETHODIMP
 nsCommandManager::AddCommandObserver(nsIObserver *aCommandObserver, const char *aCommandToObserve)
 {
-	NS_ENSURE_ARG(aCommandObserver);
+  NS_ENSURE_ARG(aCommandObserver);
-	
-	nsresult rv = NS_OK;
 
-	// XXX todo: handle special cases of aCommandToObserve being null, or empty
+  nsresult rv = NS_OK;
-	
+
-	// for each command in the table, we make a list of observers for that command
+  // XXX todo: handle special cases of aCommandToObserve being null, or empty
-  nsCStringKey hashKey(aCommandToObserve);
+
-	
+  // for each command in the table, we make a list of observers for that command
-	nsCOMPtr<nsISupports>       commandSupports  = getter_AddRefs(mCommandObserversTable.Get(&hashKey));
+  nsCOMArray<nsIObserver>* commandObservers;
-	nsCOMPtr<nsISupportsArray>  commandObservers = do_QueryInterface(commandSupports);
+  if (!mObserversTable.Get(aCommandToObserve, &commandObservers))
-	if (!commandObservers)
+  {
-	{
+    nsAutoPtr<nsCOMArray<nsIObserver> > array(new nsCOMArray<nsIObserver>);
-	  rv = NS_NewISupportsArray(getter_AddRefs(commandObservers));
+    if (!array || !mObserversTable.Put(aCommandToObserve, array))
-  	if (NS_FAILED(rv)) return rv;
+      return NS_ERROR_OUT_OF_MEMORY;
-  	
+
-  	commandSupports = do_QueryInterface(commandObservers);
+    commandObservers = array.forget();
-  	rv = mCommandObserversTable.Put(&hashKey, commandSupports);
+  }
-  	if (NS_FAILED(rv)) return rv;
+
-	}
+  // need to check that this command observer hasn't already been registered
-	
+  PRInt32 existingIndex = commandObservers->IndexOf(aCommandObserver);
-	// need to check that this command observer hasn't already been registered
-	nsCOMPtr<nsISupports> observerAsSupports = do_QueryInterface(aCommandObserver);
-	PRInt32 existingIndex = commandObservers->IndexOf(observerAsSupports);
   if (existingIndex == -1)
-    rv = commandObservers->AppendElement(observerAsSupports);
+    rv = commandObservers->AppendObject(aCommandObserver);
   else
     NS_WARNING("Registering command observer twice on the same command");
   
@@ -199,19 +182,16 @@ nsCommandManager::AddCommandObserver(nsIObserver *aCommandObserver, const char *
 NS_IMETHODIMP
 nsCommandManager::RemoveCommandObserver(nsIObserver *aCommandObserver, const char *aCommandObserved)
 {
-	NS_ENSURE_ARG(aCommandObserver);
+  NS_ENSURE_ARG(aCommandObserver);
 
-	// XXX todo: handle special cases of aCommandToObserve being null, or empty
+  // XXX todo: handle special cases of aCommandToObserve being null, or empty
-	nsCStringKey hashKey(aCommandObserved);
 
-	nsCOMPtr<nsISupports>       commandSupports  = getter_AddRefs(mCommandObserversTable.Get(&hashKey));
+  nsCOMArray<nsIObserver>* commandObservers;
-	nsCOMPtr<nsISupportsArray>  commandObservers = do_QueryInterface(commandSupports);
+  if (!mObserversTable.Get(aCommandObserved, &commandObservers))
-	if (!commandObservers)
+    return NS_ERROR_UNEXPECTED;
-	  return NS_ERROR_UNEXPECTED;
+
-	
+  return commandObservers->RemoveObject(aCommandObserver) ? NS_OK :
-	nsCOMPtr<nsISupports> observerAsSupports = do_QueryInterface(aCommandObserver);
+                                                            NS_ERROR_FAILURE;
-	nsresult removed = commandObservers->RemoveElement(observerAsSupports);
-	return (removed) ? NS_OK : NS_ERROR_FAILURE;
 }
 
 /* boolean isCommandSupported(in string aCommandName,

embedding/components/commandhandler/src/nsCommandManager.h

@@ -41,7 +41,7 @@
 
 
 #include "nsString.h"
-#include "nsHashtable.h"
+#include "nsClassHashtable.h"
 #include "nsWeakReference.h"
 
 #include "nsICommandManager.h"
@@ -49,7 +49,7 @@
 #include "nsCycleCollectionParticipant.h"
 
 class nsIController;
-
+template<class E> class nsCOMArray;
 
 
 class nsCommandManager :  public nsICommandManager,
@@ -84,7 +84,7 @@ protected:
 
 protected:
 
-  nsSupportsHashtable   mCommandObserversTable;   // hash table of nsIObservers, keyed by command name
+  nsClassHashtable<nsCharPtrHashKey, nsCOMArray<nsIObserver> > mObserversTable;
 
   nsIDOMWindow*         mWindow;      // weak ptr. The window should always outlive us
 };