Bug 1689734 - Further restrict ScriptPreloader use of CompileOptions. r=kmag,arai

The JS CompileOptions used to load cache entries must be consistent with
eachother to avoid subtle and serious bugs. This adds additional checks and
makes more consistent use of `FillCompileOptionsForCachedScript`.

This patch is a refactoring and should not change any behaviour.

Depends on D103515

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

dom/base/nsFrameMessageManager.cpp

@@ -1260,7 +1260,6 @@ void nsMessageManagerScriptExecutor::TryCacheLoadAndCompileScript(
   ScriptPreloader::FillCompileOptionsForCachedScript(options);
   options.setFileAndLine(url.get(), 1);
   options.setNonSyntacticScope(true);
-  options.setSourceIsLazy(true);
 
   JS::Rooted<JSScript*> script(cx);
   script =

js/xpconnect/loader/ScriptPreloader.cpp

@@ -858,15 +858,27 @@ void ScriptPreloader::NoteScript(const nsCString& url,
 /* static */
 void ScriptPreloader::FillCompileOptionsForCachedScript(
     JS::CompileOptions& options) {
-  // See IsMultiDecodeCompileOptionsMatching in js/src/vm/JSScript.cpp.
+  // Users of the cache do not require return values, so inform the JS parser in
+  // order for it to generate simpler bytecode.
   options.setNoScriptRval(true);
-  MOZ_ASSERT(!options.selfHostingMode);
-  MOZ_ASSERT(!options.isRunOnce);
+
+  // The ScriptPreloader trades off having bytecode available but not source
+  // text. This means the JS syntax-only parser is not used. If `toString` is
+  // called on functions in these scripts, the source-hook will fetch it over,
+  // so using `toString` of functions should be avoided in chrome js.
+  options.setSourceIsLazy(true);
 }
 
 JSScript* ScriptPreloader::GetCachedScript(
     JSContext* cx, const JS::ReadOnlyCompileOptions& options,
     const nsCString& path) {
+  // Users of ScriptPreloader must agree on a standard set of compile options so
+  // that bytecode data can safely saved from one context and loaded in another.
+  MOZ_ASSERT(options.noScriptRval);
+  MOZ_ASSERT(!options.selfHostingMode);
+  MOZ_ASSERT(!options.isRunOnce);
+  MOZ_ASSERT(options.sourceIsLazy);
+
   // If a script is used by both the parent and the child, it's stored only
   // in the child cache.
   if (mChildCache) {
@@ -1088,7 +1100,6 @@ void ScriptPreloader::DecodeNextBatch(size_t chunkSize,
 
   JS::CompileOptions options(cx);
   FillCompileOptionsForCachedScript(options);
-  options.setSourceIsLazy(true);
 
   if (!JS::CanCompileOffThread(cx, options, size) ||
       !JS::DecodeMultiOffThreadScripts(cx, options, mParsingSources,

js/xpconnect/loader/mozJSComponentLoader.cpp

@@ -731,10 +731,9 @@ nsresult mozJSComponentLoader::ObjectForLocation(
 
   CompileOptions options(cx);
   ScriptPreloader::FillCompileOptionsForCachedScript(options);
-  options.setForceStrictMode()
-      .setFileAndLine(nativePath.get(), 1)
-      .setSourceIsLazy(true)
-      .setNonSyntacticScope(true);
+  options.setFileAndLine(nativePath.get(), 1);
+  options.setForceStrictMode();
+  options.setNonSyntacticScope(true);
 
   script =
       ScriptPreloader::GetSingleton().GetCachedScript(cx, options, cachePath);

js/xpconnect/loader/mozJSSubScriptLoader.cpp

@@ -121,25 +121,6 @@ static void ReportError(JSContext* cx, const char* origMsg, nsIURI* uri) {
   ReportError(cx, msg);
 }
 
-static void FillCompileOptions(JS::CompileOptions& options, const char* uriStr,
-                               bool wantGlobalScript, bool wantReturnValue) {
-  options.setFileAndLine(uriStr, 1).setNoScriptRval(!wantReturnValue);
-
-  // This presumes that no one else might be compiling a script for this
-  // (URL, syntactic-or-not) key *not* using UTF-8.  Seeing as JS source can
-  // only be compiled as UTF-8 or UTF-16 now -- there isn't a JSAPI function to
-  // compile Latin-1 now -- this presumption seems relatively safe.
-  //
-  // This also presumes that lazy parsing is disabled, for the sake of the
-  // startup cache.  If lazy parsing is ever enabled for pertinent scripts that
-  // pass through here, we may need to disable lazy source for them.
-  options.setSourceIsLazy(true);
-
-  if (!wantGlobalScript) {
-    options.setNonSyntacticScope(true);
-  }
-}
-
 static JSScript* PrepareScript(nsIURI* uri, JSContext* cx,
                                const JS::ReadOnlyCompileOptions& options,
                                const char* buf, int64_t len) {
@@ -472,12 +453,18 @@ nsresult mozJSSubScriptLoader::DoLoadSubScriptWithOptions(
   SubscriptCachePath(cx, uri, targetObj, cachePath);
 
   JS::CompileOptions compileOptions(cx);
-  FillCompileOptions(compileOptions, uriStr.get(), JS_IsGlobalObject(targetObj),
-                     options.wantReturnValue);
+  ScriptPreloader::FillCompileOptionsForCachedScript(compileOptions);
+  compileOptions.setFileAndLine(uriStr.get(), 1);
+  compileOptions.setNonSyntacticScope(!JS_IsGlobalObject(targetObj));
+
+  if (options.wantReturnValue) {
+    compileOptions.setNoScriptRval(false);
+  }
 
   RootedScript script(cx);
   if (!options.ignoreCache) {
     if (!options.wantReturnValue) {
+      // NOTE: If we need the return value, we cannot use ScriptPreloader.
       script = ScriptPreloader::GetSingleton().GetCachedScript(
           cx, compileOptions, cachePath);
     }