Bug 658909 - Force |this| computation in SandboxCallableProxyHandler::call when using Xrays. r=bz
authorBobby Holley <bobbyholley@gmail.com>
Sat, 16 Mar 2013 22:58:12 -0700
changeset 125049 aa68b74a149563279ad94beaf6642733c912ee87
parent 125048 267ade6627346fbe23fb0373ed47dbfe3c600e93
child 125050 9f153c67ec84ac78d0b28f8e2284213704d0f421
push id24762
push userbobbyholley@gmail.com
push dateSun, 17 Mar 2013 05:58:40 +0000
treeherdermozilla-inbound@3a5f73a4f816 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs658909
milestone22.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 658909 - Force |this| computation in SandboxCallableProxyHandler::call when using Xrays. r=bz Comment says it all.
js/xpconnect/src/XPCComponents.cpp
--- a/js/xpconnect/src/XPCComponents.cpp
+++ b/js/xpconnect/src/XPCComponents.cpp
@@ -3059,22 +3059,47 @@ xpc::SandboxCallableProxyHandler::call(J
                js::GetProxyHandler(sandboxProxy) == &xpc::sandboxProxyHandler);
 
     // The parent of the sandboxProxy is the sandbox global, and the
     // target object is the original proto.
     JSObject *sandboxGlobal = JS_GetParent(sandboxProxy);
     MOZ_ASSERT(js::GetObjectJSClass(sandboxGlobal) == &SandboxClass);
 
     // If our this object is the sandbox global, we call with this set to the
-    // original proto instead.  Note that we very carefully avoid using JS_THIS
-    // or JS_THIS_OBJECT here, because we do NOT want to box undefined into the
-    // global.  Instead, we just pass it through to our callable, and it will
-    // compute the global based on its own scope chain, which will do the right
-    // thing.
-    JS::Value thisVal = JS_THIS_VALUE(cx, vp);
+    // original proto instead.
+    //
+    // There are two different ways we can compute |this|. If we use
+    // JS_THIS_VALUE, we'll get the bonafide |this| value as passed by the
+    // caller, which may be undefined if a global function was invoked without
+    // an explicit invocant. If we use JS_THIS or JS_THIS_OBJECT, the |this|
+    // in |vp| will be coerced to the global, which is not the correct
+    // behavior in ES5 strict mode. And we have no way to compute strictness
+    // here.
+    //
+    // The naive approach is simply to use JS_THIS_VALUE here. If |this| was
+    // explicit, we can remap it appropriately. If it was implicit, then we
+    // leave it as undefined, and let the callee sort it out. Since the callee
+    // is generally in the same compartment as its global (eg the Window's
+    // compartment, not the Sandbox's), the callee will generally compute the
+    // correct |this|.
+    //
+    // However, this breaks down in the Xray case. If the sandboxPrototype
+    // is an Xray wrapper, then we'll end up reifying the native methods in
+    // the Sandbox's scope, which means that they'll compute |this| to be the
+    // Sandbox, breaking old-style XPC_WN_CallMethod methods.
+    //
+    // Luckily, the intent of Xrays is to provide a vanilla view of a foreign
+    // DOM interface, which means that we don't care about script-enacted
+    // strictness in the prototype's home compartment. Indeed, since DOM
+    // methods are always non-strict, we can just assume non-strict semantics
+    // if the sandboxPrototype is an Xray Wrapper, which lets us appropriately
+    // remap |this|.
+    JS::Value thisVal =
+      WrapperFactory::IsXrayWrapper(sandboxProxy) ? JS_THIS(cx, vp)
+                                                  : JS_THIS_VALUE(cx, vp);
     if (thisVal == ObjectValue(*sandboxGlobal)) {
         thisVal = ObjectValue(*js::GetProxyTargetObject(sandboxProxy));
     }
 
     return JS::Call(cx, thisVal, js::GetProxyCall(proxy), argc,
                     JS_ARGV(cx, vp), vp);
 }