[mq]: AutoInstallingCallback draft
authorNicholas Nethercote <nnethercote@mozilla.com>
Thu, 16 Nov 2017 09:57:47 +1100
changeset 698737 57337aeb927c0e317367f1f6f8831ef44a0197d7
parent 698258 46a0be972d5ae92da4a6dacb21f6e6ff119eb970
child 698738 6655875f2f06098d8c33313df6c41c620b6cb53c
push id89343
push usernnethercote@mozilla.com
push dateThu, 16 Nov 2017 02:15:55 +0000
milestone59.0a1
[mq]: AutoInstallingCallback
modules/libpref/Preferences.cpp
--- a/modules/libpref/Preferences.cpp
+++ b/modules/libpref/Preferences.cpp
@@ -864,46 +864,51 @@ static bool
 InInitArray(const char* aKey)
 {
   size_t prefsLen;
   size_t found;
   const char** list = mozilla::dom::ContentPrefs::GetContentPrefs(&prefsLen);
   return BinarySearchIf(list, 0, prefsLen, StringComparator(aKey), &found);
 }
 
-static bool gWatchingPref = false;
-
-class WatchingPrefRAII
+static bool gInstallingCallback = false;
+
+class AutoInstallingCallback
 {
 public:
-  WatchingPrefRAII() { gWatchingPref = true; }
-  ~WatchingPrefRAII() { gWatchingPref = false; }
+  AutoInstallingCallback() { gInstallingCallback = true; }
+  ~AutoInstallingCallback() { gInstallingCallback = false; }
 };
 
-#define WATCHING_PREF_RAII() WatchingPrefRAII watchingPrefRAII
+#define AUTO_INSTALLING_CALLBACK() AutoInstallingCallback installingRAII
 
 #else // DEBUG
 
-#define WATCHING_PREF_RAII()
+#define AUTO_INSTALLING_CALLBACK()
 
 #endif // DEBUG
 
 static PrefHashEntry*
 pref_HashTableLookup(const char* aKey)
 {
   MOZ_ASSERT(NS_IsMainThread() || mozilla::ServoStyleSet::IsInServoTraversal());
   MOZ_ASSERT((XRE_IsParentProcess() || gPhase != START),
              "pref access before commandline prefs set");
 
   // If you're hitting this assertion, you've added a pref access to start up.
   // Consider moving it later or add it to the whitelist in ContentPrefs.cpp
   // and get review from a DOM peer.
+  //
+  // Note that accesses of non-whitelisted prefs that happen while installing a
+  // callback (e.g. VarCache) are considered acceptable. These accesses will
+  // fail, but once the proper pref value is set the callback will be
+  // immediately called, so things should work out.
 #ifdef DEBUG
-  if (!XRE_IsParentProcess() && gPhase <= END_INIT_PREFS && !gWatchingPref &&
-      !InInitArray(aKey)) {
+  if (!XRE_IsParentProcess() && gPhase <= END_INIT_PREFS &&
+      !gInstallingCallback && !InInitArray(aKey)) {
     MOZ_CRASH_UNSAFE_PRINTF(
       "accessing non-init pref %s before the rest of the prefs are sent", aKey);
   }
 #endif
 
   return static_cast<PrefHashEntry*>(gHashTable->Search(aKey));
 }
 
@@ -4808,19 +4813,19 @@ Preferences::RegisterCallback(PrefChange
 
 /* static */ nsresult
 Preferences::RegisterCallbackAndCall(PrefChangedFunc aCallback,
                                      const char* aPref,
                                      void* aClosure,
                                      MatchKind aMatchKind)
 {
   MOZ_ASSERT(aCallback);
-  WATCHING_PREF_RAII();
   nsresult rv = RegisterCallback(aCallback, aPref, aClosure, aMatchKind);
   if (NS_SUCCEEDED(rv)) {
+    AUTO_INSTALLING_CALLBACK();
     (*aCallback)(aPref, aClosure);
   }
   return rv;
 }
 
 /* static */ nsresult
 Preferences::UnregisterCallback(PrefChangedFunc aCallback,
                                 const char* aPref,
@@ -4852,22 +4857,24 @@ CacheDataAppendElement(CacheData* aData)
     MOZ_CRASH_UNSAFE_PRINTF("!gCacheData: %s", gCacheDataDesc);
   }
   gCacheData->AppendElement(aData);
 }
 
 /* static */ nsresult
 Preferences::AddBoolVarCache(bool* aCache, const char* aPref, bool aDefault)
 {
-  WATCHING_PREF_RAII();
   NS_ASSERTION(aCache, "aCache must not be NULL");
 #ifdef DEBUG
   AssertNotAlreadyCached("bool", aPref, aCache);
 #endif
-  *aCache = GetBool(aPref, aDefault);
+  {
+    AUTO_INSTALLING_CALLBACK();
+    *aCache = GetBool(aPref, aDefault);
+  }
   CacheData* data = new CacheData();
   data->mCacheLocation = aCache;
   data->mDefaultValueBool = aDefault;
   CacheDataAppendElement(data);
   RegisterVarCacheCallback(BoolVarChanged, aPref, data);
   return NS_OK;
 }
 
@@ -4879,22 +4886,24 @@ IntVarChanged(const char* aPref, void* a
     Preferences::GetInt(aPref, cache->mDefaultValueInt);
 }
 
 /* static */ nsresult
 Preferences::AddIntVarCache(int32_t* aCache,
                             const char* aPref,
                             int32_t aDefault)
 {
-  WATCHING_PREF_RAII();
   NS_ASSERTION(aCache, "aCache must not be NULL");
 #ifdef DEBUG
   AssertNotAlreadyCached("int", aPref, aCache);
 #endif
-  *aCache = Preferences::GetInt(aPref, aDefault);
+  {
+    AUTO_INSTALLING_CALLBACK();
+    *aCache = Preferences::GetInt(aPref, aDefault);
+  }
   CacheData* data = new CacheData();
   data->mCacheLocation = aCache;
   data->mDefaultValueInt = aDefault;
   CacheDataAppendElement(data);
   RegisterVarCacheCallback(IntVarChanged, aPref, data);
   return NS_OK;
 }
 
@@ -4906,22 +4915,24 @@ UintVarChanged(const char* aPref, void* 
     Preferences::GetUint(aPref, cache->mDefaultValueUint);
 }
 
 /* static */ nsresult
 Preferences::AddUintVarCache(uint32_t* aCache,
                              const char* aPref,
                              uint32_t aDefault)
 {
-  WATCHING_PREF_RAII();
   NS_ASSERTION(aCache, "aCache must not be NULL");
 #ifdef DEBUG
   AssertNotAlreadyCached("uint", aPref, aCache);
 #endif
-  *aCache = Preferences::GetUint(aPref, aDefault);
+  {
+    AUTO_INSTALLING_CALLBACK();
+    *aCache = Preferences::GetUint(aPref, aDefault);
+  }
   CacheData* data = new CacheData();
   data->mCacheLocation = aCache;
   data->mDefaultValueUint = aDefault;
   CacheDataAppendElement(data);
   RegisterVarCacheCallback(UintVarChanged, aPref, data);
   return NS_OK;
 }
 
@@ -4935,22 +4946,24 @@ AtomicUintVarChanged(const char* aPref, 
 }
 
 template<MemoryOrdering Order>
 /* static */ nsresult
 Preferences::AddAtomicUintVarCache(Atomic<uint32_t, Order>* aCache,
                                    const char* aPref,
                                    uint32_t aDefault)
 {
-  WATCHING_PREF_RAII();
   NS_ASSERTION(aCache, "aCache must not be NULL");
 #ifdef DEBUG
   AssertNotAlreadyCached("uint", aPref, aCache);
 #endif
-  *aCache = Preferences::GetUint(aPref, aDefault);
+  {
+    AUTO_INSTALLING_CALLBACK();
+    *aCache = Preferences::GetUint(aPref, aDefault);
+  }
   CacheData* data = new CacheData();
   data->mCacheLocation = aCache;
   data->mDefaultValueUint = aDefault;
   CacheDataAppendElement(data);
   RegisterVarCacheCallback(AtomicUintVarChanged<Order>, aPref, data);
   return NS_OK;
 }
 
@@ -4968,22 +4981,24 @@ FloatVarChanged(const char* aPref, void*
   CacheData* cache = static_cast<CacheData*>(aClosure);
   *static_cast<float*>(cache->mCacheLocation) =
     Preferences::GetFloat(aPref, cache->mDefaultValueFloat);
 }
 
 /* static */ nsresult
 Preferences::AddFloatVarCache(float* aCache, const char* aPref, float aDefault)
 {
-  WATCHING_PREF_RAII();
   NS_ASSERTION(aCache, "aCache must not be NULL");
 #ifdef DEBUG
   AssertNotAlreadyCached("float", aPref, aCache);
 #endif
-  *aCache = Preferences::GetFloat(aPref, aDefault);
+  {
+    AUTO_INSTALLING_CALLBACK();
+    *aCache = Preferences::GetFloat(aPref, aDefault);
+  }
   CacheData* data = new CacheData();
   data->mCacheLocation = aCache;
   data->mDefaultValueFloat = aDefault;
   CacheDataAppendElement(data);
   RegisterVarCacheCallback(FloatVarChanged, aPref, data);
   return NS_OK;
 }