Bug 753687 - nsCategoryCache implementation doesn't free old category entries if their contract mapping is removed using .unregisterFactory. Store the factory objects directly in the map, instead of keeping both a map and a separate list. r=froydnj

content/base/src/nsContentPolicy.cpp

@@ -117,7 +117,8 @@ nsContentPolicy::CheckPolicy(CPMethod          policyMethod,
      * their permissions.
      */
     nsresult rv;
-    const nsCOMArray<nsIContentPolicy>& entries = mPolicies.GetEntries();
+    nsCOMArray<nsIContentPolicy> entries;
+    mPolicies.GetEntries(entries);
     int32_t count = entries.Count();
     for (int32_t i = 0; i < count; i++) {
         /* check the appropriate policy */

modules/libpref/src/Preferences.cpp

@@ -15,6 +15,7 @@
 
 #include "mozilla/Preferences.h"
 #include "nsAppDirectoryServiceDefs.h"
+#include "nsDataHashtable.h"
 #include "nsDirectoryServiceDefs.h"
 #include "nsICategoryManager.h"
 #include "nsCategoryManagerUtils.h"

netwerk/base/public/nsNetUtil.h

@@ -2326,7 +2326,8 @@ NS_SniffContent(const char* aSnifferType, nsIRequest* aRequest,
     return;
   }
 
-  const nsCOMArray<nsIContentSniffer>& sniffers = cache->GetEntries();
+  nsCOMArray<nsIContentSniffer> sniffers;
+  cache->GetEntries(sniffers);
   for (int32_t i = 0; i < sniffers.Count(); ++i) {
     nsresult rv = sniffers[i]->GetMIMETypeFromContent(aRequest, aData, aLength, aSniffedType);
     if (NS_SUCCEEDED(rv) && !aSniffedType.IsEmpty()) {

netwerk/base/src/nsIOService.cpp

@@ -319,8 +319,8 @@ nsIOService::AsyncOnChannelRedirect(nsIChannel* oldChan, nsIChannel* newChan,
     }
 
     // Finally, our category
-    const nsCOMArray<nsIChannelEventSink>& entries =
-        mChannelEventSinks.GetEntries();
+    nsCOMArray<nsIChannelEventSink> entries;
+    mChannelEventSinks.GetEntries(entries);
     int32_t len = entries.Count();
     for (int32_t i = 0; i < len; ++i) {
         nsresult rv = helper->DelegateOnChannelRedirect(entries[i], oldChan,

storage/src/VacuumManager.cpp

@@ -364,8 +364,8 @@ VacuumManager::Observe(nsISupports *aSubject,
   if (strcmp(aTopic, OBSERVER_TOPIC_IDLE_DAILY) == 0) {
     // Try to run vacuum on all registered entries.  Will stop at the first
     // successful one.
-    const nsCOMArray<mozIStorageVacuumParticipant> &entries =
-      mParticipants.GetEntries();
+    nsCOMArray<mozIStorageVacuumParticipant> entries;
+    mParticipants.GetEntries(entries);
     // If there are more entries than what a month can contain, we could end up
     // skipping some, since we run daily.  So we use a starting index.
     static const char* kPrefName = PREF_VACUUM_BRANCH "index";

toolkit/components/places/nsPlacesMacros.h

@@ -16,7 +16,8 @@
 #define NOTIFY_OBSERVERS(canFire, cache, array, type, method)                  \
   PR_BEGIN_MACRO                                                               \
   if (canFire) {                                                               \
-    const nsCOMArray<type> &entries = cache.GetEntries();                      \
+    nsCOMArray<type> entries;                                                  \
+    cache.GetEntries(entries);                                                 \
     for (int32_t idx = 0; idx < entries.Count(); ++idx)                        \
         entries[idx]->method;                                                  \
     ENUMERATE_WEAKARRAY(array, type, method)                                   \

widget/xpwidgets/nsIdleService.cpp

@@ -105,7 +105,8 @@ nsIdleServiceDaily::Observe(nsISupports *,
                                          nullptr);
 
   // Notify the category observers.
-  const nsCOMArray<nsIObserver> &entries = mCategoryObservers.GetEntries();
+  nsCOMArray<nsIObserver> entries;
+  mCategoryObservers.GetEntries(entries);
   for (int32_t i = 0; i < entries.Count(); ++i) {
     (void)entries[i]->Observe(nullptr, OBSERVER_TOPIC_IDLE_DAILY, nullptr);
   }

xpcom/glue/nsCategoryCache.cpp

@@ -6,17 +6,16 @@
 #include "nsIObserverService.h"
 #include "mozilla/Services.h"
 #include "nsISupportsPrimitives.h"
+#include "nsIStringEnumerator.h"
 
 #include "nsXPCOMCID.h"
 
 #include "nsCategoryCache.h"
 
-nsCategoryObserver::nsCategoryObserver(const char* aCategory,
-                                       nsCategoryListener* aListener)
-  : mListener(nullptr), mCategory(aCategory), mObserversRemoved(false)
+nsCategoryObserver::nsCategoryObserver(const char* aCategory)
+  : mCategory(aCategory)
+  , mObserversRemoved(false)
 {
-  mListener = aListener;
-
   // First, enumerate the currently existing entries
   nsCOMPtr<nsICategoryManager> catMan =
     do_GetService(NS_CATEGORYMANAGER_CONTRACTID);
@@ -29,23 +28,22 @@ nsCategoryObserver::nsCategoryObserver(const char* aCategory,
   if (NS_FAILED(rv))
     return;
 
-  nsTArray<nsCString> entries;
-  nsCOMPtr<nsISupports> entry;
-  while (NS_SUCCEEDED(enumerator->GetNext(getter_AddRefs(entry)))) {
-    nsCOMPtr<nsISupportsCString> entryName = do_QueryInterface(entry, &rv);
+  nsCOMPtr<nsIUTF8StringEnumerator> strings = do_QueryInterface(enumerator);
+  MOZ_ASSERT(strings);
 
+  bool more;
+  while (NS_SUCCEEDED(strings->HasMore(&more)) && more) {
+    nsAutoCString entryName;
+    strings->GetNext(entryName);
+
+    nsCString entryValue;
+    rv = catMan->GetCategoryEntry(aCategory,
+				  entryName.get(),
+				  getter_Copies(entryValue));
     if (NS_SUCCEEDED(rv)) {
-      nsAutoCString categoryEntry;
-      rv = entryName->GetData(categoryEntry);
-
-      nsCString entryValue;
-      catMan->GetCategoryEntry(aCategory,
-                               categoryEntry.get(),
-                               getter_Copies(entryValue));
-
-      if (NS_SUCCEEDED(rv)) {
-        mHash.Put(categoryEntry, entryValue);
-        entries.AppendElement(entryValue);
+      nsCOMPtr<nsISupports> service = do_GetService(entryValue.get());
+      if (service) {
+	mHash.Put(entryName, service);
       }
     }
   }
@@ -59,9 +57,6 @@ nsCategoryObserver::nsCategoryObserver(const char* aCategory,
     serv->AddObserver(this, NS_XPCOM_CATEGORY_ENTRY_REMOVED_OBSERVER_ID, false);
     serv->AddObserver(this, NS_XPCOM_CATEGORY_CLEARED_OBSERVER_ID, false);
   }
-
-  for (int32_t i = entries.Length() - 1; i >= 0; --i)
-    mListener->EntryAdded(entries[i]);
 }
 
 nsCategoryObserver::~nsCategoryObserver() {
@@ -71,11 +66,10 @@ NS_IMPL_ISUPPORTS1(nsCategoryObserver, nsIObserver)
 
 void
 nsCategoryObserver::ListenerDied() {
-  mListener = nullptr;
   RemoveObservers();
 }
 
-NS_HIDDEN_(void)
+void
 nsCategoryObserver::RemoveObservers() {
   if (mObserversRemoved)
     return;
@@ -94,12 +88,8 @@ nsCategoryObserver::RemoveObservers() {
 NS_IMETHODIMP
 nsCategoryObserver::Observe(nsISupports* aSubject, const char* aTopic,
                             const PRUnichar* aData) {
-  if (!mListener)
-    return NS_OK;
-
   if (strcmp(aTopic, NS_XPCOM_SHUTDOWN_OBSERVER_ID) == 0) {
     mHash.Clear();
-    mListener->CategoryCleared();
     RemoveObservers();
 
     return NS_OK;
@@ -120,7 +110,7 @@ nsCategoryObserver::Observe(nsISupports* aSubject, const char* aTopic,
     // added and an nsCategoryObserver gets instantiated before events get
     // processed, we'd get the notification for an existing entry.
     // Do nothing in that case.
-    if (mHash.Get(str, nullptr))
+    if (mHash.GetWeak(str))
       return NS_OK;
 
     nsCOMPtr<nsICategoryManager> catMan =
@@ -133,17 +123,15 @@ nsCategoryObserver::Observe(nsISupports* aSubject, const char* aTopic,
                              str.get(),
                              getter_Copies(entryValue));
 
-    mHash.Put(str, entryValue);
-    mListener->EntryAdded(entryValue);
-  } else if (strcmp(aTopic, NS_XPCOM_CATEGORY_ENTRY_REMOVED_OBSERVER_ID) == 0) {
-    nsAutoCString val;
-    if (mHash.Get(str, &val)) {
-      mHash.Remove(str);
-      mListener->EntryRemoved(val);
+    nsCOMPtr<nsISupports> service = do_GetService(entryValue.get());
+
+    if (service) {
+      mHash.Put(str, service);
     }
+  } else if (strcmp(aTopic, NS_XPCOM_CATEGORY_ENTRY_REMOVED_OBSERVER_ID) == 0) {
+    mHash.Remove(str);
   } else if (strcmp(aTopic, NS_XPCOM_CATEGORY_CLEARED_OBSERVER_ID) == 0) {
     mHash.Clear();
-    mListener->CategoryCleared();
   }
   return NS_OK;
 }

xpcom/glue/nsCategoryCache.h

@@ -16,39 +16,30 @@
 
 #include "nsAutoPtr.h"
 #include "nsCOMArray.h"
-#include "nsDataHashtable.h"
+#include "nsInterfaceHashtable.h"
 
 #include "nsXPCOM.h"
 
-class NS_NO_VTABLE nsCategoryListener {
-  protected:
-    // no virtual destructor (people shouldn't delete through an
-    // nsCategoryListener pointer)
-    ~nsCategoryListener() {}
-
+class NS_COM_GLUE nsCategoryObserver MOZ_FINAL : public nsIObserver
+{
   public:
-    virtual void EntryAdded(const nsCString& aValue) = 0;
-    virtual void EntryRemoved(const nsCString& aValue) = 0;
-    virtual void CategoryCleared() = 0;
-};
-
-class NS_COM_GLUE nsCategoryObserver MOZ_FINAL : public nsIObserver {
-  public:
-    nsCategoryObserver(const char* aCategory,
-                       nsCategoryListener* aCategoryListener);
+    nsCategoryObserver(const char* aCategory);
     ~nsCategoryObserver();
 
     void ListenerDied();
+    nsInterfaceHashtable<nsCStringHashKey, nsISupports>& GetHash()
+    {
+      return mHash;
+    }
 
     NS_DECL_ISUPPORTS
     NS_DECL_NSIOBSERVER
   private:
-    NS_HIDDEN_(void) RemoveObservers();
+    void RemoveObservers();
 
-    nsDataHashtable<nsCStringHashKey, nsCString> mHash;
-    nsCategoryListener*                          mListener;
-    nsCString                                    mCategory;
-    bool                                         mObserversRemoved;
+    nsInterfaceHashtable<nsCStringHashKey, nsISupports> mHash;
+    nsCString mCategory;
+    bool mObserversRemoved;
 };
 
 /**
@@ -59,59 +50,46 @@ class NS_COM_GLUE nsCategoryObserver MOZ_FINAL : public nsIObserver {
  * then get the name of the category.
  */
 template<class T>
-class nsCategoryCache MOZ_FINAL : protected nsCategoryListener {
+class nsCategoryCache MOZ_FINAL
+{
   public:
-    explicit nsCategoryCache(const char* aCategory);
-    ~nsCategoryCache() { if (mObserver) mObserver->ListenerDied(); }
+    explicit nsCategoryCache(const char* aCategory)
+      : mCategoryName(aCategory)
+    {
+    }
+    ~nsCategoryCache() {
+      if (mObserver)
+	mObserver->ListenerDied();
+    }
 
-    const nsCOMArray<T>& GetEntries() {
+    void GetEntries(nsCOMArray<T>& result) {
       // Lazy initialization, so that services in this category can't
       // cause reentrant getService (bug 386376)
       if (!mObserver)
-        mObserver = new nsCategoryObserver(mCategoryName.get(), this);
-      return mEntries;
-    }
-  protected:
-    virtual void EntryAdded(const nsCString& aValue);
-    virtual void EntryRemoved(const nsCString& aValue);
-    virtual void CategoryCleared();
-  private:
-    friend class CategoryObserver;
+        mObserver = new nsCategoryObserver(mCategoryName.get());
 
+      mObserver->GetHash().EnumerateRead(EntriesToArray, &result);
+    }
+
+  private:
     // Not to be implemented
     nsCategoryCache(const nsCategoryCache<T>&);
 
+    static PLDHashOperator EntriesToArray(const nsACString& key,
+					  nsISupports* entry, void* arg)
+    {
+      nsCOMArray<T>& entries = *static_cast<nsCOMArray<T>*>(arg);
+
+      nsCOMPtr<T> service = do_QueryInterface(entry);
+      if (service) {
+	entries.AppendObject(service);
+      }
+      return PL_DHASH_NEXT;
+    }
+
     nsCString mCategoryName;
-    nsCOMArray<T> mEntries;
     nsRefPtr<nsCategoryObserver> mObserver;
+
 };
 
-// -----------------------------------
-// Implementation
-
-template<class T>
-nsCategoryCache<T>::nsCategoryCache(const char* aCategory)
-: mCategoryName(aCategory)
-{
-}
-
-template<class T>
-void nsCategoryCache<T>::EntryAdded(const nsCString& aValue) {
-  nsCOMPtr<T> catEntry = do_GetService(aValue.get());
-  if (catEntry)
-    mEntries.AppendObject(catEntry);
-}
-
-template<class T>
-void nsCategoryCache<T>::EntryRemoved(const nsCString& aValue) {
-  nsCOMPtr<T> catEntry = do_GetService(aValue.get());
-  if (catEntry)
-    mEntries.RemoveObject(catEntry);
-}
-
-template<class T>
-void nsCategoryCache<T>::CategoryCleared() {
-  mEntries.Clear();
-}
-
 #endif