Fixed nsSupportsHashtable behaviour and API to fix leaks in

nsDirectoryService. Fixes bugs #38606 and #39859.

a=waterson; r=rayw,brendan,warren

chrome/src/nsChromeRegistry.cpp

@@ -855,7 +855,7 @@ NS_IMETHODIMP nsChromeRegistry::LoadDataSource(const nsCString &aFileName,
   
   nsCOMPtr<nsISupports> supports = do_QueryInterface(remote);
   nsStringKey skey(key);
-  mDataSourceTable->Put(&skey, (void*)supports.get());
+  mDataSourceTable->Put(&skey, supports.get());
 
   return NS_OK;
 }

content/events/src/nsEventStateManager.cpp

@@ -2444,8 +2444,7 @@ nsEventStateManager::RegisterAccessKey(nsIFrame * aFrame, nsIContent* aContent,
   if (content) {
     nsVoidKey key((void*)aKey);
 
-    nsIContent* oldContent = NS_STATIC_CAST(nsIContent*, mAccessKeys->Put(&key, content));
+    mAccessKeys->Put(&key, content);
-    NS_IF_RELEASE(oldContent);
   }
 
   return NS_OK;

content/xbl/src/nsBindingManager.cpp

@@ -126,8 +126,7 @@ nsBindingManager::SetBinding(nsIContent* aContent, nsIXBLBinding* aBinding )
 
   nsISupportsKey key(aContent);
   if (aBinding) {
-    nsIXBLBinding* oldBinding = NS_STATIC_CAST(nsIXBLBinding*, mBindingTable->Put(&key, aBinding));
+    mBindingTable->Put (&key, aBinding);
-    NS_IF_RELEASE(oldBinding);
   }
   else
     mBindingTable->Remove(&key);

content/xul/document/src/nsControllerCommandManager.cpp

@@ -52,16 +52,14 @@ nsControllerCommandManager::RegisterCommand(const PRUnichar *commandName, nsICon
 {
   nsStringKey commandKey(commandName);
   
-  void* replacedCmd = mCommandsTable.Put(&commandKey, (void*)aCommand);
+  if (mCommandsTable.Put (&commandKey, aCommand))
-#if DEBUG
-  if (replacedCmd)
   {
+#if DEBUG
     nsCAutoString msg("Replacing existing command -- ");
     msg.AppendWithConversion(commandName);
     NS_WARNING(msg);
-  }
 #endif
-  
+  }  
   return NS_OK;
 }
 
@@ -71,8 +69,8 @@ nsControllerCommandManager::UnregisterCommand(const PRUnichar *commandName, nsIC
 {
   nsStringKey commandKey(commandName);
 
-  void* foundCommand = mCommandsTable.Remove(&commandKey);
+  PRBool any_object_actually_removed_p = mCommandsTable.Remove (&commandKey);
-  return (foundCommand) ? NS_OK : NS_ERROR_FAILURE;
+  return any_object_actually_removed_p? NS_OK : NS_ERROR_FAILURE;
 }
 
 
@@ -84,7 +82,7 @@ nsControllerCommandManager::FindCommandHandler(const PRUnichar *aCommandName, ns
   *outCommand = NULL;
   
   nsStringKey commandKey(aCommandName);
-  void* foundCommand = mCommandsTable.Get(&commandKey);   // this does the addref
+  nsISupports* foundCommand = mCommandsTable.Get(&commandKey);   // this does the addref
   if (!foundCommand) return NS_ERROR_FAILURE;
   
   *outCommand = NS_REINTERPRET_CAST(nsIControllerCommand*, foundCommand);

content/xul/document/src/nsXULPrototypeCache.cpp

@@ -159,8 +159,9 @@ nsXULPrototypeCache::PutPrototype(nsIXULPrototypeDocument* aDocument)
     rv = aDocument->GetURI(getter_AddRefs(uri));
 
     nsIURIKey key(uri);
-    nsIXULPrototypeDocument* oldproto = NS_STATIC_CAST(nsIXULPrototypeDocument*, mPrototypeTable.Put(&key, aDocument));
+        // Put() w/o  a third parameter with a destination for the
-    NS_IF_RELEASE(oldproto);
+        // replaced value releases it 
+    mPrototypeTable.Put(&key, aDocument);
 
     return NS_OK;
 }
@@ -191,8 +192,7 @@ nsXULPrototypeCache::PutStyleSheet(nsICSSStyleSheet* aStyleSheet)
     rv = aStyleSheet->GetURL(*getter_AddRefs(uri));
 
     nsIURIKey key(uri);
-    nsICSSStyleSheet* oldsheet = NS_STATIC_CAST(nsICSSStyleSheet*, mStyleSheetTable.Put(&key, aStyleSheet));
+    mStyleSheetTable.Put(&key, aStyleSheet);
-    NS_IF_RELEASE(oldsheet);
 
     return NS_OK;
 }
@@ -214,8 +214,7 @@ nsXULPrototypeCache::PutXBLDocument(nsIDocument *aDocument)
     uri->GetSpec(&aString);
 
     nsStringKey key(aString);
-    nsIDocument* olddoc = NS_STATIC_CAST(nsIDocument*, mXBLDocTable.Put(&key, aDocument));
+    mXBLDocTable.Put(&key, aDocument);
-    NS_IF_RELEASE(olddoc);
 
     return NS_OK;
 }
@@ -236,8 +235,7 @@ nsXULPrototypeCache::PutXBLDocScriptAccess(nsIDocument *aDocument)
     uri->GetSpec(&aString);
 
     nsStringKey key(aString);
-    nsIDocument* olddoc = NS_STATIC_CAST(nsIDocument*, mScriptAccessTable.Put(&key, aDocument));
+    mScriptAccessTable.Put(&key, aDocument);
-    NS_IF_RELEASE(olddoc);
 
     return NS_OK;
 }

layout/base/nsLayoutHistoryState.cpp

@@ -115,21 +115,22 @@ nsLayoutHistoryState::AddState(PRUint32 aContentID,
                                nsIPresState* aState, 
                                nsIStatefulFrame::StateType aStateType)
 {
-  nsresult rv = NS_OK;
   HistoryKey key(aContentID, aStateType);
-  void * res = mStates.Put(&key, (void *) aState);
+  /*
-  /* nsHashTable seems to return null when it actually added
+   * nsSupportsHashtable::Put() returns false when no object has been
-   * the element in to the table. If another element was already
+   * replaced when inserting the new one, true if it some one was.
-   * present in the table for the key, it seems to return the 
+   *
-   * element that was already present. Not sure if that was
-   * the intended behavior
    */
-  if (res)  {
+  PRBool replaced = mStates.Put (&key, aState);
-    //printf("nsLayoutHistoryState::AddState OOPS!. There was already a state in the hash table for the key\n");
+  if (replaced)
-    rv = NS_OK;
+  {
+          // done this way by indication of warren@netscape.com [ipg]
+#if 0
+      printf("nsLayoutHistoryState::AddState OOPS!. There was already a state in the hash table for the key\n");
+#endif
   }
 
-  return rv;
+  return NS_OK;
 }
 
 NS_IMETHODIMP
@@ -139,13 +140,15 @@ nsLayoutHistoryState::GetState(PRUint32 aContentID,
 {
   nsresult rv = NS_OK;
   HistoryKey key(aContentID, aStateType);
-  void * state = nsnull;
+  nsISupports *state = nsnull;
   state = mStates.Get(&key);
   if (state) {
     *aState = (nsIPresState *)state;
   }
   else {
-    // printf("nsLayoutHistoryState::GetState, ERROR getting History state for the key\n");
+#if 0
+      printf("nsLayoutHistoryState::GetState, ERROR getting History state for the key\n");
+#endif
     *aState = nsnull;
     rv = NS_OK;
   }
@@ -158,7 +161,6 @@ nsLayoutHistoryState::RemoveState(PRUint32 aContentID,
 {
   nsresult rv = NS_OK;
   HistoryKey key(aContentID, aStateType);
-  void * state = nsnull;
+  mStates.Remove(&key);
-  state = mStates.Remove(&key);
   return rv;
 }

layout/base/src/nsLayoutHistoryState.cpp

@@ -115,21 +115,22 @@ nsLayoutHistoryState::AddState(PRUint32 aContentID,
                                nsIPresState* aState, 
                                nsIStatefulFrame::StateType aStateType)
 {
-  nsresult rv = NS_OK;
   HistoryKey key(aContentID, aStateType);
-  void * res = mStates.Put(&key, (void *) aState);
+  /*
-  /* nsHashTable seems to return null when it actually added
+   * nsSupportsHashtable::Put() returns false when no object has been
-   * the element in to the table. If another element was already
+   * replaced when inserting the new one, true if it some one was.
-   * present in the table for the key, it seems to return the 
+   *
-   * element that was already present. Not sure if that was
-   * the intended behavior
    */
-  if (res)  {
+  PRBool replaced = mStates.Put (&key, aState);
-    //printf("nsLayoutHistoryState::AddState OOPS!. There was already a state in the hash table for the key\n");
+  if (replaced)
-    rv = NS_OK;
+  {
+          // done this way by indication of warren@netscape.com [ipg]
+#if 0
+      printf("nsLayoutHistoryState::AddState OOPS!. There was already a state in the hash table for the key\n");
+#endif
   }
 
-  return rv;
+  return NS_OK;
 }
 
 NS_IMETHODIMP
@@ -139,13 +140,15 @@ nsLayoutHistoryState::GetState(PRUint32 aContentID,
 {
   nsresult rv = NS_OK;
   HistoryKey key(aContentID, aStateType);
-  void * state = nsnull;
+  nsISupports *state = nsnull;
   state = mStates.Get(&key);
   if (state) {
     *aState = (nsIPresState *)state;
   }
   else {
-    // printf("nsLayoutHistoryState::GetState, ERROR getting History state for the key\n");
+#if 0
+      printf("nsLayoutHistoryState::GetState, ERROR getting History state for the key\n");
+#endif
     *aState = nsnull;
     rv = NS_OK;
   }
@@ -158,7 +161,6 @@ nsLayoutHistoryState::RemoveState(PRUint32 aContentID,
 {
   nsresult rv = NS_OK;
   HistoryKey key(aContentID, aStateType);
-  void * state = nsnull;
+  mStates.Remove(&key);
-  state = mStates.Remove(&key);
   return rv;
 }

layout/events/src/nsEventStateManager.cpp

@@ -2444,8 +2444,7 @@ nsEventStateManager::RegisterAccessKey(nsIFrame * aFrame, nsIContent* aContent,
   if (content) {
     nsVoidKey key((void*)aKey);
 
-    nsIContent* oldContent = NS_STATIC_CAST(nsIContent*, mAccessKeys->Put(&key, content));
+    mAccessKeys->Put(&key, content);
-    NS_IF_RELEASE(oldContent);
   }
 
   return NS_OK;

layout/xbl/src/nsBindingManager.cpp

@@ -126,8 +126,7 @@ nsBindingManager::SetBinding(nsIContent* aContent, nsIXBLBinding* aBinding )
 
   nsISupportsKey key(aContent);
   if (aBinding) {
-    nsIXBLBinding* oldBinding = NS_STATIC_CAST(nsIXBLBinding*, mBindingTable->Put(&key, aBinding));
+    mBindingTable->Put (&key, aBinding);
-    NS_IF_RELEASE(oldBinding);
   }
   else
     mBindingTable->Remove(&key);

mailnews/db/msgdb/src/nsMsgDatabase.cpp

@@ -132,7 +132,13 @@ nsresult nsMsgDatabase::RemoveHdrFromCache(nsIMsgDBHdr *hdr, nsMsgKey key)
 		nsCAutoString strKey;
 		strKey.AppendInt(key, 10);
 		nsStringKey hashKey(strKey.GetBuffer());
-		nsIMsgDBHdr *removedHdr = (nsIMsgDBHdr *) m_cachedHeaders->Remove(&hashKey); // does this release, or do I have to?
+            /*
+             * this does release on the held object, unless you don't
+             * want it to, by passing as third argument a non-null pointer
+             * where to store it; this way you would get the ownership
+             * of the reference held by the table for that object.
+             */
+		m_cachedHeaders->Remove(&hashKey);
 	}
 	return NS_OK;
 }

netwerk/dns/src/nsDnsService.cpp

@@ -609,7 +609,7 @@ nsDNSLookup::InitiateLookup(void)
 
     // XXX remove from hashtable and release - for now
     nsStringKey key(mHostName);
-    (void) nsDNSService::gService->mLookups.Remove(&key);
+    nsDNSService::gService->mLookups.Remove(&key);
     
 #endif /* XP_UNIX */
 
@@ -1149,7 +1149,6 @@ nsDNSService::GetLookupEntry(const char* hostName,
                              nsDNSLookup* *result)
 {
     nsresult rv;
-    void* prev;
 
     nsAutoMonitor mon(mMonitor);
 
@@ -1176,9 +1175,11 @@ nsDNSService::GetLookupEntry(const char* hostName,
     rv = lookup->Init(hostName);
     if (NS_FAILED(rv)) goto done;
 
-	NS_ADDREF(lookup);
+    NS_ADDREF(lookup);   // reference for the caller
-    prev = mLookups.Put(&key, lookup);
+    if (mLookups.Put(&key, lookup))
-    NS_ASSERTION(prev == nsnull, "already a nsDNSLookup entry");
+    {
+        NS_ASSERTION (0, "already a nsDNSLookup entry");
+    }
 
 	*result = lookup;
   done:

rdf/chrome/src/nsChromeRegistry.cpp

@@ -855,7 +855,7 @@ NS_IMETHODIMP nsChromeRegistry::LoadDataSource(const nsCString &aFileName,
   
   nsCOMPtr<nsISupports> supports = do_QueryInterface(remote);
   nsStringKey skey(key);
-  mDataSourceTable->Put(&skey, (void*)supports.get());
+  mDataSourceTable->Put(&skey, supports.get());
 
   return NS_OK;
 }

rdf/content/src/nsControllerCommandManager.cpp

@@ -52,16 +52,14 @@ nsControllerCommandManager::RegisterCommand(const PRUnichar *commandName, nsICon
 {
   nsStringKey commandKey(commandName);
   
-  void* replacedCmd = mCommandsTable.Put(&commandKey, (void*)aCommand);
+  if (mCommandsTable.Put (&commandKey, aCommand))
-#if DEBUG
-  if (replacedCmd)
   {
+#if DEBUG
     nsCAutoString msg("Replacing existing command -- ");
     msg.AppendWithConversion(commandName);
     NS_WARNING(msg);
-  }
 #endif
-  
+  }  
   return NS_OK;
 }
 
@@ -71,8 +69,8 @@ nsControllerCommandManager::UnregisterCommand(const PRUnichar *commandName, nsIC
 {
   nsStringKey commandKey(commandName);
 
-  void* foundCommand = mCommandsTable.Remove(&commandKey);
+  PRBool any_object_actually_removed_p = mCommandsTable.Remove (&commandKey);
-  return (foundCommand) ? NS_OK : NS_ERROR_FAILURE;
+  return any_object_actually_removed_p? NS_OK : NS_ERROR_FAILURE;
 }
 
 
@@ -84,7 +82,7 @@ nsControllerCommandManager::FindCommandHandler(const PRUnichar *aCommandName, ns
   *outCommand = NULL;
   
   nsStringKey commandKey(aCommandName);
-  void* foundCommand = mCommandsTable.Get(&commandKey);   // this does the addref
+  nsISupports* foundCommand = mCommandsTable.Get(&commandKey);   // this does the addref
   if (!foundCommand) return NS_ERROR_FAILURE;
   
   *outCommand = NS_REINTERPRET_CAST(nsIControllerCommand*, foundCommand);

rdf/content/src/nsXULPrototypeCache.cpp

@@ -159,8 +159,9 @@ nsXULPrototypeCache::PutPrototype(nsIXULPrototypeDocument* aDocument)
     rv = aDocument->GetURI(getter_AddRefs(uri));
 
     nsIURIKey key(uri);
-    nsIXULPrototypeDocument* oldproto = NS_STATIC_CAST(nsIXULPrototypeDocument*, mPrototypeTable.Put(&key, aDocument));
+        // Put() w/o  a third parameter with a destination for the
-    NS_IF_RELEASE(oldproto);
+        // replaced value releases it 
+    mPrototypeTable.Put(&key, aDocument);
 
     return NS_OK;
 }
@@ -191,8 +192,7 @@ nsXULPrototypeCache::PutStyleSheet(nsICSSStyleSheet* aStyleSheet)
     rv = aStyleSheet->GetURL(*getter_AddRefs(uri));
 
     nsIURIKey key(uri);
-    nsICSSStyleSheet* oldsheet = NS_STATIC_CAST(nsICSSStyleSheet*, mStyleSheetTable.Put(&key, aStyleSheet));
+    mStyleSheetTable.Put(&key, aStyleSheet);
-    NS_IF_RELEASE(oldsheet);
 
     return NS_OK;
 }
@@ -214,8 +214,7 @@ nsXULPrototypeCache::PutXBLDocument(nsIDocument *aDocument)
     uri->GetSpec(&aString);
 
     nsStringKey key(aString);
-    nsIDocument* olddoc = NS_STATIC_CAST(nsIDocument*, mXBLDocTable.Put(&key, aDocument));
+    mXBLDocTable.Put(&key, aDocument);
-    NS_IF_RELEASE(olddoc);
 
     return NS_OK;
 }
@@ -236,8 +235,7 @@ nsXULPrototypeCache::PutXBLDocScriptAccess(nsIDocument *aDocument)
     uri->GetSpec(&aString);
 
     nsStringKey key(aString);
-    nsIDocument* olddoc = NS_STATIC_CAST(nsIDocument*, mScriptAccessTable.Put(&key, aDocument));
+    mScriptAccessTable.Put(&key, aDocument);
-    NS_IF_RELEASE(olddoc);
 
     return NS_OK;
 }

rdf/tests/domds/nsRDFDOMViewerUtils.cpp

@@ -42,7 +42,7 @@ nsDOMViewerObject::SetTarget(nsIRDFResource *aProperty,
                              nsIRDFNode *aValue)
 {
   nsISupportsKey propKey(aProperty);
-  targets.Put(&propKey, (void *)aValue);
+  targets.Put(&propKey, aValue);
 
   return NS_OK;
 }

xpcom/ds/nsHashtable.cpp

@@ -385,8 +385,8 @@ nsObjectHashtable::RemoveAndDelete(nsHashKey *aKey)
 ////////////////////////////////////////////////////////////////////////////////
 // nsSupportsHashtable: an nsHashtable where the elements are nsISupports*
 
-static PRBool  PR_CALLBACK 
+PRBool PR_CALLBACK
-_ReleaseElement(nsHashKey *aKey, void *aData, void* closure)
+nsSupportsHashtable::ReleaseElement(nsHashKey *aKey, void *aData, void* closure)
 {
     nsISupports* element = NS_STATIC_CAST(nsISupports*, aData);
     NS_IF_RELEASE(element);
@@ -395,18 +395,25 @@ _ReleaseElement(nsHashKey *aKey, void *aData, void* closure)
 
 nsSupportsHashtable::~nsSupportsHashtable()
 {
-    Enumerate(_ReleaseElement, nsnull);
+    Enumerate(ReleaseElement, nsnull);
 }
 
-void*
+    // Return if we overwrote something
-nsSupportsHashtable::Put(nsHashKey *aKey, void *aData)
+
+PRBool
+nsSupportsHashtable::Put (nsHashKey *aKey, nsISupports* aData, nsISupports **value)
 {
-    nsISupports* element = NS_REINTERPRET_CAST(nsISupports*, aData);
+    NS_IF_ADDREF(aData);
-    NS_IF_ADDREF(element);
+    void *prev = nsHashtable::Put(aKey, aData);
-    return nsHashtable::Put(aKey, aData);
+    nsISupports *old = NS_REINTERPRET_CAST(nsISupports *, prev);
+    if (value)  // pass own the ownership to the caller
+        *value = old;
+    else        // the caller doesn't care, we do
+        NS_IF_RELEASE(old);
+    return prev != nsnull;
 }
 
-void*
+nsISupports *
 nsSupportsHashtable::Get(nsHashKey *aKey)
 {
     void* data = nsHashtable::Get(aKey);
@@ -414,22 +421,25 @@ nsSupportsHashtable::Get(nsHashKey *aKey)
         return nsnull;
     nsISupports* element = NS_REINTERPRET_CAST(nsISupports*, data);
     NS_IF_ADDREF(element);
-    return data;
+    return element;
 }
 
-void*
+    // Return if we found something (useful for checks)
-nsSupportsHashtable::Remove(nsHashKey *aKey)
+
+PRBool
+nsSupportsHashtable::Remove(nsHashKey *aKey, nsISupports **value)
 {
     void* data = nsHashtable::Remove(aKey);
-    if (!data)
-        return nsnull;
     nsISupports* element = NS_STATIC_CAST(nsISupports*, data);
+    if (value)            // caller wants it
+        *value = element;
+    else                  // caller doesn't care, we do
     NS_IF_RELEASE(element);
-    return data;
+    return data != nsnull;
 }
 
-static PRIntn  PR_CALLBACK 
+PRIntn  PR_CALLBACK 
-_hashEnumerateCopy(PLHashEntry *he, PRIntn i, void *arg)
+nsSupportsHashtable::EnumerateCopy(PLHashEntry *he, PRIntn i, void *arg)
 {
     nsHashtable *newHashtable = (nsHashtable *)arg;
     nsISupports* element = NS_STATIC_CAST(nsISupports*, he->value);
@@ -447,14 +457,14 @@ nsSupportsHashtable::Clone()
     nsSupportsHashtable* newHashTable = 
         new nsSupportsHashtable(hashtable->nentries, threadSafe);
 
-    PL_HashTableEnumerateEntries(hashtable, _hashEnumerateCopy, newHashTable);
+    PL_HashTableEnumerateEntries(hashtable, EnumerateCopy, newHashTable);
     return newHashTable;
 }
 
 void 
 nsSupportsHashtable::Reset()
 {
-    Enumerate(_ReleaseElement, nsnull);
+    Enumerate(ReleaseElement, nsnull);
     nsHashtable::Reset();
 }
 

xpcom/ds/nsHashtable.h

@@ -59,7 +59,7 @@ protected:
 
 public:
   nsHashtable(PRUint32 aSize = 256, PRBool threadSafe = PR_FALSE);
-  ~nsHashtable();
+  virtual ~nsHashtable();
 
   PRInt32 Count(void) { return hashtable->nentries; }
   PRBool Exists(nsHashKey *aKey);
@@ -103,17 +103,41 @@ protected:
 ////////////////////////////////////////////////////////////////////////////////
 // nsSupportsHashtable: an nsHashtable where the elements are nsISupports*
 
-class NS_COM nsSupportsHashtable : public nsHashtable {
+class nsISupports;
+
+class NS_COM nsSupportsHashtable
+ : private nsHashtable
+{
 public:
+  typedef PRBool (* PR_CALLBACK EnumFunc) (nsHashKey *aKey, void *aData, void* closure);
+
   nsSupportsHashtable(PRUint32 aSize = 256, PRBool threadSafe = PR_FALSE)
     : nsHashtable(aSize, threadSafe) {}
   ~nsSupportsHashtable();
 
-  void *Put(nsHashKey *aKey, void *aData);
+  PRInt32 Count (void)
-  void *Get(nsHashKey *aKey);
+  {
-  void *Remove(nsHashKey *aKey);
+    return nsHashtable::Count();
+  }
+  PRBool Exists (nsHashKey *aKey)
+  {
+    return nsHashtable::Exists (aKey);
+  }
+  PRBool Put (nsHashKey *aKey,
+              nsISupports *aData,
+              nsISupports **value = NULL);
+  nsISupports* Get (nsHashKey *aKey);
+  PRBool Remove (nsHashKey *aKey, nsISupports **value = NULL);
   nsHashtable *Clone();
+  void Enumerate (EnumFunc aEnumFunc, void* closure = NULL)
+  {
+    nsHashtable::Enumerate (aEnumFunc, closure);
+  }
   void Reset();
+
+ private:
+  static PRBool PR_CALLBACK ReleaseElement (nsHashKey *, void *, void *);
+  static PRIntn PR_CALLBACK EnumerateCopy (PLHashEntry *, PRIntn, void *);
 };
 
 ////////////////////////////////////////////////////////////////////////////////

xpcom/ds/nsStopwatch.cpp

@@ -100,12 +100,11 @@ NS_IMETHODIMP
 nsStopwatchService::Define(const char *prop, nsISupports *initialValue)
 {
     nsStringKey key(prop);
-    nsCOMPtr<nsIStopwatch> prev = (nsStopwatch*)mStopwatches.Get(&key);
+    if (mStopwatches.Put(&key, initialValue))
-    NS_ASSERTION(prev == nsnull, "stopwatch redefinition");
+    {
-    if (prev != nsnull)
+        NS_ASSERTION(0, "stopwatch redefinition");
         return NS_ERROR_FAILURE;
-
+    }
-    mStopwatches.Put(&key, initialValue);
     return NS_OK;
 }
 
@@ -113,12 +112,11 @@ NS_IMETHODIMP
 nsStopwatchService::Undefine(const char *prop)
 {
     nsStringKey key(prop);
-    nsCOMPtr<nsIStopwatch> prev = (nsStopwatch*)mStopwatches.Get(&key);
+    if (!mStopwatches.Remove(&key))
-    NS_ASSERTION(prev != nsnull, "stopwatch undefined");
+    {
-    if (prev == nsnull)
+        NS_ASSERTION(0, "stopwatch undefined");
         return NS_ERROR_FAILURE;
-
+    }
-    mStopwatches.Remove(&key);
     return NS_OK;
 }
 
@@ -137,12 +135,11 @@ NS_IMETHODIMP
 nsStopwatchService::Set(const char *prop, nsISupports *value)
 {
     nsStringKey key(prop);
-    nsCOMPtr<nsISupports> prev = (nsStopwatch*)mStopwatches.Get(&key);
+    if (!mStopwatches.Put(&key, value))
-    NS_ASSERTION(prev != nsnull, "stopwatch undefined");
+    {
-    if (prev == nsnull)
+        NS_ASSERTION (0, "stopwatch undefined");
         return NS_ERROR_FAILURE;
-
+    }
-    mStopwatches.Put(&key, value);
     return NS_OK;
 }
 

xpcom/io/nsDirectoryService.cpp

@@ -506,8 +506,7 @@ nsDirectoryService::Undefine(const char* prop)
     if (!mHashtable->Exists(&key))
         return NS_ERROR_FAILURE;
 
-    nsISupports* prevValue = (nsISupports*)mHashtable->Remove(&key);
+    mHashtable->Remove (&key);
-    NS_IF_RELEASE(prevValue);
     return NS_OK;
 }
 
@@ -566,7 +565,7 @@ nsDirectoryService::Get(const char* prop, const nsIID & uuid, void* *result)
     if (mHashtable->Exists(&key))
     {
       nsCOMPtr<nsIFile> ourFile;
-      nsISupports* value = (nsISupports*)mHashtable->Get(&key);
+      nsCOMPtr<nsISupports> value = getter_AddRefs (mHashtable->Get(&key));
       
       if (value && NS_SUCCEEDED(value->QueryInterface(NS_GET_IID(nsIFile), getter_AddRefs(ourFile))))
       {
@@ -590,12 +589,9 @@ nsDirectoryService::Set(const char* prop, nsISupports* value)
     value->QueryInterface(NS_GET_IID(nsIFile), getter_AddRefs(ourFile));
     if (ourFile)
     {
-        nsIFile* cloneFile;
+      nsCOMPtr<nsIFile> cloneFile;
-        ourFile->Clone(&cloneFile);
+      ourFile->Clone (getter_AddRefs (cloneFile));
-
+      mHashtable->Put(&key, cloneFile);
-        nsISupports* prevValue = (nsISupports*)mHashtable->Put(&key, 
-			                                                   NS_STATIC_CAST(nsISupports*,cloneFile));
-        NS_IF_RELEASE(prevValue);
         return NS_OK;
     }
     return NS_ERROR_FAILURE;   

xpcom/io/nsDirectoryService.h

@@ -53,7 +53,7 @@ class nsDirectoryService : public nsIDirectoryService, public nsIProperties, pub
 private:
     static nsDirectoryService* mService;
     static PRBool PR_CALLBACK ReleaseValues(nsHashKey* key, void* data, void* closure);
-    nsHashtable* mHashtable;
+    nsSupportsHashtable* mHashtable;
     nsCOMPtr<nsISupportsArray> mProviders;
 
     static nsIAtom *sCurrentProcess;