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
authorBenjamin Smedberg <benjamin@smedbergs.us>
Thu, 10 Oct 2013 08:48:03 -0400
changeset 150361 94bd7574eff132ca85d9a63b873af16765716f7e
parent 150360 44c21dcf12745f2f0b8bc8dbc367ac587d14551d
child 150362 266e7e5e3a2b484ae6de3d1b73fa2d0f3935f210
push idunknown
push userunknown
push dateunknown
reviewersfroydnj
bugs753687
milestone27.0a1
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
modules/libpref/src/Preferences.cpp
netwerk/base/public/nsNetUtil.h
netwerk/base/src/nsIOService.cpp
storage/src/VacuumManager.cpp
toolkit/components/places/nsPlacesMacros.h
widget/xpwidgets/nsIdleService.cpp
xpcom/glue/nsCategoryCache.cpp
xpcom/glue/nsCategoryCache.h
--- a/content/base/src/nsContentPolicy.cpp
+++ b/content/base/src/nsContentPolicy.cpp
@@ -112,17 +112,18 @@ nsContentPolicy::CheckPolicy(CPMethod   
         }
     }
 
     /* 
      * Enumerate mPolicies and ask each of them, taking the logical AND of
      * 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 */
         rv = (entries[i]->*policyMethod)(contentType, contentLocation,
                                          requestingLocation, requestingContext,
                                          mimeType, extra, requestPrincipal,
                                          decision);
 
--- a/modules/libpref/src/Preferences.cpp
+++ b/modules/libpref/src/Preferences.cpp
@@ -10,16 +10,17 @@
 #include "mozilla/Attributes.h"
 #include "mozilla/Util.h"
 #include "mozilla/HashFunctions.h"
 
 #include "nsXULAppAPI.h"
 
 #include "mozilla/Preferences.h"
 #include "nsAppDirectoryServiceDefs.h"
+#include "nsDataHashtable.h"
 #include "nsDirectoryServiceDefs.h"
 #include "nsICategoryManager.h"
 #include "nsCategoryManagerUtils.h"
 #include "nsNetUtil.h"
 #include "nsIFile.h"
 #include "nsIInputStream.h"
 #include "nsIObserverService.h"
 #include "nsIStringEnumerator.h"
--- a/netwerk/base/public/nsNetUtil.h
+++ b/netwerk/base/public/nsNetUtil.h
@@ -2321,17 +2321,18 @@ NS_SniffContent(const char* aSnifferType
     }
     cache = gDataSniffers;
   } else {
     // Invalid content sniffer type was requested
     MOZ_ASSERT(false);
     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()) {
       return;
     }
   }
 
   aSniffedType.Truncate();
--- a/netwerk/base/src/nsIOService.cpp
+++ b/netwerk/base/src/nsIOService.cpp
@@ -314,18 +314,18 @@ nsIOService::AsyncOnChannelRedirect(nsIC
     if (sink) {
         nsresult rv = helper->DelegateOnChannelRedirect(sink, oldChan,
                                                         newChan, flags);
         if (NS_FAILED(rv))
             return rv;
     }
 
     // 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,
                                                         newChan, flags);
         if (NS_FAILED(rv))
             return rv;
     }
     return NS_OK;
--- a/storage/src/VacuumManager.cpp
+++ b/storage/src/VacuumManager.cpp
@@ -359,18 +359,18 @@ VacuumManager::~VacuumManager()
 NS_IMETHODIMP
 VacuumManager::Observe(nsISupports *aSubject,
                        const char *aTopic,
                        const PRUnichar *aData)
 {
   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";
     int32_t startIndex = Preferences::GetInt(kPrefName, 0);
     if (startIndex >= entries.Count()) {
       startIndex = 0;
     }
     int32_t index;
--- a/toolkit/components/places/nsPlacesMacros.h
+++ b/toolkit/components/places/nsPlacesMacros.h
@@ -11,17 +11,18 @@
 #define __FUNCTION__ __func__
 #endif
 
 // Call a method on each observer in a category cache, then call the same
 // method on the observer array.
 #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)                                   \
   }                                                                            \
   PR_END_MACRO;
 
 #define PLACES_FACTORY_SINGLETON_IMPLEMENTATION(_className, _sInstance)        \
   _className * _className::_sInstance = nullptr;                                \
--- a/widget/xpwidgets/nsIdleService.cpp
+++ b/widget/xpwidgets/nsIdleService.cpp
@@ -100,17 +100,18 @@ nsIdleServiceDaily::Observe(nsISupports 
   nsCOMPtr<nsIObserverService> observerService =
     mozilla::services::GetObserverService();
   NS_ENSURE_STATE(observerService);
   (void)observerService->NotifyObservers(nullptr,
                                          OBSERVER_TOPIC_IDLE_DAILY,
                                          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);
   }
 
   // Stop observing idle for today.
   (void)mIdleService->RemoveIdleObserver(this, mIdleDailyTriggerWait);
 
   // Set the last idle-daily time pref.
--- a/xpcom/glue/nsCategoryCache.cpp
+++ b/xpcom/glue/nsCategoryCache.cpp
@@ -1,86 +1,80 @@
 /* vim:set st=2 sts=2 ts=2 et cin: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #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);
   if (!catMan)
     return;
 
   nsCOMPtr<nsISimpleEnumerator> enumerator;
   nsresult rv = catMan->EnumerateCategory(aCategory,
                                           getter_AddRefs(enumerator));
   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);
 
-    if (NS_SUCCEEDED(rv)) {
-      nsAutoCString categoryEntry;
-      rv = entryName->GetData(categoryEntry);
+  bool more;
+  while (NS_SUCCEEDED(strings->HasMore(&more)) && more) {
+    nsAutoCString entryName;
+    strings->GetNext(entryName);
 
-      nsCString entryValue;
-      catMan->GetCategoryEntry(aCategory,
-                               categoryEntry.get(),
-                               getter_Copies(entryValue));
-
-      if (NS_SUCCEEDED(rv)) {
-        mHash.Put(categoryEntry, entryValue);
-        entries.AppendElement(entryValue);
+    nsCString entryValue;
+    rv = catMan->GetCategoryEntry(aCategory,
+				  entryName.get(),
+				  getter_Copies(entryValue));
+    if (NS_SUCCEEDED(rv)) {
+      nsCOMPtr<nsISupports> service = do_GetService(entryValue.get());
+      if (service) {
+	mHash.Put(entryName, service);
       }
     }
   }
 
   // Now, listen for changes
   nsCOMPtr<nsIObserverService> serv =
     mozilla::services::GetObserverService();
   if (serv) {
     serv->AddObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID, false);
     serv->AddObserver(this, NS_XPCOM_CATEGORY_ENTRY_ADDED_OBSERVER_ID, false);
     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() {
 }
 
 NS_IMPL_ISUPPORTS1(nsCategoryObserver, nsIObserver)
 
 void
 nsCategoryObserver::ListenerDied() {
-  mListener = nullptr;
   RemoveObservers();
 }
 
-NS_HIDDEN_(void)
+void
 nsCategoryObserver::RemoveObservers() {
   if (mObserversRemoved)
     return;
 
   mObserversRemoved = true;
   nsCOMPtr<nsIObserverService> obsSvc =
     mozilla::services::GetObserverService();
   if (obsSvc) {
@@ -89,22 +83,18 @@ nsCategoryObserver::RemoveObservers() {
     obsSvc->RemoveObserver(this, NS_XPCOM_CATEGORY_ENTRY_REMOVED_OBSERVER_ID);
     obsSvc->RemoveObserver(this, NS_XPCOM_CATEGORY_CLEARED_OBSERVER_ID);
   }
 }
 
 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;
   }
 
   if (!aData ||
       !nsDependentString(aData).Equals(NS_ConvertASCIItoUTF16(mCategory)))
     return NS_OK;
@@ -115,35 +105,33 @@ nsCategoryObserver::Observe(nsISupports*
     strWrapper->GetData(str);
 
   if (strcmp(aTopic, NS_XPCOM_CATEGORY_ENTRY_ADDED_OBSERVER_ID) == 0) {
     // We may get an add notification even when we already have an entry. This
     // is due to the notification happening asynchronously, so if the entry gets
     // 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 =
       do_GetService(NS_CATEGORYMANAGER_CONTRACTID);
     if (!catMan)
       return NS_OK;
 
     nsCString entryValue;
     catMan->GetCategoryEntry(mCategory.get(),
                              str.get(),
                              getter_Copies(entryValue));
 
-    mHash.Put(str, entryValue);
-    mListener->EntryAdded(entryValue);
+    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) {
-    nsAutoCString val;
-    if (mHash.Get(str, &val)) {
-      mHash.Remove(str);
-      mListener->EntryRemoved(val);
-    }
+    mHash.Remove(str);
   } else if (strcmp(aTopic, NS_XPCOM_CATEGORY_CLEARED_OBSERVER_ID) == 0) {
     mHash.Clear();
-    mListener->CategoryCleared();
   }
   return NS_OK;
 }
--- a/xpcom/glue/nsCategoryCache.h
+++ b/xpcom/glue/nsCategoryCache.h
@@ -11,107 +11,85 @@
 #include "nsIObserver.h"
 #include "nsISimpleEnumerator.h"
 #include "nsISupportsPrimitives.h"
 
 #include "nsServiceManagerUtils.h"
 
 #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;
 };
 
 /**
  * This is a helper class that caches services that are registered in a certain
  * category. The intended usage is that a service stores a variable of type
  * nsCategoryCache<nsIFoo> in a member variable, where nsIFoo is the interface
  * that these services should implement. The constructor of this class should
  * 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;
+        mObserver = new nsCategoryObserver(mCategoryName.get());
+
+      mObserver->GetHash().EnumerateRead(EntriesToArray, &result);
     }
-  protected:
-    virtual void EntryAdded(const nsCString& aValue);
-    virtual void EntryRemoved(const nsCString& aValue);
-    virtual void CategoryCleared();
+
   private:
-    friend class CategoryObserver;
-
     // 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