Bug 658909 - Force |this| computation in SandboxCallableProxyHandler::call when using Xrays. r=bz
authorBobby Holley <bobbyholley@gmail.com>
Thu, 21 Mar 2013 08:20:41 -0700
changeset 125799 6325487c131eb71bf225a9c2846b2fdb88065618
parent 125798 4489ed4085abae8302f5b80e4fdba4d23af0907c
child 125800 ff717ac709fa0d5ad0281007b1a53c84f07bc113
push id25109
push userryanvm@gmail.com
push dateThu, 21 Mar 2013 19:52:05 +0000
treeherdermozilla-inbound@a83cbe4e0576 [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);
 }