Bug 1436643 - Use a PLDHashTable `initEntry` function for Pref. r=froydnj draft
authorNicholas Nethercote <nnethercote@mozilla.com>
Thu, 08 Feb 2018 15:39:14 +1100
changeset 752401 113d236e73a24258f071e72d17a50f12251eed5e
parent 752400 c8e7ab77ed3fccf4d99711de3507dae353e16181
push id98252
push usernnethercote@mozilla.com
push dateThu, 08 Feb 2018 04:39:34 +0000
reviewersfroydnj
bugs1436643
milestone60.0a1
Bug 1436643 - Use a PLDHashTable `initEntry` function for Pref. r=froydnj This lets us have a proper constructor for Pref, which is nice. The patch also adds a missing case to PrefTypeToString(), and reorders the fields in Pref to be more sensible. MozReview-Commit-ID: A01ULF4q08O
modules/libpref/Preferences.cpp
--- a/modules/libpref/Preferences.cpp
+++ b/modules/libpref/Preferences.cpp
@@ -225,16 +225,18 @@ union PrefValue {
   }
 };
 
 #ifdef DEBUG
 const char*
 PrefTypeToString(PrefType aType)
 {
   switch (aType) {
+    case PrefType::None:
+      return "none";
     case PrefType::String:
       return "string";
     case PrefType::Int:
       return "int";
     case PrefType::Bool:
       return "bool";
     default:
       MOZ_CRASH("Unhandled enum value");
@@ -294,21 +296,25 @@ StrEscape(const char* aOriginal, nsCStri
 }
 
 static ArenaAllocator<8192, 1> gPrefNameArena;
 
 class Pref : public PLDHashEntryHdr
 {
 public:
   explicit Pref(const char* aName)
+    : mType(static_cast<uint32_t>(PrefType::None))
+    , mIsSticky(false)
+    , mIsLocked(false)
+    , mHasDefaultValue(false)
+    , mHasUserValue(false)
+    , mName(ArenaStrdup(aName, gPrefNameArena))
+    , mDefaultValue()
+    , mUserValue()
   {
-    mName = ArenaStrdup(aName, gPrefNameArena);
-
-    // We don't set the other fields because PLDHashTable always zeroes new
-    // entries.
   }
 
   ~Pref()
   {
     // There's no need to free mName because it's allocated in memory owned by
     // gPrefNameArena.
 
     mDefaultValue.Clear(Type());
@@ -318,16 +324,17 @@ public:
   const char* Name() { return mName; }
 
   // Types.
 
   PrefType Type() const { return static_cast<PrefType>(mType); }
   void SetType(PrefType aType) { mType = static_cast<uint32_t>(aType); }
 
   bool IsType(PrefType aType) const { return Type() == aType; }
+  bool IsTypeNone() const { return IsType(PrefType::None); }
   bool IsTypeString() const { return IsType(PrefType::String); }
   bool IsTypeInt() const { return IsType(PrefType::Int); }
   bool IsTypeBool() const { return IsType(PrefType::Bool); }
 
   // Other properties.
 
   bool IsLocked() const { return mIsLocked; }
   void SetIsLocked(bool aValue) { mIsLocked = aValue; }
@@ -348,16 +355,23 @@ public:
 
     if (!pref->mName || !aKey) {
       return false;
     }
 
     return strcmp(pref->mName, key) == 0;
   }
 
+  static void InitEntry(PLDHashEntryHdr* aEntry, const void* aKey)
+  {
+    auto pref = static_cast<Pref*>(aEntry);
+    auto prefName = static_cast<const char*>(aKey);
+    new (pref) Pref(prefName);
+  }
+
   static void ClearEntry(PLDHashTable* aTable, PLDHashEntryHdr* aEntry)
   {
     auto pref = static_cast<Pref*>(aEntry);
     pref->~Pref();
     memset(pref, 0, sizeof(Pref)); // zero just to be extra safe
   }
 
   nsresult GetBoolValue(PrefValueKind aKind, bool* aResult)
@@ -643,18 +657,18 @@ public:
 
 private:
   // These fields go first to minimize this struct's size on 64-bit. (Because
   // this a subclass of PLDHashEntryHdr, there's a preceding 32-bit mKeyHash
   // field.)
   uint32_t mType : 2;
   uint32_t mIsSticky : 1;
   uint32_t mIsLocked : 1;
+  uint32_t mHasDefaultValue : 1;
   uint32_t mHasUserValue : 1;
-  uint32_t mHasDefaultValue : 1;
 
   const char* mName; // allocated in gPrefNameArena
 
   PrefValue mDefaultValue;
   PrefValue mUserValue;
 };
 
 struct CallbackNode
@@ -695,17 +709,17 @@ static bool gIsAnyPrefLocked = false;
 static bool gCallbacksInProgress = false;
 static bool gShouldCleanupDeadNodes = false;
 
 static PLDHashTableOps pref_HashTableOps = {
   PLDHashTable::HashStringKey,
   Pref::MatchEntry,
   PLDHashTable::MoveEntryStub,
   Pref::ClearEntry,
-  nullptr,
+  Pref::InitEntry,
 };
 
 static Pref*
 pref_HashTableLookup(const char* aPrefName);
 
 static void
 NotifyCallbacks(const char* aPrefName);
 
@@ -818,19 +832,18 @@ pref_SetPref(const char* aPrefName,
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   auto pref = static_cast<Pref*>(gHashTable->Add(aPrefName, fallible));
   if (!pref) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
-  if (!pref->Name()) {
-    // New (zeroed) entry. Partially initialize it.
-    new (pref) Pref(aPrefName);
+  if (pref->IsTypeNone()) {
+    // New entry. Set the type.
     pref->SetType(aType);
   }
 
   bool valueChanged = false;
   nsresult rv;
   if (aKind == PrefValueKind::Default) {
     rv = pref->SetDefaultValue(aType, aValue, aIsSticky, &valueChanged);
   } else {
@@ -3160,21 +3173,16 @@ Preferences::SetPreference(const dom::Pr
 
   const char* prefName = aDomPref.name().get();
 
   auto pref = static_cast<Pref*>(gHashTable->Add(prefName, fallible));
   if (!pref) {
     return;
   }
 
-  if (!pref->Name()) {
-    // New (zeroed) entry. Partially initialize it.
-    new (pref) Pref(prefName);
-  }
-
   bool valueChanged = false;
   pref->FromDomPref(aDomPref, &valueChanged);
 
   // When the parent process clears a pref's user value we get a DomPref here
   // with no default value and no user value. There are two possibilities.
   //
   // - There was an existing pref with only a user value. FromDomPref() will
   //   have just cleared that user value, so the pref can be removed.