From 9766fa5987e8a86baddb1cb568790e6f573ffd60 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Tue, 22 Apr 2014 14:08:27 -0700 Subject: [PATCH] Bug 990353 - Make the decision to discard source entirely per-global, rather than per-script. r=luke This is effectively a policy decision based on the kind of code we expect to be running somewhere. This is in contrast to lazy source, which is often a practical per-script consideration of whether or not we can retrieve the source if requested. More importantly, tracking this information on the global is much easier to get right than tracking it on the script. --- content/xul/content/src/nsXULElement.cpp | 3 +- js/src/frontend/BytecodeCompiler.cpp | 22 ++++++------- js/src/jit-test/tests/basic/withSourceHook.js | 6 ++-- .../jit-test/tests/debug/Source-text-lazy.js | 2 +- js/src/jsapi-tests/testSourcePolicy.cpp | 2 +- js/src/jsapi.cpp | 8 ++++- js/src/jsapi.h | 26 ++++++++++----- js/src/jsfriendapi.h | 16 +++++----- js/src/shell/js.cpp | 32 ++++--------------- js/src/vm/SelfHosting.cpp | 6 ++-- js/xpconnect/loader/mozJSComponentLoader.cpp | 9 ++++-- js/xpconnect/loader/mozJSSubScriptLoader.cpp | 5 ++- 12 files changed, 67 insertions(+), 70 deletions(-) diff --git a/content/xul/content/src/nsXULElement.cpp b/content/xul/content/src/nsXULElement.cpp index 6b5bcc68b2e6..b9887068c405 100644 --- a/content/xul/content/src/nsXULElement.cpp +++ b/content/xul/content/src/nsXULElement.cpp @@ -2645,8 +2645,7 @@ nsXULPrototypeScript::Compile(const char16_t* aText, // If the script was inline, tell the JS parser to save source for // Function.prototype.toSource(). If it's out of line, we retrieve the // source from the files on demand. - options.setSourcePolicy(mOutOfLine ? JS::CompileOptions::LAZY_SOURCE - : JS::CompileOptions::SAVE_SOURCE); + options.setSourceIsLazy(mOutOfLine); JS::Rooted scope(cx, JS::CurrentGlobalOrNull(cx)); if (scope) { JS::ExposeObjectToActiveJS(scope); diff --git a/js/src/frontend/BytecodeCompiler.cpp b/js/src/frontend/BytecodeCompiler.cpp index a0731c609f03..4c8045251407 100644 --- a/js/src/frontend/BytecodeCompiler.cpp +++ b/js/src/frontend/BytecodeCompiler.cpp @@ -146,7 +146,8 @@ CanLazilyParse(ExclusiveContext *cx, const ReadOnlyCompileOptions &options) { return options.canLazilyParse && options.compileAndGo && - options.sourcePolicy == CompileOptions::SAVE_SOURCE && + !cx->compartment()->options().discardSource() && + !options.sourceIsLazy && !(cx->compartment()->debugMode() && cx->compartment()->runtimeFromAnyThread()->debugHooks.newScriptHook); } @@ -212,7 +213,7 @@ frontend::CompileScript(ExclusiveContext *cx, LifoAlloc *alloc, HandleObject sco if (!CheckLength(cx, length)) return nullptr; - JS_ASSERT_IF(staticLevel != 0, options.sourcePolicy != CompileOptions::LAZY_SOURCE); + JS_ASSERT_IF(staticLevel != 0, !options.sourceIsLazy); RootedScriptSource sourceObject(cx, CreateScriptSourceObject(cx, options)); if (!sourceObject) @@ -223,16 +224,11 @@ frontend::CompileScript(ExclusiveContext *cx, LifoAlloc *alloc, HandleObject sco SourceCompressionTask mysct(cx); SourceCompressionTask *sct = extraSct ? extraSct : &mysct; - switch (options.sourcePolicy) { - case CompileOptions::SAVE_SOURCE: - if (!ss->setSourceCopy(cx, chars, length, false, sct)) + if (!cx->compartment()->options().discardSource()) { + if (options.sourceIsLazy) + ss->setSourceRetrievable(); + else if (!ss->setSourceCopy(cx, chars, length, false, sct)) return nullptr; - break; - case CompileOptions::LAZY_SOURCE: - ss->setSourceRetrievable(); - break; - case CompileOptions::NO_SOURCE: - break; } bool canLazilyParse = CanLazilyParse(cx, options); @@ -517,8 +513,8 @@ CompileFunctionBody(JSContext *cx, MutableHandleFunction fun, const ReadOnlyComp ScriptSource *ss = sourceObject->source(); SourceCompressionTask sct(cx); - JS_ASSERT(options.sourcePolicy != CompileOptions::LAZY_SOURCE); - if (options.sourcePolicy == CompileOptions::SAVE_SOURCE) { + JS_ASSERT(!options.sourceIsLazy); + if (!cx->compartment()->options().discardSource()) { if (!ss->setSourceCopy(cx, chars, length, true, &sct)) return false; } diff --git a/js/src/jit-test/tests/basic/withSourceHook.js b/js/src/jit-test/tests/basic/withSourceHook.js index 816f25614fe0..460c4efe1373 100644 --- a/js/src/jit-test/tests/basic/withSourceHook.js +++ b/js/src/jit-test/tests/basic/withSourceHook.js @@ -35,20 +35,20 @@ withSourceHook(function (url) { }, function () { log += 'I'; return evaluate('(function inner() { 2; })', - { fileName: 'inner', sourcePolicy: 'LAZY_SOURCE' }) + { fileName: 'inner', sourceIsLazy: true }) .toSource(); }), '(function inner() { 1; })'); // Verify that the source hook that throws has been reinstated. evaluate('(function middle() { })', - { fileName: 'middle', sourcePolicy: 'LAZY_SOURCE' }) + { fileName: 'middle', sourceIsLazy: true }) .toSource(); }); }, 'borborygmus'); // Verify that the outermost source hook has been restored. assertEq(evaluate('(function outer() { 4; })', - { fileName: 'outer', sourcePolicy: 'LAZY_SOURCE' }) + { fileName: 'outer', sourceIsLazy: true }) .toSource(), '(function outer() { 3; })'); }); diff --git a/js/src/jit-test/tests/debug/Source-text-lazy.js b/js/src/jit-test/tests/debug/Source-text-lazy.js index 5a0455c0c1a2..21896b212a8e 100644 --- a/js/src/jit-test/tests/debug/Source-text-lazy.js +++ b/js/src/jit-test/tests/debug/Source-text-lazy.js @@ -28,7 +28,7 @@ function test(source) { } g.evaluate(source, { fileName: "BanalBivalve.jsm", - sourcePolicy: "LAZY_SOURCE"}); + sourceIsLazy: true }); }); assertEq(log, 'ds'); diff --git a/js/src/jsapi-tests/testSourcePolicy.cpp b/js/src/jsapi-tests/testSourcePolicy.cpp index 0a451911bca8..99838c13a58a 100644 --- a/js/src/jsapi-tests/testSourcePolicy.cpp +++ b/js/src/jsapi-tests/testSourcePolicy.cpp @@ -9,7 +9,7 @@ BEGIN_TEST(testBug795104) { JS::CompileOptions opts(cx); - opts.setSourcePolicy(JS::CompileOptions::NO_SOURCE); + JS::CompartmentOptionsRef(cx->compartment()).setDiscardSource(true); const size_t strLen = 60002; char *s = static_cast(JS_malloc(cx, strLen)); CHECK(s); diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 56b490e176ad..355dcf1bb289 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -2479,6 +2479,12 @@ JS::CompartmentOptionsRef(JSCompartment *compartment) return compartment->options(); } +JS::CompartmentOptions & +JS::CompartmentOptionsRef(JSObject *obj) +{ + return obj->compartment()->options(); +} + JS::CompartmentOptions & JS::CompartmentOptionsRef(JSContext *cx) { @@ -4348,7 +4354,7 @@ JS::ReadOnlyCompileOptions::copyPODOptions(const ReadOnlyCompileOptions &rhs) asmJSOption = rhs.asmJSOption; forceAsync = rhs.forceAsync; installedFile = rhs.installedFile; - sourcePolicy = rhs.sourcePolicy; + sourceIsLazy = rhs.sourceIsLazy; introductionType = rhs.introductionType; introductionLineno = rhs.introductionLineno; introductionOffset = rhs.introductionOffset; diff --git a/js/src/jsapi.h b/js/src/jsapi.h index d85425f51798..e496c862fc7f 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -2535,6 +2535,7 @@ class JS_PUBLIC_API(CompartmentOptions) : version_(JSVERSION_UNKNOWN) , invisibleToDebugger_(false) , mergeable_(false) + , discardSource_(false) , traceGlobal_(nullptr) , singletonsAsTemplates_(true) { @@ -2568,6 +2569,15 @@ class JS_PUBLIC_API(CompartmentOptions) return *this; } + // For certain globals, we know enough about the code that will run in them + // that we can discard script source entirely. + bool discardSource() const { return discardSource_; } + CompartmentOptions &setDiscardSource(bool flag) { + discardSource_ = flag; + return *this; + } + + bool cloneSingletons(JSContext *cx) const; Override &cloneSingletonsOverride() { return cloneSingletonsOverride_; } @@ -2598,6 +2608,7 @@ class JS_PUBLIC_API(CompartmentOptions) JSVersion version_; bool invisibleToDebugger_; bool mergeable_; + bool discardSource_; Override cloneSingletonsOverride_; union { ZoneSpecifier spec; @@ -2614,6 +2625,9 @@ class JS_PUBLIC_API(CompartmentOptions) JS_PUBLIC_API(CompartmentOptions &) CompartmentOptionsRef(JSCompartment *compartment); +JS_PUBLIC_API(CompartmentOptions &) +CompartmentOptionsRef(JSObject *obj); + JS_PUBLIC_API(CompartmentOptions &) CompartmentOptionsRef(JSContext *cx); @@ -3388,7 +3402,7 @@ class JS_FRIEND_API(ReadOnlyCompileOptions) asmJSOption(false), forceAsync(false), installedFile(false), - sourcePolicy(SAVE_SOURCE), + sourceIsLazy(false), introductionType(nullptr), introductionLineno(0), introductionOffset(0), @@ -3428,11 +3442,7 @@ class JS_FRIEND_API(ReadOnlyCompileOptions) bool asmJSOption; bool forceAsync; bool installedFile; // 'true' iff pre-compiling js file in packaged app - enum SourcePolicy { - NO_SOURCE, - LAZY_SOURCE, - SAVE_SOURCE - } sourcePolicy; + bool sourceIsLazy; // |introductionType| is a statically allocated C string: // one of "eval", "Function", or "GeneratorFunction". @@ -3524,7 +3534,7 @@ class JS_FRIEND_API(OwningCompileOptions) : public ReadOnlyCompileOptions OwningCompileOptions &setNoScriptRval(bool nsr) { noScriptRval = nsr; return *this; } OwningCompileOptions &setSelfHostingMode(bool shm) { selfHostingMode = shm; return *this; } OwningCompileOptions &setCanLazilyParse(bool clp) { canLazilyParse = clp; return *this; } - OwningCompileOptions &setSourcePolicy(SourcePolicy sp) { sourcePolicy = sp; return *this; } + OwningCompileOptions &setSourceIsLazy(bool l) { sourceIsLazy = l; return *this; } OwningCompileOptions &setIntroductionType(const char *t) { introductionType = t; return *this; } bool setIntroductionInfo(JSContext *cx, const char *introducerFn, const char *intro, unsigned line, JSScript *script, uint32_t offset) @@ -3610,7 +3620,7 @@ class MOZ_STACK_CLASS JS_FRIEND_API(CompileOptions) : public ReadOnlyCompileOpti CompileOptions &setNoScriptRval(bool nsr) { noScriptRval = nsr; return *this; } CompileOptions &setSelfHostingMode(bool shm) { selfHostingMode = shm; return *this; } CompileOptions &setCanLazilyParse(bool clp) { canLazilyParse = clp; return *this; } - CompileOptions &setSourcePolicy(SourcePolicy sp) { sourcePolicy = sp; return *this; } + CompileOptions &setSourceIsLazy(bool l) { sourceIsLazy = l; return *this; } CompileOptions &setIntroductionType(const char *t) { introductionType = t; return *this; } CompileOptions &setIntroductionInfo(const char *introducerFn, const char *intro, unsigned line, JSScript *script, uint32_t offset) diff --git a/js/src/jsfriendapi.h b/js/src/jsfriendapi.h index 9f12e06adaf2..61698a44719f 100644 --- a/js/src/jsfriendapi.h +++ b/js/src/jsfriendapi.h @@ -383,13 +383,13 @@ proxy_Slice(JSContext *cx, JS::HandleObject proxy, uint32_t begin, uint32_t end, /* * A class of objects that return source code on demand. * - * When code is compiled with CompileOptions::LAZY_SOURCE, SpiderMonkey - * doesn't retain the source code (and doesn't do lazy bytecode - * generation). If we ever need the source code, say, in response to a call - * to Function.prototype.toSource or Debugger.Source.prototype.text, then - * we call the 'load' member function of the instance of this class that - * has hopefully been registered with the runtime, passing the code's URL, - * and hope that it will be able to find the source. + * When code is compiled with setSourceIsLazy(true), SpiderMonkey doesn't + * retain the source code (and doesn't do lazy bytecode generation). If we ever + * need the source code, say, in response to a call to Function.prototype. + * toSource or Debugger.Source.prototype.text, then we call the 'load' member + * function of the instance of this class that has hopefully been registered + * with the runtime, passing the code's URL, and hope that it will be able to + * find the source. */ class SourceHook { public: @@ -404,7 +404,7 @@ class SourceHook { }; /* - * Have |rt| use |hook| to retrieve LAZY_SOURCE source code. See the + * Have |rt| use |hook| to retrieve lazily-retrieved source code. See the * comments for SourceHook. The runtime takes ownership of the hook, and * will delete it when the runtime itself is deleted, or when a new hook is * set. diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index 867fa9caa7de..91046039c015 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -866,28 +866,10 @@ ParseCompileOptions(JSContext *cx, CompileOptions &options, HandleObject opts, options.setLine(u); } - if (!JS_GetProperty(cx, opts, "sourcePolicy", &v)) + if (!JS_GetProperty(cx, opts, "sourceIsLazy", &v)) return false; - if (!v.isUndefined()) { - RootedString s(cx, ToString(cx, v)); - if (!s) - return false; - - JSAutoByteString bytes; - char *policy = bytes.encodeUtf8(cx, s); - if (!policy) - return false; - if (strcmp(policy, "NO_SOURCE") == 0) { - options.setSourcePolicy(CompileOptions::NO_SOURCE); - } else if (strcmp(policy, "LAZY_SOURCE") == 0) { - options.setSourcePolicy(CompileOptions::LAZY_SOURCE); - } else if (strcmp(policy, "SAVE_SOURCE") == 0) { - options.setSourcePolicy(CompileOptions::SAVE_SOURCE); - } else { - JS_ReportError(cx, "bad 'sourcePolicy' option: '%s'", policy); - return false; - } - } + if (v.isBoolean()) + options.setSourceIsLazy(v.toBoolean()); return true; } @@ -3636,7 +3618,7 @@ OffThreadCompileScript(JSContext *cx, unsigned argc, jsval *vp) // These option settings must override whatever the caller requested. options.setCompileAndGo(true) - .setSourcePolicy(CompileOptions::SAVE_SOURCE); + .setSourceIsLazy(false); // We assume the caller wants caching if at all possible, ignoring // heuristics that make sense for a real browser. @@ -4389,9 +4371,9 @@ static const JSFunctionSpecWithHelp shell_functions[] = { " provide that as the code's source map URL. If omitted, attach no\n" " source map URL to the code (although the code may provide one itself,\n" " via a //#sourceMappingURL comment).\n" -" sourcePolicy: if present, the value converted to a string must be either\n" -" 'NO_SOURCE', 'LAZY_SOURCE', or 'SAVE_SOURCE'; use the given source\n" -" retention policy for this compilation.\n" +" sourceIsLazy: if present and true, indicates that, after compilation, \n" + "script source should not be cached by the JS engine and should be \n" + "lazily loaded from the embedding as-needed.\n" " loadBytecode: if true, and if the source is a CacheEntryObject,\n" " the bytecode would be loaded and decoded from the cache entry instead\n" " of being parsed, then it would be executed as usual.\n" diff --git a/js/src/vm/SelfHosting.cpp b/js/src/vm/SelfHosting.cpp index c9f6adb9237d..f446a98da3a0 100644 --- a/js/src/vm/SelfHosting.cpp +++ b/js/src/vm/SelfHosting.cpp @@ -904,7 +904,6 @@ js::FillSelfHostingCompileOptions(CompileOptions &options) options.setFileAndLine("self-hosted", 1); options.setSelfHostingMode(true); options.setCanLazilyParse(false); - options.setSourcePolicy(CompileOptions::NO_SOURCE); options.setVersion(JSVERSION_LATEST); options.werrorOption = true; options.strictOption = true; @@ -934,8 +933,11 @@ JSRuntime::initSelfHosting(JSContext *cx) RootedObject savedGlobal(cx, receivesDefaultObject ? js::DefaultObjectForContextOrNull(cx) : nullptr); + JS::CompartmentOptions compartmentOptions; + compartmentOptions.setDiscardSource(true); if (!(selfHostingGlobal_ = JS_NewGlobalObject(cx, &self_hosting_global_class, - nullptr, JS::DontFireOnNewGlobalHook))) + nullptr, JS::DontFireOnNewGlobalHook, + compartmentOptions))) return false; JSAutoCompartment ac(cx, selfHostingGlobal_); if (receivesDefaultObject) diff --git a/js/xpconnect/loader/mozJSComponentLoader.cpp b/js/xpconnect/loader/mozJSComponentLoader.cpp index b6d0004e3b05..1bd6b798a4bc 100644 --- a/js/xpconnect/loader/mozJSComponentLoader.cpp +++ b/js/xpconnect/loader/mozJSComponentLoader.cpp @@ -814,13 +814,16 @@ mozJSComponentLoader::ObjectForLocation(nsIFile *aComponentFile, if (aPropagateExceptions) ContextOptionsRef(cx).setDontReportUncaught(true); + // Note - if mReuseLoaderGlobal is true, then we can't do lazy source, + // because we compile things as functions (rather than script), and lazy + // source isn't supported in that configuration. That's ok though, + // because we only do mReuseLoaderGlobal on b2g, where we invoke + // setDiscardSource(true) on the entire global. CompileOptions options(cx); options.setNoScriptRval(mReuseLoaderGlobal ? false : true) .setVersion(JSVERSION_LATEST) .setFileAndLine(nativePath.get(), 1) - .setSourcePolicy(mReuseLoaderGlobal ? - CompileOptions::NO_SOURCE : - CompileOptions::LAZY_SOURCE); + .setSourceIsLazy(!mReuseLoaderGlobal); if (realFile) { #ifdef HAVE_PR_MEMMAP diff --git a/js/xpconnect/loader/mozJSSubScriptLoader.cpp b/js/xpconnect/loader/mozJSSubScriptLoader.cpp index e51b48eaa941..0cf2de6f673a 100644 --- a/js/xpconnect/loader/mozJSSubScriptLoader.cpp +++ b/js/xpconnect/loader/mozJSSubScriptLoader.cpp @@ -164,10 +164,10 @@ mozJSSubScriptLoader::ReadScript(nsIURI *uri, JSContext *cx, JSObject *targetObj script.Length()); } } else { - // We only use LAZY_SOURCE when no special encoding is specified because + // We only use lazy source when no special encoding is specified because // the lazy source loader doesn't know the encoding. if (!reuseGlobal) { - options.setSourcePolicy(JS::CompileOptions::LAZY_SOURCE); + options.setSourceIsLazy(true); *scriptp = JS::Compile(cx, target_obj, options, buf.get(), len); } else { *functionp = JS::CompileFunction(cx, target_obj, options, @@ -498,7 +498,6 @@ ScriptPrecompiler::OnStreamComplete(nsIStreamLoader* aLoader, JSAutoCompartment ac(cx, js::UncheckedUnwrap(&v.toObject())); JS::CompileOptions options(cx, JSVERSION_DEFAULT); - options.setSourcePolicy(CompileOptions::NO_SOURCE); options.forceAsync = true; options.compileAndGo = true; options.installedFile = true;