Bug 1441754 - Privatize CallbackNode's field. r=glandium
authorNicholas Nethercote <nnethercote@mozilla.com>
Wed, 28 Feb 2018 16:10:11 +1100
changeset 406509 13e0252c67c9caeb7fa73ae5b2e16080c84cb728
parent 406508 17fe71b2e622c2ae4bfcc963156ddc8815f40dd1
child 406510 e6680fcb17a0392208b7413a214a662f8b8ca284
push id100446
push usernnethercote@mozilla.com
push dateMon, 05 Mar 2018 02:52:02 +0000
treeherdermozilla-inbound@76a740cc4454 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglandium
bugs1441754
milestone60.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1441754 - Privatize CallbackNode's field. r=glandium This isn't compelling on its own, but it's necessary for the next patch. MozReview-Commit-ID: CFON8DCdGoA
modules/libpref/Preferences.cpp
--- a/modules/libpref/Preferences.cpp
+++ b/modules/libpref/Preferences.cpp
@@ -741,36 +741,52 @@ public:
   {
     auto entry = static_cast<PrefEntry*>(aEntry);
 
     delete entry->mPref;
     entry->mPref = nullptr;
   }
 };
 
-struct CallbackNode
+class CallbackNode
 {
+public:
   CallbackNode(const char* aDomain,
                PrefChangedFunc aFunc,
                void* aData,
                Preferences::MatchKind aMatchKind)
     : mDomain(moz_xstrdup(aDomain))
     , mFunc(aFunc)
     , mData(aData)
     , mMatchKind(aMatchKind)
     , mNext(nullptr)
   {
   }
 
+  // mDomain is a UniquePtr<>, so any uses of Domain() should only be temporary
+  // borrows.
+  const char* Domain() const { return mDomain.get(); }
+
+  PrefChangedFunc Func() const { return mFunc; }
+  void ClearFunc() { mFunc = nullptr; }
+
+  void* Data() const { return mData; }
+
+  Preferences::MatchKind MatchKind() const { return mMatchKind; }
+
+  CallbackNode* Next() const { return mNext; }
+  void SetNext(CallbackNode* aNext) { mNext = aNext; }
+
   void AddSizeOfIncludingThis(MallocSizeOf aMallocSizeOf, PrefsSizes& aSizes)
   {
     aSizes.mCallbacksObjects += aMallocSizeOf(this);
     aSizes.mCallbacksDomains += aMallocSizeOf(mDomain.get());
   }
 
+private:
   UniqueFreePtr<const char> mDomain;
 
   // If someone attempts to remove the node from the callback list while
   // NotifyCallbacks() is running, |func| is set to nullptr. Such nodes will
   // be removed at the end of NotifyCallbacks().
   PrefChangedFunc mFunc;
   void* mData;
   Preferences::MatchKind mMatchKind;
@@ -959,26 +975,26 @@ pref_SetPref(const char* aPrefName,
 
   return NS_OK;
 }
 
 // Removes |node| from callback list. Returns the node after the deleted one.
 static CallbackNode*
 pref_RemoveCallbackNode(CallbackNode* aNode, CallbackNode* aPrevNode)
 {
-  NS_PRECONDITION(!aPrevNode || aPrevNode->mNext == aNode, "invalid params");
+  NS_PRECONDITION(!aPrevNode || aPrevNode->Next() == aNode, "invalid params");
   NS_PRECONDITION(aPrevNode || gFirstCallback == aNode, "invalid params");
 
   NS_ASSERTION(
     !gCallbacksInProgress,
     "modifying the callback list while gCallbacksInProgress is true");
 
-  CallbackNode* next_node = aNode->mNext;
+  CallbackNode* next_node = aNode->Next();
   if (aPrevNode) {
-    aPrevNode->mNext = next_node;
+    aPrevNode->SetNext(next_node);
   } else {
     gFirstCallback = next_node;
   }
   if (gLastPriorityNode == aNode) {
     gLastPriorityNode = aPrevNode;
   }
   delete aNode;
   return next_node;
@@ -990,41 +1006,40 @@ NotifyCallbacks(const char* aPrefName)
   bool reentered = gCallbacksInProgress;
 
   // Nodes must not be deleted while gCallbacksInProgress is true.
   // Nodes that need to be deleted are marked for deletion by nulling
   // out the |func| pointer. We release them at the end of this function
   // if we haven't reentered.
   gCallbacksInProgress = true;
 
-  for (CallbackNode* node = gFirstCallback; node; node = node->mNext) {
-    if (node->mFunc) {
-      bool matches = node->mMatchKind == Preferences::ExactMatch
-                       ? strcmp(node->mDomain.get(), aPrefName) == 0
-                       : strncmp(node->mDomain.get(),
-                                 aPrefName,
-                                 strlen(node->mDomain.get())) == 0;
+  for (CallbackNode* node = gFirstCallback; node; node = node->Next()) {
+    if (node->Func()) {
+      bool matches =
+        node->MatchKind() == Preferences::ExactMatch
+          ? strcmp(node->Domain(), aPrefName) == 0
+          : strncmp(node->Domain(), aPrefName, strlen(node->Domain())) == 0;
       if (matches) {
-        (node->mFunc)(aPrefName, node->mData);
+        (node->Func())(aPrefName, node->Data());
       }
     }
   }
 
   gCallbacksInProgress = reentered;
 
   if (gShouldCleanupDeadNodes && !gCallbacksInProgress) {
     CallbackNode* prev_node = nullptr;
     CallbackNode* node = gFirstCallback;
 
     while (node) {
-      if (!node->mFunc) {
+      if (!node->Func()) {
         node = pref_RemoveCallbackNode(node, prev_node);
       } else {
         prev_node = node;
-        node = node->mNext;
+        node = node->Next();
       }
     }
     gShouldCleanupDeadNodes = false;
   }
 }
 
 //===========================================================================
 // Prefs parsing
@@ -2700,17 +2715,17 @@ PreferenceServiceReporter::CollectReport
     sizes.mCacheData += gCacheData->ShallowSizeOfIncludingThis(mallocSizeOf);
     for (uint32_t i = 0, count = gCacheData->Length(); i < count; ++i) {
       sizes.mCacheData += mallocSizeOf((*gCacheData)[i]);
     }
   }
 
   sizes.mPrefNameArena += gPrefNameArena.SizeOfExcludingThis(mallocSizeOf);
 
-  for (CallbackNode* node = gFirstCallback; node; node = node->mNext) {
+  for (CallbackNode* node = gFirstCallback; node; node = node->Next()) {
     node->AddSizeOfIncludingThis(mallocSizeOf, sizes);
   }
 
   MOZ_COLLECT_REPORT("explicit/preferences/hash-table",
                      KIND_HEAP,
                      UNITS_BYTES,
                      sizes.mHashTable,
                      "Memory used by libpref's hash table.");
@@ -3006,17 +3021,17 @@ Preferences::~Preferences()
   delete gCacheData;
   gCacheData = nullptr;
 
   NS_ASSERTION(!gCallbacksInProgress,
                "~Preferences was called while gCallbacksInProgress is true!");
 
   CallbackNode* node = gFirstCallback;
   while (node) {
-    CallbackNode* next_node = node->mNext;
+    CallbackNode* next_node = node->Next();
     delete node;
     node = next_node;
   }
   gLastPriorityNode = gFirstCallback = nullptr;
 
   delete gHashTable;
   gHashTable = nullptr;
 
@@ -4350,28 +4365,28 @@ Preferences::RegisterCallback(PrefChange
   NS_ENSURE_ARG(aCallback);
 
   NS_ENSURE_TRUE(InitStaticMembers(), NS_ERROR_NOT_AVAILABLE);
 
   auto node = new CallbackNode(aPrefNode, aCallback, aData, aMatchKind);
 
   if (aIsPriority) {
     // Add to the start of the list.
-    node->mNext = gFirstCallback;
+    node->SetNext(gFirstCallback);
     gFirstCallback = node;
     if (!gLastPriorityNode) {
       gLastPriorityNode = node;
     }
   } else {
     // Add to the start of the non-priority part of the list.
     if (gLastPriorityNode) {
-      node->mNext = gLastPriorityNode->mNext;
-      gLastPriorityNode->mNext = node;
+      node->SetNext(gLastPriorityNode->Next());
+      gLastPriorityNode->SetNext(node);
     } else {
-      node->mNext = gFirstCallback;
+      node->SetNext(gFirstCallback);
       gFirstCallback = node;
     }
   }
 
   return NS_OK;
 }
 
 /* static */ nsresult
@@ -4401,33 +4416,33 @@ Preferences::UnregisterCallback(PrefChan
   }
   NS_ENSURE_TRUE(sPreferences, NS_ERROR_NOT_AVAILABLE);
 
   nsresult rv = NS_ERROR_FAILURE;
   CallbackNode* node = gFirstCallback;
   CallbackNode* prev_node = nullptr;
 
   while (node) {
-    if (node->mFunc == aCallback && node->mData == aData &&
-        node->mMatchKind == aMatchKind &&
-        strcmp(node->mDomain.get(), aPrefNode) == 0) {
+    if (node->Func() == aCallback && node->Data() == aData &&
+        node->MatchKind() == aMatchKind &&
+        strcmp(node->Domain(), aPrefNode) == 0) {
       if (gCallbacksInProgress) {
-        // postpone the node removal until after
-        // callbacks enumeration is finished.
-        node->mFunc = nullptr;
+        // Postpone the node removal until after callbacks enumeration is
+        // finished.
+        node->ClearFunc();
         gShouldCleanupDeadNodes = true;
         prev_node = node;
-        node = node->mNext;
+        node = node->Next();
       } else {
         node = pref_RemoveCallbackNode(node, prev_node);
       }
       rv = NS_OK;
     } else {
       prev_node = node;
-      node = node->mNext;
+      node = node->Next();
     }
   }
   return rv;
 }
 
 static void
 CacheDataAppendElement(CacheData* aData)
 {