Bug 1255800 - Make computeThis return a boolean for easier error handling. r=jorendorff
authorTom Schuster <evilpies@gmail.com>
Fri, 23 Mar 2018 13:09:04 +0100
changeset 410802 edcef7625d066d8006452d561f9732adee54e0b9
parent 410801 e4c46aab1b2f39349c5fcbb937136be7a9340da8
child 410803 a90440b814dc199417bfebdebb95689cfed8f229
push id33740
push usernbeleuzu@mozilla.com
push dateFri, 30 Mar 2018 21:49:44 +0000
treeherdermozilla-central@f4fcdaef6168 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1255800
milestone61.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 1255800 - Make computeThis return a boolean for easier error handling. r=jorendorff
ipc/testshell/XPCShellEnvironment.cpp
js/public/CallArgs.h
js/src/jsapi.cpp
js/xpconnect/src/ExportHelpers.cpp
js/xpconnect/src/Sandbox.cpp
js/xpconnect/src/XPCShellImpl.cpp
js/xpconnect/src/XPCWrappedNativeJSOps.cpp
--- a/ipc/testshell/XPCShellEnvironment.cpp
+++ b/ipc/testshell/XPCShellEnvironment.cpp
@@ -127,18 +127,20 @@ Dump(JSContext *cx, unsigned argc, JS::V
 
 static bool
 Load(JSContext *cx,
      unsigned argc,
      JS::Value *vp)
 {
     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;
     }
 
     for (unsigned i = 0; i < args.length(); i++) {
         JS::Rooted<JSString*> str(cx, JS::ToString(cx, args[i]));
         if (!str)
             return false;
--- a/js/public/CallArgs.h
+++ b/js/public/CallArgs.h
@@ -82,18 +82,18 @@ namespace JS {
 extern JS_PUBLIC_DATA(const HandleValue) UndefinedHandleValue;
 
 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)
 CheckIsValidConstructible(const Value& v);
 #endif
 
 class MOZ_STACK_CLASS IncludeUsedRval
 {
@@ -194,21 +194,23 @@ class MOZ_STACK_CLASS CallArgsBase
      */
     HandleValue thisv() const {
         // Some internal code uses thisv() in constructing cases, so don't do
         // this yet.
         // MOZ_ASSERT(!argv_[-1].isMagic(JS_IS_CONSTRUCTING));
         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
 
         /* Returns the number of arguments. */
     unsigned length() const { return argc_; }
 
     /* Returns the i-th zero-indexed argument. */
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -1334,27 +1334,28 @@ JS::CurrentGlobalOrNull(JSContext* cx)
 {
     AssertHeapIsIdleOrIterating();
     CHECK_REQUEST(cx);
     if (!cx->compartment())
         return nullptr;
     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 thisv;
+        return false;
+
+    thisObject.set(&thisv.toObject());
+    return true;
 }
 
 JS_PUBLIC_API(void*)
 JS_malloc(JSContext* cx, size_t nbytes)
 {
     AssertHeapIsIdle();
     CHECK_REQUEST(cx);
     return static_cast<void*>(cx->zone()->pod_malloc<uint8_t>(nbytes));
--- a/js/xpconnect/src/ExportHelpers.cpp
+++ b/js/xpconnect/src/ExportHelpers.cpp
@@ -285,17 +285,20 @@ FunctionForwarder(JSContext* cx, unsigne
         return false;
 
     // Grab and unwrap the underlying callable.
     RootedValue v(cx, js::GetFunctionNativeReserved(&args.callee(), 0));
     RootedObject unwrappedFun(cx, js::UncheckedUnwrap(&v.toObject()));
 
     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);
     }
 
     {
         // We manually implement the contents of CrossCompartmentWrapper::call
         // here, because certain function wrappers (notably content->nsEP) are
         // not callable.
         JSAutoCompartment ac(cx, unwrappedFun);
         if (!CheckSameOriginArg(cx, options, thisVal) || !JS_WrapValue(cx, &thisVal))
--- a/js/xpconnect/src/Sandbox.cpp
+++ b/js/xpconnect/src/Sandbox.cpp
@@ -211,22 +211,20 @@ SandboxImport(JSContext* cx, unsigned ar
 
     RootedId id(cx);
     if (!JS_StringToId(cx, funname, &id))
         return false;
 
     // 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;
 
     args.rval().setUndefined();
     return true;
 }
 
 static bool
@@ -557,17 +555,25 @@ xpc::SandboxCallableProxyHandler::call(J
     //
     // 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|.
     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));
     }
 
     RootedValue func(cx, js::GetProxyPrivate(proxy));
     return JS::Call(cx, thisVal, func, args, args.rval());
 }
 
--- a/js/xpconnect/src/XPCShellImpl.cpp
+++ b/js/xpconnect/src/XPCShellImpl.cpp
@@ -332,18 +332,20 @@ Dump(JSContext* cx, unsigned argc, Value
     return true;
 }
 
 static bool
 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;
     }
 
     RootedString str(cx);
     for (unsigned i = 0; i < args.length(); i++) {
         str = ToString(cx, args[i]);
         if (!str)
--- a/js/xpconnect/src/XPCWrappedNativeJSOps.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeJSOps.cpp
@@ -65,22 +65,19 @@ ToStringGuts(XPCCallContext& ccx)
 
 /***************************************************************************/
 
 static bool
 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())
         return Throw(NS_ERROR_XPC_BAD_OP_ON_WN_PROTO, cx);
     ccx.SetName(ccx.GetContext()->GetStringID(XPCJSContext::IDX_TO_STRING));
     ccx.SetArgsAndResultPtr(args.length(), args.array(), vp);
     return ToStringGuts(ccx);
 }
@@ -890,22 +887,19 @@ FixUpThisIfBroken(JSObject* obj, JSObjec
 
 bool
 XPC_WN_CallMethod(JSContext* cx, unsigned argc, Value* vp)
 {
     JS::CallArgs args = JS::CallArgsFromVp(argc, 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(),
                        args.array(), vp);
     XPCWrappedNative* wrapper = ccx.GetWrapper();
     THROW_AND_RETURN_IF_BAD_WRAPPER(cx, wrapper);
 
     RefPtr<XPCNativeInterface> iface;