Bug 1480244: Part 2 - Replace ExecuteInGlobalAndReturnScope with ExecuteInScopeChainAndReturnNewScope. r?tcampbell draft
authorKris Maglione <maglione.k@gmail.com>
Fri, 10 Aug 2018 13:54:23 -0700
changeset 828428 3c0bfe59abd1a357297cf8be3e7933cd372494d0
parent 828427 e93e24a62aca9596f41e6d16045771177f38611c
child 828429 daf637730cfe4f8210a9f3c3a9fcc2322decb842
push id118679
push usermaglione.k@gmail.com
push dateFri, 10 Aug 2018 21:19:41 +0000
reviewerstcampbell
bugs1480244
milestone63.0a1
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
@@ -1311,17 +1311,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,26 +452,38 @@ 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));
+    AutoObjectVector envChain(cx);
+    if (!envChain.append(objArg))
+        return false;
+
+    RootedObject env(cx);
+    if (!js::CreateObjectsForEnvironmentChain(cx, envChain, varEnv, &env))
+        return false;
+
+    // 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);
+    RootedObject lexEnv(cx, realm.getOrCreateNonSyntacticLexicalEnvironment(cx, env, varEnv, objArg));
     if (!lexEnv)
         return false;
 
     if (!ExecuteInExtensibleLexicalEnvironment(cx, scriptArg, lexEnv))
         return false;
 
     envArg.set(lexEnv);
     return true;
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -3883,25 +3883,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
@@ -190,76 +190,83 @@ 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;
 
         if (!map->init()) {
             ReportOutOfMemory(cx);
             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_;