Bug 1411480 (attempt 2) - Remove the machinery for choosing the dirty callback. r=glandium. draft
authorNicholas Nethercote <nnethercote@mozilla.com>
Thu, 26 Oct 2017 16:14:01 +1100
changeset 688528 995d28d0d3829f6adbb6b7baeeb53eb0fca18fde
parent 688527 ced0f446b7889ba1ebd32274c084efb129c9c7ed
child 688529 eafe1ca063d9ac83c4db12c565d71dcd2cd58242
push id86771
push userbmo:dlee@mozilla.com
push dateMon, 30 Oct 2017 07:11:09 +0000
reviewersglandium
bugs1411480
milestone58.0a1
Bug 1411480 (attempt 2) - Remove the machinery for choosing the dirty callback. r=glandium. It's unnecessarily general, because we only ever use Preferences::DirtyCallback() as the callback. And because it's no longer a callback, the patch renames DirtyCallback() as HandleDirty(). MozReview-Commit-ID: Hl50dcxfVQq
modules/libpref/Preferences.cpp
modules/libpref/Preferences.h
--- a/modules/libpref/Preferences.cpp
+++ b/modules/libpref/Preferences.cpp
@@ -328,39 +328,16 @@ static bool gShouldCleanupDeadNodes = fa
 static PLDHashTableOps pref_HashTableOps = {
   PLDHashTable::HashStringKey,
   MatchPrefEntry,
   PLDHashTable::MoveEntryStub,
   ClearPrefEntry,
   nullptr,
 };
 
-typedef void (*PrefsDirtyFunc)();
-static PrefsDirtyFunc gDirtyCallback = nullptr;
-
-static inline void
-MakeDirtyCallback()
-{
-  // Right now the callback function is always set, so we don't need
-  // to complicate the code to cover the scenario where we set the callback
-  // after we've already tried to make it dirty.  If this assert triggers
-  // we will add that code.
-  MOZ_ASSERT(gDirtyCallback);
-  if (gDirtyCallback) {
-    gDirtyCallback();
-  }
-}
-
-// Callback for whenever we change a preference.
-static void
-PREF_SetDirtyCallback(PrefsDirtyFunc aFunc)
-{
-  gDirtyCallback = aFunc;
-}
-
 //---------------------------------------------------------------------------
 
 static bool
 pref_ValueChanged(PrefValue aOldValue, PrefValue aNewValue, PrefType aType);
 
 static nsresult
 pref_DoCallback(const char* aChangedPref);
 
@@ -847,17 +824,17 @@ PREF_DeleteBranch(const char* aBranchNam
     // "ldap" (if such a leaf node exists) but not "ldap_1.xxx".
     if (PL_strncmp(entry->mKey, to_delete, len) == 0 ||
         (len - 1 == strlen(entry->mKey) &&
          PL_strncmp(entry->mKey, to_delete, len - 1) == 0)) {
       iter.Remove();
     }
   }
 
-  MakeDirtyCallback();
+  Preferences::HandleDirty();
   return NS_OK;
 }
 
 // Clears the given pref (reverts it to its default value).
 static nsresult
 PREF_ClearUserPref(const char* aPrefName)
 {
   if (!gHashTable) {
@@ -868,17 +845,17 @@ PREF_ClearUserPref(const char* aPrefName
   if (pref && pref->mPrefFlags.HasUserValue()) {
     pref->mPrefFlags.SetHasUserValue(false);
 
     if (!pref->mPrefFlags.HasDefault()) {
       gHashTable->RemoveEntry(pref);
     }
 
     pref_DoCallback(aPrefName);
-    MakeDirtyCallback();
+    Preferences::HandleDirty();
   }
   return NS_OK;
 }
 
 // Clears all user prefs.
 static nsresult
 PREF_ClearAllUserPrefs()
 {
@@ -901,17 +878,17 @@ PREF_ClearAllUserPrefs()
       }
     }
   }
 
   for (std::string& prefString : prefStrings) {
     pref_DoCallback(prefString.c_str());
   }
 
-  MakeDirtyCallback();
+  Preferences::HandleDirty();
   return NS_OK;
 }
 
 // Function that sets whether or not the preference is locked and therefore
 // cannot be changed.
 static nsresult
 PREF_LockPref(const char* aKey, bool aLockIt)
 {
@@ -1118,28 +1095,28 @@ pref_HashPref(const char* aKey,
     if ((pref->mPrefFlags.HasDefault()) &&
         !(pref->mPrefFlags.HasStickyDefault()) &&
         !pref_ValueChanged(pref->mDefaultPref, aValue, aType) &&
         !(aFlags & kPrefForceSet)) {
       if (pref->mPrefFlags.HasUserValue()) {
         // XXX should we free a user-set string value if there is one?
         pref->mPrefFlags.SetHasUserValue(false);
         if (!pref->mPrefFlags.IsLocked()) {
-          MakeDirtyCallback();
+          Preferences::HandleDirty();
           valueChanged = true;
         }
       }
     } else if (!pref->mPrefFlags.HasUserValue() ||
                !pref->mPrefFlags.IsPrefType(aType) ||
                pref_ValueChanged(pref->mUserPref, aValue, aType)) {
       pref->mPrefFlags =
         pref_SetValue(&pref->mUserPref, pref->mPrefFlags, aValue, aType)
           .SetHasUserValue(true);
       if (!pref->mPrefFlags.IsLocked()) {
-        MakeDirtyCallback();
+        Preferences::HandleDirty();
         valueChanged = true;
       }
     }
   }
 
   if (valueChanged) {
     return pref_DoCallback(aKey);
   }
@@ -3228,17 +3205,17 @@ nsRelativeFilePref::SetRelativeToKey(con
 
 namespace mozilla {
 
 #define INITIAL_PREF_FILES 10
 
 static NS_DEFINE_CID(kZipReaderCID, NS_ZIPREADER_CID);
 
 void
-Preferences::DirtyCallback()
+Preferences::HandleDirty()
 {
   if (!XRE_IsParentProcess()) {
     // TODO: this should really assert because you can't set prefs in a
     // content process. But so much code currently does this that we just
     // ignore it for now.
     return;
   }
 
@@ -3536,17 +3513,17 @@ public:
       nsresult rvCopy = rv;
       nsCOMPtr<nsIFile> fileCopy(mFile);
       SystemGroup::Dispatch(
         TaskCategory::Other,
         NS_NewRunnableFunction("Preferences::WriterRunnable",
                                [fileCopy, rvCopy] {
                                  MOZ_RELEASE_ASSERT(NS_IsMainThread());
                                  if (NS_FAILED(rvCopy)) {
-                                   Preferences::DirtyCallback();
+                                   Preferences::HandleDirty();
                                  }
                                }));
     }
     return rv;
   }
 
 protected:
   nsCOMPtr<nsIFile> mFile;
@@ -3923,17 +3900,16 @@ static InfallibleTArray<Preferences::Pre
 Preferences::SetInitPreferences(nsTArray<PrefSetting>* aPrefs)
 {
   gInitPrefs = new InfallibleTArray<PrefSetting>(mozilla::Move(*aPrefs));
 }
 
 Result<Ok, const char*>
 Preferences::Init()
 {
-  PREF_SetDirtyCallback(&DirtyCallback);
   PREF_Init();
 
   MOZ_TRY(pref_InitInitialObjects());
 
   if (XRE_IsContentProcess()) {
     MOZ_ASSERT(gInitPrefs);
     for (unsigned int i = 0; i < gInitPrefs->Length(); i++) {
       Preferences::SetPreference(gInitPrefs->ElementAt(i));
@@ -4397,17 +4373,17 @@ Preferences::SavePrefFileInternal(nsIFil
 
     // The mDirty flag tells us if we should write to mCurrentFile. We only
     // check this flag when the caller wants to write to the default.
     if (!mDirty) {
       return NS_OK;
     }
 
     // Check for profile shutdown after mDirty because the runnables from
-    // DirtyCallback can still be pending.
+    // HandleDirty() can still be pending.
     if (mProfileShutdown) {
       NS_WARNING("Cannot save pref file after profile shutdown.");
       return NS_ERROR_ILLEGAL_DURING_SHUTDOWN;
     }
 
     // It's possible that we never got a prefs file.
     nsresult rv = NS_OK;
     if (mCurrentFile) {
--- a/modules/libpref/Preferences.h
+++ b/modules/libpref/Preferences.h
@@ -329,17 +329,17 @@ public:
 #ifdef DEBUG
   static void SetInitPhase(pref_initPhase phase);
   static pref_initPhase InitPhase();
 #endif
 
   static int64_t SizeOfIncludingThisAndOtherStuff(
     mozilla::MallocSizeOf aMallocSizeOf);
 
-  static void DirtyCallback();
+  static void HandleDirty();
 
   // Explicitly choosing synchronous or asynchronous (if allowed) preferences
   // file write. Only for the default file.  The guarantee for the "blocking"
   // is that when it returns, the file on disk reflect the current state of
   // preferences.
   nsresult SavePrefFileBlocking();
   nsresult SavePrefFileAsynchronous();