From 1adf442e9249afed3d557ba0c679150e4d85c60e Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Sun, 23 Apr 2023 17:53:38 -0500 Subject: 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) --- dom/base/nsJSUtils.cpp | 156 +--------------------------------------- dom/base/nsJSUtils.h | 65 +++-------------- dom/script/ScriptLoader.cpp | 2 +- dom/xbl/nsXBLProtoImplField.cpp | 12 ---- 4 files changed, 11 insertions(+), 224 deletions(-) (limited to 'dom') 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,166 +389,12 @@ nsJSUtils::ExecutionContext::ExecScript(JS::MutableHandle 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 aEvaluationGlobal, - JS::CompileOptions& aCompileOptions, - const EvaluateOptions& aEvaluateOptions, - JS::MutableHandle 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 aEvaluationGlobal, - JS::CompileOptions& aCompileOptions, - const EvaluateOptions& aEvaluateOptions, - JS::MutableHandle 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 - 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 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 aEvaluationGlobal, - JS::CompileOptions& aCompileOptions, - const EvaluateOptions& aEvaluateOptions, - JS::MutableHandle aRetValue) -{ - return EvaluateString(aCx, aSrcBuf, aEvaluationGlobal, aCompileOptions, - aEvaluateOptions, aRetValue, nullptr); -} - -nsresult -nsJSUtils::EvaluateString(JSContext* aCx, - const nsAString& aScript, - JS::Handle 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 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, 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 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 aEvaluationGlobal, - JS::CompileOptions &aCompileOptions, - const EvaluateOptions& aEvaluateOptions, - JS::MutableHandle aRetValue); - - static nsresult EvaluateString(JSContext* aCx, - JS::SourceBufferHolder& aSrcBuf, - JS::Handle aEvaluationGlobal, - JS::CompileOptions &aCompileOptions, - const EvaluateOptions& aEvaluateOptions, - JS::MutableHandle aRetValue); - - - static nsresult EvaluateString(JSContext* aCx, - const nsAString& aScript, - JS::Handle aEvaluationGlobal, - JS::CompileOptions &aCompileOptions); - - static nsresult EvaluateString(JSContext* aCx, - JS::SourceBufferHolder& aSrcBuf, - JS::Handle aEvaluationGlobal, - JS::CompileOptions &aCompileOptions, - void **aOffThreadToken); - static nsresult CompileModule(JSContext* aCx, JS::SourceBufferHolder& aSrcBuf, JS::Handle 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 aEvaluationGlobal, - JS::CompileOptions& aCompileOptions, - const EvaluateOptions& aEvaluateOptions, - JS::MutableHandle aRetValue, - void **aOffThreadToken); }; template 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 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 aBoundNode, exec.Compile(options, nsDependentString(mFieldText, mFieldTextLength)); rv = exec.ExecScript(&result); } -#endif if (NS_FAILED(rv)) { return rv; -- cgit v1.2.3