Bug 1178653 - Refactor construction code to use an interface consistent with the spec, with the one exception using an out-of-the-way, differently-named method. r=efaust

--HG-- extra : rebase_source : bbd77489c8ccffe65eb951b50b494557bd30cc03
2015-07-25 02:38:10 -07:00 · 2015-07-25 02:38:10 -07:00 · 4e26d6ccea
--- a/js/src/builtin/Reflect.cpp
+++ b/js/src/builtin/Reflect.cpp
@ -24,8 +24,9 @@ using namespace js;
 * The elementTypes argument is not supported. The result list is
 * pushed to *args.
 */
+template <class InvokeArgs>
 static bool
-InitArgsFromArrayLike(JSContext* cx, HandleValue v, InvokeArgs* args, bool construct)
+InitArgsFromArrayLike(JSContext* cx, HandleValue v, InvokeArgs* args)
 {
    // Step 3.
    RootedObject obj(cx, NonNullObject(cx, v));
@ -42,7 +43,7 @@ InitArgsFromArrayLike(JSContext* cx, HandleValue v, InvokeArgs* args, bool const
        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_TOO_MANY_FUN_APPLY_ARGS);
        return false;
    }
-    if (!args->init(len, construct))
+    if (!args->init(len))
        return false;

    // Steps 6-8.
@ -71,7 +72,7 @@ Reflect_apply(JSContext* cx, unsigned argc, Value* vp)
    // Steps 2-3.
    FastInvokeGuard fig(cx, args.get(0));
    InvokeArgs& invokeArgs = fig.args();
-    if (!InitArgsFromArrayLike(cx, args.get(2), &invokeArgs, false))
+    if (!InitArgsFromArrayLike(cx, args.get(2), &invokeArgs))
        return false;
    invokeArgs.setCallee(args.get(0));
    invokeArgs.setThis(args.get(1));
@ -108,18 +109,12 @@ Reflect_construct(JSContext* cx, unsigned argc, Value* vp)
    }

    // Step 4-5.
-    InvokeArgs invokeArgs(cx);
-    if (!InitArgsFromArrayLike(cx, args.get(1), &invokeArgs, true))
+    ConstructArgs constructArgs(cx);
+    if (!InitArgsFromArrayLike(cx, args.get(1), &constructArgs))
        return false;
-    invokeArgs.setCallee(args.get(0));
-    invokeArgs.setThis(MagicValue(JS_THIS_POISON));
-    invokeArgs.newTarget().set(newTarget);

    // Step 6.
-    if (!InvokeConstructor(cx, invokeArgs))
-        return false;
-    args.rval().set(invokeArgs.rval());
-    return true;
+    return Construct(cx, args.get(0), constructArgs, newTarget, args.rval());
 }

 /* ES6 26.1.3 Reflect.defineProperty(target, propertyKey, attributes) */
--- a/js/src/jit-test/tests/basic/testBug593559.js
+++ b/js/src/jit-test/tests/basic/testBug593559.js
@ -3,6 +3,7 @@ var t = gen.throw;
 try {
    new t;
 } catch (e) {
-    actual = "" + e;
+    actual = e;
 }
-assertEq(actual, "TypeError: t is not a constructor");
+assertEq(actual.name, "TypeError");
+assertEq(/is not a constructor/.test(actual.message), true);
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@ -10003,7 +10003,25 @@ DoCallFallback(JSContext* cx, BaselineFrame* frame, ICCall_Fallback* stub_, uint
    }

    if (op == JSOP_NEW) {
-        if (!InvokeConstructor(cx, callee, argc, args, true, res))
+        // Callees from the stack could have any old non-constructor callee.
+        if (!IsConstructor(callee)) {
+            ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_IGNORE_STACK, callee, nullptr);
+            return false;
+        }
+
+        ConstructArgs cargs(cx);
+        if (!cargs.init(argc))
+            return false;
+
+        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))
            return false;
    } else if ((op == JSOP_EVAL || op == JSOP_STRICTEVAL) &&
               frame->scopeChain()->global().valueIsEval(callee))
--- a/js/src/jit/VMFunctions.cpp
+++ b/js/src/jit/VMFunctions.cpp
@ -63,16 +63,37 @@ InvokeFunction(JSContext* cx, HandleObject obj, bool constructing, uint32_t argc
    AutoArrayRooter argvRoot(cx, argc + 1 + constructing, argv);

    // Data in the argument vector is arranged for a JIT -> JIT call.
-    Value thisv = argv[0];
+    RootedValue thisv(cx, argv[0]);
    Value* argvWithoutThis = argv + 1;

-    // For constructing functions, |this| is constructed at caller side and we can just call Invoke.
-    // When creating this failed / is impossible at caller site, i.e. MagicValue(JS_IS_CONSTRUCTING),
-    // we use InvokeConstructor that creates it at the callee side.
-    if (thisv.isMagic(JS_IS_CONSTRUCTING))
-        return InvokeConstructor(cx, ObjectValue(*obj), argc, argvWithoutThis, true, rval);
+    RootedValue fval(cx, ObjectValue(*obj));
+    if (constructing) {
+        if (!IsConstructor(fval)) {
+            ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_IGNORE_STACK, fval, nullptr);
+            return false;
+        }

-    return Invoke(cx, thisv, ObjectValue(*obj), argc, argvWithoutThis, rval);
+        ConstructArgs cargs(cx);
+        if (!cargs.init(argc))
+            return false;
+
+        for (uint32_t i = 0; i < argc; i++)
+            cargs[i].set(argvWithoutThis[i]);
+
+        RootedValue newTarget(cx, argvWithoutThis[argc]);
+
+        // If |this| hasn't been created, we can use normal construction code.
+        if (thisv.isMagic(JS_IS_CONSTRUCTING))
+            return Construct(cx, fval, cargs, newTarget, rval);
+
+        // 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);
+    }
+
+    return Invoke(cx, thisv, fval, argc, argvWithoutThis, rval);
 }

 bool
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@ -4615,7 +4615,16 @@ JS::Construct(JSContext* cx, HandleValue fval, const JS::HandleValueArray& args,
    assertSameCompartment(cx, fval, args);
    AutoLastFrameCheck lfc(cx);

-    return InvokeConstructor(cx, fval, args.length(), args.begin(), false, rval);
+    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);
 }

 JS_PUBLIC_API(bool)
@ -4627,26 +4636,22 @@ JS::Construct(JSContext* cx, HandleValue fval, HandleObject newTarget, const JS:
    assertSameCompartment(cx, fval, newTarget, args);
    AutoLastFrameCheck lfc(cx);

-    // Reflect.construct ensures that the supplied new.target value is a
-    // constructor. Frankly, this makes good sense, so we reproduce the check.
-    if (!newTarget->isConstructor()) {
-        RootedValue val(cx, ObjectValue(*newTarget));
-        ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_IGNORE_STACK, val, nullptr);
+    if (!IsConstructor(fval)) {
+        ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_IGNORE_STACK, fval, nullptr);
        return false;
    }

-    // This is a littlesilly, but we need to convert from what's useful for our
-    // consumers to what we can actually handle internally.
-    AutoValueVector argv(cx);
-    unsigned argc = args.length();
-    if (!argv.reserve(argc + 1))
+    RootedValue newTargetVal(cx, ObjectValue(*newTarget));
+    if (!IsConstructor(newTargetVal)) {
+        ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_IGNORE_STACK, newTargetVal, nullptr);
        return false;
-    for (unsigned i = 0; i < argc; i++) {
-        argv.infallibleAppend(args[i]);
    }
-    argv.infallibleAppend(ObjectValue(*newTarget));

-    return InvokeConstructor(cx, fval, argc, argv.begin(), true, rval);
+    ConstructArgs cargs(cx);
+    if (!FillArgumentsFromArraylike(cx, cargs, args))
+        return false;
+
+    return js::Construct(cx, fval, cargs, newTargetVal, rval);
 }

 static JSObject*
@ -4656,36 +4661,21 @@ JS_NewHelper(JSContext* cx, HandleObject ctor, const JS::HandleValueArray& input
    CHECK_REQUEST(cx);
    assertSameCompartment(cx, ctor, inputArgs);

-    // This is not a simple variation of JS_CallFunctionValue because JSOP_NEW
-    // is not a simple variation of JSOP_CALL. We have to determine what class
-    // of object to create, create it, and clamp the return value to an object,
-    // among other details. InvokeConstructor does the hard work.
-    InvokeArgs args(cx);
-    if (!args.init(inputArgs.length(), true))
-        return nullptr;
-
-    args.setCallee(ObjectValue(*ctor));
-    args.setThis(NullValue());
-    PodCopy(args.array(), inputArgs.begin(), inputArgs.length());
-    args.newTarget().setObject(*ctor);
-
-    if (!InvokeConstructor(cx, args))
-        return nullptr;
-
-    if (!args.rval().isObject()) {
-        /*
-         * Although constructors may return primitives (via proxies), this
-         * API is asking for an object, so we report an error.
-         */
-        JSAutoByteString bytes;
-        if (ValueToPrintable(cx, args.rval(), &bytes)) {
-            JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_BAD_NEW_RESULT,
-                                 bytes.ptr());
-        }
+    RootedValue ctorVal(cx, ObjectValue(*ctor));
+    if (!IsConstructor(ctorVal)) {
+        ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_IGNORE_STACK, ctorVal, nullptr);
        return nullptr;
    }

-    return &args.rval().toObject();
+    ConstructArgs args(cx);
+    if (!FillArgumentsFromArraylike(cx, args, inputArgs))
+        return nullptr;
+
+    RootedValue rval(cx);
+    if (!js::Construct(cx, ctorVal, args, ctorVal, &rval))
+        return nullptr;
+
+    return &rval.toObject();
 }

 JS_PUBLIC_API(JSObject*)
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@ -2988,13 +2988,16 @@ array_of(JSContext* cx, unsigned argc, Value* vp)
    // Step 4.
    RootedObject obj(cx);
    {
+        ConstructArgs cargs(cx);
+        if (!cargs.init(1))
+            return false;
+        cargs[0].setNumber(args.length());
+
        RootedValue v(cx);
-        Value argv[1] = {NumberValue(args.length())};
-        if (!InvokeConstructor(cx, args.thisv(), 1, argv, false, &v))
-            return false;
-        obj = ToObject(cx, v);
-        if (!obj)
+        if (!Construct(cx, args.thisv(), cargs, args.thisv(), &v))
            return false;
+
+        obj = &v.toObject();
    }

    // Step 8.
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@ -1504,9 +1504,10 @@ js::CallOrConstructBoundFunction(JSContext* cx, unsigned argc, Value* vp)
    MOZ_ASSERT(fun->isBoundFunction());

    /* 15.3.4.5.1 step 1, 15.3.4.5.2 step 3. */
-    unsigned argslen = fun->getBoundFunctionArgumentCount();
+    unsigned boundArgsLen = fun->getBoundFunctionArgumentCount();

-    if (args.length() + argslen > ARGS_LENGTH_MAX) {
+    uint32_t argsLen = args.length();
+    if (argsLen + boundArgsLen > ARGS_LENGTH_MAX) {
        ReportAllocationOverflow(cx);
        return false;
    }
@ -1517,31 +1518,44 @@ js::CallOrConstructBoundFunction(JSContext* cx, unsigned argc, Value* vp)
    /* 15.3.4.5.1 step 2. */
    const Value& boundThis = fun->getBoundFunctionThis();

+    if (args.isConstructing()) {
+        ConstructArgs cargs(cx);
+        if (!cargs.init(argsLen + boundArgsLen))
+            return false;
+
+        /* 15.3.4.5.1, 15.3.4.5.2 step 4. */
+        for (uint32_t i = 0; i < boundArgsLen; i++)
+            cargs[i].set(fun->getBoundFunctionArgument(i));
+        for (uint32_t i = 0; i < argsLen; i++)
+            cargs[boundArgsLen + i].set(args[i]);
+
+        RootedValue targetv(cx, ObjectValue(*target));
+
+        /* ES6 9.4.1.2 step 5 */
+        RootedValue newTarget(cx);
+        if (&args.newTarget().toObject() == fun)
+            newTarget.set(targetv);
+        else
+            newTarget.set(args.newTarget());
+
+        return Construct(cx, targetv, cargs, newTarget, args.rval());
+    }
+
    InvokeArgs invokeArgs(cx);
-    if (!invokeArgs.init(args.length() + argslen, args.isConstructing()))
+    if (!invokeArgs.init(argsLen + boundArgsLen))
        return false;

    /* 15.3.4.5.1, 15.3.4.5.2 step 4. */
-    for (unsigned i = 0; i < argslen; i++)
+    for (uint32_t i = 0; i < boundArgsLen; i++)
        invokeArgs[i].set(fun->getBoundFunctionArgument(i));
-    PodCopy(invokeArgs.array() + argslen, vp + 2, args.length());
+    for (uint32_t i = 0; i < argsLen; i++)
+        invokeArgs[boundArgsLen + i].set(args[i]);

    /* 15.3.4.5.1, 15.3.4.5.2 step 5. */
    invokeArgs.setCallee(ObjectValue(*target));
+    invokeArgs.setThis(boundThis);

-    bool constructing = args.isConstructing();
-    if (!constructing)
-        invokeArgs.setThis(boundThis);
-
-    /* ES6 9.4.1.2 step 5 */
-    if (constructing) {
-        if (&args.newTarget().toObject() == fun)
-            invokeArgs.newTarget().setObject(*target);
-        else
-            invokeArgs.newTarget().set(args.newTarget());
-    }
-
-    if (constructing ? !InvokeConstructor(cx, invokeArgs) : !Invoke(cx, invokeArgs))
+    if (!Invoke(cx, invokeArgs))
        return false;

    args.rval().set(invokeArgs.rval());
--- a/js/src/proxy/DirectProxyHandler.cpp
+++ b/js/src/proxy/DirectProxyHandler.cpp
@ -81,8 +81,18 @@ bool
 DirectProxyHandler::construct(JSContext* cx, HandleObject proxy, const CallArgs& args) const
 {
    assertEnteredPolicy(cx, proxy, JSID_VOID, CALL);
+
    RootedValue target(cx, proxy->as<ProxyObject>().private_());
-    return InvokeConstructor(cx, target, args.length(), args.array(), true, args.rval());
+    if (!IsConstructor(target)) {
+        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());
 }

 bool
--- a/js/src/proxy/ScriptedDirectProxyHandler.cpp
+++ b/js/src/proxy/ScriptedDirectProxyHandler.cpp
@ -1052,8 +1052,12 @@ ScriptedDirectProxyHandler::construct(JSContext* cx, HandleObject proxy, const C

    // step 6
    if (trap.isUndefined()) {
+        ConstructArgs cargs(cx);
+        if (!FillArgumentsFromArraylike(cx, cargs, args))
+            return false;
+
        RootedValue targetv(cx, ObjectValue(*target));
-        return InvokeConstructor(cx, targetv, args.length(), args.array(), true, args.rval());
+        return Construct(cx, targetv, cargs, args.newTarget(), args.rval());
    }

    // step 8-9
--- a/js/src/proxy/ScriptedIndirectProxyHandler.cpp
+++ b/js/src/proxy/ScriptedIndirectProxyHandler.cpp
@ -463,11 +463,24 @@ bool
 CallableScriptedIndirectProxyHandler::construct(JSContext* cx, HandleObject proxy, const CallArgs& args) const
 {
    assertEnteredPolicy(cx, proxy, JSID_VOID, CALL);
+
    RootedObject ccHolder(cx, &proxy->as<ProxyObject>().extra(0).toObject());
    MOZ_ASSERT(ccHolder->getClass() == &CallConstructHolder);
+
    RootedValue construct(cx, ccHolder->as<NativeObject>().getReservedSlot(1));
-    MOZ_ASSERT(construct.isObject() && construct.toObject().isCallable());
-    return InvokeConstructor(cx, construct, args.length(), args.array(), true, args.rval());
+
+    // We could enforce this at proxy creation time, but lipstick on a pig.
+    // Plus, let's delay in-the-field bustage as long as possible.
+    if (!IsConstructor(construct)) {
+        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());
 }

 const CallableScriptedIndirectProxyHandler CallableScriptedIndirectProxyHandler::singleton;
--- a/js/src/tests/ecma_6/extensions/new-cross-compartment.js
+++ b/js/src/tests/ecma_6/extensions/new-cross-compartment.js
@ -0,0 +1,42 @@
+// Any copyright is dedicated to the Public Domain.
+// http://creativecommons.org/licenses/publicdomain/
+
+//-----------------------------------------------------------------------------
+var BUGNUMBER = 1178653;
+var summary =
+  "|new| on a cross-compartment wrapper to a non-constructor shouldn't assert";
+
+print(BUGNUMBER + ": " + summary);
+
+/**************
+ * BEGIN TEST *
+ **************/
+
+var g = newGlobal();
+
+var otherStr = new g.String("foo");
+assertEq(otherStr instanceof g.String, true);
+assertEq(otherStr.valueOf(), "foo");
+
+// THIS IS WRONG.  |new| itself should throw if !IsConstructor(constructor),
+// meaning this global's TypeError should be used.  The problem ultimately is
+// that wrappers conflate callable/constructible, so any old function from
+// another global appears to be both.  Somebody fix bug XXXXXX!
+try
+{
+  var constructor = g.parseInt;
+  new constructor();
+  throw new Error("no error thrown");
+}
+catch (e)
+{
+  assertEq(e instanceof g.TypeError, true,
+           "THIS REALLY SHOULD BE |e instanceof TypeError|");
+}
+
+/******************************************************************************/
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);
+
+print("Tests complete");
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@ -832,26 +832,23 @@ js::Invoke(JSContext* cx, const Value& thisv, const Value& fval, unsigned argc,
    return true;
 }

-bool
-js::InvokeConstructor(JSContext* cx, CallArgs args)
+static bool
+InternalConstruct(JSContext* cx, const CallArgs& args)
 {
+    MOZ_ASSERT(args.array() + args.length() + 1 == args.end(),
+               "must pass constructing arguments to a construction attempt");
    MOZ_ASSERT(!JSFunction::class_.construct);

-    args.setThis(MagicValue(JS_IS_CONSTRUCTING));
-
-    // +2 here and below to pass over |this| and |new.target|
-    if (!args.calleev().isObject())
-        return ReportIsNotFunction(cx, args.calleev(), args.length() + 2, CONSTRUCT);
-
-    MOZ_ASSERT(args.newTarget().isObject());
+    // Callers are responsible for enforcing these preconditions.
+    MOZ_ASSERT(IsConstructor(args.calleev()),
+               "trying to construct a value that isn't a constructor");
+    MOZ_ASSERT(IsConstructor(args.newTarget()),
+               "provided new.target value must be a constructor");

    JSObject& callee = args.callee();
    if (callee.is<JSFunction>()) {
        RootedFunction fun(cx, &callee.as<JSFunction>());

-        if (!fun->isConstructor())
-            return ReportIsNotFunction(cx, args.calleev(), args.length() + 2, CONSTRUCT);
-
        if (fun->isNative())
            return CallJSNativeConstructor(cx, fun->native(), args);

@ -863,29 +860,65 @@ js::InvokeConstructor(JSContext* cx, CallArgs args)
    }

    JSNative construct = callee.constructHook();
-    if (!construct)
-        return ReportIsNotFunction(cx, args.calleev(), args.length() + 2, CONSTRUCT);
+    MOZ_ASSERT(construct != nullptr, "IsConstructor without a construct hook?");

    return CallJSNativeConstructor(cx, construct, args);
 }

-bool
-js::InvokeConstructor(JSContext* cx, Value fval, unsigned argc, const Value* argv,
-                      bool newTargetInArgv, MutableHandleValue rval)
+// Check that |callee|, the callee in a |new| expression, is a constructor.
+static bool
+StackCheckIsConstructorCalleeNewTarget(JSContext* cx, HandleValue callee, HandleValue newTarget)
 {
-    InvokeArgs args(cx);
-    if (!args.init(argc, true))
+    // Calls from the stack could have any old non-constructor callee.
+    if (!IsConstructor(callee)) {
+        ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_SEARCH_STACK, callee, nullptr);
+        return false;
+    }
+
+    // The new.target for a stack construction attempt is just the callee: no
+    // need to check that it's a constructor.
+    MOZ_ASSERT(&callee.toObject() == &newTarget.toObject());
+
+    return true;
+}
+
+static bool
+ConstructFromStack(JSContext* cx, const CallArgs& args)
+{
+    if (!StackCheckIsConstructorCalleeNewTarget(cx, args.calleev(), args.newTarget()))
        return false;

-    args.setCallee(fval);
-    args.setThis(MagicValue(JS_THIS_POISON));
-    PodCopy(args.array(), argv, argc);
-    if (newTargetInArgv)
-        args.newTarget().set(argv[argc]);
-    else
-        args.newTarget().set(fval);
+    args.setThis(MagicValue(JS_IS_CONSTRUCTING));
+    return InternalConstruct(cx, args);
+}

-    if (!InvokeConstructor(cx, args))
+bool
+js::Construct(JSContext* cx, HandleValue fval, const ConstructArgs& args, HandleValue newTarget,
+              MutableHandleValue rval)
+{
+    args.setCallee(fval);
+    args.setThis(MagicValue(JS_IS_CONSTRUCTING));
+    args.newTarget().set(newTarget);
+    if (!InternalConstruct(cx, args))
+        return false;
+
+    rval.set(args.rval());
+    return true;
+}
+
+bool
+js::InternalConstructWithProvidedThis(JSContext* cx, HandleValue fval, HandleValue thisv,
+                                      const ConstructArgs& args, HandleValue newTarget,
+                                      MutableHandleValue rval)
+{
+    args.setCallee(fval);
+
+    MOZ_ASSERT(thisv.isObject());
+    args.setThis(thisv);
+
+    args.newTarget().set(newTarget);
+
+    if (!InternalConstruct(cx, args))
        return false;

    rval.set(args.rval());
@ -3029,7 +3062,7 @@ CASE(JSOP_FUNCALL)
        (!construct && maybeFun->isClassConstructor()))
    {
        if (construct) {
-            if (!InvokeConstructor(cx, args))
+            if (!ConstructFromStack(cx, args))
                goto error;
        } else {
            if (!Invoke(cx, args))
@ -3943,7 +3976,7 @@ CASE(JSOP_CLASSHERITAGE)
        if (!GetBuiltinPrototype(cx, JSProto_Function, &funcProto))
            goto error;
    } else {
-        if (!val.isObject() || !val.toObject().isConstructor()) {
+        if (!IsConstructor(val)) {
            ReportIsNotFunction(cx, val, 0, CONSTRUCT);
            goto error;
        }
@ -4667,44 +4700,53 @@ js::SpreadCallOperation(JSContext* cx, HandleScript script, jsbytecode* pc, Hand
        MOZ_ASSERT(!aobj->getDenseElement(i).isMagic());
 #endif

-    InvokeArgs args(cx);
-
-    if (!args.init(length, constructing))
-        return false;
-
-    args.setCallee(callee);
-    args.setThis(thisv);
-
-    if (!GetElements(cx, aobj, length, args.array()))
-        return false;
-
-    if (constructing)
-        args.newTarget().set(newTarget);
-
-    switch (op) {
-      case JSOP_SPREADNEW:
-        if (!InvokeConstructor(cx, args))
+    if (op == JSOP_SPREADNEW) {
+        if (!StackCheckIsConstructorCalleeNewTarget(cx, callee, newTarget))
            return false;
-        break;
-      case JSOP_SPREADCALL:
-        if (!Invoke(cx, args))
+
+        ConstructArgs cargs(cx);
+        if (!cargs.init(length))
            return false;
-        break;
-      case JSOP_SPREADEVAL:
-      case JSOP_STRICTSPREADEVAL:
-        if (cx->global()->valueIsEval(args.calleev())) {
-            if (!DirectEval(cx, args))
-                return false;
-        } else {
+
+        if (!GetElements(cx, aobj, length, cargs.array()))
+            return false;
+
+        if (!Construct(cx, callee, cargs, newTarget, res))
+            return false;
+    } else {
+        InvokeArgs args(cx);
+
+        if (!args.init(length))
+            return false;
+
+        args.setCallee(callee);
+        args.setThis(thisv);
+
+        if (!GetElements(cx, aobj, length, args.array()))
+            return false;
+
+        switch (op) {
+          case JSOP_SPREADCALL:
            if (!Invoke(cx, args))
                return false;
+            break;
+          case JSOP_SPREADEVAL:
+          case JSOP_STRICTSPREADEVAL:
+            if (cx->global()->valueIsEval(args.calleev())) {
+                if (!DirectEval(cx, args))
+                    return false;
+            } else {
+                if (!Invoke(cx, args))
+                    return false;
+            }
+            break;
+          default:
+            MOZ_CRASH("bad spread opcode");
        }
-        break;
-      default:
-        MOZ_CRASH("bad spread opcode");
+
+        res.set(args.rval());
    }

-    res.set(args.rval());
    TypeScript::Monitor(cx, script, pc, res);
    return true;
 }
--- a/js/src/vm/Interpreter.h
+++ b/js/src/vm/Interpreter.h
@ -85,17 +85,26 @@ InvokeGetter(JSContext* cx, JSObject* obj, Value fval, MutableHandleValue rval);
 extern bool
 InvokeSetter(JSContext* cx, const Value& thisv, Value fval, HandleValue v);

-/*
- * InvokeConstructor implement a function call from a constructor context
- * (e.g. 'new') handling the the creation of the new 'this' object.
- */
+// 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
-InvokeConstructor(JSContext* cx, CallArgs args);
+Construct(JSContext* cx, HandleValue fval, const ConstructArgs& args, HandleValue newTarget,
+          MutableHandleValue rval);

-/* See the fval overload of Invoke. */
+// 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
-InvokeConstructor(JSContext* cx, Value fval, unsigned argc, const Value* argv,
-                  bool newTargetInArgv, MutableHandleValue rval);
+InternalConstructWithProvidedThis(JSContext* cx, HandleValue fval, HandleValue thisv,
+                                  const ConstructArgs& args, HandleValue newTarget,
+                                  MutableHandleValue rval);

 /*
 * Executes a script with the given scopeChain/this. The 'type' indicates
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@ -1083,24 +1083,62 @@ void MarkInterpreterActivations(JSRuntime* rt, JSTracer* trc);

 /*****************************************************************************/

-class InvokeArgs : public JS::CallArgs
+namespace detail {
+
+class GenericInvokeArgs : public JS::CallArgs
 {
+  protected:
    AutoValueVector v_;

-  public:
-    explicit InvokeArgs(JSContext* cx, bool construct = false) : v_(cx) {}
+    explicit GenericInvokeArgs(JSContext* cx) : v_(cx) {}

-    bool init(unsigned argc, bool construct = false) {
+    bool init(unsigned argc, bool construct) {
        MOZ_ASSERT(2 + argc + construct > argc);  // no overflow
        if (!v_.resize(2 + argc + construct))
            return false;
-        ImplicitCast<CallArgs>(*this) = CallArgsFromVp(argc, v_.begin());
-        // Set the internal flag, since we are not initializing from a made array
+
+        *static_cast<JS::CallArgs*>(this) = CallArgsFromVp(argc, v_.begin());
        constructing_ = construct;
        return true;
    }
 };

+} // namespace detail
+
+class InvokeArgs : public detail::GenericInvokeArgs
+{
+  public:
+    explicit InvokeArgs(JSContext* cx) : detail::GenericInvokeArgs(cx) {}
+
+    bool init(unsigned argc) {
+        return detail::GenericInvokeArgs::init(argc, false);
+    }
+};
+
+class ConstructArgs : public detail::GenericInvokeArgs
+{
+  public:
+    explicit ConstructArgs(JSContext* cx) : detail::GenericInvokeArgs(cx) {}
+
+    bool init(unsigned argc) {
+        return detail::GenericInvokeArgs::init(argc, true);
+    }
+};
+
+template <class Args, class Arraylike>
+inline bool
+FillArgumentsFromArraylike(JSContext* cx, Args& args, const Arraylike& arraylike)
+{
+    uint32_t len = arraylike.length();
+    if (!args.init(len))
+        return false;
+
+    for (uint32_t i = 0; i < len; i++)
+        args[i].set(arraylike[i]);
+
+    return true;
+}
+
 template <>
 struct DefaultHasher<AbstractFramePtr> {
    typedef AbstractFramePtr Lookup;