From cdfffa27fe52d8ba54e873ce9311fd346e68ef98 Mon Sep 17 00:00:00 2001 From: Jon Coppeard Date: Tue, 17 Jul 2018 14:30:23 +0100 Subject: [PATCH] Bug 1475228 - Make asynchronous compile APIs take SourceBufferHolders r=jandem --- dom/script/ScriptLoader.cpp | 12 ++++++---- dom/xul/XULDocument.cpp | 11 +++------- dom/xul/nsXULElement.cpp | 2 +- js/src/NamespaceImports.h | 2 +- .../jsapi-tests/testCompileNonSyntactic.cpp | 4 ++-- js/src/jsapi.cpp | 12 +++++----- js/src/jsapi.h | 6 ++--- js/src/shell/js.cpp | 8 +++++-- js/src/vm/HelperThreads.cpp | 22 +++++++++---------- js/src/vm/HelperThreads.h | 12 +++++----- js/xpconnect/loader/ChromeScriptLoader.cpp | 6 ++--- 11 files changed, 49 insertions(+), 48 deletions(-) diff --git a/dom/script/ScriptLoader.cpp b/dom/script/ScriptLoader.cpp index b339d2a0a215..2c4ea44d61b0 100644 --- a/dom/script/ScriptLoader.cpp +++ b/dom/script/ScriptLoader.cpp @@ -1830,9 +1830,11 @@ ScriptLoader::AttemptAsyncScriptCompile(ScriptLoadRequest* aRequest) if (aRequest->IsModuleRequest()) { MOZ_ASSERT(aRequest->IsTextSource()); + JS::SourceBufferHolder srcBuf(aRequest->ScriptText().begin(), + aRequest->ScriptText().length(), + JS::SourceBufferHolder::NoOwnership); if (!JS::CompileOffThreadModule(cx, options, - aRequest->ScriptText().begin(), - aRequest->ScriptText().length(), + srcBuf, OffThreadScriptLoaderCallback, static_cast(runnable))) { return NS_ERROR_OUT_OF_MEMORY; @@ -1858,9 +1860,11 @@ ScriptLoader::AttemptAsyncScriptCompile(ScriptLoadRequest* aRequest) #endif } else { MOZ_ASSERT(aRequest->IsTextSource()); + JS::SourceBufferHolder srcBuf(aRequest->ScriptText().begin(), + aRequest->ScriptText().length(), + JS::SourceBufferHolder::NoOwnership); if (!JS::CompileOffThread(cx, options, - aRequest->ScriptText().begin(), - aRequest->ScriptText().length(), + srcBuf, OffThreadScriptLoaderCallback, static_cast(runnable))) { return NS_ERROR_OUT_OF_MEMORY; diff --git a/dom/xul/XULDocument.cpp b/dom/xul/XULDocument.cpp index a5a9c65db5e6..dc6474b38dc0 100644 --- a/dom/xul/XULDocument.cpp +++ b/dom/xul/XULDocument.cpp @@ -2241,15 +2241,10 @@ XULDocument::OnStreamComplete(nsIStreamLoader* aLoader, rv = mCurrentScriptProto->Compile(srcBuf, uri, 1, this, this); if (NS_SUCCEEDED(rv) && !mCurrentScriptProto->HasScriptObject()) { // We will be notified via OnOffThreadCompileComplete when the - // compile finishes. Keep the contents of the compiled script - // alive until the compilation finishes. + // compile finishes. The JS engine has taken ownership of the + // source buffer. + MOZ_RELEASE_ASSERT(!srcBuf.ownsChars()); mOffThreadCompiling = true; - // If the JS engine did not take the source buffer, then take - // it back here to ensure it remains alive. - mOffThreadCompileStringBuf = srcBuf.take(); - if (mOffThreadCompileStringBuf) { - mOffThreadCompileStringLength = srcBuf.length(); - } BlockOnload(); return NS_OK; } diff --git a/dom/xul/nsXULElement.cpp b/dom/xul/nsXULElement.cpp index 9ba1eb52cdbe..0c8c522429a5 100644 --- a/dom/xul/nsXULElement.cpp +++ b/dom/xul/nsXULElement.cpp @@ -2257,7 +2257,7 @@ nsXULPrototypeScript::Compile(JS::SourceBufferHolder& aSrcBuf, if (aOffThreadReceiver && JS::CanCompileOffThread(cx, options, aSrcBuf.length())) { if (!JS::CompileOffThread(cx, options, - aSrcBuf.get(), aSrcBuf.length(), + aSrcBuf, OffThreadScriptReceiverCallback, static_cast(aOffThreadReceiver))) { return NS_ERROR_OUT_OF_MEMORY; diff --git a/js/src/NamespaceImports.h b/js/src/NamespaceImports.h index e9d5645e9213..60ee0c2a1357 100644 --- a/js/src/NamespaceImports.h +++ b/js/src/NamespaceImports.h @@ -38,7 +38,7 @@ using ValueVector = JS::GCVector; using IdVector = JS::GCVector; using ScriptVector = JS::GCVector; -class MOZ_STACK_CLASS SourceBufferHolder; +class SourceBufferHolder; class HandleValueArray; diff --git a/js/src/jsapi-tests/testCompileNonSyntactic.cpp b/js/src/jsapi-tests/testCompileNonSyntactic.cpp index d74c4f435451..436f3ba64818 100644 --- a/js/src/jsapi-tests/testCompileNonSyntactic.cpp +++ b/js/src/jsapi-tests/testCompileNonSyntactic.cpp @@ -108,8 +108,8 @@ testCompile(bool nonSyntactic) OffThreadTask task; OffThreadToken* token; - CHECK(CompileOffThread(cx, options, src_16, length, - task.OffThreadCallback, &task)); + SourceBufferHolder srcBuf(src_16, length, SourceBufferHolder::NoOwnership); + CHECK(CompileOffThread(cx, options, srcBuf, task.OffThreadCallback, &task)); CHECK(token = task.waitUntilDone(cx)); CHECK(script = FinishOffThreadScript(cx, token)); CHECK_EQUAL(script->hasNonSyntacticScope(), nonSyntactic); diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index c4d78606e6a9..adca77bae565 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -4215,11 +4215,11 @@ JS::CanDecodeBinASTOffThread(JSContext* cx, const ReadOnlyCompileOptions& option JS_PUBLIC_API(bool) JS::CompileOffThread(JSContext* cx, const ReadOnlyCompileOptions& options, - const char16_t* chars, size_t length, + JS::SourceBufferHolder& srcBuf, OffThreadCompileCallback callback, void* callbackData) { - MOZ_ASSERT(CanCompileOffThread(cx, options, length)); - return StartOffThreadParseScript(cx, options, chars, length, callback, callbackData); + MOZ_ASSERT(CanCompileOffThread(cx, options, srcBuf.length())); + return StartOffThreadParseScript(cx, options, srcBuf, callback, callbackData); } JS_PUBLIC_API(JSScript*) @@ -4240,11 +4240,11 @@ JS::CancelOffThreadScript(JSContext* cx, JS::OffThreadToken* token) JS_PUBLIC_API(bool) JS::CompileOffThreadModule(JSContext* cx, const ReadOnlyCompileOptions& options, - const char16_t* chars, size_t length, + JS::SourceBufferHolder& srcBuf, OffThreadCompileCallback callback, void* callbackData) { - MOZ_ASSERT(CanCompileOffThread(cx, options, length)); - return StartOffThreadParseModule(cx, options, chars, length, callback, callbackData); + MOZ_ASSERT(CanCompileOffThread(cx, options, srcBuf.length())); + return StartOffThreadParseModule(cx, options, srcBuf, callback, callbackData); } JS_PUBLIC_API(JSObject*) diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 830b1e3c0d98..1c3fb82e3fa1 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -378,7 +378,7 @@ namespace JS { * JS::SourceBufferHolder srcBuf(chars, length, JS::SourceBufferHolder::GiveOwnership); * JS::Compile(cx, options, srcBuf); */ -class MOZ_STACK_CLASS SourceBufferHolder final +class SourceBufferHolder final { public: enum Ownership { @@ -3648,7 +3648,7 @@ CanDecodeOffThread(JSContext* cx, const ReadOnlyCompileOptions& options, size_t extern JS_PUBLIC_API(bool) CompileOffThread(JSContext* cx, const ReadOnlyCompileOptions& options, - const char16_t* chars, size_t length, + JS::SourceBufferHolder& srcBuf, OffThreadCompileCallback callback, void* callbackData); extern JS_PUBLIC_API(JSScript*) @@ -3659,7 +3659,7 @@ CancelOffThreadScript(JSContext* cx, OffThreadToken* token); extern JS_PUBLIC_API(bool) CompileOffThreadModule(JSContext* cx, const ReadOnlyCompileOptions& options, - const char16_t* chars, size_t length, + JS::SourceBufferHolder& srcBuf, OffThreadCompileCallback callback, void* callbackData); extern JS_PUBLIC_API(JSObject*) diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index 5e9a869f7bbf..e032dbf00017 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -4743,7 +4743,9 @@ OffThreadCompileScript(JSContext* cx, unsigned argc, Value* vp) if (!job) return false; - if (!JS::CompileOffThread(cx, options, job->sourceChars(), length, + JS::SourceBufferHolder srcBuf(job->sourceChars(), length, + JS::SourceBufferHolder::NoOwnership); + if (!JS::CompileOffThread(cx, options, srcBuf, OffThreadCompileScriptCallback, job)) { job->cancel(); @@ -4829,7 +4831,9 @@ OffThreadCompileModule(JSContext* cx, unsigned argc, Value* vp) if (!job) return false; - if (!JS::CompileOffThreadModule(cx, options, job->sourceChars(), length, + JS::SourceBufferHolder srcBuf(job->sourceChars(), length, + JS::SourceBufferHolder::NoOwnership); + if (!JS::CompileOffThreadModule(cx, options, srcBuf, OffThreadCompileScriptCallback, job)) { job->cancel(); diff --git a/js/src/vm/HelperThreads.cpp b/js/src/vm/HelperThreads.cpp index 86e0f3b9eceb..67d32fd58978 100644 --- a/js/src/vm/HelperThreads.cpp +++ b/js/src/vm/HelperThreads.cpp @@ -482,22 +482,21 @@ ParseTask::sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const errors.sizeOfExcludingThis(mallocSizeOf); } -ScriptParseTask::ScriptParseTask(JSContext* cx, const char16_t* chars, size_t length, +ScriptParseTask::ScriptParseTask(JSContext* cx, JS::SourceBufferHolder& srcBuf, JS::OffThreadCompileCallback callback, void* callbackData) : ParseTask(ParseTaskKind::Script, cx, callback, callbackData), - data(TwoByteChars(chars, length)) + data(std::move(srcBuf)) {} void ScriptParseTask::parse(JSContext* cx) { - SourceBufferHolder srcBuf(data.begin().get(), data.length(), SourceBufferHolder::NoOwnership); Rooted sourceObject(cx); ScopeKind scopeKind = options.nonSyntacticScope ? ScopeKind::NonSyntactic : ScopeKind::Global; JSScript* script = frontend::CompileGlobalScript(cx, alloc, scopeKind, - options, srcBuf, + options, data, /* sourceObjectOut = */ &sourceObject.get()); if (script) scripts.infallibleAppend(script); @@ -505,19 +504,18 @@ ScriptParseTask::parse(JSContext* cx) sourceObjects.infallibleAppend(sourceObject); } -ModuleParseTask::ModuleParseTask(JSContext* cx, const char16_t* chars, size_t length, +ModuleParseTask::ModuleParseTask(JSContext* cx, JS::SourceBufferHolder& srcBuf, JS::OffThreadCompileCallback callback, void* callbackData) : ParseTask(ParseTaskKind::Module, cx, callback, callbackData), - data(TwoByteChars(chars, length)) + data(std::move(srcBuf)) {} void ModuleParseTask::parse(JSContext* cx) { - SourceBufferHolder srcBuf(data.begin().get(), data.length(), SourceBufferHolder::NoOwnership); Rooted sourceObject(cx); - ModuleObject* module = frontend::CompileModule(cx, options, srcBuf, alloc, &sourceObject.get()); + ModuleObject* module = frontend::CompileModule(cx, options, data, alloc, &sourceObject.get()); if (module) { scripts.infallibleAppend(module->script()); if (sourceObject) @@ -825,10 +823,10 @@ StartOffThreadParseTask(JSContext* cx, ParseTask* task, const ReadOnlyCompileOpt bool js::StartOffThreadParseScript(JSContext* cx, const ReadOnlyCompileOptions& options, - const char16_t* chars, size_t length, + JS::SourceBufferHolder& srcBuf, JS::OffThreadCompileCallback callback, void* callbackData) { - auto task = cx->make_unique(cx, chars, length, callback, callbackData); + auto task = cx->make_unique(cx, srcBuf, callback, callbackData); if (!task || !StartOffThreadParseTask(cx, task.get(), options)) return false; @@ -838,10 +836,10 @@ js::StartOffThreadParseScript(JSContext* cx, const ReadOnlyCompileOptions& optio bool js::StartOffThreadParseModule(JSContext* cx, const ReadOnlyCompileOptions& options, - const char16_t* chars, size_t length, + JS::SourceBufferHolder& srcBuf, JS::OffThreadCompileCallback callback, void* callbackData) { - auto task = cx->make_unique(cx, chars, length, callback, callbackData); + auto task = cx->make_unique(cx, srcBuf, callback, callbackData); if (!task || !StartOffThreadParseTask(cx, task.get(), options)) return false; diff --git a/js/src/vm/HelperThreads.h b/js/src/vm/HelperThreads.h index 426ee2cbd3a3..dae4182a7ebe 100644 --- a/js/src/vm/HelperThreads.h +++ b/js/src/vm/HelperThreads.h @@ -591,12 +591,12 @@ CancelOffThreadParses(JSRuntime* runtime); */ bool StartOffThreadParseScript(JSContext* cx, const ReadOnlyCompileOptions& options, - const char16_t* chars, size_t length, + JS::SourceBufferHolder& srcBuf, JS::OffThreadCompileCallback callback, void* callbackData); bool StartOffThreadParseModule(JSContext* cx, const ReadOnlyCompileOptions& options, - const char16_t* chars, size_t length, + JS::SourceBufferHolder& srcBuf, JS::OffThreadCompileCallback callback, void* callbackData); bool @@ -722,18 +722,18 @@ struct ParseTask : public mozilla::LinkedListElement, public JS::OffT struct ScriptParseTask : public ParseTask { - JS::TwoByteChars data; + JS::SourceBufferHolder data; - ScriptParseTask(JSContext* cx, const char16_t* chars, size_t length, + ScriptParseTask(JSContext* cx, JS::SourceBufferHolder& srcBuf, JS::OffThreadCompileCallback callback, void* callbackData); void parse(JSContext* cx) override; }; struct ModuleParseTask : public ParseTask { - JS::TwoByteChars data; + JS::SourceBufferHolder data; - ModuleParseTask(JSContext* cx, const char16_t* chars, size_t length, + ModuleParseTask(JSContext* cx, JS::SourceBufferHolder& srcBuf, JS::OffThreadCompileCallback callback, void* callbackData); void parse(JSContext* cx) override; }; diff --git a/js/xpconnect/loader/ChromeScriptLoader.cpp b/js/xpconnect/loader/ChromeScriptLoader.cpp index 4ec0bfca8280..faeb90acb892 100644 --- a/js/xpconnect/loader/ChromeScriptLoader.cpp +++ b/js/xpconnect/loader/ChromeScriptLoader.cpp @@ -131,8 +131,10 @@ AsyncScriptCompiler::StartCompile(JSContext* aCx) { Rooted global(aCx, mGlobalObject->GetGlobalJSObject()); + JS::SourceBufferHolder srcBuf(mScriptText.get(), mScriptLength, + JS::SourceBufferHolder::NoOwnership); if (JS::CanCompileOffThread(aCx, mOptions, mScriptLength)) { - if (!JS::CompileOffThread(aCx, mOptions, mScriptText.get(), mScriptLength, + if (!JS::CompileOffThread(aCx, mOptions, srcBuf, OffThreadScriptLoaderCallback, static_cast(this))) { return false; @@ -143,8 +145,6 @@ AsyncScriptCompiler::StartCompile(JSContext* aCx) } Rooted script(aCx); - JS::SourceBufferHolder srcBuf(mScriptText.get(), mScriptLength, - JS::SourceBufferHolder::NoOwnership); if (!JS::Compile(aCx, mOptions, srcBuf, &script)) { return false; }