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, a=sledru
authorJeff Walden <jwalden@mit.edu>
Tue, 18 Aug 2015 13:47:49 -0700
changeset 288861 5da240e25d30052604756ba3c49d325c4a0eb491
parent 288860 7c4f341f35f1b549a0753a50c062f92662ce81f0
child 288862 5c614f699e6b60e6864eabdc7bc5e25d40b10d06
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersefaust, sledru
bugs1178653
milestone42.0a2
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, a=sledru
js/src/builtin/Reflect.cpp
js/src/jit-test/tests/basic/testBug593559.js
js/src/jit/BaselineIC.cpp
js/src/jit/VMFunctions.cpp
js/src/jsapi.cpp
js/src/jsarray.cpp
js/src/jsfun.cpp
js/src/proxy/DirectProxyHandler.cpp
js/src/proxy/ScriptedDirectProxyHandler.cpp
js/src/proxy/ScriptedIndirectProxyHandler.cpp
js/src/tests/ecma_3/Exceptions/15.11.5.js
js/src/tests/ecma_6/extensions/new-cross-compartment.js
js/src/tests/js1_5/extensions/regress-429739.js
js/src/vm/Interpreter.cpp
js/src/vm/Interpreter.h
js/src/vm/Stack.h
--- a/js/src/builtin/Reflect.cpp
+++ b/js/src/builtin/Reflect.cpp
@@ -19,35 +19,36 @@ using namespace js;
 
 /*** Reflect methods *****************************************************************************/
 
 /*
  * ES draft rev 32 (2015 Feb 2) 7.3.17 CreateListFromArrayLike.
  * 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));
     if (!obj)
         return false;
 
     // Steps 4-5.
     uint32_t len;
     if (!GetLengthProperty(cx, obj, &len))
         return false;
 
     // Allocate space for the arguments.
     if (len > ARGS_LENGTH_MAX) {
         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.
     for (uint32_t index = 0; index < len; index++) {
         if (!GetElement(cx, obj, obj, index, (*args)[index]))
             return false;
     }
 
@@ -66,17 +67,17 @@ Reflect_apply(JSContext* cx, unsigned ar
         JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_NOT_FUNCTION,
                              "Reflect.apply argument");
         return false;
     }
 
     // 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));
 
     // Steps 4-5. This is specified to be a tail call, but isn't.
     if (!fig.invoke(cx))
         return false;
     args.rval().set(invokeArgs.rval());
@@ -103,28 +104,22 @@ Reflect_construct(JSContext* cx, unsigne
         if (!IsConstructor(newTarget)) {
             JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_NOT_CONSTRUCTOR,
                                  "Reflect.construct argument 3");
             return false;
         }
     }
 
     // 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) */
 static bool
 Reflect_defineProperty(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
--- a/js/src/jit-test/tests/basic/testBug593559.js
+++ b/js/src/jit-test/tests/basic/testBug593559.js
@@ -1,8 +1,9 @@
 var gen = (function () {yield})();
 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
@@ -9998,17 +9998,35 @@ DoCallFallback(JSContext* cx, BaselineFr
     bool handled = false;
     if (!TryAttachCallStub(cx, stub, script, pc, op, argc, vp, constructing, false,
                            createSingleton, &handled))
     {
         return false;
     }
 
     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))
     {
         if (!DirectEval(cx, CallArgsFromVp(argc, vp)))
             return false;
         res.set(vp[0]);
     } else {
--- a/js/src/jit/VMFunctions.cpp
+++ b/js/src/jit/VMFunctions.cpp
@@ -58,26 +58,47 @@ VMFunction::addToFunctions()
 
 bool
 InvokeFunction(JSContext* cx, HandleObject obj, bool constructing, uint32_t argc, Value* argv,
                MutableHandleValue rval)
 {
     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;
+        }
+
+        ConstructArgs cargs(cx);
+        if (!cargs.init(argc))
+            return false;
+
+        for (uint32_t i = 0; i < argc; i++)
+            cargs[i].set(argvWithoutThis[i]);
 
-    return Invoke(cx, thisv, ObjectValue(*obj), argc, argvWithoutThis, rval);
+        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
 InvokeFunctionShuffleNewTarget(JSContext* cx, HandleObject obj, uint32_t numActualArgs,
                                uint32_t numFormalArgs, Value* argv, MutableHandleValue rval)
 {
     MOZ_ASSERT(numFormalArgs > numActualArgs);
     argv[1 + numActualArgs] = argv[1 + numFormalArgs];
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -4628,87 +4628,77 @@ JS_PUBLIC_API(bool)
 JS::Construct(JSContext* cx, HandleValue fval, const JS::HandleValueArray& args,
               MutableHandleValue rval)
 {
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     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)
 JS::Construct(JSContext* cx, HandleValue fval, HandleObject newTarget, const JS::HandleValueArray& args,
               MutableHandleValue rval)
 {
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     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*
 JS_NewHelper(JSContext* cx, HandleObject ctor, const JS::HandleValueArray& inputArgs)
 {
     AssertHeapIsIdle(cx);
     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*)
 JS_New(JSContext* cx, HandleObject ctor, const JS::HandleValueArray& inputArgs)
 {
     RootedObject obj(cx);
     {
         AutoLastFrameCheck lfc(cx);
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -3094,23 +3094,26 @@ array_of(JSContext* cx, unsigned argc, V
         // IsArrayConstructor(this) will usually be true in practice. This is
         // the most common path.
         return ArrayFromCallArgs(cx, args);
     }
 
     // Step 4.
     RootedObject obj(cx);
     {
-        RootedValue v(cx);
-        Value argv[1] = {NumberValue(args.length())};
-        if (!InvokeConstructor(cx, args.thisv(), 1, argv, false, &v))
+        ConstructArgs cargs(cx);
+        if (!cargs.init(1))
             return false;
-        obj = ToObject(cx, v);
-        if (!obj)
+        cargs[0].setNumber(args.length());
+
+        RootedValue v(cx);
+        if (!Construct(cx, args.thisv(), cargs, args.thisv(), &v))
             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/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -1500,54 +1500,68 @@ JSFunction::maybeRelazify(JSRuntime* rt)
 bool
 js::CallOrConstructBoundFunction(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     RootedFunction fun(cx, &args.callee().as<JSFunction>());
     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;
     }
 
     /* 15.3.4.5.1 step 3, 15.3.4.5.2 step 1. */
     RootedObject target(cx, fun->getBoundFunctionTarget());
 
     /* 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));
-
-    bool constructing = args.isConstructing();
-    if (!constructing)
-        invokeArgs.setThis(boundThis);
+    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());
     return true;
 }
 
 static bool
 fun_isGenerator(JSContext* cx, unsigned argc, Value* vp)
--- a/js/src/proxy/DirectProxyHandler.cpp
+++ b/js/src/proxy/DirectProxyHandler.cpp
@@ -76,18 +76,28 @@ DirectProxyHandler::call(JSContext* cx, 
     RootedValue target(cx, proxy->as<ProxyObject>().private_());
     return Invoke(cx, args.thisv(), target, args.length(), args.array(), args.rval());
 }
 
 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
 DirectProxyHandler::nativeCall(JSContext* cx, IsAcceptableThis test, NativeImpl impl,
                                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
@@ -1047,18 +1047,22 @@ ScriptedDirectProxyHandler::construct(JS
 
     // step 4-5
     RootedValue trap(cx);
     if (!GetProperty(cx, handler, handler, cx->names().construct, &trap))
         return false;
 
     // 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
     Value constructArgv[] = {
         ObjectValue(*target),
         ObjectValue(*argsArray),
         args.newTarget()
     };
--- a/js/src/proxy/ScriptedIndirectProxyHandler.cpp
+++ b/js/src/proxy/ScriptedIndirectProxyHandler.cpp
@@ -458,21 +458,34 @@ CallableScriptedIndirectProxyHandler::ca
     MOZ_ASSERT(call.isObject() && call.toObject().isCallable());
     return Invoke(cx, args.thisv(), call, args.length(), args.array(), args.rval());
 }
 
 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;
 
 bool
 js::proxy_create(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
--- a/js/src/tests/ecma_3/Exceptions/15.11.5.js
+++ b/js/src/tests/ecma_3/Exceptions/15.11.5.js
@@ -16,20 +16,21 @@ var summary = 'Error instances have no s
 test();
 //-----------------------------------------------------------------------------
 
 function test()
 {
   enterFunc ('test');
   printStatus (summary);
 
-  var actual = '';
-  var expect = 'TypeError: Error.prototype is not a constructor';
+  var actual = { name: "no error", message: "no message" };
   try {
       new Error.prototype;
   } catch (e) {
-      actual = '' + e;
+      actual = e;
   }
 
-  reportCompare(actual, expect, "not a constructor");
+  reportCompare("TypeError", actual.name, "must be a TypeError");
+  reportCompare(true, /not a constructor/.test(actual.message),
+                "message must indicate not a constructor");
 
   exitFunc ('test');
 }
new file mode 100644
--- /dev/null
+++ 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/tests/js1_5/extensions/regress-429739.js
+++ b/js/src/tests/js1_5/extensions/regress-429739.js
@@ -1,37 +1,36 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //-----------------------------------------------------------------------------
 var BUGNUMBER = 429739;
 var summary = 'Do not assert: OBJ_GET_CLASS(cx, obj)->flags & JSCLASS_HAS_PRIVATE';
-var actual = '';
-var expect = '';
 
 //-----------------------------------------------------------------------------
 test();
 //-----------------------------------------------------------------------------
 
 function test()
 {
   enterFunc ('test');
   printBugNumber(BUGNUMBER);
   printStatus (summary);
- 
-  expect = 'TypeError: o.y is not a constructor';
 
+  var actual;
   try
   {
     var o = { __noSuchMethod__: Function }; 
     actual = (new o.y) + '';
+    throw new Error("didn't throw, produced a value");
   }
   catch(ex)
   {
-    actual = ex + '';
+    actual = ex;
   }
 
-  reportCompare(expect, actual, summary);
+  reportCompare("TypeError", actual.name, "bad error name");
+  reportCompare(true, /is not a constructor/.test(actual), summary);
 
   exitFunc ('test');
 }
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -827,70 +827,103 @@ js::Invoke(JSContext* cx, const Value& t
 
     if (!Invoke(cx, args))
         return false;
 
     rval.set(args.rval());
     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);
 
         if (!Invoke(cx, args, CONSTRUCT))
             return false;
 
         MOZ_ASSERT(args.rval().isObject());
         return true;
     }
 
     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);
 }
 
+// Check that |callee|, the callee in a |new| expression, is a constructor.
+static bool
+StackCheckIsConstructorCalleeNewTarget(JSContext* cx, HandleValue callee, HandleValue newTarget)
+{
+    // 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.setThis(MagicValue(JS_IS_CONSTRUCTING));
+    return InternalConstruct(cx, args);
+}
+
 bool
-js::InvokeConstructor(JSContext* cx, Value fval, unsigned argc, const Value* argv,
-                      bool newTargetInArgv, MutableHandleValue rval)
-{
-    InvokeArgs args(cx);
-    if (!args.init(argc, true))
-        return false;
-
+js::Construct(JSContext* cx, HandleValue fval, const ConstructArgs& args, HandleValue newTarget,
+              MutableHandleValue rval)
+{
     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);
-
-    if (!InvokeConstructor(cx, args))
+    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());
     return true;
 }
 
 bool
 js::InvokeGetter(JSContext* cx, JSObject* obj, Value fval, MutableHandleValue rval)
@@ -3024,17 +3057,17 @@ CASE(JSOP_FUNCALL)
     JSFunction* maybeFun;
     bool isFunction = IsFunctionObject(args.calleev(), &maybeFun);
 
     /* Don't bother trying to fast-path calls to scripted non-constructors. */
     if (!isFunction || !maybeFun->isInterpreted() || !maybeFun->isConstructor() ||
         (!construct && maybeFun->isClassConstructor()))
     {
         if (construct) {
-            if (!InvokeConstructor(cx, args))
+            if (!ConstructFromStack(cx, args))
                 goto error;
         } else {
             if (!Invoke(cx, args))
                 goto error;
         }
         Value* newsp = args.spAfterCall();
         TypeScript::Monitor(cx, script, REGS.pc, newsp[-1]);
         REGS.sp = newsp;
@@ -3938,17 +3971,17 @@ CASE(JSOP_CLASSHERITAGE)
 
     ReservedRooted<Value> objProto(&rootValue1);
     ReservedRooted<JSObject*> funcProto(&rootObject0);
     if (val.isNull()) {
         objProto = NullValue();
         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;
         }
 
         funcProto = &val.toObject();
 
         if (!GetProperty(cx, funcProto, funcProto, cx->names().prototype, &objProto))
             goto error;
@@ -4662,54 +4695,63 @@ js::SpreadCallOperation(JSContext* cx, H
     // The object must be an array with dense elements and no holes. Baseline's
     // optimized spread call stubs rely on this.
     MOZ_ASSERT(aobj->getDenseInitializedLength() == length);
     MOZ_ASSERT(!aobj->isIndexed());
     for (uint32_t i = 0; i < length; i++)
         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;
+
+        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))
             return false;
-        break;
-      case JSOP_SPREADCALL:
-        if (!Invoke(cx, args))
+    } else {
+        InvokeArgs args(cx);
+
+        if (!args.init(length))
             return false;
-        break;
-      case JSOP_SPREADEVAL:
-      case JSOP_STRICTSPREADEVAL:
-        if (cx->global()->valueIsEval(args.calleev())) {
-            if (!DirectEval(cx, args))
-                return false;
-        } else {
+
+        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;
 }
 
 JSObject*
 js::NewObjectOperation(JSContext* cx, HandleScript script, jsbytecode* pc,
                        NewObjectKind newKind /* = GenericObject */)
 {
--- a/js/src/vm/Interpreter.h
+++ b/js/src/vm/Interpreter.h
@@ -80,27 +80,36 @@ Invoke(JSContext* cx, const Value& thisv
  * getter/setter calls.
  */
 extern bool
 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
  * whether this is eval code or global code. To support debugging, the
  * evalFrame parameter can point to an arbitrary frame in the context's call
  * stack to simulate executing an eval in that frame.
  */
 extern bool
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -1080,34 +1080,72 @@ class InterpreterStack
         return allocator_.sizeOfExcludingThis(mallocSizeOf);
     }
 };
 
 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;
 
     static js::HashNumber hash(const Lookup& key) {
         return size_t(key.raw());
     }