Bug 1877193 part 18 - Assert startup prefs are set before JS_Init*. r=mgaudet,necko-reviewers,valentin

Looking at bug 1877605 made me realize we should define better when (startup) prefs
are set exactly.

This patch adds assertions to check startup prefs can only be set before `JS_Init` and
also fixes the embeddings to follow this new rule. This makes the JS shell and browser
behavior more consistent, and makes it possible to rely on pref values during startup.

Differential Revision: https://phabricator.services.mozilla.com/D200148

js/public/Prefs.h

@@ -52,8 +52,8 @@
 // Setting Prefs
 // =============
 // Embedders can override pref values. For startup prefs, this should only be
-// done during startup to avoid races with worker threads and to avoid confusing
-// code with unexpected pref changes:
+// done during startup (before calling JS_Init*) to avoid races with worker
+// threads and to avoid confusing code with unexpected pref changes:
 //
 //   JS::Prefs::setAtStartup_experimental_my_new_feature(true);
 //
@@ -86,11 +86,22 @@ class Prefs {
   // For each pref, define a static |pref_| member.
   JS_PREF_CLASS_FIELDS;
 
+#ifdef DEBUG
+  static void assertCanSetStartupPref();
+#else
+  static void assertCanSetStartupPref() {}
+#endif
+
  public:
   // For each pref, define static getter/setter accessors.
-#define DEF_GETSET(NAME, CPP_NAME, TYPE, SETTER) \
-  static TYPE CPP_NAME() { return CPP_NAME##_; } \
-  static void SETTER(TYPE value) { CPP_NAME##_ = value; }
+#define DEF_GETSET(NAME, CPP_NAME, TYPE, SETTER, IS_STARTUP_PREF) \
+  static TYPE CPP_NAME() { return CPP_NAME##_; }                  \
+  static void SETTER(TYPE value) {                                \
+    if (IS_STARTUP_PREF) {                                        \
+      assertCanSetStartupPref();                                  \
+    }                                                             \
+    CPP_NAME##_ = value;                                          \
+  }
   FOR_EACH_JS_PREF(DEF_GETSET)
 #undef DEF_GETSET
 };

js/src/GeneratePrefs.py

@@ -122,10 +122,14 @@ def generate_prefs_header(c_out, yaml_path):
             "{} JS::Prefs::{}_{{{}}};".format(field_type, cpp_name, init_value)
         )
 
+        is_startup_pref_bool = "true" if is_startup_pref else "false"
+
         # Generate a MACRO invocation like this:
-        #   MACRO("arraybuffer_transfer", arraybuffer_transfer, bool, setAtStartup_arraybuffer_transfer)
+        #   MACRO("arraybuffer_transfer", arraybuffer_transfer, bool, setAtStartup_arraybuffer_transfer, true)
         macro_entries.append(
-            'MACRO("{}", {}, {}, {})'.format(name, cpp_name, type, setter_name)
+            'MACRO("{}", {}, {}, {}, {})'.format(
+                name, cpp_name, type, setter_name, is_startup_pref_bool
+            )
         )
 
         # Generate a C++ statement to set the JS pref based on Gecko's StaticPrefs:
@@ -134,7 +138,7 @@ def generate_prefs_header(c_out, yaml_path):
         if pref.get("do_not_use_directly", False):
             browser_pref_cpp_name += "_DoNotUseDirectly"
 
-        statement = "JS::Prefs::{}(StaticPrefs::{}());".format(
+        statement = "JS::Prefs::{}(mozilla::StaticPrefs::{}());".format(
             setter_name, browser_pref_cpp_name
         )
         browser_set_statements.append(statement)

js/src/builtin/TestingFunctions.cpp

@@ -8407,9 +8407,9 @@ static bool GetAllPrefNames(JSContext* cx, unsigned argc, Value* vp) {
     return values.append(StringValue(s));
   };
 
-#define ADD_NAME(NAME, CPP_NAME, TYPE, SETTER) \
-  if (!addPref(NAME)) {                        \
-    return false;                              \
+#define ADD_NAME(NAME, CPP_NAME, TYPE, SETTER, IS_STARTUP_PREF) \
+  if (!addPref(NAME)) {                                         \
+    return false;                                               \
   }
   FOR_EACH_JS_PREF(ADD_NAME)
 #undef ADD_NAME
@@ -8450,10 +8450,10 @@ static bool GetPrefValue(JSContext* cx, unsigned argc, Value* vp) {
   };
 
   // Search for a matching pref and return its value.
-#define CHECK_PREF(NAME, CPP_NAME, TYPE, SETTER) \
-  if (StringEqualsAscii(name, NAME)) {           \
-    setReturnValue(JS::Prefs::CPP_NAME());       \
-    return true;                                 \
+#define CHECK_PREF(NAME, CPP_NAME, TYPE, SETTER, IS_STARTUP_PREF) \
+  if (StringEqualsAscii(name, NAME)) {                            \
+    setReturnValue(JS::Prefs::CPP_NAME());                        \
+    return true;                                                  \
   }
   FOR_EACH_JS_PREF(CHECK_PREF)
 #undef CHECK_PREF

js/src/fuzz-tests/tests.cpp

@@ -71,15 +71,15 @@ static void jsfuzz_uninit(JSContext* cx) {
 }
 
 int main(int argc, char* argv[]) {
+  // Override prefs for fuzz-tests.
+  JS::Prefs::setAtStartup_weakrefs(true);
+  JS::Prefs::setAtStartup_experimental_weakrefs_expose_cleanupSome(true);
+
   if (!JS_Init()) {
     fprintf(stderr, "Error: Call to jsfuzz_init() failed\n");
     return 1;
   }
 
-  // Override prefs for fuzz-tests.
-  JS::Prefs::setAtStartup_weakrefs(true);
-  JS::Prefs::setAtStartup_experimental_weakrefs_expose_cleanupSome(true);
-
   if (!jsfuzz_init(&gCx, &gGlobal)) {
     fprintf(stderr, "Error: Call to jsfuzz_init() failed\n");
     return 1;

js/src/jsapi-tests/tests.cpp

@@ -230,6 +230,13 @@ int main(int argc, char* argv[]) {
     return 0;
   }
 
+  // Override prefs for jsapi-tests.
+  JS::Prefs::setAtStartup_weakrefs(true);
+  JS::Prefs::setAtStartup_experimental_weakrefs_expose_cleanupSome(true);
+#ifdef NIGHTLY_BUILD
+  JS::Prefs::setAtStartup_experimental_symbols_as_weakmap_keys(true);
+#endif
+
   if (!options.frontendOnly) {
     if (!JS_Init()) {
       printf("TEST-UNEXPECTED-FAIL | jsapi-tests | JS_Init() failed.\n");
@@ -248,13 +255,6 @@ int main(int argc, char* argv[]) {
     return 0;
   }
 
-  // Override prefs for jsapi-tests.
-  JS::Prefs::setAtStartup_weakrefs(true);
-  JS::Prefs::setAtStartup_experimental_weakrefs_expose_cleanupSome(true);
-#ifdef NIGHTLY_BUILD
-  JS::Prefs::setAtStartup_experimental_symbols_as_weakmap_keys(true);
-#endif
-
   // Reinitializing the global for every test is quite slow, due to having to
   // recompile all self-hosted builtins. Allow tests to opt-in to reusing the
   // global.

js/src/shell/js.cpp

@@ -11345,7 +11345,7 @@ static bool SetJSPref(const char* pref) {
   const char* valStart = assign + 1;  // Skip '='.
 
   // Search for a matching pref and try to set it.
-#define CHECK_PREF(NAME, CPP_NAME, TYPE, SETTER)                          \
+#define CHECK_PREF(NAME, CPP_NAME, TYPE, SETTER, IS_STARTUP_PREF)         \
   if (nameLen == strlen(NAME) && memcmp(pref, NAME, strlen(NAME)) == 0) { \
     TYPE v;                                                               \
     if (!ParsePrefValue<TYPE>(NAME, valStart, &v)) {                      \
@@ -11374,7 +11374,7 @@ static void ListJSPrefs() {
     }
   };
 
-#define PRINT_PREF(NAME, CPP_NAME, TYPE, SETTER) \
+#define PRINT_PREF(NAME, CPP_NAME, TYPE, SETTER, IS_STARTUP_PREF) \
   printPref(NAME, JS::Prefs::CPP_NAME());
   FOR_EACH_JS_PREF(PRINT_PREF)
 #undef PRINT_PREF

js/src/vm/Prefs.cpp

@@ -6,5 +6,18 @@
 
 #include "js/Prefs.h"
 
+#include "js/Initialization.h"
+#include "vm/Runtime.h"
+
 // Set all static JS::Prefs fields to the default pref values.
 JS_PREF_CLASS_FIELDS_INIT;
+
+#ifdef DEBUG
+// static
+void JS::Prefs::assertCanSetStartupPref() {
+  MOZ_ASSERT(detail::libraryInitState == detail::InitState::Uninitialized,
+             "startup prefs must be set before calling JS_Init");
+  MOZ_ASSERT(!JSRuntime::hasLiveRuntimes(),
+             "startup prefs must be set before creating a JSContext");
+}
+#endif

js/xpconnect/src/XPCJSContext.cpp

@@ -835,8 +835,8 @@ static void LoadStartupJSPrefs(XPCJSContext* xpccx) {
   //
   // 'Live' prefs are handled by ReloadPrefsCallback below.
 
-  // Set all JS::Prefs.
-  SET_JS_PREFS_FROM_BROWSER_PREFS;
+  // Note: JS::Prefs are set earlier in startup, in InitializeJS in
+  // XPCOMInit.cpp.
 
   JSContext* cx = xpccx->Context();
 

netwerk/ipc/SocketProcessChild.cpp

@@ -30,6 +30,7 @@
 #include "mozilla/ipc/ProcessUtils.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/RemoteLazyInputStreamChild.h"
+#include "mozilla/StaticPrefs_javascript.h"
 #include "mozilla/StaticPrefs_network.h"
 #include "mozilla/Telemetry.h"
 #include "NetworkConnectivityService.h"
@@ -46,6 +47,7 @@
 #include "SocketProcessBridgeParent.h"
 #include "jsapi.h"
 #include "js/Initialization.h"
+#include "js/Prefs.h"
 #include "XPCSelfHostedShmem.h"
 
 #if defined(XP_WIN)
@@ -695,6 +697,9 @@ mozilla::ipc::IPCResult SocketProcessChild::RecvInitProxyAutoConfigChild(
   if (!sInitializedJS) {
     JS::DisableJitBackend();
 
+    // Set all JS::Prefs.
+    SET_JS_PREFS_FROM_BROWSER_PREFS;
+
     const char* jsInitFailureReason = JS_InitWithFailureDiagnostic();
     if (jsInitFailureReason) {
       MOZ_CRASH_UNSAFE(jsInitFailureReason);

xpcom/build/XPCOMInit.cpp

@@ -105,6 +105,7 @@
 
 #include "jsapi.h"
 #include "js/Initialization.h"
+#include "js/Prefs.h"
 #include "mozilla/StaticPrefs_javascript.h"
 #include "XPCSelfHostedShmem.h"
 
@@ -232,6 +233,9 @@ static void InitializeJS() {
   JS::SetAVXEnabled(mozilla::StaticPrefs::javascript_options_wasm_simd_avx());
 #endif
 
+  // Set all JS::Prefs.
+  SET_JS_PREFS_FROM_BROWSER_PREFS;
+
   const char* jsInitFailureReason = JS_InitWithFailureDiagnostic();
   if (jsInitFailureReason) {
     MOZ_CRASH_UNSAFE(jsInitFailureReason);