Bug 1158463 - Reorder property creation in js::FromPropertyDescriptorToObject. r=Waldo

js/src/jit-test/tests/arrays/sort-getter-only.js

@@ -5,10 +5,10 @@ load(libdir + "asserts.js");
 var a = ["A", , "B", "C", "D"];
 var normalArrayElementDesc = Object.getOwnPropertyDescriptor(a, 0);
 var getterDesc = {
-    configurable: false,
-    enumerable: true,
     get: function () { return "F"; },
-    set: undefined
+    set: undefined,
+    enumerable: true,
+    configurable: false
 };
 Object.defineProperty(a, 1, getterDesc);
 

js/src/jit-test/tests/collections/Array-of-nonconfigurable-2.js

@@ -9,8 +9,8 @@ function C() {
 }
 try { Array.of.call(C, 1); } catch (e) {}
 assertDeepEq(Object.getOwnPropertyDescriptor(obj, 0), {
-    configurable: false,
-    enumerable: false,
     value: "v",
-    writable: false
+    writable: false,
+    enumerable: false,
+    configurable: false
 });

js/src/jit-test/tests/proxy/testDirectProxyGetOwnPropertyDescriptor11.js

@@ -7,8 +7,8 @@ var p = new Proxy({}, {
 });
 var desc = Object.getOwnPropertyDescriptor(p, "x");
 assertDeepEq(desc, {
-    configurable: true,
-    enumerable: false,
     value: undefined,
-    writable: false
+    writable: false,
+    enumerable: false,
+    configurable: true
 });

js/src/jit-test/tests/proxy/testIndirectProxyGetOwnPropertyDescriptor.js

@@ -5,8 +5,8 @@ load(libdir + "asserts.js");
 var p = Proxy.create({ getOwnPropertyDescriptor() { return {}; } });
 var desc = Object.getOwnPropertyDescriptor(p, "x");
 assertDeepEq(desc, {
-    configurable: false,
-    enumerable: false,
     value: undefined,
-    writable: false
+    writable: false,
+    enumerable: false,
+    configurable: false
 });

js/src/jsobj.cpp

@@ -127,9 +127,11 @@ js::InformalValueTypeName(const Value& v)
     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;
@@ -142,31 +144,28 @@ 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);
@@ -175,6 +174,8 @@ js::FromPropertyDescriptorToObject(JSContext* cx, Handle<PropertyDescriptor> des
         if (!DefineProperty(cx, obj, names.get, v))
             return false;
     }
+
+    // Step 7.
     if (desc.hasSetterObject()) {
         if (JSObject* set = desc.setterObject())
             v.setObject(*set);
@@ -183,6 +184,21 @@ js::FromPropertyDescriptorToObject(JSContext* cx, Handle<PropertyDescriptor> des
         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;
 }

js/src/tests/ecma_6/Object/getOwnPropertyDescriptor.js

@@ -18,17 +18,17 @@ if (typeof Symbol === "function") {
 }
 
 assertDeepEq(Object.getOwnPropertyDescriptor("foo", "length"), {
-    configurable: false,
-    enumerable: false,
     value: 3,
-    writable: false
+    writable: false,
+    enumerable: false,
+    configurable: false
 });
 
 assertDeepEq(Object.getOwnPropertyDescriptor("foo", 0), {
-    configurable: false,
-    enumerable: true,
     value: "f",
-    writable: false
+    writable: false,
+    enumerable: true,
+    configurable: false
 });
 
 if (typeof reportCompare === "function")

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);

js/src/tests/ecma_6/Symbol/property-basics.js

@@ -25,10 +25,10 @@ for (var sym of symbols) {
     assertEq(obj.hasOwnProperty(sym), true);
     assertEq(obj[sym], "ok");
     assertDeepEq(Object.getOwnPropertyDescriptor(obj, sym), {
-        configurable: true,
-        enumerable: true,
         value: "ok",
-        writable: true
+        writable: true,
+        enumerable: true,
+        configurable: true
     });
 
     // assign again, observe value is overwritten

js/src/tests/ecma_6/Symbol/property-reflection.js

@@ -21,17 +21,17 @@ var descs = new D;
 var s1 = Symbol("s1");
 var hits = 0;
 descs[s1] = {
-    configurable: true,
-    enumerable: true,
     get: () => hits++,
-    set: undefined
+    set: undefined,
+    enumerable: true,
+    configurable: true
 };
 var s2 = Symbol("s2");
 descs[s2] = {
-    configurable: true,
-    enumerable: false,
     value: {},
-    writable: true
+    writable: true,
+    enumerable: false,
+    configurable: true
 };
 var s3 = Symbol("s3");
 D.prototype[s3] = {value: "FAIL"};

js/src/tests/ecma_6/TypedArray/of.js

@@ -14,10 +14,10 @@ for (var constructor of constructors) {
     assertEq(constructor.of.length, 0);
 
     assertDeepEq(Object.getOwnPropertyDescriptor(constructor.__proto__, "of"), {
-        configurable: true,
-        enumerable: false,
         value: constructor.of,
-        writable: true
+        writable: true,
+        enumerable: false,
+        configurable: true
     });
 
     // Basic tests.