Bug 1255800 - Make computeThis return a boolean for easier error handling. r=jorendorff

--HG--
extra : rebase_source : 1b4d227aba2fd0e1a458849addb634df9d72111c
extra : histedit_source : 29e8c520ae4d3644131b05794a447a0e7a32796f

ipc/testshell/XPCShellEnvironment.cpp

@@ -132,8 +132,10 @@ Load(JSContext *cx,
 {
     JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
 
-    JS::RootedValue thisv(cx, args.computeThis(cx));
-    if (!thisv.isObject() || !JS_IsGlobalObject(&thisv.toObject())) {
+    JS::RootedObject thisObject(cx);
+    if (!args.computeThis(cx, &thisObject))
+        return false;
+    if (!JS_IsGlobalObject(thisObject)) {
         JS_ReportErrorASCII(cx, "Trying to load() into a non-global object");
         return false;
     }

js/public/CallArgs.h

@@ -87,8 +87,8 @@ namespace detail {
  * Compute |this| for the |vp| inside a JSNative, either boxing primitives or
  * replacing with the global object as necessary.
  */
-extern JS_PUBLIC_API(Value)
-ComputeThis(JSContext* cx, JS::Value* vp);
+extern JS_PUBLIC_API(bool)
+ComputeThis(JSContext* cx, JS::Value* vp, MutableHandleObject thisObject);
 
 #ifdef JS_DEBUG
 extern JS_PUBLIC_API(void)
@@ -199,11 +199,13 @@ class MOZ_STACK_CLASS CallArgsBase
         return HandleValue::fromMarkedLocation(&argv_[-1]);
     }
 
-    Value computeThis(JSContext* cx) const {
-        if (thisv().isObject())
-            return thisv();
+    bool computeThis(JSContext* cx, MutableHandleObject thisObject) const {
+        if (thisv().isObject()) {
+            thisObject.set(&thisv().toObject());
+            return true;
+        }
 
-        return ComputeThis(cx, base());
+        return ComputeThis(cx, base(), thisObject);
     }
 
     // ARGUMENTS

js/src/jsapi.cpp

@@ -1339,17 +1339,18 @@ JS::CurrentGlobalOrNull(JSContext* cx)
     return cx->global();
 }
 
-JS_PUBLIC_API(Value)
-JS::detail::ComputeThis(JSContext* cx, Value* vp)
+JS_PUBLIC_API(bool)
+JS::detail::ComputeThis(JSContext* cx, Value* vp, MutableHandleObject thisObject)
 {
     AssertHeapIsIdle();
     assertSameCompartment(cx, JSValueArray(vp, 2));
 
     MutableHandleValue thisv = MutableHandleValue::fromMarkedLocation(&vp[1]);
     if (!BoxNonStrictThis(cx, thisv, thisv))
-        return NullValue();
+        return false;
 
-    return thisv;
+    thisObject.set(&thisv.toObject());
+    return true;
 }
 
 JS_PUBLIC_API(void*)

js/xpconnect/src/ExportHelpers.cpp

@@ -290,7 +290,10 @@ FunctionForwarder(JSContext* cx, unsigned argc, Value* vp)
 
     RootedValue thisVal(cx, NullValue());
     if (!args.isConstructing()) {
-        thisVal.set(args.computeThis(cx));
+        RootedObject thisObject(cx);
+        if (!args.computeThis(cx, &thisObject))
+            return false;
+        thisVal.setObject(*thisObject);
     }
 
     {

js/xpconnect/src/Sandbox.cpp

@@ -216,12 +216,10 @@ SandboxImport(JSContext* cx, unsigned argc, Value* vp)
     // We need to resolve the this object, because this function is used
     // unbound and should still work and act on the original sandbox.
 
-    RootedValue thisv(cx, args.computeThis(cx));
-    if (!thisv.isObject()) {
-        XPCThrower::Throw(NS_ERROR_UNEXPECTED, cx);
+    RootedObject thisObject(cx);
+    if (!args.computeThis(cx, &thisObject))
         return false;
-    }
-    RootedObject thisObject(cx, &thisv.toObject());
+
     if (!JS_SetPropertyById(cx, thisObject, id, args[0]))
         return false;
 
@@ -562,7 +560,15 @@ xpc::SandboxCallableProxyHandler::call(JSContext* cx, JS::Handle<JSObject*> prox
     // if the sandboxPrototype is an Xray Wrapper, which lets us appropriately
     // remap |this|.
     bool isXray = WrapperFactory::IsXrayWrapper(sandboxProxy);
-    RootedValue thisVal(cx, isXray ? args.computeThis(cx) : args.thisv());
+    RootedValue thisVal(cx, args.thisv());
+    if (isXray) {
+        RootedObject thisObject(cx);
+        if (!args.computeThis(cx, &thisObject)) {
+            return false;
+        }
+        thisVal.setObject(*thisObject);
+    }
+
     if (thisVal == ObjectValue(*sandboxGlobal)) {
         thisVal = ObjectValue(*js::GetProxyTargetObject(sandboxProxy));
     }

js/xpconnect/src/XPCShellImpl.cpp

@@ -337,8 +337,10 @@ Load(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
-    RootedValue thisv(cx, args.computeThis(cx));
-    if (!thisv.isObject() || !JS_IsGlobalObject(&thisv.toObject())) {
+    JS::RootedObject thisObject(cx);
+    if (!args.computeThis(cx, &thisObject))
+        return false;
+    if (!JS_IsGlobalObject(thisObject)) {
         JS_ReportErrorASCII(cx, "Trying to load() into a non-global object");
         return false;
     }

js/xpconnect/src/XPCWrappedNativeJSOps.cpp

@@ -70,12 +70,9 @@ XPC_WN_Shared_ToString(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
-    RootedValue thisv(cx, args.computeThis(cx));
-    if (!thisv.isObject()) {
-        JS_ReportErrorASCII(cx, "Called on incompatible |this|");
+    RootedObject obj(cx);
+    if (!args.computeThis(cx, &obj))
         return false;
-    }
-    RootedObject obj(cx, &thisv.toObject());
 
     XPCCallContext ccx(cx, obj);
     if (!ccx.IsValid())
@@ -895,12 +892,9 @@ XPC_WN_CallMethod(JSContext* cx, unsigned argc, Value* vp)
     MOZ_ASSERT(JS_TypeOfValue(cx, args.calleev()) == JSTYPE_FUNCTION, "bad function");
     RootedObject funobj(cx, &args.callee());
 
-    RootedValue thisv(cx, args.computeThis(cx));
-    if (!thisv.isObject()) {
-        JS_ReportErrorASCII(cx, "Called on incompatible |this|");
+    RootedObject obj(cx);
+    if (!args.computeThis(cx, &obj))
         return false;
-    }
-    RootedObject obj(cx, &thisv.toObject());
 
     obj = FixUpThisIfBroken(obj, funobj);
     XPCCallContext ccx(cx, obj, funobj, JSID_VOIDHANDLE, args.length(),