Bug 1480244: Part 2 - Replace ExecuteInGlobalAndReturnScope with ExecuteInScopeChainAndReturnNewScope. r=tcampbell
authorKris Maglione <maglione.k@gmail.com>
Fri, 10 Aug 2018 13:54:23 -0700
changeset 431543 809dc9048fea945398495e575a7d7cc9f71fd5cc
parent 431542 2aac9238b59e5e7b1422d82d7f71e3a001465300
child 431544 d17af8236abe8b1a53674955d2baee603abc2b62
push id34443
push usercsabou@mozilla.com
push dateWed, 15 Aug 2018 00:53:32 +0000
treeherdermozilla-central@b80906e2fbc9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1480244
milestone63.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
Bug 1480244: Part 2 - Replace ExecuteInGlobalAndReturnScope with ExecuteInScopeChainAndReturnNewScope. r=tcampbell This patch series replaces message manager globals with ordinary JS objects which live in the shared JSM global. Once that happens, ExecuteInGlobalAndReturnScope will no longer have useful behavior for them, since the base global has none of the methods that they rely on, and it provides no way to insert another plain object into the scope chain. This patch changes the scope chain for frame scripts to instead look like: -+- Shared JSM global | +- LexicalEnvironment[this=global] | +- NonSyntacticVariablesObject | +- WithEnvironmentObject[target=messageManager] | +- LexicalEnvironment[this=messageManager] Where lexical assignments end up on the lexical scope, and both qualified and unqualified assignments wind up on the NSVO. This has some slight behavioral differences from the previous model, in that properties defined on the message manager can mask properties on the NSVO. But those differences are minor, and probably not worth worrying about, since frame scripts are being deprecated as part of the Fission project. MozReview-Commit-ID: ACEOY2hExco
dom/base/nsFrameMessageManager.cpp
js/src/builtin/Eval.cpp
js/src/builtin/TestingFunctions.cpp
js/src/jit-test/tests/debug/Environment-object-01.js
js/src/jsfriendapi.h
js/src/vm/EnvironmentObject.h
js/src/vm/Realm.cpp
js/src/vm/Realm.h
--- a/dom/base/nsFrameMessageManager.cpp
+++ b/dom/base/nsFrameMessageManager.cpp
@@ -1299,17 +1299,17 @@ nsMessageManagerScriptExecutor::LoadScri
   AutoEntryScript aes(aGlobal, "message manager script load");
   JSContext* cx = aes.cx();
   if (script) {
     if (aRunInGlobalScope) {
       JS::RootedValue rval(cx);
       JS::CloneAndExecuteScript(cx, script, &rval);
     } else {
       JS::Rooted<JSObject*> scope(cx);
-      bool ok = js::ExecuteInGlobalAndReturnScope(cx, aGlobal, script, &scope);
+      bool ok = js::ExecuteInFrameScriptEnvironment(cx, aGlobal, script, &scope);
       if (ok) {
         // Force the scope to stay alive.
         mAnonymousGlobalScopes.AppendElement(scope);
       }
     }
   }
 }
 
--- a/js/src/builtin/Eval.cpp
+++ b/js/src/builtin/Eval.cpp
@@ -452,33 +452,45 @@ ExecuteInExtensibleLexicalEnvironment(JS
     }
 
     RootedValue rval(cx);
     return ExecuteKernel(cx, script, *env, UndefinedValue(),
                          NullFramePtr() /* evalInFrame */, rval.address());
 }
 
 JS_FRIEND_API(bool)
-js::ExecuteInGlobalAndReturnScope(JSContext* cx, HandleObject global, HandleScript scriptArg,
-                                  MutableHandleObject envArg)
+js::ExecuteInFrameScriptEnvironment(JSContext* cx, HandleObject objArg, HandleScript scriptArg,
+                                    MutableHandleObject envArg)
 {
     RootedObject varEnv(cx, NonSyntacticVariablesObject::create(cx));
     if (!varEnv)
         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)
+    AutoObjectVector envChain(cx);
+    if (!envChain.append(objArg))
+        return false;
+
+    RootedObject env(cx);
+    if (!js::CreateObjectsForEnvironmentChain(cx, envChain, varEnv, &env))
         return false;
 
-    if (!ExecuteInExtensibleLexicalEnvironment(cx, scriptArg, lexEnv))
+    // Create lexical environment with |this| == objArg, which should be a Gecko
+    // MessageManager.
+    // NOTE: This is required behavior for Gecko FrameScriptLoader, where some
+    // callers try to bind methods from the message manager in their scope chain
+    // to |this|, and will fail if it is not bound to a message manager.
+    ObjectRealm& realm = ObjectRealm::get(varEnv);
+    env = realm.getOrCreateNonSyntacticLexicalEnvironment(cx, env, varEnv, objArg);
+    if (!env)
         return false;
 
-    envArg.set(lexEnv);
+    if (!ExecuteInExtensibleLexicalEnvironment(cx, scriptArg, env))
+        return false;
+
+    envArg.set(env);
     return true;
 }
 
 JS_FRIEND_API(JSObject*)
 js::NewJSMEnvironment(JSContext* cx)
 {
     RootedObject varEnv(cx, NonSyntacticVariablesObject::create(cx));
     if (!varEnv)
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -3878,25 +3878,28 @@ EvalReturningScope(JSContext* cx, unsign
     } else {
         global = JS::CurrentGlobalOrNull(cx);
     }
 
     RootedObject varObj(cx);
     RootedObject lexicalScope(cx);
 
     {
-        // If we're switching globals here, ExecuteInGlobalAndReturnScope will
+        // If we're switching globals here, ExecuteInFrameScriptEnvironment will
         // take care of cloning the script into that compartment before
         // executing it.
         AutoRealm ar(cx, global);
-
-        if (!js::ExecuteInGlobalAndReturnScope(cx, global, script, &lexicalScope))
+        JS::RootedObject obj(cx, JS_NewPlainObject(cx));
+        if (!obj)
             return false;
 
-        varObj = lexicalScope->enclosingEnvironment();
+        if (!js::ExecuteInFrameScriptEnvironment(cx, obj, script, &lexicalScope))
+            return false;
+
+        varObj = lexicalScope->enclosingEnvironment()->enclosingEnvironment();
     }
 
     RootedObject rv(cx, JS_NewPlainObject(cx));
     if (!rv)
         return false;
 
     RootedValue varObjVal(cx, ObjectValue(*varObj));
     if (!cx->compartment()->wrap(cx, &varObjVal))
--- a/js/src/jit-test/tests/debug/Environment-object-01.js
+++ b/js/src/jit-test/tests/debug/Environment-object-01.js
@@ -1,8 +1,9 @@
 var g = newGlobal();
 var dbg = new Debugger(g);
 
 dbg.onDebuggerStatement = (frame) => {
-  assertEq(frame.environment.parent.type, "object");
-  assertEq(frame.environment.parent.object.getOwnPropertyDescriptor("x").value, 42);
+  assertEq(frame.environment.parent.type, "with");
+  assertEq(frame.environment.parent.parent.type, "object");
+  assertEq(frame.environment.parent.parent.object.getOwnPropertyDescriptor("x").value, 42);
 }
 g.evalReturningScope("x = 42; debugger;");
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -2793,18 +2793,19 @@ ForwardToNative(JSContext* cx, JSNative 
 JS_FRIEND_API(bool)
 SetPropertyIgnoringNamedGetter(JSContext* cx, JS::HandleObject obj, JS::HandleId id,
                                JS::HandleValue v, JS::HandleValue receiver,
                                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);
+ExecuteInFrameScriptEnvironment(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.
--- a/js/src/vm/EnvironmentObject.h
+++ b/js/src/vm/EnvironmentObject.h
@@ -222,27 +222,30 @@ EnvironmentCoordinateFunctionScript(JSSc
  *   LexicalEnvironmentObject[this=nsvo]
  *       |
  *   WithEnvironmentObject wrapping target
  *       |
  *   LexicalEnvironmentObject[this=target]
  *
  * D. Frame scripts
  *
- * XUL frame scripts are always loaded with a NonSyntacticVariablesObject as a
- * "polluting global". This is done exclusively in
- * js::ExecuteInGlobalAndReturnScope.
+ * XUL frame scripts are loaded in the same global as components, with a
+ * NonSyntacticVariablesObject as a "polluting global", and a with environment
+ * wrapping a message manager object. This is done exclusively in
+ * js::ExecuteInScopeChainAndReturnNewScope.
  *
- *   Loader global
+ *   BackstagePass global
  *       |
  *   LexicalEnvironmentObject[this=global]
  *       |
  *   NonSyntacticVariablesObject
  *       |
- *   LexicalEnvironmentObject[this=global]
+ *   WithEnvironmentObject wrapping messageManager
+ *       |
+ *   LexicalEnvironmentObject[this=messageManager]
  *
  * D. XBL and DOM event handlers
  *
  * XBL methods are compiled as functions with XUL elements on the env chain,
  * and DOM event handlers are compiled as functions with HTML elements on the
  * env chain. For a chain of elements e0,e1,...:
  *
  *      ...
--- a/js/src/vm/Realm.cpp
+++ b/js/src/vm/Realm.cpp
@@ -186,71 +186,78 @@ namespace {
 struct CheckGCThingAfterMovingGCFunctor {
     template <class T> void operator()(T* t) { CheckGCThingAfterMovingGC(*t); }
 };
 } // namespace (anonymous)
 
 #endif // JSGC_HASH_TABLE_CHECKS
 
 LexicalEnvironmentObject*
-ObjectRealm::getOrCreateNonSyntacticLexicalEnvironment(JSContext* cx, HandleObject enclosing)
+ObjectRealm::getOrCreateNonSyntacticLexicalEnvironment(JSContext* cx, HandleObject enclosing,
+                                                       HandleObject key, HandleObject thisv)
 {
     MOZ_ASSERT(&ObjectRealm::get(enclosing) == this);
 
     if (!nonSyntacticLexicalEnvironments_) {
         auto map = cx->make_unique<ObjectWeakMap>(cx);
         if (!map)
             return nullptr;
 
         nonSyntacticLexicalEnvironments_ = std::move(map);
     }
 
-    // If a wrapped WithEnvironmentObject was passed in, unwrap it, as we may
-    // be creating different WithEnvironmentObject wrappers each time.
-    RootedObject key(cx, enclosing);
-    if (enclosing->is<WithEnvironmentObject>()) {
-        MOZ_ASSERT(!enclosing->as<WithEnvironmentObject>().isSyntactic());
-        key = &enclosing->as<WithEnvironmentObject>().object();
-    }
     RootedObject lexicalEnv(cx, nonSyntacticLexicalEnvironments_->lookup(key));
 
     if (!lexicalEnv) {
-        // NOTE: The default global |this| value is set to key for compatibility
-        // with existing users of the lexical environment cache.
-        //  - When used by shared-global JSM loader, |this| must be the
-        //    NonSyntacticVariablesObject passed as enclosing.
-        //  - When used by SubscriptLoader, |this| must be the target object of
-        //    the WithEnvironmentObject wrapper.
-        //  - When used by XBL/DOM Events, we execute directly as a function and
-        //    do not access the |this| value.
-        // See js::GetFunctionThis / js::GetNonSyntacticGlobalThis
         MOZ_ASSERT(key->is<NonSyntacticVariablesObject>() || !key->is<EnvironmentObject>());
-        lexicalEnv = LexicalEnvironmentObject::createNonSyntactic(cx, enclosing, /*thisv = */key);
+        lexicalEnv = LexicalEnvironmentObject::createNonSyntactic(cx, enclosing, thisv);
         if (!lexicalEnv)
             return nullptr;
         if (!nonSyntacticLexicalEnvironments_->add(cx, key, lexicalEnv))
             return nullptr;
     }
 
     return &lexicalEnv->as<LexicalEnvironmentObject>();
 }
 
 LexicalEnvironmentObject*
-ObjectRealm::getNonSyntacticLexicalEnvironment(JSObject* enclosing) const
+ObjectRealm::getOrCreateNonSyntacticLexicalEnvironment(JSContext* cx, HandleObject enclosing)
 {
-    MOZ_ASSERT(&ObjectRealm::get(enclosing) == this);
+    // If a wrapped WithEnvironmentObject was passed in, unwrap it, as we may
+    // be creating different WithEnvironmentObject wrappers each time.
+    RootedObject key(cx, enclosing);
+    if (enclosing->is<WithEnvironmentObject>()) {
+        MOZ_ASSERT(!enclosing->as<WithEnvironmentObject>().isSyntactic());
+        key = &enclosing->as<WithEnvironmentObject>().object();
+    }
+
+    // NOTE: The default global |this| value is set to key for compatibility
+    // with existing users of the lexical environment cache.
+    //  - When used by shared-global JSM loader, |this| must be the
+    //    NonSyntacticVariablesObject passed as enclosing.
+    //  - When used by SubscriptLoader, |this| must be the target object of
+    //    the WithEnvironmentObject wrapper.
+    //  - When used by XBL/DOM Events, we execute directly as a function and
+    //    do not access the |this| value.
+    // See js::GetFunctionThis / js::GetNonSyntacticGlobalThis
+    return getOrCreateNonSyntacticLexicalEnvironment(cx, enclosing, key, /*thisv = */key);
+}
+
+LexicalEnvironmentObject*
+ObjectRealm::getNonSyntacticLexicalEnvironment(JSObject* key) const
+{
+    MOZ_ASSERT(&ObjectRealm::get(key) == this);
 
     if (!nonSyntacticLexicalEnvironments_)
         return nullptr;
     // If a wrapped WithEnvironmentObject was passed in, unwrap it as in
     // getOrCreateNonSyntacticLexicalEnvironment.
-    JSObject* key = enclosing;
-    if (enclosing->is<WithEnvironmentObject>()) {
-        MOZ_ASSERT(!enclosing->as<WithEnvironmentObject>().isSyntactic());
-        key = &enclosing->as<WithEnvironmentObject>().object();
+    if (key->is<WithEnvironmentObject>()) {
+        MOZ_ASSERT(!key->as<WithEnvironmentObject>().isSyntactic());
+        key = &key->as<WithEnvironmentObject>().object();
     }
     JSObject* lexicalEnv = nonSyntacticLexicalEnvironments_->lookup(key);
     if (!lexicalEnv)
         return nullptr;
     return &lexicalEnv->as<LexicalEnvironmentObject>();
 }
 
 bool
--- a/js/src/vm/Realm.h
+++ b/js/src/vm/Realm.h
@@ -287,17 +287,20 @@ class ObjectRealm
                                 size_t* lazyArrayBuffersArg,
                                 size_t* objectMetadataTablesArg,
                                 size_t* nonSyntacticLexicalEnvironmentsArg);
 
     MOZ_ALWAYS_INLINE bool objectMaybeInIteration(JSObject* obj);
 
     js::LexicalEnvironmentObject*
     getOrCreateNonSyntacticLexicalEnvironment(JSContext* cx, js::HandleObject enclosing);
-    js::LexicalEnvironmentObject* getNonSyntacticLexicalEnvironment(JSObject* enclosing) const;
+    js::LexicalEnvironmentObject*
+    getOrCreateNonSyntacticLexicalEnvironment(JSContext* cx, js::HandleObject enclosing,
+                                              js::HandleObject key, js::HandleObject thisv);
+    js::LexicalEnvironmentObject* getNonSyntacticLexicalEnvironment(JSObject* key) const;
 };
 
 } // namespace js
 
 class JS::Realm : public JS::shadow::Realm
 {
     JS::Zone* zone_;
     JSRuntime* runtime_;