Backed out 5 changesets (bug 1398601) for unexpected crashtest assertions
authorPhil Ringnalda <philringnalda@gmail.com>
Tue, 12 Sep 2017 20:11:49 -0700
changeset 429981 a45742d015d35f371c1092610da7678772622a6f
parent 429980 8e8f901c19ca74728fc76cc2f7998b4ee464b6be
child 429982 649a61cd93814faaf5aba92d2f1df6310209c37c
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1398601
milestone57.0a1
backs outb728872f4d9ad911854b90ca9893cdb7604fa9bf
05957a92b1a56cbf8f7c71afa4fef2b3f46b2ae3
f5c26c3407c02ecb15b4d0080d6f7fa20aa7c082
eddc38b0afd0839774d0c2b04157647edab6c96e
776a65d43a5eaa098a1b7956ef6486d64729a0db
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
Backed out 5 changesets (bug 1398601) for unexpected crashtest assertions Backed out changeset b728872f4d9a (bug 1398601) Backed out changeset 05957a92b1a5 (bug 1398601) Backed out changeset f5c26c3407c0 (bug 1398601) Backed out changeset eddc38b0afd0 (bug 1398601) Backed out changeset 776a65d43a5e (bug 1398601) MozReview-Commit-ID: 9lfUtdlZy7F
js/src/builtin/Eval.cpp
js/src/jsapi-tests/testExecuteInJSMEnvironment.cpp
js/src/jsfriendapi.h
js/src/vm/EnvironmentObject.cpp
js/xpconnect/loader/mozJSComponentLoader.cpp
js/xpconnect/loader/mozJSSubScriptLoader.cpp
js/xpconnect/tests/unit/environment_loadscript.jsm
js/xpconnect/tests/unit/test_SubscriptLoaderJSMEnvironment.js
js/xpconnect/tests/unit/xpcshell.ini
--- a/js/src/builtin/Eval.cpp
+++ b/js/src/builtin/Eval.cpp
@@ -440,34 +440,39 @@ js::DirectEval(JSContext* cx, HandleValu
 
 bool
 js::IsAnyBuiltinEval(JSFunction* fun)
 {
     return fun->maybeNative() == IndirectEval;
 }
 
 static bool
-ExecuteInExtensibleLexicalEnvironment(JSContext* cx, HandleScript scriptArg, HandleObject env)
+ExecuteInNonSyntacticGlobalInternal(JSContext* cx, HandleObject global, HandleScript scriptArg,
+                                    HandleObject varEnv, HandleObject lexEnv)
 {
     CHECK_REQUEST(cx);
-    assertSameCompartment(cx, env);
-    MOZ_ASSERT(IsExtensibleLexicalEnvironment(env));
+    assertSameCompartment(cx, global, varEnv, lexEnv);
+    MOZ_ASSERT(global->is<GlobalObject>());
+    MOZ_ASSERT(varEnv->is<NonSyntacticVariablesObject>());
+    MOZ_ASSERT(IsExtensibleLexicalEnvironment(lexEnv));
+    MOZ_ASSERT(lexEnv->enclosingEnvironment() == varEnv);
     MOZ_RELEASE_ASSERT(scriptArg->hasNonSyntacticScope());
 
     RootedScript script(cx, scriptArg);
+    Rooted<GlobalObject*> globalRoot(cx, &global->as<GlobalObject>());
     if (script->compartment() != cx->compartment()) {
         script = CloneGlobalScript(cx, ScopeKind::NonSyntactic, script);
         if (!script)
             return false;
 
         Debugger::onNewScript(cx, script);
     }
 
     RootedValue rval(cx);
-    return ExecuteKernel(cx, script, *env, UndefinedValue(),
+    return ExecuteKernel(cx, script, *lexEnv, UndefinedValue(),
                          NullFramePtr() /* evalInFrame */, rval.address());
 }
 
 JS_FRIEND_API(bool)
 js::ExecuteInGlobalAndReturnScope(JSContext* cx, HandleObject global, HandleScript scriptArg,
                                   MutableHandleObject envArg)
 {
     RootedObject varEnv(cx, NonSyntacticVariablesObject::create(cx));
@@ -475,17 +480,17 @@ js::ExecuteInGlobalAndReturnScope(JSCont
         return false;
 
     // Create lexical environment with |this| == global.
     // NOTE: This is required behavior for Gecko FrameScriptLoader
     RootedObject lexEnv(cx, LexicalEnvironmentObject::createNonSyntactic(cx, varEnv, global));
     if (!lexEnv)
         return false;
 
-    if (!ExecuteInExtensibleLexicalEnvironment(cx, scriptArg, lexEnv))
+    if (!ExecuteInNonSyntacticGlobalInternal(cx, global, scriptArg, varEnv, lexEnv))
         return false;
 
     envArg.set(lexEnv);
     return true;
 }
 
 JS_FRIEND_API(JSObject*)
 js::NewJSMEnvironment(JSContext* cx)
@@ -500,60 +505,23 @@ js::NewJSMEnvironment(JSContext* cx)
         return nullptr;
 
     return varEnv;
 }
 
 JS_FRIEND_API(bool)
 js::ExecuteInJSMEnvironment(JSContext* cx, HandleScript scriptArg, HandleObject varEnv)
 {
-    AutoObjectVector emptyChain(cx);
-    return ExecuteInJSMEnvironment(cx, scriptArg, varEnv, emptyChain);
-}
-
-JS_FRIEND_API(bool)
-js::ExecuteInJSMEnvironment(JSContext* cx, HandleScript scriptArg, HandleObject varEnv,
-                            AutoObjectVector& targetObj)
-{
     assertSameCompartment(cx, varEnv);
     MOZ_ASSERT(cx->compartment()->getNonSyntacticLexicalEnvironment(varEnv));
-    MOZ_DIAGNOSTIC_ASSERT(scriptArg->noScriptRval());
-
-    RootedObject env(cx, JS_ExtensibleLexicalEnvironment(varEnv));
 
-    // If the Gecko subscript loader specifies target objects, we need to add
-    // them to the environment. These are added after the NSVO environment.
-    if (!targetObj.empty()) {
-        // The environment chain will be as follows:
-        //      GlobalObject / BackstagePass
-        //      LexicalEnvironmentObject[this=global]
-        //      NonSyntacticVariablesObject (the JSMEnvironment)
-        //      LexicalEnvironmentObject[this=nsvo]
-        //      WithEnvironmentObject[target=targetObj]
-        //      LexicalEnvironmentObject[this=targetObj] (*)
-        //
-        //  (*) This environment intentionally intercepts JSOP_GLOBALTHIS, but
-        //  not JSOP_FUNCTIONTHIS (which instead will fallback to the NSVO). I
-        //  don't make the rules, I just record them.
+    RootedObject global(cx, &varEnv->global());
+    RootedObject lexEnv(cx, JS_ExtensibleLexicalEnvironment(varEnv));
 
-        // Wrap the target objects in WithEnvironments.
-        if (!js::CreateObjectsForEnvironmentChain(cx, targetObj, env, &env))
-            return false;
-
-        // See CreateNonSyntacticEnvironmentChain
-        if (!JSObject::setQualifiedVarObj(cx, env))
-            return false;
-
-        // Create an extensible LexicalEnvironmentObject for target object
-        env = cx->compartment()->getOrCreateNonSyntacticLexicalEnvironment(cx, env);
-        if (!env)
-            return false;
-    }
-
-    return ExecuteInExtensibleLexicalEnvironment(cx, scriptArg, env);
+    return ExecuteInNonSyntacticGlobalInternal(cx, global, scriptArg, varEnv, lexEnv);
 }
 
 JS_FRIEND_API(JSObject*)
 js::GetJSMEnvironmentOfScriptedCaller(JSContext* cx)
 {
     FrameIter iter(cx);
     if (iter.done())
         return nullptr;
@@ -563,16 +531,8 @@ js::GetJSMEnvironmentOfScriptedCaller(JS
     MOZ_RELEASE_ASSERT(!iter.isWasm());
 
     RootedObject env(cx, iter.environmentChain(cx));
     while (env && !env->is<NonSyntacticVariablesObject>())
         env = env->enclosingEnvironment();
 
     return env;
 }
-
-JS_FRIEND_API(bool)
-js::IsJSMEnvironment(JSObject* obj)
-{
-    // NOTE: This also returns true if the NonSyntacticVariablesObject was
-    // created for reasons other than the JSM loader.
-    return obj->is<NonSyntacticVariablesObject>();
-}
--- a/js/src/jsapi-tests/testExecuteInJSMEnvironment.cpp
+++ b/js/src/jsapi-tests/testExecuteInJSMEnvironment.cpp
@@ -19,17 +19,16 @@ BEGIN_TEST(testExecuteInJSMEnvironment_B
         "eval('this.e = 5');\n"
         "(0,eval)('this.f = 6');\n"
         "(function() { this.g = 7; })();\n"
         "function f_h() { this.h = 8; }; f_h();\n"
         ;
 
     JS::CompileOptions options(cx);
     options.setFileAndLine(__FILE__, __LINE__);
-    options.setNoScriptRval(true);
 
     JS::RootedScript script(cx);
     CHECK(JS::CompileForNonSyntacticScope(cx, options, src, sizeof(src)-1, &script));
 
     JS::RootedObject varEnv(cx, js::NewJSMEnvironment(cx));
     JS::RootedObject lexEnv(cx, JS_ExtensibleLexicalEnvironment(varEnv));
     CHECK(varEnv && varEnv->is<js::NonSyntacticVariablesObject>());
     CHECK(lexEnv && js::IsExtensibleLexicalEnvironment(lexEnv));
@@ -78,17 +77,16 @@ BEGIN_TEST(testExecuteInJSMEnvironment_C
     static const char src[] =
         "var output = callback();\n"
         ;
 
     CHECK(JS_DefineFunctions(cx, global, testFunctions));
 
     JS::CompileOptions options(cx);
     options.setFileAndLine(__FILE__, __LINE__);
-    options.setNoScriptRval(true);
 
     JS::RootedScript script(cx);
     CHECK(JS::CompileForNonSyntacticScope(cx, options, src, sizeof(src)-1, &script));
 
     JS::RootedObject nsvo(cx, js::NewJSMEnvironment(cx));
     CHECK(nsvo);
     CHECK(js::ExecuteInJSMEnvironment(cx, script, nsvo));
 
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -2883,79 +2883,26 @@ SetPropertyIgnoringNamedGetter(JSContext
                                JS::Handle<JS::PropertyDescriptor> ownDesc,
                                JS::ObjectOpResult& result);
 
 // This function is for one specific use case, please don't use this for anything else!
 extern JS_FRIEND_API(bool)
 ExecuteInGlobalAndReturnScope(JSContext* cx, JS::HandleObject obj, JS::HandleScript script,
                               JS::MutableHandleObject scope);
 
-// These functions are provided for the JSM component loader in Gecko.
-//
-// A 'JSMEnvironment' refers to an environment chain constructed for JSM loading
-// in a shared global. Internally it is a NonSyntacticVariablesObject with a
-// corresponding extensible LexicalEnvironmentObject that is accessible by
-// JS_ExtensibleLexicalEnvironment. The |this| value of that lexical environment
-// is the NSVO itself.
-//
-// Normal global environment (ES6):     JSM "global" environment:
-//
-//                                      * - extensible lexical environment
-//                                      |   (code runs in this environment;
-//                                      |    `let/const` bindings go here)
-//                                      |
-//                                      * - JSMEnvironment (=== `this`)
-//                                      |   (`var` bindings go here)
-//                                      |
-// * - extensible lexical environment   * - extensible lexical environment
-// |   (code runs in this environment;  |   (empty)
-// |    `let/const` bindings go here)   |
-// |                                    |
-// * - actual global (=== `this`)       * - shared JSM global
-//     (var bindings go here; and           (Object, Math, etc. live here)
-//      Object, Math, etc. live here)
-
-// Allocate a new environment in current compartment that is compatible with JSM
-// shared loading.
+// These functions are only for JSM component loader, please don't use this for anything else!
 extern JS_FRIEND_API(JSObject*)
 NewJSMEnvironment(JSContext* cx);
 
-// Execute the given script (copied into compartment if necessary) in the given
-// JSMEnvironment. The script must have been compiled for hasNonSyntacticScope.
-// The |jsmEnv| must have been previously allocated by NewJSMEnvironment.
-//
-// NOTE: The associated extensible lexical environment is reused.
 extern JS_FRIEND_API(bool)
-ExecuteInJSMEnvironment(JSContext* cx, JS::HandleScript script, JS::HandleObject jsmEnv);
-
-// Additionally, target objects may be specified as required by the Gecko
-// subscript loader. These are wrapped in non-syntactic WithEnvironments and
-// temporarily placed on environment chain.
-//
-// See also: JS::CloneAndExecuteScript(...)
-extern JS_FRIEND_API(bool)
-ExecuteInJSMEnvironment(JSContext* cx, JS::HandleScript script, JS::HandleObject jsmEnv,
-                        JS::AutoObjectVector& targetObj);
-
-// Used by native methods to determine the JSMEnvironment of caller if possible
-// by looking at stack frames. Returns nullptr if top frame isn't a scripted
-// caller in a JSM.
-//
-// NOTE: This may find NonSyntacticVariablesObject generated by other embedding
-// such as a Gecko FrameScript. Caller can check the compartment if needed.
+ExecuteInJSMEnvironment(JSContext* cx, JS::HandleScript script, JS::HandleObject nsvo);
+
 extern JS_FRIEND_API(JSObject*)
 GetJSMEnvironmentOfScriptedCaller(JSContext* cx);
 
-// Determine if obj is a JSMEnvironment
-//
-// NOTE: This may return true for an NonSyntacticVariablesObject generated by
-// other embedding such as a Gecko FrameScript. Caller can check compartment.
-extern JS_FRIEND_API(bool)
-IsJSMEnvironment(JSObject* obj);
-
 
 #if defined(XP_WIN) && defined(_WIN64)
 // Parameters use void* types to avoid #including windows.h. The return value of
 // this function is returned from the exception handler.
 typedef long
 (*JitExceptionHandler)(void* exceptionRecord,  // PEXECTION_RECORD
                        void* context);         // PCONTEXT
 
--- a/js/src/vm/EnvironmentObject.cpp
+++ b/js/src/vm/EnvironmentObject.cpp
@@ -3182,18 +3182,17 @@ js::GetDebugEnvironmentForGlobalLexicalE
 
 bool
 js::CreateObjectsForEnvironmentChain(JSContext* cx, AutoObjectVector& chain,
                                      HandleObject terminatingEnv, MutableHandleObject envObj)
 {
 #ifdef DEBUG
     for (size_t i = 0; i < chain.length(); ++i) {
         assertSameCompartment(cx, chain[i]);
-        MOZ_ASSERT(!chain[i]->is<GlobalObject>() &&
-                   !chain[i]->is<NonSyntacticVariablesObject>());
+        MOZ_ASSERT(!chain[i]->is<GlobalObject>());
     }
 #endif
 
     // Construct With object wrappers for the things on this environment chain
     // and use the result as the thing to scope the function to.
     Rooted<WithEnvironmentObject*> withEnv(cx);
     RootedObject enclosingEnv(cx, terminatingEnv);
     for (size_t i = chain.length(); i > 0; ) {
--- a/js/xpconnect/loader/mozJSComponentLoader.cpp
+++ b/js/xpconnect/loader/mozJSComponentLoader.cpp
@@ -441,24 +441,19 @@ mozJSComponentLoader::LoadModule(FileLoc
 }
 
 void
 mozJSComponentLoader::FindTargetObject(JSContext* aCx,
                                        MutableHandleObject aTargetObject)
 {
     aTargetObject.set(js::GetJSMEnvironmentOfScriptedCaller(aCx));
 
-    // The above could fail if the scripted caller is not a component/JSM (it
-    // could be a DOM scope, for instance).
-    //
-    // If the target object was not in the JSM shared global, return the global
-    // instead. This is needed when calling the subscript loader within a frame
-    // script, since it the FrameScript NSVO will have been found.
-    if (!aTargetObject ||
-        !IsLoaderGlobal(js::GetGlobalForObjectCrossCompartment(aTargetObject))) {
+    // The above could fail if the scripted caller is not a
+    // component/JSM (it could be a DOM scope, for instance).
+    if (!aTargetObject) {
         aTargetObject.set(CurrentGlobalOrNull(aCx));
     }
 }
 
 // This requires that the keys be strings and the values be pointers.
 template <class Key, class Data, class UserData>
 static size_t
 SizeOfTableExcludingThis(const nsBaseHashtable<Key, Data, UserData>& aTable,
--- a/js/xpconnect/loader/mozJSSubScriptLoader.cpp
+++ b/js/xpconnect/loader/mozJSSubScriptLoader.cpp
@@ -35,18 +35,16 @@
 #include "GeckoProfiler.h"
 
 using namespace mozilla::scache;
 using namespace JS;
 using namespace xpc;
 using namespace mozilla;
 using namespace mozilla::dom;
 
-#define JSSUB_CACHE_PREFIX(aType) "jssubloader/" aType
-
 class MOZ_STACK_CLASS LoadSubScriptOptions : public OptionsBase {
 public:
     explicit LoadSubScriptOptions(JSContext* cx = xpc_GetSafeJSContext(),
                                   JSObject* options = nullptr)
         : OptionsBase(cx, options)
         , target(cx)
         , charset(NullString())
         , ignoreCache(false)
@@ -175,64 +173,46 @@ EvalScript(JSContext* cx,
            bool startupCache,
            bool preloadCache,
            MutableHandleScript script)
 {
     if (JS_IsGlobalObject(targetObj)) {
         if (!JS::CloneAndExecuteScript(cx, script, retval)) {
             return false;
         }
-    } else if (js::IsJSMEnvironment(targetObj)) {
-        if (!ExecuteInJSMEnvironment(cx, script, targetObj)) {
-            return false;
-        }
-        retval.setUndefined();
     } else {
         JS::AutoObjectVector envChain(cx);
         if (!envChain.append(targetObj)) {
             return false;
         }
-        if (!loadScope) {
-            // A null loadScope means we are cross-compartment. In this case, we
-            // should check the target isn't in the JSM loader shared-global or
-            // we will contaiminate all JSMs in the compartment.
-            //
-            // NOTE: If loadScope is already a shared-global JSM, we can't
-            // determine which JSM the target belongs to and have to assume it
-            // is in our JSM.
-            JSObject* targetGlobal = js::GetGlobalForObjectCrossCompartment(targetObj);
-            MOZ_DIAGNOSTIC_ASSERT(!mozJSComponentLoader::Get()->IsLoaderGlobal(targetGlobal),
-                                  "Don't load subscript into target in a shared-global JSM");
-            if (!JS::CloneAndExecuteScript(cx, envChain, script, retval)) {
+        if (loadScope != targetObj &&
+            loadScope &&
+            !JS_IsGlobalObject(loadScope))
+        {
+            MOZ_DIAGNOSTIC_ASSERT(js::GetObjectCompartment(loadScope) ==
+                                  js::GetObjectCompartment(targetObj));
+
+            if (!envChain.append(loadScope)) {
                 return false;
             }
-        } else if (JS_IsGlobalObject(loadScope)) {
-            if (!JS::CloneAndExecuteScript(cx, envChain, script, retval)) {
-                return false;
-            }
-        } else {
-            MOZ_ASSERT(js::IsJSMEnvironment(loadScope));
-            if (!js::ExecuteInJSMEnvironment(cx, script, loadScope, envChain)) {
-                return false;
-            }
-            retval.setUndefined();
+        }
+        if (!JS::CloneAndExecuteScript(cx, envChain, script, retval)) {
+            return false;
         }
     }
 
     JSAutoCompartment rac(cx, targetObj);
     if (!JS_WrapValue(cx, retval)) {
         return false;
     }
 
     if (script && (startupCache || preloadCache)) {
-        bool hasNonSyntacticScope = !JS_IsGlobalObject(targetObj);
-        nsAutoCString cachePath(hasNonSyntacticScope ? JSSUB_CACHE_PREFIX("non-syntactic")
-                                                     : JSSUB_CACHE_PREFIX("global"));
+        nsAutoCString cachePath;
         JSVersion version = JS_GetVersion(cx);
-        cachePath.AppendPrintf("/%d", version);
+        cachePath.AppendPrintf("jssubloader/%d", version);
         PathifyURI(uri, cachePath);
 
         nsCString uriStr;
         if (preloadCache && NS_SUCCEEDED(uri->GetSpec(uriStr))) {
             // Note that, when called during startup, this will keep the
             // original JSScript object alive for an indefinite amount of time.
             // This has the side-effect of keeping the global that the script
             // was compiled for alive, too.
deleted file mode 100644
--- a/js/xpconnect/tests/unit/environment_loadscript.jsm
+++ /dev/null
@@ -1,18 +0,0 @@
-Components.utils.import("resource://gre/modules/Services.jsm");
-
-var EXPORTED_SYMBOLS = ["target", "bound"];
-
-var bound = "";
-var target = {};
-Services.scriptloader.loadSubScript("resource://test/environment_script.js", target);
-
-// Check global bindings
-try { void vu; bound += "vu,"; } catch (e) {}
-try { void vq; bound += "vq,"; } catch (e) {}
-try { void vl; bound += "vl,"; } catch (e) {}
-try { void gt; bound += "gt,"; } catch (e) {}
-try { void ed; bound += "ed,"; } catch (e) {}
-try { void ei; bound += "ei,"; } catch (e) {}
-try { void fo; bound += "fo,"; } catch (e) {}
-try { void fi; bound += "fi,"; } catch (e) {}
-try { void fd; bound += "fd,"; } catch (e) {}
deleted file mode 100644
--- a/js/xpconnect/tests/unit/test_SubscriptLoaderJSMEnvironment.js
+++ /dev/null
@@ -1,23 +0,0 @@
-let tgt_load = {};
-let tgt_check = {};
-Components.utils.import("resource://test/environment_loadscript.jsm", tgt_load);
-Components.utils.import("resource://test/environment_checkscript.jsm", tgt_check);
-
-// Check target bindings
-var tgt_subscript_bound = "";
-for (var name of ["vu", "vq", "vl", "gt", "ed", "ei", "fo", "fi", "fd"])
-    if (tgt_load.target.hasOwnProperty(name))
-        tgt_subscript_bound += name + ",";
-
-// Expected subscript loader behavior is as follows:
-//  - Qualified vars and |this| access occur on target object
-//  - Lexical vars occur on ExtensibleLexicalEnvironment of target object
-//  - Bareword assignments and global |this| access occur on caller's global
-if (tgt_load.bound != "vu,ei,fo,fi,")
-    throw new Error("Unexpected global binding set - " + tgt_load.bound);
-if (tgt_subscript_bound != "vq,gt,ed,fd,")
-    throw new Error("Unexpected target binding set - " + tgt_subscript_bound);
-
-// Components should not share namespace
-if (tgt_check.bound != "")
-    throw new Error("Unexpected shared binding set - " + tgt_check.bound);
--- a/js/xpconnect/tests/unit/xpcshell.ini
+++ b/js/xpconnect/tests/unit/xpcshell.ini
@@ -7,17 +7,16 @@ support-files =
   bug451678_subscript.js
   component-blob.js
   component-blob.manifest
   component-file.js
   component-file.manifest
   component_import.js
   component_import.manifest
   environment_script.js
-  environment_loadscript.jsm
   environment_checkscript.jsm
   file_simple_script.js
   importer.jsm
   recursive_importA.jsm
   recursive_importB.jsm
   subScriptWithEarlyError.js
   syntax_error.jsm
 
@@ -138,10 +137,9 @@ head = head_watchdog.js
 [test_xray_SavedFrame-02.js]
 [test_xray_regexp.js]
 [test_resolve_dead_promise.js]
 [test_asyncLoadSubScriptError.js]
 [test_function_names.js]
 [test_FrameScriptEnvironment.js]
 [test_SubscriptLoaderEnvironment.js]
 [test_SubscriptLoaderSandboxEnvironment.js]
-[test_SubscriptLoaderJSMEnvironment.js]
 [test_ComponentEnvironment.js]