Bug 1212533 - Change the out-param of js/JS::Construct from MutableHandleValue to MutableHandleObject. r=arai, r=jorendorff
☠☠ backed out by f3b54b8458bf ☠ ☠
authorsimplyblue <bmanojkumar24@gmail.com>
Wed, 28 Oct 2015 16:38:27 +0530
changeset 281825 66d4205c2958d3bf81432988b7f385d961cdf3fc
parent 281824 30f1acd9387f270279d71da436bba1d0aedcc617
child 281826 1d205c7574e0cef5b0d35f7da0a58c4a28d4e245
push id19510
push usergwagner@mozilla.com
push dateMon, 08 Feb 2016 15:56:48 +0000
treeherderb2g-inbound@a3d54e32dee1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersarai, jorendorff
bugs1212533
milestone47.0a1
Bug 1212533 - Change the out-param of js/JS::Construct from MutableHandleValue to MutableHandleObject. r=arai, r=jorendorff
js/ipc/WrapperAnswer.cpp
js/src/builtin/Reflect.cpp
js/src/jit/BaselineIC.cpp
js/src/jit/VMFunctions.cpp
js/src/jsapi-tests/testDifferentNewTargetInvokeConstructor.cpp
js/src/jsapi-tests/testNewTargetInvokeConstructor.cpp
js/src/jsapi.cpp
js/src/jsapi.h
js/src/jsarray.cpp
js/src/proxy/DirectProxyHandler.cpp
js/src/proxy/ScriptedDirectProxyHandler.cpp
js/src/proxy/ScriptedIndirectProxyHandler.cpp
js/src/vm/Interpreter.cpp
js/src/vm/Interpreter.h
js/src/vm/SelfHosting.cpp
js/src/vm/TypedArrayObject.cpp
js/xpconnect/src/ExportHelpers.cpp
--- a/js/ipc/WrapperAnswer.cpp
+++ b/js/ipc/WrapperAnswer.cpp
@@ -420,23 +420,25 @@ WrapperAnswer::RecvCallOrConstruct(const
     }
 
     RootedValue rval(cx);
     {
         AutoSaveContextOptions asco(cx);
         ContextOptionsRef(cx).setDontReportUncaught(true);
 
         HandleValueArray args = HandleValueArray::subarray(vals, 2, vals.length() - 2);
-        bool success;
-        if (construct)
-            success = JS::Construct(cx, vals[0], args, &rval);
-        else
-            success = JS::Call(cx, vals[1], vals[0], args, &rval);
-        if (!success)
-            return fail(aes, rs);
+        if (construct) {
+            RootedObject obj(cx);
+            if (!JS::Construct(cx, vals[0], args, &obj))
+                return fail(aes, rs);
+            rval.setObject(*obj);
+        } else {
+            if(!JS::Call(cx, vals[1], vals[0], args, &rval))
+                return fail(aes, rs);
+        }
     }
 
     if (!toVariant(cx, rval, result))
         return fail(aes, rs);
 
     // Prefill everything with a dummy jsval.
     for (size_t i = 0; i < outobjects.length(); i++)
         outparams->AppendElement(JSParam(void_t()));
--- a/js/src/builtin/Reflect.cpp
+++ b/js/src/builtin/Reflect.cpp
@@ -109,17 +109,22 @@ Reflect_construct(JSContext* cx, unsigne
     }
 
     // Step 4-5.
     ConstructArgs constructArgs(cx);
     if (!InitArgsFromArrayLike(cx, args.get(1), &constructArgs))
         return false;
 
     // Step 6.
-    return Construct(cx, args.get(0), constructArgs, newTarget, args.rval());
+    RootedObject obj(cx);
+    if (!Construct(cx, args.get(0), constructArgs, newTarget, &obj))
+        return false;
+
+    args.rval().setObject(*obj);
+    return true;
 }
 
 /* ES6 26.1.3 Reflect.defineProperty(target, propertyKey, attributes) */
 static bool
 Reflect_defineProperty(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@@ -6085,18 +6085,22 @@ DoCallFallback(JSContext* cx, BaselineFr
         for (uint32_t i = 0; i < argc; i++)
             cargs[i].set(args[i]);
 
         RootedValue newTarget(cx, args[argc]);
         MOZ_ASSERT(IsConstructor(newTarget),
                    "either callee == newTarget, or the initial |new| checked "
                    "that IsConstructor(newTarget)");
 
-        if (!Construct(cx, callee, cargs, newTarget, res))
+        RootedObject obj(cx);
+        if (!Construct(cx, callee, cargs, newTarget, &obj))
             return false;
+
+        res.setObject(*obj);
+
     } else if ((op == JSOP_EVAL || op == JSOP_STRICTEVAL) &&
                frame->scopeChain()->global().valueIsEval(callee))
     {
         if (!DirectEval(cx, CallArgsFromVp(argc, vp)))
             return false;
         res.set(vp[0]);
     } else {
         MOZ_ASSERT(op == JSOP_CALL ||
--- a/js/src/jit/VMFunctions.cpp
+++ b/js/src/jit/VMFunctions.cpp
@@ -78,17 +78,23 @@ InvokeFunction(JSContext* cx, HandleObje
         RootedValue newTarget(cx, argvWithoutThis[argc]);
 
         // If |this| hasn't been created, or is JS_UNINITIALIZED_LEXICAL,
         // we can use normal construction code without creating an extraneous
         // object.
         if (thisv.isMagic()) {
             MOZ_ASSERT(thisv.whyMagic() == JS_IS_CONSTRUCTING ||
                        thisv.whyMagic() == JS_UNINITIALIZED_LEXICAL);
-            return Construct(cx, fval, cargs, newTarget, rval);
+
+            RootedObject obj(cx);
+            if (!Construct(cx, fval, cargs, newTarget, &obj))
+                return false;
+
+            rval.setObject(*obj);
+            return true;
         }
 
         // Otherwise the default |this| has already been created.  We could
         // almost perform a *call* at this point, but we'd break |new.target|
         // in the function.  So in this one weird case we call a one-off
         // construction path that *won't* set |this| to JS_IS_CONSTRUCTING.
         return InternalConstructWithProvidedThis(cx, fval, thisv, cargs, newTarget, rval);
     }
--- a/js/src/jsapi-tests/testDifferentNewTargetInvokeConstructor.cpp
+++ b/js/src/jsapi-tests/testDifferentNewTargetInvokeConstructor.cpp
@@ -14,24 +14,24 @@ BEGIN_TEST(testDifferentNewTargetInvokeC
     EVAL("(function() { /* This is a different new.target function */ })", &otherFunc);
 
     EVAL("(function(expected) { if (expected !== new.target) throw new Error('whoops'); })",
          &func);
 
     JS::AutoValueArray<1> args(cx);
     args[0].set(otherFunc);
 
-    JS::RootedValue rval(cx);
+    JS::RootedObject obj(cx);
 
     JS::RootedObject newTarget(cx, &otherFunc.toObject());
 
-    CHECK(JS::Construct(cx, func, newTarget, args, &rval));
+    CHECK(JS::Construct(cx, func, newTarget, args, &obj));
 
     // It should fail, though, if newTarget is not a constructor
     JS::RootedValue plain(cx);
     EVAL("({})", &plain);
     args[0].set(plain);
     newTarget = &plain.toObject();
-    CHECK(!JS::Construct(cx, func, newTarget, args, &rval));
+    CHECK(!JS::Construct(cx, func, newTarget, args, &obj));
 
     return true;
 }
 END_TEST(testDifferentNewTargetInvokeConstructor)
--- a/js/src/jsapi-tests/testNewTargetInvokeConstructor.cpp
+++ b/js/src/jsapi-tests/testNewTargetInvokeConstructor.cpp
@@ -11,15 +11,15 @@ BEGIN_TEST(testNewTargetInvokeConstructo
     JS::RootedValue func(cx);
 
     EVAL("(function(expected) { if (expected !== new.target) throw new Error('whoops'); })",
          &func);
 
     JS::AutoValueArray<1> args(cx);
     args[0].set(func);
 
-    JS::RootedValue rval(cx);
+    JS::RootedObject obj(cx);
 
-    CHECK(JS::Construct(cx, func, args, &rval));
+    CHECK(JS::Construct(cx, func, args, &obj));
 
     return true;
 }
 END_TEST(testNewTargetInvokeConstructor)
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -2879,17 +2879,17 @@ JS::Call(JSContext* cx, HandleValue this
     assertSameCompartment(cx, thisv, fval, args);
     AutoLastFrameCheck lfc(cx);
 
     return Invoke(cx, thisv, fval, args.length(), args.begin(), rval);
 }
 
 JS_PUBLIC_API(bool)
 JS::Construct(JSContext* cx, HandleValue fval, HandleObject newTarget, const JS::HandleValueArray& args,
-              MutableHandleValue rval)
+              MutableHandleObject objp)
 {
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, fval, newTarget, args);
     AutoLastFrameCheck lfc(cx);
 
     if (!IsConstructor(fval)) {
         ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_IGNORE_STACK, fval, nullptr);
@@ -2901,38 +2901,38 @@ JS::Construct(JSContext* cx, HandleValue
         ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_IGNORE_STACK, newTargetVal, nullptr);
         return false;
     }
 
     ConstructArgs cargs(cx);
     if (!FillArgumentsFromArraylike(cx, cargs, args))
         return false;
 
-    return js::Construct(cx, fval, cargs, newTargetVal, rval);
+    return js::Construct(cx, fval, cargs, newTargetVal, objp);
 }
 
 JS_PUBLIC_API(bool)
 JS::Construct(JSContext* cx, HandleValue fval, const JS::HandleValueArray& args,
-              MutableHandleValue rval)
+              MutableHandleObject objp)
 {
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, fval, args);
     AutoLastFrameCheck lfc(cx);
 
     if (!IsConstructor(fval)) {
         ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_IGNORE_STACK, fval, nullptr);
         return false;
     }
 
     ConstructArgs cargs(cx);
     if (!FillArgumentsFromArraylike(cx, cargs, args))
         return false;
 
-    return js::Construct(cx, fval, cargs, fval, rval);
+    return js::Construct(cx, fval, cargs, fval, objp);
 }
 
 
 /* * */
 
 JS_PUBLIC_API(bool)
 JS_AlreadyHasOwnPropertyById(JSContext* cx, HandleObject obj, HandleId id, bool* foundp)
 {
@@ -4572,21 +4572,21 @@ JS_NewHelper(JSContext* cx, HandleObject
         ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_IGNORE_STACK, ctorVal, nullptr);
         return nullptr;
     }
 
     ConstructArgs args(cx);
     if (!FillArgumentsFromArraylike(cx, args, inputArgs))
         return nullptr;
 
-    RootedValue rval(cx);
-    if (!js::Construct(cx, ctorVal, args, ctorVal, &rval))
+    RootedObject obj(cx);
+    if (!js::Construct(cx, ctorVal, args, ctorVal, &obj))
         return nullptr;
 
-    return &rval.toObject();
+    return obj;
 }
 
 JS_PUBLIC_API(JSObject*)
 JS_New(JSContext* cx, HandleObject ctor, const JS::HandleValueArray& inputArgs)
 {
     RootedObject obj(cx);
     {
         AutoLastFrameCheck lfc(cx);
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -3320,31 +3320,28 @@ Call(JSContext* cx, JS::HandleValue this
  * implementing a subclass or a proxy handler's construct() method, this is the
  * right function to call.)
  *
  * Implements: ES6 7.3.13 Construct(F, [argumentsList], [newTarget]).
  * Use this function to invoke the [[Construct]] internal method.
  */
 extern JS_PUBLIC_API(bool)
 Construct(JSContext* cx, JS::HandleValue fun, HandleObject newTarget,
-          const JS::HandleValueArray &args, MutableHandleValue rval);
+          const JS::HandleValueArray &args, MutableHandleObject objp);
 
 /**
  * Invoke a constructor. This is the C++ equivalent of
  * `rval = new fun(...args)`.
  *
- * The value left in rval on success is always an object in practice,
- * though at the moment this is not enforced by the C++ type system.
- *
  * Implements: ES6 7.3.13 Construct(F, [argumentsList], [newTarget]), when
  * newTarget is omitted.
  */
 extern JS_PUBLIC_API(bool)
 Construct(JSContext* cx, JS::HandleValue fun, const JS::HandleValueArray& args,
-          MutableHandleValue rval);
+          MutableHandleObject objp);
 
 } /* namespace JS */
 
 /**
  * Invoke a constructor, like the JS expression `new ctor(...args)`. Returns
  * the new object, or null on error.
  */
 extern JS_PUBLIC_API(JSObject*)
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -3090,21 +3090,18 @@ array_of(JSContext* cx, unsigned argc, V
     // Step 4.
     RootedObject obj(cx);
     {
         ConstructArgs cargs(cx);
         if (!cargs.init(1))
             return false;
         cargs[0].setNumber(args.length());
 
-        RootedValue v(cx);
-        if (!Construct(cx, args.thisv(), cargs, args.thisv(), &v))
+        if (!Construct(cx, args.thisv(), cargs, args.thisv(), &obj))
             return false;
-
-        obj = &v.toObject();
     }
 
     // Step 8.
     for (unsigned k = 0; k < args.length(); k++) {
         if (!DefineElement(cx, obj, k, args[k]))
             return false;
     }
 
--- a/js/src/proxy/DirectProxyHandler.cpp
+++ b/js/src/proxy/DirectProxyHandler.cpp
@@ -87,17 +87,22 @@ DirectProxyHandler::construct(JSContext*
         ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_IGNORE_STACK, target, nullptr);
         return false;
     }
 
     ConstructArgs cargs(cx);
     if (!FillArgumentsFromArraylike(cx, cargs, args))
         return false;
 
-    return Construct(cx, target, cargs, args.newTarget(), args.rval());
+    RootedObject obj(cx);
+    if (!Construct(cx, target, cargs, args.newTarget(), &obj))
+        return false;
+
+    args.rval().setObject(*obj);
+    return true;
 }
 
 bool
 DirectProxyHandler::nativeCall(JSContext* cx, IsAcceptableThis test, NativeImpl impl,
                                const CallArgs& args) const
 {
     args.setThis(ObjectValue(*args.thisv().toObject().as<ProxyObject>().target()));
     if (!test(args.thisv())) {
--- a/js/src/proxy/ScriptedDirectProxyHandler.cpp
+++ b/js/src/proxy/ScriptedDirectProxyHandler.cpp
@@ -1045,17 +1045,22 @@ ScriptedDirectProxyHandler::construct(JS
 
     // step 6
     if (trap.isUndefined()) {
         ConstructArgs cargs(cx);
         if (!FillArgumentsFromArraylike(cx, cargs, args))
             return false;
 
         RootedValue targetv(cx, ObjectValue(*target));
-        return Construct(cx, targetv, cargs, args.newTarget(), args.rval());
+        RootedObject obj(cx);
+        if (!Construct(cx, targetv, cargs, args.newTarget(), &obj))
+            return false;
+
+        args.rval().setObject(*obj);
+        return true;
     }
 
     // step 8-9
     Value constructArgv[] = {
         ObjectValue(*target),
         ObjectValue(*argsArray),
         args.newTarget()
     };
--- a/js/src/proxy/ScriptedIndirectProxyHandler.cpp
+++ b/js/src/proxy/ScriptedIndirectProxyHandler.cpp
@@ -475,17 +475,22 @@ CallableScriptedIndirectProxyHandler::co
         ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_IGNORE_STACK, construct, nullptr);
         return false;
     }
 
     ConstructArgs cargs(cx);
     if (!FillArgumentsFromArraylike(cx, cargs, args))
         return false;
 
-    return Construct(cx, construct, cargs, args.newTarget(), args.rval());
+    RootedObject obj(cx);
+    if (!Construct(cx, construct, cargs, args.newTarget(), &obj))
+        return false;
+
+    args.rval().setObject(*obj);
+    return true;
 }
 
 const CallableScriptedIndirectProxyHandler CallableScriptedIndirectProxyHandler::singleton;
 
 bool
 js::proxy_create(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -588,25 +588,26 @@ ConstructFromStack(JSContext* cx, const 
         return false;
 
     args.setThis(MagicValue(JS_IS_CONSTRUCTING));
     return InternalConstruct(cx, args);
 }
 
 bool
 js::Construct(JSContext* cx, HandleValue fval, const ConstructArgs& args, HandleValue newTarget,
-              MutableHandleValue rval)
+              MutableHandleObject objp)
 {
     args.setCallee(fval);
     args.setThis(MagicValue(JS_IS_CONSTRUCTING));
     args.newTarget().set(newTarget);
     if (!InternalConstruct(cx, args))
         return false;
 
-    rval.set(args.rval());
+    MOZ_ASSERT(args.rval().isObject());
+    objp.set(&args.rval().toObject());
     return true;
 }
 
 bool
 js::InternalConstructWithProvidedThis(JSContext* cx, HandleValue fval, HandleValue thisv,
                                       const ConstructArgs& args, HandleValue newTarget,
                                       MutableHandleValue rval)
 {
@@ -4572,18 +4573,20 @@ js::SpreadCallOperation(JSContext* cx, H
 
         ConstructArgs cargs(cx);
         if (!cargs.init(length))
             return false;
 
         if (!GetElements(cx, aobj, length, cargs.array()))
             return false;
 
-        if (!Construct(cx, callee, cargs, newTarget, res))
+        RootedObject obj(cx);
+        if (!Construct(cx, callee, cargs, newTarget, &obj))
             return false;
+        res.setObject(*obj);
     } else {
         InvokeArgs args(cx);
 
         if (!args.init(length))
             return false;
 
         args.setCallee(callee);
         args.setThis(thisv);
--- a/js/src/vm/Interpreter.h
+++ b/js/src/vm/Interpreter.h
@@ -82,17 +82,17 @@ InvokeSetter(JSContext* cx, const Value&
 // ES6 7.3.13 Construct(F, argumentsList, newTarget).  All parameters are
 // required, hopefully forcing callers to be careful not to (say) blindly pass
 // callee as |newTarget| when a different value should have been passed.
 //
 // NOTE: As with the ES6 spec operation, it's the caller's responsibility to
 //       ensure |fval| and |newTarget| are both |IsConstructor|.
 extern bool
 Construct(JSContext* cx, HandleValue fval, const ConstructArgs& args, HandleValue newTarget,
-          MutableHandleValue rval);
+          MutableHandleObject objp);
 
 // Call Construct(fval, args, newTarget), but use the given |thisv| as |this|
 // during construction of |fval|.
 //
 // This method exists only for very rare cases where a |this| was created
 // caller-side for construction of |fval|: basically only for JITs using
 // |CreateThis|.  If that's not you, use Construct()!
 extern bool
@@ -474,16 +474,13 @@ ReportRuntimeRedeclaration(JSContext* cx
 
 bool
 ThrowUninitializedThis(JSContext* cx, AbstractFramePtr frame);
 
 bool
 DefaultClassConstructor(JSContext* cx, unsigned argc, Value* vp);
 
 bool
-DefaultDerivedClassConstructor(JSContext* cx, unsigned argc, Value* vp);
-
-bool
 Debug_CheckSelfHosted(JSContext* cx, HandleValue v);
 
 }  /* namespace js */
 
 #endif /* vm_Interpreter_h */
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -1371,17 +1371,22 @@ intrinsic_ConstructFunction(JSContext* c
     RootedArrayObject argsList(cx, &args[1].toObject().as<ArrayObject>());
     uint32_t len = argsList->length();
     ConstructArgs constructArgs(cx);
     if (!constructArgs.init(len))
         return false;
     for (uint32_t index = 0; index < len; index++)
         constructArgs[index].set(argsList->getDenseElement(index));
 
-    return Construct(cx, args[0], constructArgs, args.rval());
+    RootedObject res(cx);
+    if (!Construct(cx, args[0], constructArgs, args[0], &res))
+        return false;
+
+    args.rval().setObject(*res);
+    return true;
 }
 
 
 static bool
 intrinsic_IsConstructing(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     MOZ_ASSERT(args.length() == 0);
--- a/js/src/vm/TypedArrayObject.cpp
+++ b/js/src/vm/TypedArrayObject.cpp
@@ -2517,14 +2517,13 @@ JS_NewDataView(JSContext* cx, HandleObje
     if (!GetBuiltinConstructor(cx, key, &constructor))
         return nullptr;
 
     cargs[0].setObject(*arrayBuffer);
     cargs[1].setNumber(byteOffset);
     cargs[2].setInt32(byteLength);
 
     RootedValue fun(cx, ObjectValue(*constructor));
-    RootedValue rval(cx);
-    if (!Construct(cx, fun, cargs, fun, &rval))
+    RootedObject obj(cx);
+    if (!Construct(cx, fun, cargs, fun, &obj))
         return nullptr;
-    MOZ_ASSERT(rval.isObject());
-    return &rval.toObject();
+    return obj;
 }
--- a/js/xpconnect/src/ExportHelpers.cpp
+++ b/js/xpconnect/src/ExportHelpers.cpp
@@ -340,18 +340,20 @@ FunctionForwarder(JSContext* cx, unsigne
 
         for (size_t n = 0;  n < args.length(); ++n) {
             if (!CheckSameOriginArg(cx, options, args[n]) || !JS_WrapValue(cx, args[n]))
                 return false;
         }
 
         RootedValue fval(cx, ObjectValue(*unwrappedFun));
         if (args.isConstructing()) {
-            if (!JS::Construct(cx, fval, args, args.rval()))
+            RootedObject obj(cx);
+            if (!JS::Construct(cx, fval, args, &obj))
                 return false;
+            args.rval().setObject(*obj);
         } else {
             if (!JS_CallFunctionValue(cx, thisObj, fval, args, args.rval()))
                 return false;
         }
     }
 
     // Rewrap the return value into our compartment.
     return JS_WrapValue(cx, args.rval());