Bug 1051102 - Make mPackagesHash be a more modern hashtable. r=bsmedberg
authorBlake Kaplan <mrbkap@gmail.com>
Mon, 11 Aug 2014 11:13:36 -0700
changeset 220588 7f5a7dc734207085c2e8a7b8c4103285307b8f8f
parent 220587 d20cfaa4220b62da01767efde0e6df99e1145cdb
child 220589 74aae2dec3c9f2a50a70fa101f2d5986d7c6048f
push id1
push usersledru@mozilla.com
push dateThu, 04 Dec 2014 17:57:20 +0000
reviewersbsmedberg
bugs1051102
milestone34.0a1
Bug 1051102 - Make mPackagesHash be a more modern hashtable. r=bsmedberg
chrome/nsChromeRegistryChrome.cpp
chrome/nsChromeRegistryChrome.h
--- a/chrome/nsChromeRegistryChrome.cpp
+++ b/chrome/nsChromeRegistryChrome.cpp
@@ -44,23 +44,17 @@
 #define SELECTED_LOCALE_PREF "general.useragent.locale"
 #define SELECTED_SKIN_PREF   "general.skins.selectedSkin"
 #define PACKAGE_OVERRIDE_BRANCH "chrome.override_package."
 
 using namespace mozilla;
 using mozilla::dom::ContentParent;
 using mozilla::dom::PContentParent;
 
-static PLDHashOperator
-RemoveAll(PLDHashTable *table, PLDHashEntryHdr *entry, uint32_t number, void *arg)
-{
-  return (PLDHashOperator) (PL_DHASH_NEXT | PL_DHASH_REMOVE);
-}
-
-// We use a "best-fit" algorithm for matching locales and themes. 
+// We use a "best-fit" algorithm for matching locales and themes.
 // 1) the exact selected locale/theme
 // 2) (locales only) same language, different country
 //    e.g. en-GB is the selected locale, only en-US is available
 // 3) any available locale/theme
 
 /**
  * Match the language-part of two lang-COUNTRY codes, hopefully but
  * not guaranteed to be in the form ab-CD or abz-CD. "ab" should also
@@ -77,17 +71,17 @@ LanguagesMatch(const nsACString& a, cons
   a.BeginReading(as);
   a.EndReading(ae);
   b.BeginReading(bs);
   b.EndReading(be);
 
   while (*as == *bs) {
     if (*as == '-')
       return true;
- 
+
     ++as; ++bs;
 
     // reached the end
     if (as == ae && bs == be)
       return true;
 
     // "a" is short
     if (as == ae)
@@ -100,37 +94,32 @@ LanguagesMatch(const nsACString& a, cons
 
   return false;
 }
 
 nsChromeRegistryChrome::nsChromeRegistryChrome()
   : mProfileLoaded(false)
   , mDynamicRegistration(true)
 {
-  mPackagesHash.ops = nullptr;
 }
 
 nsChromeRegistryChrome::~nsChromeRegistryChrome()
 {
-  if (mPackagesHash.ops)
-    PL_DHashTableFinish(&mPackagesHash);
 }
 
 nsresult
 nsChromeRegistryChrome::Init()
 {
   nsresult rv = nsChromeRegistry::Init();
   if (NS_FAILED(rv))
     return rv;
 
   mSelectedLocale = NS_LITERAL_CSTRING("en-US");
   mSelectedSkin = NS_LITERAL_CSTRING("classic/1.0");
 
-  PL_DHashTableInit(&mPackagesHash, &kTableOps, nullptr, sizeof(PackageEntry));
-
   bool safeMode = false;
   nsCOMPtr<nsIXULRuntime> xulrun (do_GetService(XULAPPINFO_SERVICE_CONTRACTID));
   if (xulrun)
     xulrun->GetInSafeMode(&safeMode);
 
   nsCOMPtr<nsIPrefService> prefserv (do_GetService(NS_PREFSERVICE_CONTRACTID));
   nsCOMPtr<nsIPrefBranch> prefs;
 
@@ -194,22 +183,18 @@ nsChromeRegistryChrome::GetLocalesForPac
   nsresult rv = OverrideLocalePackage(aPackage, realpackage);
   if (NS_FAILED(rv))
     return rv;
 
   nsTArray<nsCString> *a = new nsTArray<nsCString>;
   if (!a)
     return NS_ERROR_OUT_OF_MEMORY;
 
-  PackageEntry* entry =
-      static_cast<PackageEntry*>(PL_DHashTableOperate(&mPackagesHash,
-                                                      & realpackage,
-                                                      PL_DHASH_LOOKUP));
-
-  if (PL_DHASH_ENTRY_IS_BUSY(entry)) {
+  PackageEntry* entry;
+  if (mPackagesHash.Get(realpackage, &entry)) {
     entry->locales.EnumerateToArray(a);
   }
 
   rv = NS_NewAdoptingUTF8StringEnumerator(aResult, a);
   if (NS_FAILED(rv))
     delete a;
 
   return rv;
@@ -243,17 +228,17 @@ nsChromeRegistryChrome::IsLocaleRTL(cons
 
   // first check the intl.uidirection.<locale> preference, and if that is not
   // set, check the same preference but with just the first two characters of
   // the locale. If that isn't set, default to left-to-right.
   nsAutoCString prefString = NS_LITERAL_CSTRING("intl.uidirection.") + locale;
   nsCOMPtr<nsIPrefBranch> prefBranch (do_GetService(NS_PREFSERVICE_CONTRACTID));
   if (!prefBranch)
     return NS_OK;
-  
+
   nsXPIDLCString dir;
   prefBranch->GetCharPref(prefString.get(), getter_Copies(dir));
   if (dir.IsEmpty()) {
     int32_t hyphen = prefString.FindChar('-');
     if (hyphen >= 1) {
       nsAutoCString shortPref(Substring(prefString, 0, hyphen));
       prefBranch->GetCharPref(shortPref.get(), getter_Copies(dir));
     }
@@ -265,22 +250,18 @@ nsChromeRegistryChrome::IsLocaleRTL(cons
 nsresult
 nsChromeRegistryChrome::GetSelectedLocale(const nsACString& aPackage,
                                           nsACString& aLocale)
 {
   nsCString realpackage;
   nsresult rv = OverrideLocalePackage(aPackage, realpackage);
   if (NS_FAILED(rv))
     return rv;
-  PackageEntry* entry =
-      static_cast<PackageEntry*>(PL_DHashTableOperate(&mPackagesHash,
-                                                      & realpackage,
-                                                      PL_DHASH_LOOKUP));
-
-  if (PL_DHASH_ENTRY_IS_FREE(entry))
+  PackageEntry* entry;
+  if (!mPackagesHash.Get(realpackage, &entry))
     return NS_ERROR_FILE_NOT_FOUND;
 
   aLocale = entry->locales.GetSelected(mSelectedLocale, nsProviderArray::LOCALE);
   if (aLocale.IsEmpty())
     return NS_ERROR_FAILURE;
 
   return NS_OK;
 }
@@ -383,17 +364,17 @@ nsChromeRegistryChrome::Observe(nsISuppo
   }
 
   return rv;
 }
 
 NS_IMETHODIMP
 nsChromeRegistryChrome::CheckForNewChrome()
 {
-  PL_DHashTableEnumerate(&mPackagesHash, RemoveAll, nullptr);
+  mPackagesHash.Clear();
   mOverlayHash.Clear();
   mStyleHash.Clear();
   mOverrideTable.Clear();
 
   mDynamicRegistration = false;
 
   nsComponentManagerImpl::gComponentManager->RereadChromeManifests();
 
@@ -439,17 +420,17 @@ EnumerateOverride(nsIURI* aURIKey,
 {
   nsTArray<OverrideMapping>* overrides =
       static_cast<nsTArray<OverrideMapping>*>(aArg);
 
   SerializedURI chromeURI, overrideURI;
 
   SerializeURI(aURIKey, chromeURI);
   SerializeURI(aURI, overrideURI);
-        
+
   OverrideMapping override = {
     chromeURI, overrideURI
   };
   overrides->AppendElement(override);
   return (PLDHashOperator)PL_DHASH_NEXT;
 }
 
 struct EnumerationArgs
@@ -465,17 +446,17 @@ nsChromeRegistryChrome::SendRegisteredCh
 {
   InfallibleTArray<ChromePackage> packages;
   InfallibleTArray<ResourceMapping> resources;
   InfallibleTArray<OverrideMapping> overrides;
 
   EnumerationArgs args = {
     packages, mSelectedLocale, mSelectedSkin
   };
-  PL_DHashTableEnumerate(&mPackagesHash, CollectPackages, &args);
+  mPackagesHash.EnumerateRead(CollectPackages, &args);
 
   nsCOMPtr<nsIIOService> io (do_GetIOService());
   NS_ENSURE_TRUE_VOID(io);
 
   nsCOMPtr<nsIProtocolHandler> ph;
   nsresult rv = io->GetProtocolHandler("resource", getter_AddRefs(ph));
   NS_ENSURE_SUCCESS_VOID(rv);
 
@@ -501,45 +482,44 @@ nsChromeRegistryChrome::SendRegisteredCh
         parents[i]->SendRegisterChrome(packages, resources, overrides,
                                        mSelectedLocale, true);
       NS_WARN_IF_FALSE(success, "couldn't reset a child's registered chrome");
     }
   }
 }
 
 /* static */ void
-nsChromeRegistryChrome::ChromePackageFromPackageEntry(PackageEntry* aPackage,
+nsChromeRegistryChrome::ChromePackageFromPackageEntry(const nsACString& aPackageName,
+                                                      PackageEntry* aPackage,
                                                       ChromePackage* aChromePackage,
                                                       const nsCString& aSelectedLocale,
                                                       const nsCString& aSelectedSkin)
 {
   SerializeURI(aPackage->baseURI, aChromePackage->contentBaseURI);
   SerializeURI(aPackage->locales.GetBase(aSelectedLocale,
                                          nsProviderArray::LOCALE),
                aChromePackage->localeBaseURI);
   SerializeURI(aPackage->skins.GetBase(aSelectedSkin, nsProviderArray::ANY),
                aChromePackage->skinBaseURI);
-  aChromePackage->package = aPackage->package;
+  aChromePackage->package = aPackageName;
   aChromePackage->flags = aPackage->flags;
 }
 
 PLDHashOperator
-nsChromeRegistryChrome::CollectPackages(PLDHashTable *table,
-                                  PLDHashEntryHdr *entry,
-                                  uint32_t number,
-                                  void *arg)
+nsChromeRegistryChrome::CollectPackages(const nsACString &aKey,
+                                        PackageEntry *package,
+                                        void *arg)
 {
   EnumerationArgs* args = static_cast<EnumerationArgs*>(arg);
-  PackageEntry* package = static_cast<PackageEntry*>(entry);
 
   ChromePackage chromePackage;
-  ChromePackageFromPackageEntry(package, &chromePackage,
+  ChromePackageFromPackageEntry(aKey, package, &chromePackage,
                                 args->selectedLocale, args->selectedSkin);
   args->packages.AppendElement(chromePackage);
-  return (PLDHashOperator)PL_DHASH_NEXT;
+  return PL_DHASH_NEXT;
 }
 
 static bool
 CanLoadResource(nsIURI* aResourceURI)
 {
   bool isLocalResource = false;
   (void)NS_URIChainHasFlags(aResourceURI,
                             nsIProtocolHandler::URI_IS_LOCAL_RESOURCE,
@@ -547,22 +527,18 @@ CanLoadResource(nsIURI* aResourceURI)
   return isLocalResource;
 }
 
 nsIURI*
 nsChromeRegistryChrome::GetBaseURIFromPackage(const nsCString& aPackage,
                                               const nsCString& aProvider,
                                               const nsCString& aPath)
 {
-  PackageEntry* entry =
-      static_cast<PackageEntry*>(PL_DHashTableOperate(&mPackagesHash,
-                                                      &aPackage,
-                                                      PL_DHASH_LOOKUP));
-
-  if (PL_DHASH_ENTRY_IS_FREE(entry)) {
+  PackageEntry* entry;
+  if (!mPackagesHash.Get(aPackage, &entry)) {
     if (!mInitialized)
       return nullptr;
 
     LogMessage("No chrome package registered for chrome://%s/%s/%s",
                aPackage.get(), aProvider.get(), aPath.get());
 
     return nullptr;
   }
@@ -578,72 +554,24 @@ nsChromeRegistryChrome::GetBaseURIFromPa
   }
   return nullptr;
 }
 
 nsresult
 nsChromeRegistryChrome::GetFlagsFromPackage(const nsCString& aPackage,
                                             uint32_t* aFlags)
 {
-  PackageEntry* entry =
-      static_cast<PackageEntry*>(PL_DHashTableOperate(&mPackagesHash,
-                                                      & (nsACString&) aPackage,
-                                                      PL_DHASH_LOOKUP));
-  if (PL_DHASH_ENTRY_IS_FREE(entry))
+  PackageEntry* entry;
+  if (!mPackagesHash.Get(aPackage, &entry))
     return NS_ERROR_FILE_NOT_FOUND;
 
   *aFlags = entry->flags;
   return NS_OK;
 }
 
-PLHashNumber
-nsChromeRegistryChrome::HashKey(PLDHashTable *table, const void *key)
-{
-  const nsACString& str = *reinterpret_cast<const nsACString*>(key);
-  return HashString(str);
-}
-
-bool
-nsChromeRegistryChrome::MatchKey(PLDHashTable *table, const PLDHashEntryHdr *entry,
-                           const void *key)
-{
-  const nsACString& str = *reinterpret_cast<const nsACString*>(key);
-  const PackageEntry* pentry = static_cast<const PackageEntry*>(entry);
-  return str.Equals(pentry->package);
-}
-
-void
-nsChromeRegistryChrome::ClearEntry(PLDHashTable *table, PLDHashEntryHdr *entry)
-{
-  PackageEntry* pentry = static_cast<PackageEntry*>(entry);
-  pentry->~PackageEntry();
-}
-
-bool
-nsChromeRegistryChrome::InitEntry(PLDHashTable *table, PLDHashEntryHdr *entry,
-                            const void *key)
-{
-  const nsACString& str = *reinterpret_cast<const nsACString*>(key);
-
-  new (entry) PackageEntry(str);
-  return true;
-}
-
-const PLDHashTableOps
-nsChromeRegistryChrome::kTableOps = {
-  PL_DHashAllocTable,
-  PL_DHashFreeTable,
-  HashKey,
-  MatchKey,
-  PL_DHashMoveEntryStub,
-  ClearEntry,
-  PL_DHashFinalizeStub,
-  InitEntry
-};
-
 nsChromeRegistryChrome::ProviderEntry*
 nsChromeRegistryChrome::nsProviderArray::GetProvider(const nsACString& aPreferred, MatchType aType)
 {
   size_t i = mArray.Length();
   if (!i)
     return nullptr;
 
   ProviderEntry* found = nullptr;  // Only set if we find a partial-match locale
@@ -849,33 +777,29 @@ nsChromeRegistryChrome::ManifestContent(
 
   if (!CanLoadResource(resolved)) {
     LogMessageWithContext(resolved, lineno, nsIScriptError::warningFlag,
                           "During chrome registration, cannot register non-local URI '%s' as content.",
                           uri);
     return;
   }
 
-  PackageEntry* entry =
-    static_cast<PackageEntry*>(PL_DHashTableOperate(&mPackagesHash,
-                                                    & (const nsACString&) nsDependentCString(package),
-                                                    PL_DHASH_ADD));
-  if (!entry)
-    return;
-
+  nsDependentCString packageName(package);
+  PackageEntry* entry = mPackagesHash.LookupOrAdd(packageName);
   entry->baseURI = resolved;
 
   if (platform)
     entry->flags |= PLATFORM_PACKAGE;
   if (contentaccessible)
     entry->flags |= CONTENT_ACCESSIBLE;
 
   if (mDynamicRegistration) {
     ChromePackage chromePackage;
-    ChromePackageFromPackageEntry(entry, &chromePackage, mSelectedLocale, mSelectedSkin);
+    ChromePackageFromPackageEntry(packageName, entry, &chromePackage,
+                                  mSelectedLocale, mSelectedSkin);
     SendManifestEntry(chromePackage);
   }
 }
 
 void
 nsChromeRegistryChrome::ManifestLocale(ManifestProcessingContext& cx, int lineno,
                                        char *const * argv, bool platform,
                                        bool contentaccessible)
@@ -895,28 +819,24 @@ nsChromeRegistryChrome::ManifestLocale(M
 
   if (!CanLoadResource(resolved)) {
     LogMessageWithContext(resolved, lineno, nsIScriptError::warningFlag,
                           "During chrome registration, cannot register non-local URI '%s' as content.",
                           uri);
     return;
   }
 
-  PackageEntry* entry =
-    static_cast<PackageEntry*>(PL_DHashTableOperate(&mPackagesHash,
-                                                    & (const nsACString&) nsDependentCString(package),
-                                                    PL_DHASH_ADD));
-  if (!entry)
-    return;
-
+  nsDependentCString packageName(package);
+  PackageEntry* entry = mPackagesHash.LookupOrAdd(packageName);
   entry->locales.SetBase(nsDependentCString(provider), resolved);
 
   if (mDynamicRegistration) {
     ChromePackage chromePackage;
-    ChromePackageFromPackageEntry(entry, &chromePackage, mSelectedLocale, mSelectedSkin);
+    ChromePackageFromPackageEntry(packageName, entry, &chromePackage,
+                                  mSelectedLocale, mSelectedSkin);
     SendManifestEntry(chromePackage);
   }
 }
 
 void
 nsChromeRegistryChrome::ManifestSkin(ManifestProcessingContext& cx, int lineno,
                                      char *const * argv, bool platform,
                                      bool contentaccessible)
@@ -936,28 +856,24 @@ nsChromeRegistryChrome::ManifestSkin(Man
 
   if (!CanLoadResource(resolved)) {
     LogMessageWithContext(resolved, lineno, nsIScriptError::warningFlag,
                           "During chrome registration, cannot register non-local URI '%s' as content.",
                           uri);
     return;
   }
 
-  PackageEntry* entry =
-    static_cast<PackageEntry*>(PL_DHashTableOperate(&mPackagesHash,
-                                                    & (const nsACString&) nsDependentCString(package),
-                                                    PL_DHASH_ADD));
-  if (!entry)
-    return;
-
+  nsDependentCString packageName(package);
+  PackageEntry* entry = mPackagesHash.LookupOrAdd(packageName);
   entry->skins.SetBase(nsDependentCString(provider), resolved);
 
   if (mDynamicRegistration) {
     ChromePackage chromePackage;
-    ChromePackageFromPackageEntry(entry, &chromePackage, mSelectedLocale, mSelectedSkin);
+    ChromePackageFromPackageEntry(packageName, entry, &chromePackage,
+                                  mSelectedLocale, mSelectedSkin);
     SendManifestEntry(chromePackage);
   }
 }
 
 void
 nsChromeRegistryChrome::ManifestOverlay(ManifestProcessingContext& cx, int lineno,
                                         char *const * argv, bool platform,
                                         bool contentaccessible)
@@ -1058,17 +974,17 @@ nsChromeRegistryChrome::ManifestResource
     NS_WARNING("No IO service trying to process chrome manifests");
     return;
   }
 
   nsCOMPtr<nsIProtocolHandler> ph;
   nsresult rv = io->GetProtocolHandler("resource", getter_AddRefs(ph));
   if (NS_FAILED(rv))
     return;
-  
+
   nsCOMPtr<nsIResProtocolHandler> rph = do_QueryInterface(ph);
 
   bool exists = false;
   rv = rph->HasSubstitution(host, &exists);
   if (exists) {
     LogMessageWithContext(cx.GetManifestURI(), lineno, nsIScriptError::warningFlag,
                           "Duplicate resource declaration for '%s' ignored.", package);
     return;
--- a/chrome/nsChromeRegistryChrome.h
+++ b/chrome/nsChromeRegistryChrome.h
@@ -5,16 +5,17 @@
 
 #ifndef nsChromeRegistryChrome_h
 #define nsChromeRegistryChrome_h
 
 #include "nsCOMArray.h"
 #include "nsChromeRegistry.h"
 #include "nsTArray.h"
 #include "mozilla/Move.h"
+#include "nsClassHashtable.h"
 
 namespace mozilla {
 namespace dom {
 class PContentParent;
 }
 }
 
 class nsIPrefBranch;
@@ -48,42 +49,35 @@ class nsChromeRegistryChrome : public ns
 
   // If aChild is non-null then it is a new child to notify. If aChild is
   // null, then we have installed new chrome and we are resetting all of our
   // children's registered chrome.
   void SendRegisteredChrome(mozilla::dom::PContentParent* aChild);
 
  private:
   struct PackageEntry;
-  static void ChromePackageFromPackageEntry(PackageEntry* aPackage,
+  static void ChromePackageFromPackageEntry(const nsACString& aPackageName,
+                                            PackageEntry* aPackage,
                                             ChromePackage* aChromePackage,
                                             const nsCString& aSelectedLocale,
                                             const nsCString& aSelectedSkin);
-  static PLDHashOperator CollectPackages(PLDHashTable *table,
-                                         PLDHashEntryHdr *entry,
-                                         uint32_t number, void *arg);
+  static PLDHashOperator CollectPackages(const nsACString &aKey,
+                                         PackageEntry *package,
+                                         void *arg);
 
   nsresult OverrideLocalePackage(const nsACString& aPackage,
                                  nsACString& aOverride);
   nsresult SelectLocaleFromPref(nsIPrefBranch* prefs);
   nsresult UpdateSelectedLocale() MOZ_OVERRIDE;
   nsIURI* GetBaseURIFromPackage(const nsCString& aPackage,
                                  const nsCString& aProvider,
                                  const nsCString& aPath) MOZ_OVERRIDE;
   nsresult GetFlagsFromPackage(const nsCString& aPackage,
                                uint32_t* aFlags) MOZ_OVERRIDE;
 
-  static const PLDHashTableOps kTableOps;
-  static PLDHashNumber HashKey(PLDHashTable *table, const void *key);
-  static bool          MatchKey(PLDHashTable *table, const PLDHashEntryHdr *entry,
-                                const void *key);
-  static void          ClearEntry(PLDHashTable *table, PLDHashEntryHdr *entry);
-  static bool          InitEntry(PLDHashTable *table, PLDHashEntryHdr *entry,
-                                 const void *key);
-
   struct ProviderEntry
   {
     ProviderEntry(const nsACString& aProvider, nsIURI* aBase) :
     provider(aProvider),
     baseURI(aBase) { }
 
     nsCString        provider;
     nsCOMPtr<nsIURI> baseURI;
@@ -112,21 +106,20 @@ class nsChromeRegistryChrome : public ns
    private:
     ProviderEntry* GetProvider(const nsACString& aPreferred, MatchType aType);
 
     nsTArray<ProviderEntry> mArray;
   };
 
   struct PackageEntry : public PLDHashEntryHdr
   {
-    PackageEntry(const nsACString& package)
-    : package(package), flags(0) { }
+    PackageEntry()
+    : flags(0) { }
     ~PackageEntry() { }
 
-    nsCString        package;
     nsCOMPtr<nsIURI> baseURI;
     uint32_t         flags;
     nsProviderArray  locales;
     nsProviderArray  skins;
   };
 
   class OverlayListEntry : public nsURIHashKey
   {
@@ -165,17 +158,17 @@ class nsChromeRegistryChrome : public ns
 
   bool mProfileLoaded;
   bool mDynamicRegistration;
 
   nsCString mSelectedLocale;
   nsCString mSelectedSkin;
 
   // Hash of package names ("global") to PackageEntry objects
-  PLDHashTable mPackagesHash;
+  nsClassHashtable<nsCStringHashKey, PackageEntry> mPackagesHash;
 
   virtual void ManifestContent(ManifestProcessingContext& cx, int lineno,
                                char *const * argv, bool platform,
                                bool contentaccessible);
   virtual void ManifestLocale(ManifestProcessingContext& cx, int lineno,
                               char *const * argv, bool platform,
                               bool contentaccessible);
   virtual void ManifestSkin(ManifestProcessingContext& cx, int lineno,