Bug 1397385 - Refactor js::GetThisValue draft
authorTed Campbell <tcampbell@mozilla.com>
Thu, 07 Sep 2017 23:40:26 -0400
changeset 661490 c4ae9d9822c855a2b5f78cad2ad1c10b48ce40ee
parent 660208 93dd2e456c0ecca00fb4d28744e88078a77deaf7
child 661491 eab21506d9dcc653c0fea1a5d09ae3d6b8f36181
push id78779
push userbmo:tcampbell@mozilla.com
push dateFri, 08 Sep 2017 14:53:54 +0000
bugs1397385
milestone57.0a1
Bug 1397385 - Refactor js::GetThisValue Split GetThisValue into three concerns: 1) Sanity check |this| and convert Window to WindowProxy 2) Find the |this| of an extensible LexicalEnvironmentObject 3) Find the target of a WithEnvironmentObject MozReview-Commit-ID: I2U54IxClSy
js/src/jscompartment.cpp
js/src/jsobj.cpp
js/src/jsobj.h
js/src/vm/EnvironmentObject.cpp
js/src/vm/Interpreter.cpp
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -577,17 +577,27 @@ JSCompartment::getOrCreateNonSyntacticLe
     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) {
-        lexicalEnv = LexicalEnvironmentObject::createNonSyntactic(cx, enclosing, enclosing);
+        // 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);
         if (!lexicalEnv)
             return nullptr;
         if (!nonSyntacticLexicalEnvironments_->add(cx, key, lexicalEnv))
             return nullptr;
     }
 
     return &lexicalEnv->as<LexicalEnvironmentObject>();
 }
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -3255,30 +3255,42 @@ js::ToObjectSlow(JSContext* cx, JS::Hand
 
 Value
 js::GetThisValue(JSObject* obj)
 {
     if (obj->is<GlobalObject>())
         return ObjectValue(*ToWindowProxyIfWindow(obj));
 
     if (obj->is<LexicalEnvironmentObject>()) {
-        if (!obj->as<LexicalEnvironmentObject>().isExtensible())
-            return UndefinedValue();
-        return obj->as<LexicalEnvironmentObject>().thisValue();
+        MOZ_ASSERT(!obj->as<LexicalEnvironmentObject>().isExtensible());
+        return UndefinedValue();
     }
 
     if (obj->is<ModuleEnvironmentObject>())
         return UndefinedValue();
 
-    if (obj->is<WithEnvironmentObject>())
-        return ObjectValue(*obj->as<WithEnvironmentObject>().withThis());
+    MOZ_ASSERT(!obj->is<WithEnvironmentObject>());
 
     return ObjectValue(*obj);
 }
 
+Value
+js::GetThisValueOfLexical(JSObject* env)
+{
+    MOZ_ASSERT(IsExtensibleLexicalEnvironment(env));
+    return env->as<LexicalEnvironmentObject>().thisValue();
+}
+
+Value
+js::GetThisValueOfWith(JSObject* env)
+{
+    MOZ_ASSERT(env->is<WithEnvironmentObject>());
+    return GetThisValue(env->as<WithEnvironmentObject>().withThis());
+}
+
 class GetObjectSlotNameFunctor : public JS::CallbackTracer::ContextFunctor
 {
     JSObject* obj;
 
   public:
     explicit GetObjectSlotNameFunctor(JSObject* ctx) : obj(ctx) {}
     virtual void operator()(JS::CallbackTracer* trc, char* buf, size_t bufsize) override;
 };
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -1069,29 +1069,31 @@ ToPrimitive(JSContext* cx, JSType prefer
 /*
  * toString support. (This isn't called GetClassName because there's a macro in
  * <windows.h> with that name.)
  */
 MOZ_ALWAYS_INLINE const char*
 GetObjectClassName(JSContext* cx, HandleObject obj);
 
 /*
- * Return an object that may be used as `this` in place of obj. For most
- * objects this just returns obj.
+ * Prepare a |this| value to be returned to script. This includes replacing
+ * Windows with their corresponding WindowProxy.
  *
- * Some JSObjects shouldn't be exposed directly to script. This includes (at
- * least) WithEnvironmentObjects and Window objects. However, since both of
- * those can be on scope chains, we sometimes would expose those as `this` if
- * we were not so vigilant about calling GetThisValue where appropriate.
- *
- * See comments at ComputeImplicitThis.
+ * Helpers are also provided to first extract the |this| from specific
+ * types of environment.
  */
 Value
 GetThisValue(JSObject* obj);
 
+Value
+GetThisValueOfLexical(JSObject* env);
+
+Value
+GetThisValueOfWith(JSObject* env);
+
 /* * */
 
 typedef JSObject* (*ClassInitializerOp)(JSContext* cx, JS::HandleObject obj);
 
 /* Fast access to builtin constructors and prototypes. */
 bool
 GetBuiltinConstructor(JSContext* cx, JSProtoKey key, MutableHandleObject objp);
 
--- a/js/src/vm/EnvironmentObject.cpp
+++ b/js/src/vm/EnvironmentObject.cpp
@@ -1064,21 +1064,21 @@ LexicalEnvironmentObject::isExtensible()
 }
 
 Value
 LexicalEnvironmentObject::thisValue() const
 {
     MOZ_ASSERT(isExtensible());
     Value v = getReservedSlot(THIS_VALUE_OR_SCOPE_SLOT);
     if (v.isObject()) {
-        // If `v` is a Window, return the WindowProxy instead. We called
-        // GetThisValue (which also does ToWindowProxyIfWindow) when storing
-        // the value in THIS_VALUE_OR_SCOPE_SLOT, but it's possible the
-        // WindowProxy was attached to the global *after* we set
-        // THIS_VALUE_OR_SCOPE_SLOT.
+        // A WindowProxy may have been attached after this environment was
+        // created so check ToWindowProxyIfWindow again. For example,
+        // GlobalObject::createInternal will construct its lexical environment
+        // before SetWindowProxy can be called.
+        // See also: js::GetThisValue / js::GetThisValueOfLexical
         return ObjectValue(*ToWindowProxyIfWindow(&v.toObject()));
     }
     return v;
 }
 
 const Class LexicalEnvironmentObject::class_ = {
     "LexicalEnvironment",
     JSCLASS_HAS_RESERVED_SLOTS(LexicalEnvironmentObject::RESERVED_SLOTS) |
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -127,23 +127,24 @@ js::GetFunctionThis(JSContext* cx, Abstr
     }
 
     RootedValue thisv(cx, frame.thisArgument());
 
     // If there is a NSVO on environment chain, use it as basis for fallback
     // global |this|. This gives a consistent definition of global lexical
     // |this| between function and global contexts.
     //
-    // NOTE: If only non-syntactic WithEnvironments are on the chain, we use
-    // the global lexical |this| value.
+    // NOTE: If only non-syntactic WithEnvironments are on the chain, we use the
+    // global lexical |this| value. This is for compatibility with the Subscript
+    // Loader.
     if (frame.script()->hasNonSyntacticScope() && thisv.isNullOrUndefined()) {
         RootedObject env(cx, frame.environmentChain());
         while (true) {
             if (IsNSVOLexicalEnvironment(env) || IsGlobalLexicalEnvironment(env)) {
-                res.set(GetThisValue(env));
+                res.set(GetThisValueOfLexical(env));
                 return true;
             }
             if (!env->enclosingEnvironment()) {
                 // This can only happen in Debugger eval frames: in that case we
                 // don't always have a global lexical env, see EvaluateInEnv.
                 MOZ_ASSERT(env->is<GlobalObject>());
                 res.set(GetThisValue(env));
                 return true;
@@ -156,17 +157,17 @@ js::GetFunctionThis(JSContext* cx, Abstr
 }
 
 void
 js::GetNonSyntacticGlobalThis(JSContext* cx, HandleObject envChain, MutableHandleValue res)
 {
     RootedObject env(cx, envChain);
     while (true) {
         if (IsExtensibleLexicalEnvironment(env)) {
-            res.set(env->as<LexicalEnvironmentObject>().thisValue());
+            res.set(GetThisValueOfLexical(env));
             return;
         }
         if (!env->enclosingEnvironment()) {
             // This can only happen in Debugger eval frames: in that case we
             // don't always have a global lexical env, see EvaluateInEnv.
             MOZ_ASSERT(env->is<GlobalObject>());
             res.set(GetThisValue(env));
             return;
@@ -1490,16 +1491,19 @@ static inline Value
 ComputeImplicitThis(JSObject* obj)
 {
     if (obj->is<GlobalObject>())
         return UndefinedValue();
 
     if (obj->is<NonSyntacticVariablesObject>())
         return UndefinedValue();
 
+    if (obj->is<WithEnvironmentObject>())
+        return GetThisValueOfWith(obj);
+
     if (IsCacheableEnvironment(obj))
         return UndefinedValue();
 
     // Debugger environments need special casing, as despite being
     // non-syntactic, they wrap syntactic environments and should not be
     // treated like other embedding-specific non-syntactic environments.
     if (obj->is<DebugEnvironmentProxy>())
         return ComputeImplicitThis(&obj->as<DebugEnvironmentProxy>().environment());