☠☠ backed out by 178c1abf3fdb ☠ ☠ | |
author | Shu-yu Guo <shu@rfrn.org> |
Wed, 17 Jun 2015 21:26:58 -0700 | |
changeset 249503 | 0a3b1cd87c26062a78899f43f2b4a41b1d49bea2 |
parent 249502 | 662ec8b0561ebaefa8c8e563d4868b2b426fdff0 |
child 249504 | 83954c7df8ab17e6abbbb730b963fb976a8ce063 |
push id | 28927 |
push user | cbook@mozilla.com |
push date | Thu, 18 Jun 2015 13:13:33 +0000 |
treeherder | mozilla-central@efe86609e776 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | luke |
bugs | 1171177 |
milestone | 41.0a1 |
first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
--- a/js/src/jsapi-tests/testJSEvaluateScript.cpp +++ b/js/src/jsapi-tests/testJSEvaluateScript.cpp @@ -6,48 +6,30 @@ using mozilla::ArrayLength; BEGIN_TEST(testJSEvaluateScript) { JS::RootedObject obj(cx, JS_NewPlainObject(cx)); CHECK(obj); - CHECK(JS::RuntimeOptionsRef(cx).varObjFix()); - static const char16_t src[] = MOZ_UTF16("var x = 5;"); JS::RootedValue retval(cx); JS::CompileOptions opts(cx); JS::AutoObjectVector scopeChain(cx); CHECK(scopeChain.append(obj)); CHECK(JS::Evaluate(cx, scopeChain, opts.setFileAndLine(__FILE__, __LINE__), src, ArrayLength(src) - 1, &retval)); bool hasProp = true; CHECK(JS_AlreadyHasOwnProperty(cx, obj, "x", &hasProp)); - CHECK(!hasProp); + CHECK(hasProp); hasProp = false; CHECK(JS_HasProperty(cx, global, "x", &hasProp)); - CHECK(hasProp); - - // Now do the same thing, but without JSOPTION_VAROBJFIX - JS::RuntimeOptionsRef(cx).setVarObjFix(false); - - static const char16_t src2[] = MOZ_UTF16("var y = 5;"); - - CHECK(JS::Evaluate(cx, scopeChain, opts.setFileAndLine(__FILE__, __LINE__), - src2, ArrayLength(src2) - 1, &retval)); - - hasProp = false; - CHECK(JS_AlreadyHasOwnProperty(cx, obj, "y", &hasProp)); - CHECK(hasProp); - - hasProp = true; - CHECK(JS_AlreadyHasOwnProperty(cx, global, "y", &hasProp)); CHECK(!hasProp); return true; } END_TEST(testJSEvaluateScript)
--- a/js/src/jsapi-tests/tests.h +++ b/js/src/jsapi-tests/tests.h @@ -280,17 +280,16 @@ class JSAPITest } virtual JSRuntime * createRuntime() { JSRuntime* rt = JS_NewRuntime(8L * 1024 * 1024); if (!rt) return nullptr; JS_SetErrorReporter(rt, &reportError); setNativeStackQuota(rt); - JS::RuntimeOptionsRef(rt).setVarObjFix(true); return rt; } virtual void destroyRuntime() { MOZ_RELEASE_ASSERT(!cx); MOZ_RELEASE_ASSERT(rt); JS_DestroyRuntime(rt); rt = nullptr;
--- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -3291,16 +3291,26 @@ CreateNonSyntacticScopeChain(JSContext* return false; if (scopeChain.empty()) { staticScopeObj.set(nullptr); } else { staticScopeObj.set(StaticNonSyntacticScopeObjects::create(cx, nullptr)); if (!staticScopeObj) return false; + + // The XPConnect subscript loader, which may pass in its own dynamic + // scopes to load scripts in, expects the dynamic scope chain to be + // the holder of "var" declarations. In SpiderMonkey, such objects are + // called "qualified varobjs", the "qualified" part meaning the + // declaration was qualified by "var". There is only sadness. + // + // See JSObject::isQualifiedVarObj. + if (!dynamicScopeObj->setQualifiedVarObj(cx)) + return false; } return true; } static bool IsFunctionCloneable(HandleFunction fun, HandleObject dynamicScope) {
--- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -1162,18 +1162,17 @@ class JS_PUBLIC_API(RuntimeOptions) { : baseline_(true), ion_(true), asmJS_(true), nativeRegExp_(true), unboxedArrays_(false), asyncStack_(true), werror_(false), strictMode_(false), - extraWarnings_(false), - varObjFix_(false) + extraWarnings_(false) { } bool baseline() const { return baseline_; } RuntimeOptions& setBaseline(bool flag) { baseline_ = flag; return *this; } @@ -1245,37 +1244,26 @@ class JS_PUBLIC_API(RuntimeOptions) { extraWarnings_ = flag; return *this; } RuntimeOptions& toggleExtraWarnings() { extraWarnings_ = !extraWarnings_; return *this; } - bool varObjFix() const { return varObjFix_; } - RuntimeOptions& setVarObjFix(bool flag) { - varObjFix_ = flag; - return *this; - } - RuntimeOptions& toggleVarObjFix() { - varObjFix_ = !varObjFix_; - return *this; - } - private: bool baseline_ : 1; bool ion_ : 1; bool asmJS_ : 1; bool nativeRegExp_ : 1; bool unboxedArrays_ : 1; bool asyncStack_ : 1; bool werror_ : 1; bool strictMode_ : 1; bool extraWarnings_ : 1; - bool varObjFix_ : 1; }; JS_PUBLIC_API(RuntimeOptions&) RuntimeOptionsRef(JSRuntime* rt); JS_PUBLIC_API(RuntimeOptions&) RuntimeOptionsRef(JSContext* cx); @@ -3809,34 +3797,18 @@ JS_DecompileFunctionBody(JSContext* cx, * NB: JS_ExecuteScript and the JS::Evaluate APIs come in two flavors: either * they use the global as the scope, or they take an AutoObjectVector of objects * to use as the scope chain. In the former case, the global is also used as * the "this" keyword value and the variables object (ECMA parlance for where * 'var' and 'function' bind names) of the execution context for script. In the * latter case, the first object in the provided list is used, unless the list * is empty, in which case the global is used. * - * Using a non-global object as the variables object is problematic: in this - * case, variables created by 'var x = 0', e.g., go in that object, but - * variables created by assignment to an unbound id, 'x = 0', go in the global. - * - * ECMA requires that "global code" be executed with the variables object equal - * to the global object. But these JS API entry points provide freedom to - * execute code against a "sub-global", i.e., a parented or scoped object, in - * which case the variables object will differ from the global, resulting in - * confusing and non-ECMA explicit vs. implicit variable creation. - * - * Caveat embedders: unless you already depend on this buggy variables object - * binding behavior, you should call RuntimeOptionsRef(rt).setVarObjFix(true) - * for each context in the application, if you use the versions of - * JS_ExecuteScript and JS::Evaluate that take a scope chain argument, or may - * ever use them in the future. - * - * Why a runtime option? The alternative is to add APIs duplicating those below - * for the other value of varobjfix, and that doesn't seem worth the code bloat + * Why a runtime option? The alternative is to add APIs duplicating those + * for the other value of flags, and that doesn't seem worth the code bloat * cost. Such new entry points would probably have less obvious names, too, so * would not tend to be used. The RuntimeOptionsRef adjustment, OTOH, can be * more easily hacked into existing code that does not depend on the bug; such * code can continue to use the familiar JS::Evaluate, etc., entry points. */ /* * Evaluate a script in the scope of the current global of cx.
--- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -912,22 +912,16 @@ js::Execute(JSContext* cx, HandleScript #ifdef DEBUG JSObject* s = scopeChain; do { assertSameCompartment(cx, s); MOZ_ASSERT_IF(!s->enclosingScope(), s->is<GlobalObject>()); } while ((s = s->enclosingScope())); #endif - /* The VAROBJFIX option makes varObj == globalObj in global code. */ - if (!cx->runtime()->options().varObjFix()) { - if (!scopeChain->setQualifiedVarObj(cx)) - return false; - } - /* Use the scope chain as 'this', modulo outerization. */ JSObject* thisObj = GetThisObject(cx, scopeChain); if (!thisObj) return false; Value thisv = ObjectValue(*thisObj); return ExecuteKernel(cx, script, *scopeChain, thisv, NullValue(), EXECUTE_GLOBAL, NullFramePtr() /* evalInFrame */, rval);