Bug 1550422 - P26. Make setter only usable on main process. r=njn
☠☠ backed out by af54b2de7028 ☠ ☠
authorJean-Yves Avenard <jyavenard@mozilla.com>
Fri, 24 May 2019 11:39:31 +0000
changeset 475537 a162952960355c80a50bc502aae46912b90fd7b8
parent 475536 3b70307c0db51a13c491e2bf5ccd24a83f7c6721
child 475538 0b40296717100fa26c8f7bdea8e365eeb523cfbd
push id36062
push useraciure@mozilla.com
push dateSat, 25 May 2019 09:39:13 +0000
treeherdermozilla-central@af54b2de7028 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnjn
bugs1550422
milestone69.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 1550422 - P26. Make setter only usable on main process. r=njn And set the underlying preference. StaticPrefs::Set becomes a convenience access to the original preference which is what gfxPrefs was actually doing. Differential Revision: https://phabricator.services.mozilla.com/D31749
modules/libpref/Preferences.cpp
modules/libpref/StaticPrefs.h
--- a/modules/libpref/Preferences.cpp
+++ b/modules/libpref/Preferences.cpp
@@ -5262,31 +5262,16 @@ nsresult Preferences::AddFloatVarCache(f
 nsresult Preferences::AddAtomicFloatVarCache(std::atomic<float>* aCache,
                                              const nsACString& aPref,
                                              float aDefault,
                                              bool aSkipAssignment) {
   AssertNotAlreadyCached("float", aPref, aCache);
   return AddVarCache(aCache, aPref, aDefault, aSkipAssignment);
 }
 
-// For a VarCache pref like this:
-//
-//   VARCACHE_PREF("my.varcache", my_varcache, int32_t, 99)
-//
-// we generate a static variable definition:
-//
-//   int32_t StaticPrefs::sVarCache_my_varcache(99);
-//
-#define PREF(name, cpp_type, value)
-#define VARCACHE_PREF(policy, name, id, cpp_type, value) \
-  cpp_type StaticPrefs::sVarCache_##id(value);
-#include "mozilla/StaticPrefList.h"
-#undef PREF
-#undef VARCACHE_PREF
-
 // The SetPref_*() functions below end in a `_<type>` suffix because they are
 // used by the PREF macro definition in InitAll() below.
 
 static void SetPref_bool(const char* aName, bool aDefaultValue) {
   PrefValue value;
   value.mBoolVal = aDefaultValue;
   pref_SetPref(aName, PrefType::Bool, PrefValueKind::Default, value,
                /* isSticky */ false,
@@ -5354,16 +5339,50 @@ static void InitVarCachePref(StaticPrefs
 
   *aCache = aDefaultValue;
   if (MOZ_LIKELY(aIsStartup) &&
       MOZ_LIKELY(aPolicy == StaticPrefs::UpdatePolicy::Live)) {
     AddVarCache(aCache, aName, aDefaultValue, true);
   }
 }
 
+// For a VarCache pref like this:
+//
+//   VARCACHE_PREF(Once, "my.varcache", my_varcache, int32_t, 99)
+//
+// we generate a static variable definition and a setter like:
+//
+//   int32_t StaticPrefs::sVarCache_my_varcache(99);
+//   void StaticPrefs::Setmy_varcache(int32_t aValue) {
+//     SetPref(Getmy_varcachePrefName(), aValue);
+//     if (UpdatePolicy::policy == UpdatePolicy::Once) {
+//       sVarCache_my_varcache =
+//           GetPref(Getmy_varcachePrefName(), sVarCache_my_varcache);
+//     }
+//     return;
+//   }
+
+#define PREF(name, cpp_type, value)
+#define VARCACHE_PREF(policy, name, id, cpp_type, value)                       \
+  cpp_type StaticPrefs::sVarCache_##id(value);                                 \
+  void StaticPrefs::Set##id(StripAtomic<cpp_type> aValue) {                    \
+    MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread() && XRE_IsParentProcess(),          \
+                          "pref '" name "' being set outside parent process"); \
+    SetPref(Get##id##PrefName(), aValue);                                      \
+    if (UpdatePolicy::policy == UpdatePolicy::Once) {                          \
+      sVarCache_##id =                                                         \
+          GetPref(Get##id##PrefName(), StripAtomic<cpp_type>(sVarCache_##id)); \
+    }                                                                          \
+    /* The StaticPrefs storage will be updated by the registered callback */   \
+    return;                                                                    \
+  }
+#include "mozilla/StaticPrefList.h"
+#undef PREF
+#undef VARCACHE_PREF
+
 /* static */
 void StaticPrefs::InitAll(bool aIsStartup) {
   // For prefs like these:
   //
   //   PREF("foo.bar.baz", bool, true)
   //   VARCACHE_PREF(Live, "my.varcache", my_varcache, int32_t, 99)
   //
   // we generate registration calls:
--- a/modules/libpref/StaticPrefs.h
+++ b/modules/libpref/StaticPrefs.h
@@ -71,27 +71,23 @@ class StaticPrefs {
   // For a VarCache pref like this:
   //
   //   VARCACHE_PREF("my.varcache", my_varcache, int32_t, 99)
   //
   // we generate a static variable declaration, a getter and a setter
   // definition. A StaticPref can be set by using the corresponding Set method.
   // For example, if the accessor is Foo() then calling SetFoo(...) will update
   // the preference and also change the return value of subsequent Foo() calls.
-  // Changing StaticPrefs values in one process will not affect the result in
-  // other processes.
-  // Important Note: The use of the Setter is strongly discouraged.
+  // Changing StaticPrefs is only allowed on the parent process' main thread.
   //
   //   private:
   //     static int32_t sVarCache_my_varcache;
   //   public:
   //     static int32_t my_varcache() { return sVarCache_my_varcache; }
-  //     static void Setmy_varcache(int32_t aValue) {
-  //	     sVarCache_my_varcache = aValue;
-  //     }
+  //     static void Setmy_varcache(int32_t aValue);
   //     static const char* Getmy_varcachePrefName() { return "my.varcache"; }
   //     static int32_t Getmy_varcachePrefDefault() { return 99; }
   //
 
  public:
   // Enums for the update policy.
   enum class UpdatePolicy {
     Skip,  // Set the value to default, skip any Preferences calls.
@@ -107,22 +103,17 @@ class StaticPrefs {
  public:                                                                      \
   static StripAtomic<cpp_type> id() {                                         \
     MOZ_DIAGNOSTIC_ASSERT(UpdatePolicy::policy != UpdatePolicy::Live ||       \
                               IsAtomic<cpp_type>::value || NS_IsMainThread(), \
                           "Non-atomic static pref '" str                      \
                           "' being accessed on background thread by getter"); \
     return sVarCache_##id;                                                    \
   }                                                                           \
-  static void Set##id(StripAtomic<cpp_type> aValue) {                         \
-    MOZ_DIAGNOSTIC_ASSERT(IsAtomic<cpp_type>::value || NS_IsMainThread(),     \
-                          "Non-atomic static pref '" str                      \
-                          "' being accessed on background thread by setter"); \
-    sVarCache_##id = aValue;                                                  \
-  }                                                                           \
+  static void Set##id(StripAtomic<cpp_type> aValue);                          \
   static const char* Get##id##PrefName() { return str; }                      \
   static StripAtomic<cpp_type> Get##id##PrefDefault() { return default_value; }
 
 #include "mozilla/StaticPrefList.h"
 #undef PREF
 #undef VARCACHE_PREF
 
  public: