Bug 1158463 - Reorder property creation in js::FromPropertyDescriptorToObject. r=Waldo
authorTom Schuster <evilpies@gmail.com>
Fri, 01 May 2015 12:32:53 +0200
changeset 273438 a240e100cb2a662aeb187543dd8965f2af692741
parent 273437 257edfa8e77b5775e19c20aee3b7f8acd0954d93
child 273439 5028ca29deecc4eae67e804088564b9e2590ee37
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersWaldo
bugs1158463
milestone40.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 1158463 - Reorder property creation in js::FromPropertyDescriptorToObject. r=Waldo
js/src/jit-test/tests/arrays/sort-getter-only.js
js/src/jit-test/tests/collections/Array-of-nonconfigurable-2.js
js/src/jit-test/tests/proxy/testDirectProxyGetOwnPropertyDescriptor11.js
js/src/jit-test/tests/proxy/testIndirectProxyGetOwnPropertyDescriptor.js
js/src/jsobj.cpp
js/src/tests/ecma_6/Object/getOwnPropertyDescriptor.js
js/src/tests/ecma_6/Object/property-descriptor-order.js
js/src/tests/ecma_6/Symbol/property-basics.js
js/src/tests/ecma_6/Symbol/property-reflection.js
js/src/tests/ecma_6/TypedArray/of.js
--- a/js/src/jit-test/tests/arrays/sort-getter-only.js
+++ b/js/src/jit-test/tests/arrays/sort-getter-only.js
@@ -1,19 +1,19 @@
 // The property assignments in Array.prototype.sort are strict assignments.
 
 load(libdir + "asserts.js");
 
 var a = ["A", , "B", "C", "D"];
 var normalArrayElementDesc = Object.getOwnPropertyDescriptor(a, 0);
 var getterDesc = {
-    configurable: false,
+    get: function () { return "F"; },
+    set: undefined,
     enumerable: true,
-    get: function () { return "F"; },
-    set: undefined
+    configurable: false
 };
 Object.defineProperty(a, 1, getterDesc);
 
 // a.sort is permitted to try to delete a[1] or to try to assign a[1], but it
 // must try one or the other. Either one will fail, throwing a TypeError.
 assertThrowsInstanceOf(() => a.sort(), TypeError);
 
 // a.sort() is not permitted to delete the nonconfigurable property.
--- a/js/src/jit-test/tests/collections/Array-of-nonconfigurable-2.js
+++ b/js/src/jit-test/tests/collections/Array-of-nonconfigurable-2.js
@@ -4,13 +4,13 @@ load(libdir + "asserts.js");
 
 var obj;
 function C() {
     obj = this;
     Object.defineProperty(this, 0, {value: "v", configurable: false});
 }
 try { Array.of.call(C, 1); } catch (e) {}
 assertDeepEq(Object.getOwnPropertyDescriptor(obj, 0), {
-    configurable: false,
+    value: "v",
+    writable: false,
     enumerable: false,
-    value: "v",
-    writable: false
+    configurable: false
 });
--- a/js/src/jit-test/tests/proxy/testDirectProxyGetOwnPropertyDescriptor11.js
+++ b/js/src/jit-test/tests/proxy/testDirectProxyGetOwnPropertyDescriptor11.js
@@ -2,13 +2,13 @@
 
 load(libdir + "asserts.js");
 
 var p = new Proxy({}, {
     getOwnPropertyDescriptor() { return {configurable: true}; }
 });
 var desc = Object.getOwnPropertyDescriptor(p, "x");
 assertDeepEq(desc, {
-    configurable: true,
+    value: undefined,
+    writable: false,
     enumerable: false,
-    value: undefined,
-    writable: false
+    configurable: true
 });
--- a/js/src/jit-test/tests/proxy/testIndirectProxyGetOwnPropertyDescriptor.js
+++ b/js/src/jit-test/tests/proxy/testIndirectProxyGetOwnPropertyDescriptor.js
@@ -1,12 +1,12 @@
 // Bug 1133294 - Object.getOwnPropertyDescriptor should never return an incomplete descriptor.
 
 load(libdir + "asserts.js");
 
 var p = Proxy.create({ getOwnPropertyDescriptor() { return {}; } });
 var desc = Object.getOwnPropertyDescriptor(p, "x");
 assertDeepEq(desc, {
-    configurable: false,
+    value: undefined,
+    writable: false,
     enumerable: false,
-    value: undefined,
-    writable: false
+    configurable: false
 });
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -122,72 +122,88 @@ js::InformalValueTypeName(const Value& v
         return "boolean";
     if (v.isNull())
         return "null";
     if (v.isUndefined())
         return "undefined";
     return "value";
 }
 
+// ES6 draft rev37 6.2.4.4 FromPropertyDescriptor
 bool
 js::FromPropertyDescriptor(JSContext* cx, Handle<PropertyDescriptor> desc, MutableHandleValue vp)
 {
+    // Step 1.
     if (!desc.object()) {
         vp.setUndefined();
         return true;
     }
 
     return FromPropertyDescriptorToObject(cx, desc, vp);
 }
 
 bool
 js::FromPropertyDescriptorToObject(JSContext* cx, Handle<PropertyDescriptor> desc,
                                    MutableHandleValue vp)
 {
+    // Step 2-3.
     RootedObject obj(cx, NewBuiltinClassInstance<PlainObject>(cx));
     if (!obj)
         return false;
 
     const JSAtomState& names = cx->names();
-    RootedValue v(cx);
-    if (desc.hasConfigurable()) {
-        v.setBoolean(desc.configurable());
-        if (!DefineProperty(cx, obj, names.configurable, v))
-            return false;
-    }
-    if (desc.hasEnumerable()) {
-        v.setBoolean(desc.enumerable());
-        if (!DefineProperty(cx, obj, names.enumerable, v))
-            return false;
-    }
+
+    // Step 4.
     if (desc.hasValue()) {
         if (!DefineProperty(cx, obj, names.value, desc.value()))
             return false;
     }
+
+    // Step 5.
+    RootedValue v(cx);
     if (desc.hasWritable()) {
         v.setBoolean(desc.writable());
         if (!DefineProperty(cx, obj, names.writable, v))
             return false;
     }
+
+    // Step 6.
     if (desc.hasGetterObject()) {
         if (JSObject* get = desc.getterObject())
             v.setObject(*get);
         else
             v.setUndefined();
         if (!DefineProperty(cx, obj, names.get, v))
             return false;
     }
+
+    // Step 7.
     if (desc.hasSetterObject()) {
         if (JSObject* set = desc.setterObject())
             v.setObject(*set);
         else
             v.setUndefined();
         if (!DefineProperty(cx, obj, names.set, v))
             return false;
     }
+
+    // Step 8.
+    if (desc.hasEnumerable()) {
+        v.setBoolean(desc.enumerable());
+        if (!DefineProperty(cx, obj, names.enumerable, v))
+            return false;
+    }
+
+    // Step 9.
+    if (desc.hasConfigurable()) {
+        v.setBoolean(desc.configurable());
+        if (!DefineProperty(cx, obj, names.configurable, v))
+            return false;
+    }
+
     vp.setObject(*obj);
     return true;
 }
 
 bool
 js::GetFirstArgumentAsObject(JSContext* cx, const CallArgs& args, const char* method,
                              MutableHandleObject objp)
 {
--- a/js/src/tests/ecma_6/Object/getOwnPropertyDescriptor.js
+++ b/js/src/tests/ecma_6/Object/getOwnPropertyDescriptor.js
@@ -13,23 +13,23 @@ assertThrowsInstanceOf(() => Object.getO
 
 Object.getOwnPropertyDescriptor(1);
 Object.getOwnPropertyDescriptor(true);
 if (typeof Symbol === "function") {
     Object.getOwnPropertyDescriptor(Symbol("foo"));
 }
 
 assertDeepEq(Object.getOwnPropertyDescriptor("foo", "length"), {
-    configurable: false,
+    value: 3,
+    writable: false,
     enumerable: false,
-    value: 3,
-    writable: false
+    configurable: false
 });
 
 assertDeepEq(Object.getOwnPropertyDescriptor("foo", 0), {
-    configurable: false,
+    value: "f",
+    writable: false,
     enumerable: true,
-    value: "f",
-    writable: false
+    configurable: false
 });
 
 if (typeof reportCompare === "function")
     reportCompare(true, true);
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/Object/property-descriptor-order.js
@@ -0,0 +1,17 @@
+var names = Object.getOwnPropertyNames(Object.getOwnPropertyDescriptor({foo: 0}, "foo"));
+assertDeepEq(names, ["value", "writable", "enumerable", "configurable"]);
+
+names = Object.getOwnPropertyNames(Object.getOwnPropertyDescriptor({get foo(){}}, "foo"));
+assertDeepEq(names, ["get", "set", "enumerable", "configurable"]);
+
+var proxy = new Proxy({}, {
+    defineProperty(target, key, desc) {
+        var names = Object.getOwnPropertyNames(desc);
+        assertDeepEq(names, ["set", "configurable"]);
+        return true;
+    }
+});
+
+Object.defineProperty(proxy, "foo", {configurable: true, set: function() {}});
+
+reportCompare(true, true);
--- a/js/src/tests/ecma_6/Symbol/property-basics.js
+++ b/js/src/tests/ecma_6/Symbol/property-basics.js
@@ -20,20 +20,20 @@ for (var sym of symbols) {
     assertEq(Object.getOwnPropertyDescriptor(obj, sym), undefined);
 
     // assign, then try accessing again
     obj[sym] = "ok";
     assertEq(sym in obj, true);
     assertEq(obj.hasOwnProperty(sym), true);
     assertEq(obj[sym], "ok");
     assertDeepEq(Object.getOwnPropertyDescriptor(obj, sym), {
-        configurable: true,
+        value: "ok",
+        writable: true,
         enumerable: true,
-        value: "ok",
-        writable: true
+        configurable: true
     });
 
     // assign again, observe value is overwritten
     obj[sym] = 12;
     assertEq(obj[sym], 12);
 
     // increment
     assertEq(obj[sym]++, 12);
--- a/js/src/tests/ecma_6/Symbol/property-reflection.js
+++ b/js/src/tests/ecma_6/Symbol/property-reflection.js
@@ -16,27 +16,27 @@ assertEq(Symbol.for("name") in f, true);
 assertEq(f[Symbol.for("name")], "eff");
 
 // Object.defineProperties
 function D() {}
 var descs = new D;
 var s1 = Symbol("s1");
 var hits = 0;
 descs[s1] = {
-    configurable: true,
+    get: () => hits++,
+    set: undefined,
     enumerable: true,
-    get: () => hits++,
-    set: undefined
+    configurable: true
 };
 var s2 = Symbol("s2");
 descs[s2] = {
-    configurable: true,
+    value: {},
+    writable: true,
     enumerable: false,
-    value: {},
-    writable: true
+    configurable: true
 };
 var s3 = Symbol("s3");
 D.prototype[s3] = {value: "FAIL"};
 assertEq(Object.defineProperties(f, descs), f);
 assertEq(s1 in f, true);
 assertEq(f[s1], 0);
 assertEq(hits, 1);
 assertEq(s2 in f, true);
--- a/js/src/tests/ecma_6/TypedArray/of.js
+++ b/js/src/tests/ecma_6/TypedArray/of.js
@@ -9,20 +9,20 @@ const constructors = [
     Float32Array,
     Float64Array
 ];
 
 for (var constructor of constructors) {
     assertEq(constructor.of.length, 0);
 
     assertDeepEq(Object.getOwnPropertyDescriptor(constructor.__proto__, "of"), {
-        configurable: true,
+        value: constructor.of,
+        writable: true,
         enumerable: false,
-        value: constructor.of,
-        writable: true
+        configurable: true
     });
 
     // Basic tests.
     assertEq(constructor.of().constructor, constructor);
     assertEq(constructor.of() instanceof constructor, true);
     assertDeepEq(constructor.of(10), new constructor([10]));
     assertDeepEq(constructor.of(1, 2, 3), new constructor([1, 2, 3]));
     assertDeepEq(constructor.of("1", "2", "3"), new constructor([1, 2, 3]));