Bug 1103344 - Object.assign should stop when an exception is thrown. r=till
authorTom Schuster <evilpies@gmail.com>
Tue, 24 Mar 2015 22:37:35 +0100
changeset 265750 c279eefced07397e78484b0c631d8533abffde7a
parent 265749 ac30c147653ab95fe615a7cfc4464c7d3d822b88
child 265751 c5736c8981543df372213f51cfb7ae6f40232864
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstill
bugs1103344
milestone39.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 1103344 - Object.assign should stop when an exception is thrown. r=till
js/src/builtin/Object.cpp
js/src/builtin/Object.h
js/src/builtin/Object.js
js/src/tests/ecma_6/Object/assign.js
js/src/vm/SelfHosting.cpp
--- a/js/src/builtin/Object.cpp
+++ b/js/src/builtin/Object.cpp
@@ -42,18 +42,18 @@ js::obj_construct(JSContext *cx, unsigne
             return false;
     }
 
     args.rval().setObject(*obj);
     return true;
 }
 
 /* ES5 15.2.4.7. */
-static bool
-obj_propertyIsEnumerable(JSContext *cx, unsigned argc, Value *vp)
+bool
+js::obj_propertyIsEnumerable(JSContext *cx, unsigned argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
     HandleValue idValue = args.get(0);
 
     // As an optimization, provide a fast path when rooting is not necessary and
     // we can safely retrieve the attributes from the object's shape.
 
--- a/js/src/builtin/Object.h
+++ b/js/src/builtin/Object.h
@@ -18,16 +18,19 @@ class Value;
 
 namespace js {
 
 // Object constructor native. Exposed only so the JIT can know its address.
 bool
 obj_construct(JSContext *cx, unsigned argc, JS::Value *vp);
 
 bool
+obj_propertyIsEnumerable(JSContext *cx, unsigned argc, Value *vp);
+
+bool
 obj_valueOf(JSContext *cx, unsigned argc, JS::Value *vp);
 
 PlainObject *
 ObjectCreateImpl(JSContext *cx, HandleObject proto, NewObjectKind newKind = GenericObject,
                  HandleObjectGroup group = js::NullPtr());
 
 PlainObject *
 ObjectCreateWithTemplate(JSContext *cx, HandlePlainObject templateObj);
--- a/js/src/builtin/Object.js
+++ b/js/src/builtin/Object.js
@@ -1,75 +1,46 @@
 /* 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/. */
 
-/* ES6 draft 2014-07-18 19.1.2.1. */
+// ES6 draft rev36 2015-03-17 19.1.2.1
 function ObjectStaticAssign(target, firstSource) {
     // Steps 1-2.
     var to = ToObject(target);
 
     // Step 3.
     if (arguments.length < 2)
         return to;
 
-    // Step 4.
-    var i = 1;
-    do {
-        // Step 5.a-b, plus an unspecified flourish to skip null/undefined, so
-        // any site depending on agreed-upon (but not-yet-drafted) semantics
-        // from TC39 meeting minutes will work. (Yes, implausibly, such a site
-        // exists. See bug 1054426.)
+    // Steps 4-5.
+    for (var i = 1; i < arguments.length; i++) {
+        // Step 5.a.
         var nextSource = arguments[i];
         if (nextSource === null || nextSource === undefined)
             continue;
 
+        // Steps 5.b.i-ii.
         var from = ToObject(nextSource);
 
-        // Step 5.c-d.
-        var keysArray = OwnPropertyKeys(from);
-
-        // Steps 5.e-f.
-        var len = keysArray.length;
-
-        // Step 5.h.
-        var nextIndex = 0;
+        // Steps 5.b.iii-iv.
+        var keys = OwnPropertyKeys(from);
 
-        // Step 5.i (Modified a bit because we can't catch and store the
-        // actual Completion Record). Instead we have a marker object.
-        const MISSING = {};
-        var pendingException = MISSING;
-
-        // Step 5.j.
-        while (nextIndex < len) {
-            // Step 5.j.i-ii.
-            var nextKey = keysArray[nextIndex];
+        // Step 5.c.
+        for (var nextIndex = 0, len = keys.length; nextIndex < len; nextIndex++) {
+            var nextKey = keys[nextIndex];
 
-            // Step 5.j.iii-v.
-            try {
-                // We'd like to use Object.propertyIsEnumerable here, but we
-                // can't, because if a property doesn't exist, it won't properly
-                // call getOwnPropertyDescriptor (important if |from| is a
-                // proxy).
-                var desc = std_Object_getOwnPropertyDescriptor(from, nextKey);
-                if (desc !== undefined && desc.enumerable)
-                    to[nextKey] = from[nextKey];
-            } catch (e) {
-                if (pendingException === MISSING)
-                    pendingException = e;
+            // Steps 5.c.i-iii. We abbreviate this by calling propertyIsEnumerable
+            // which is faster and returns false for not defined properties.
+            if (callFunction(std_Object_propertyIsEnumerable, from, nextKey)) {
+                // Steps 5.c.iii.1-4.
+                to[nextKey] = from[nextKey];
             }
-
-            // Step 5.j.vi.
-            nextIndex++;
         }
-
-        // Step 5.k.
-        if (pendingException !== MISSING)
-            throw pendingException;
-    } while (++i < arguments.length);
+    }
 
     // Step 6.
     return to;
 }
 
 function ObjectDefineSetter(name, setter) {
     var object;
     if (this === null || this === undefined)
--- a/js/src/tests/ecma_6/Object/assign.js
+++ b/js/src/tests/ecma_6/Object/assign.js
@@ -279,80 +279,25 @@ testDeletedAndRecreatedPropertiesCopied2
 function testStringAndSymbolPropertiesCopied() {
     var keyA = "str-prop";
     var source = {"str-prop": 1};
     var target = Object.assign({}, source);
     checkDataProperty(target, keyA, 1, true, true, true);
 }
 testStringAndSymbolPropertiesCopied();
 
-// Intermediate exceptions do not stop property traversal, first exception is reported (1)
+// Intermediate exceptions stop traversal and throw exception
 function testExceptionsDoNotStopFirstReported1() {
-    var ErrorA = function ErrorA() {};
-    var ErrorB = function ErrorB() {};
-    var log = "";
+    var TestError = function TestError() {};
     var source = new Proxy({}, {
         getOwnPropertyDescriptor: function(t, pk) {
-            log += pk;
-            throw new (pk === "a" ? ErrorA : ErrorB);
+            assertEq(pk, "b");
+            throw new TestError();
         },
         ownKeys: () => ["b", "a"]
     });
-    assertThrowsInstanceOf(() => Object.assign({}, source), ErrorB);
-    assertEq(log, "ba");
+    assertThrowsInstanceOf(() => Object.assign({}, source), TestError);
 }
 testExceptionsDoNotStopFirstReported1();
 
-// Properties are retrieved through Get()
-// Intermediate exceptions do not stop property traversal, first exception is reported (2)
-function testExceptionsDoNotStopFirstReported2() {
-    var ErrorA = function ErrorA() {};
-    var ErrorB = function ErrorB() {};
-    var log = "";
-    var source = new Proxy({
-        get a() { log += "a"; throw new ErrorA },
-        get b() { log += "b"; throw new ErrorB },
-    }, {
-        ownKeys: () => ["b", "a"]
-    });
-    assertThrowsInstanceOf(() => Object.assign({}, source), ErrorB);
-    assertEq(log, "ba");
-}
-testExceptionsDoNotStopFirstReported2();
-
-// Intermediate exceptions do not stop property traversal, first exception is reported (3)
-function testExceptionsDoNotStopFirstReported3() {
-    var ErrorA = function ErrorA() {};
-    var ErrorB = function ErrorB() {};
-    var log = "";
-    var source = new Proxy({a: 1, b: 2}, {
-        ownKeys: () => ["b", "a"]
-    });
-    var target = {
-        set a(v) { log += "a"; throw new ErrorA },
-        set b(v) { log += "b"; throw new ErrorB },
-    };
-    assertThrowsInstanceOf(() => Object.assign(target, source), ErrorB);
-    assertEq(log, "ba");
-}
-testExceptionsDoNotStopFirstReported3();
-
-// Intermediate exceptions do not stop property traversal, first exception is reported (4)
-function testExceptionsDoNotStopFirstReported4() {
-    var ErrorGetOwnProperty = function ErrorGetOwnProperty() {};
-    var ErrorGet = function ErrorGet() {};
-    var ErrorSet = function ErrorSet() {};
-    var source = new Proxy({
-        get a() { throw new ErrorGet }
-    }, {
-        getOwnPropertyDescriptor: function(t, pk) {
-            throw new ErrorGetOwnProperty;
-        }
-    });
-    var target = {
-        set a(v) { throw new ErrorSet }
-    };
-    assertThrowsInstanceOf(() => Object.assign({}, source), ErrorGetOwnProperty);
-}
-testExceptionsDoNotStopFirstReported4();
 
 if (typeof reportCompare == "function")
     reportCompare(true, true);
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -909,16 +909,17 @@ static const JSFunctionSpec intrinsic_fu
     JS_FN("std_Math_log2",                       math_log2,                    1,0),
 
     JS_FN("std_Map_has",                         MapObject::has,               1,0),
     JS_FN("std_Map_iterator",                    MapObject::entries,           0,0),
 
     JS_FN("std_Number_valueOf",                  num_valueOf,                  0,0),
 
     JS_FN("std_Object_create",                   obj_create,                   2,0),
+    JS_FN("std_Object_propertyIsEnumerable",     obj_propertyIsEnumerable,     1,0),
     JS_FN("std_Object_defineProperty",           obj_defineProperty,           3,0),
     JS_FN("std_Object_getPrototypeOf",           obj_getPrototypeOf,           1,0),
     JS_FN("std_Object_getOwnPropertyNames",      obj_getOwnPropertyNames,      1,0),
     JS_FN("std_Object_getOwnPropertyDescriptor", obj_getOwnPropertyDescriptor, 2,0),
     JS_FN("std_Object_hasOwnProperty",           obj_hasOwnProperty,           1,0),
 
     JS_FN("std_Set_has",                         SetObject::has,               1,0),
     JS_FN("std_Set_iterator",                    SetObject::values,            0,0),