Issue #1691 - Part 12: Fix return value in ExecScript() and debug assert in ParseTask.

https://bugzilla.mozilla.org/show_bug.cgi?id=1331662
Replace nsJSUtils::EvaluateString calls by ExecutionContext scopes.
ExecutionContext: The mRetValue is not used even though it is checked in various asserts.
This is how it is in the Mozilla code even years later, only the passed in value is used.
ParseTask: JoinDecode() and JoinCompile() run the same code but the type is different
causing a debug assert when it checks the type Script vs ScriptDecode.

(cherry picked from commit 8f577428b7834b7d0f0d5afacbe060fbb1ff2136)

4 files changed, 11 insertions, 224 deletions

diff --git a/dom/base/nsJSUtils.cpp b/dom/base/nsJSUtils.cpp
index b43f94422d..43efe6f4e3 100644
--- a/dom/base/nsJSUtils.cpp
+++ b/dom/base/nsJSUtils.cpp
@@ -389,167 +389,13 @@ nsJSUtils::ExecutionContext::ExecScript(JS::MutableHandle<JS::Value> aRetValue)
       mSkip = true;
       return EvaluationExceptionToNSResult(mCx);
     }
-    mRetValue.set(JS::StringValue(str));
+    aRetValue.set(JS::StringValue(str));
   }
 
-  aRetValue.set(mRetValue);
   return NS_OK;
 }
 
 nsresult
-nsJSUtils::EvaluateString(JSContext* aCx,
-                          const nsAString& aScript,
-                          JS::Handle<JSObject*> aEvaluationGlobal,
-                          JS::CompileOptions& aCompileOptions,
-                          const EvaluateOptions& aEvaluateOptions,
-                          JS::MutableHandle<JS::Value> aRetValue)
-{
-  const nsPromiseFlatString& flatScript = PromiseFlatString(aScript);
-  JS::SourceBufferHolder srcBuf(flatScript.get(), aScript.Length(),
-                                JS::SourceBufferHolder::NoOwnership);
-  return EvaluateString(aCx, srcBuf, aEvaluationGlobal, aCompileOptions,
-                        aEvaluateOptions, aRetValue, nullptr);
-}
-
-nsresult
-nsJSUtils::EvaluateString(JSContext* aCx,
-                          JS::SourceBufferHolder& aSrcBuf,
-                          JS::Handle<JSObject*> aEvaluationGlobal,
-                          JS::CompileOptions& aCompileOptions,
-                          const EvaluateOptions& aEvaluateOptions,
-                          JS::MutableHandle<JS::Value> aRetValue,
-                          void **aOffThreadToken)
-{
-  PROFILER_LABEL("nsJSUtils", "EvaluateString",
-    js::ProfileEntry::Category::JS);
-
-  MOZ_ASSERT_IF(aCompileOptions.versionSet,
-                aCompileOptions.version != JSVERSION_UNKNOWN);
-  MOZ_ASSERT_IF(aEvaluateOptions.coerceToString, !aCompileOptions.noScriptRval);
-  MOZ_ASSERT(aCx == nsContentUtils::GetCurrentJSContext());
-  MOZ_ASSERT(aSrcBuf.get());
-  MOZ_ASSERT(js::GetGlobalForObjectCrossCompartment(aEvaluationGlobal) ==
-             aEvaluationGlobal);
-  MOZ_ASSERT_IF(aOffThreadToken, aCompileOptions.noScriptRval);
-  MOZ_ASSERT(NS_IsMainThread());
-  MOZ_ASSERT(CycleCollectedJSContext::Get() &&
-             CycleCollectedJSContext::Get()->MicroTaskLevel());
-
-  // Unfortunately, the JS engine actually compiles scripts with a return value
-  // in a different, less efficient way.  Furthermore, it can't JIT them in many
-  // cases.  So we need to be explicitly told whether the caller cares about the
-  // return value.  Callers can do this by calling the other overload of
-  // EvaluateString() which calls this function with
-  // aCompileOptions.noScriptRval set to true.
-  aRetValue.setUndefined();
-
-  nsresult rv = NS_OK;
-
-  NS_ENSURE_TRUE(xpc::Scriptability::Get(aEvaluationGlobal).Allowed(), NS_OK);
-
-  bool ok = true;
-  // Scope the JSAutoCompartment so that we can later wrap the return value
-  // into the caller's cx.
-  {
-    JSAutoCompartment ac(aCx, aEvaluationGlobal);
-
-    // Now make sure to wrap the scope chain into the right compartment.
-    JS::AutoObjectVector scopeChain(aCx);
-    if (!scopeChain.reserve(aEvaluateOptions.scopeChain.length())) {
-      return NS_ERROR_OUT_OF_MEMORY;
-    }
-
-    for (size_t i = 0; i < aEvaluateOptions.scopeChain.length(); ++i) {
-      JS::ExposeObjectToActiveJS(aEvaluateOptions.scopeChain[i]);
-      scopeChain.infallibleAppend(aEvaluateOptions.scopeChain[i]);
-      if (!JS_WrapObject(aCx, scopeChain[i])) {
-        ok = false;
-        break;
-      }
-    }
-
-    if (ok && aOffThreadToken) {
-      JS::Rooted<JSScript*>
-        script(aCx, JS::FinishOffThreadScript(aCx, *aOffThreadToken));
-      *aOffThreadToken = nullptr; // Mark the token as having been finished.
-      if (script) {
-        ok = JS_ExecuteScript(aCx, scopeChain, script);
-      } else {
-        ok = false;
-      }
-    } else if (ok) {
-      ok = JS::Evaluate(aCx, scopeChain, aCompileOptions, aSrcBuf, aRetValue);
-    }
-
-    if (ok && aEvaluateOptions.coerceToString && !aRetValue.isUndefined()) {
-      JS::Rooted<JS::Value> value(aCx, aRetValue);
-      JSString* str = JS::ToString(aCx, value);
-      ok = !!str;
-      aRetValue.set(ok ? JS::StringValue(str) : JS::UndefinedValue());
-    }
-  }
-
-  if (!ok) {
-    if (JS_IsExceptionPending(aCx)) {
-      rv = NS_SUCCESS_DOM_SCRIPT_EVALUATION_THREW;
-    } else {
-      rv = NS_SUCCESS_DOM_SCRIPT_EVALUATION_THREW_UNCATCHABLE;
-    }
-
-    if (!aCompileOptions.noScriptRval) {
-      aRetValue.setUndefined();
-    }
-  }
-
-  // Wrap the return value into whatever compartment aCx was in.
-  if (ok && !aCompileOptions.noScriptRval) {
-    if (!JS_WrapValue(aCx, aRetValue)) {
-      return NS_ERROR_OUT_OF_MEMORY;
-    }
-  }
-  return rv;
-}
-
-nsresult
-nsJSUtils::EvaluateString(JSContext* aCx,
-                          JS::SourceBufferHolder& aSrcBuf,
-                          JS::Handle<JSObject*> aEvaluationGlobal,
-                          JS::CompileOptions& aCompileOptions,
-                          const EvaluateOptions& aEvaluateOptions,
-                          JS::MutableHandle<JS::Value> aRetValue)
-{
-  return EvaluateString(aCx, aSrcBuf, aEvaluationGlobal, aCompileOptions,
-                        aEvaluateOptions, aRetValue, nullptr);
-}
-
-nsresult
-nsJSUtils::EvaluateString(JSContext* aCx,
-                          const nsAString& aScript,
-                          JS::Handle<JSObject*> aEvaluationGlobal,
-                          JS::CompileOptions& aCompileOptions)
-{
-  EvaluateOptions options(aCx);
-  aCompileOptions.setNoScriptRval(true);
-  JS::RootedValue unused(aCx);
-  return EvaluateString(aCx, aScript, aEvaluationGlobal, aCompileOptions,
-                        options, &unused);
-}
-
-nsresult
-nsJSUtils::EvaluateString(JSContext* aCx,
-                          JS::SourceBufferHolder& aSrcBuf,
-                          JS::Handle<JSObject*> aEvaluationGlobal,
-                          JS::CompileOptions& aCompileOptions,
-                          void **aOffThreadToken)
-{
-  EvaluateOptions options(aCx);
-  aCompileOptions.setNoScriptRval(true);
-  JS::RootedValue unused(aCx);
-  return EvaluateString(aCx, aSrcBuf, aEvaluationGlobal, aCompileOptions,
-                        options, &unused, aOffThreadToken);
-}
-
-nsresult
 nsJSUtils::CompileModule(JSContext* aCx,
                        JS::SourceBufferHolder& aSrcBuf,
                        JS::Handle<JSObject*> aEvaluationGlobal,
diff --git a/dom/base/nsJSUtils.h b/dom/base/nsJSUtils.h
index 8d5ccc95ef..db70cd924d 100644
--- a/dom/base/nsJSUtils.h
+++ b/dom/base/nsJSUtils.h
@@ -128,6 +128,15 @@ public:
       return *this;
     }
 
+    // When set, this flag records and encodes the bytecode as soon as it is
+    // being compiled, and before it is being executed. The bytecode can then be
+    // requested by using |JS::FinishIncrementalEncoding| with the mutable
+    // handle |aScript| argument of |CompileAndExec| or |JoinAndExec|.
+    ExecutionContext& SetEncodeBytecode(bool aEncodeBytecode) {
+      mEncodeBytecode = aEncodeBytecode;
+      return *this;
+    }
+
     // Set the scope chain in which the code should be executed.
     void SetScopeChain(const JS::AutoObjectVector& aScopeChain);
 
@@ -177,52 +186,6 @@ public:
     MOZ_MUST_USE nsresult ExecScript(JS::MutableHandle<JS::Value> aRetValue);
   };
 
-  struct MOZ_STACK_CLASS EvaluateOptions {
-    bool coerceToString;
-    JS::AutoObjectVector scopeChain;
-
-    explicit EvaluateOptions(JSContext* cx)
-      : coerceToString(false)
-      , scopeChain(cx)
-    {}
-
-    EvaluateOptions& setCoerceToString(bool aCoerce) {
-      coerceToString = aCoerce;
-      return *this;
-    }
-  };
-
-  // aEvaluationGlobal is the global to evaluate in.  The return value
-  // will then be wrapped back into the compartment aCx is in when
-  // this function is called.  For all the EvaluateString overloads,
-  // the JSContext must come from an AutoJSAPI that has had
-  // TakeOwnershipOfErrorReporting() called on it.
-  static nsresult EvaluateString(JSContext* aCx,
-                                 const nsAString& aScript,
-                                 JS::Handle<JSObject*> aEvaluationGlobal,
-                                 JS::CompileOptions &aCompileOptions,
-                                 const EvaluateOptions& aEvaluateOptions,
-                                 JS::MutableHandle<JS::Value> aRetValue);
-
-  static nsresult EvaluateString(JSContext* aCx,
-                                 JS::SourceBufferHolder& aSrcBuf,
-                                 JS::Handle<JSObject*> aEvaluationGlobal,
-                                 JS::CompileOptions &aCompileOptions,
-                                 const EvaluateOptions& aEvaluateOptions,
-                                 JS::MutableHandle<JS::Value> aRetValue);
-
-
-  static nsresult EvaluateString(JSContext* aCx,
-                                 const nsAString& aScript,
-                                 JS::Handle<JSObject*> aEvaluationGlobal,
-                                 JS::CompileOptions &aCompileOptions);
-
-  static nsresult EvaluateString(JSContext* aCx,
-                                 JS::SourceBufferHolder& aSrcBuf,
-                                 JS::Handle<JSObject*> aEvaluationGlobal,
-                                 JS::CompileOptions &aCompileOptions,
-                                 void **aOffThreadToken);
-
   static nsresult CompileModule(JSContext* aCx,
                                 JS::SourceBufferHolder& aSrcBuf,
                                 JS::Handle<JSObject*> aEvaluationGlobal,
@@ -242,16 +205,6 @@ public:
                                       JS::AutoObjectVector& aScopeChain);
 
   static void ResetTimeZone();
-
-private:
-  // Implementation for our EvaluateString bits
-  static nsresult EvaluateString(JSContext* aCx,
-                                 JS::SourceBufferHolder& aSrcBuf,
-                                 JS::Handle<JSObject*> aEvaluationGlobal,
-                                 JS::CompileOptions& aCompileOptions,
-                                 const EvaluateOptions& aEvaluateOptions,
-                                 JS::MutableHandle<JS::Value> aRetValue,
-                                 void **aOffThreadToken);
 };
 
 template<typename T>
diff --git a/dom/script/ScriptLoader.cpp b/dom/script/ScriptLoader.cpp
index 1136152096..e43803e7c8 100644
--- a/dom/script/ScriptLoader.cpp
+++ b/dom/script/ScriptLoader.cpp
@@ -2349,7 +2349,7 @@ ScriptLoader::EvaluateScript(ScriptLoadRequest* aRequest)
         {
           nsJSUtils::ExecutionContext exec(cx, global);
           if (aRequest->mOffThreadToken) {
-            rv = exec.JoinDecode(&aRequest->mOffThreadToken);
+            rv = exec.JoinCompile(&aRequest->mOffThreadToken);
           } else {
             nsAutoString inlineData;
             SourceBufferHolder srcBuf = GetScriptSource(aRequest, inlineData);
diff --git a/dom/xbl/nsXBLProtoImplField.cpp b/dom/xbl/nsXBLProtoImplField.cpp
index 2ace609cc9..a68ff67aea 100644
--- a/dom/xbl/nsXBLProtoImplField.cpp
+++ b/dom/xbl/nsXBLProtoImplField.cpp
@@ -434,17 +434,6 @@ nsXBLProtoImplField::InstallField(JS::Handle<JSObject*> aBoundNode,
   JS::CompileOptions options(cx);
   options.setFileAndLine(uriSpec.get(), mLineNumber)
          .setVersion(JSVERSION_LATEST);
-#if 1
-  nsJSUtils::EvaluateOptions evalOptions(cx);
-  if (!nsJSUtils::GetScopeChainForElement(cx, boundElement,
-                                          evalOptions.scopeChain)) {
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
-  rv = nsJSUtils::EvaluateString(cx, nsDependentString(mFieldText,
-                                                       mFieldTextLength),
-                                 scopeObject, options, evalOptions, &result);
-#endif
-#if 0
   JS::AutoObjectVector scopeChain(cx);
   if (!nsJSUtils::GetScopeChainForElement(cx, boundElement, scopeChain)) {
     return NS_ERROR_OUT_OF_MEMORY;
@@ -456,7 +445,6 @@ nsXBLProtoImplField::InstallField(JS::Handle<JSObject*> aBoundNode,
     exec.Compile(options, nsDependentString(mFieldText, mFieldTextLength));
     rv = exec.ExecScript(&result);
   }
-#endif
 
   if (NS_FAILED(rv)) {
     return rv;