Bug 1008441 - Make Object.defineProperty(proxy, desc) only parse desc once. r=efaust.
authorJason Orendorff <jorendorff@mozilla.com>
Fri, 06 Jun 2014 11:15:21 -0400
changeset 207528 3825abd9a302a6270bd41f83bf666313af5cd758
parent 207527 c48f50085fecd32758628f3898ff3c02f8674209
child 207529 dbec390460f655702a1d953efaefae07bcad21ac
push id494
push userraliiev@mozilla.com
push dateMon, 25 Aug 2014 18:42:16 +0000
treeherdermozilla-release@a3cc3e46b571 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersefaust
bugs1008441
milestone32.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 1008441 - Make Object.defineProperty(proxy, desc) only parse desc once. r=efaust.
js/src/jsobj.cpp
js/src/jsproxy.cpp
js/src/jsproxy.h
js/src/tests/ecma_6/Object/browser.js
js/src/tests/ecma_6/Object/defineProperty-proxy.js
js/src/tests/ecma_6/Object/shell.js
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -1062,18 +1062,20 @@ js::DefineProperty(JSContext *cx, Handle
 {
     if (obj->is<ArrayObject>()) {
         Rooted<ArrayObject*> arr(cx, &obj->as<ArrayObject>());
         return DefinePropertyOnArray(cx, arr, id, desc, throwError, rval);
     }
 
     if (obj->getOps()->lookupGeneric) {
         if (obj->is<ProxyObject>()) {
-            RootedValue pd(cx, desc.descriptorValue());
-            return Proxy::defineProperty(cx, obj, id, pd);
+            Rooted<PropertyDescriptor> pd(cx);
+            desc.populatePropertyDescriptor(obj, &pd);
+            pd.object().set(obj);
+            return Proxy::defineProperty(cx, obj, id, &pd);
         }
         return Reject(cx, obj, JSMSG_OBJECT_NOT_EXTENSIBLE, throwError, rval);
     }
 
     return DefinePropertyOnObject(cx, obj, id, desc, throwError, rval);
 }
 
 bool
@@ -1144,19 +1146,20 @@ js::DefineProperties(JSContext *cx, Hand
             if (!DefinePropertyOnArray(cx, arr, ids[i], descs[i], true, &dummy))
                 return false;
         }
         return true;
     }
 
     if (obj->getOps()->lookupGeneric) {
         if (obj->is<ProxyObject>()) {
+            Rooted<PropertyDescriptor> pd(cx);
             for (size_t i = 0, len = ids.length(); i < len; i++) {
-                RootedValue pd(cx, descs[i].descriptorValue());
-                if (!Proxy::defineProperty(cx, obj, ids[i], pd))
+                descs[i].populatePropertyDescriptor(obj, &pd);
+                if (!Proxy::defineProperty(cx, obj, ids[i], &pd))
                     return false;
             }
             return true;
         }
         bool dummy;
         return Reject(cx, obj, JSMSG_OBJECT_NOT_EXTENSIBLE, true, &dummy);
     }
 
--- a/js/src/jsproxy.cpp
+++ b/js/src/jsproxy.cpp
@@ -2379,25 +2379,16 @@ Proxy::defineProperty(JSContext *cx, Han
     BaseProxyHandler *handler = proxy->as<ProxyObject>().handler();
     AutoEnterPolicy policy(cx, handler, proxy, id, BaseProxyHandler::SET, true);
     if (!policy.allowed())
         return policy.returnValue();
     return proxy->as<ProxyObject>().handler()->defineProperty(cx, proxy, id, desc);
 }
 
 bool
-Proxy::defineProperty(JSContext *cx, HandleObject proxy, HandleId id, HandleValue v)
-{
-    JS_CHECK_RECURSION(cx, return false);
-    Rooted<PropertyDescriptor> desc(cx);
-    return ParsePropertyDescriptorObject(cx, proxy, v, &desc) &&
-           Proxy::defineProperty(cx, proxy, id, &desc);
-}
-
-bool
 Proxy::getOwnPropertyNames(JSContext *cx, HandleObject proxy, AutoIdVector &props)
 {
     JS_CHECK_RECURSION(cx, return false);
     BaseProxyHandler *handler = proxy->as<ProxyObject>().handler();
     AutoEnterPolicy policy(cx, handler, proxy, JSID_VOIDHANDLE, BaseProxyHandler::ENUMERATE, true);
     if (!policy.allowed())
         return policy.returnValue();
     return proxy->as<ProxyObject>().handler()->getOwnPropertyNames(cx, proxy, props);
--- a/js/src/jsproxy.h
+++ b/js/src/jsproxy.h
@@ -309,17 +309,16 @@ class Proxy
     static bool getPropertyDescriptor(JSContext *cx, HandleObject proxy, HandleId id,
                                       MutableHandleValue vp);
     static bool getOwnPropertyDescriptor(JSContext *cx, HandleObject proxy, HandleId id,
                                          MutableHandle<JSPropertyDescriptor> desc);
     static bool getOwnPropertyDescriptor(JSContext *cx, HandleObject proxy, HandleId id,
                                          MutableHandleValue vp);
     static bool defineProperty(JSContext *cx, HandleObject proxy, HandleId id,
                                MutableHandle<JSPropertyDescriptor> desc);
-    static bool defineProperty(JSContext *cx, HandleObject proxy, HandleId id, HandleValue v);
     static bool getOwnPropertyNames(JSContext *cx, HandleObject proxy, AutoIdVector &props);
     static bool delete_(JSContext *cx, HandleObject proxy, HandleId id, bool *bp);
     static bool enumerate(JSContext *cx, HandleObject proxy, AutoIdVector &props);
 
     /* ES5 Harmony derived proxy traps. */
     static bool has(JSContext *cx, HandleObject proxy, HandleId id, bool *bp);
     static bool hasOwn(JSContext *cx, HandleObject proxy, HandleId id, bool *bp);
     static bool get(JSContext *cx, HandleObject proxy, HandleObject receiver, HandleId id,
new file mode 100644
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/Object/defineProperty-proxy.js
@@ -0,0 +1,54 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/ */
+
+// Test details of the implementation of ToPropertyDescriptor exposed to scripts
+// thanks to scriptable proxies.
+
+// A LoggingProxy object logs certain operations performed on it.
+var log = [];
+function LoggingProxy(target) {
+    return new Proxy(target, {
+        has: function (t, id) {
+            log.push("has " + id);
+            return id in t;
+        },
+        get: function (t, id) {
+            log.push("get " + id);
+            return t[id];
+        }
+    });
+}
+
+// Tragically, we use separate code to implement Object.defineProperty on
+// arrays and on proxies. So run the test three times.
+var testSubjects = [
+    {},
+    [],
+    new Proxy({}, {})
+];
+
+for (var obj of testSubjects) {
+    log = [];
+
+    // Object.defineProperty is one public method that performs a
+    // ToPropertyDescriptor call.
+    Object.defineProperty(obj, "x", new LoggingProxy({
+        enumerable: true,
+        configurable: true,
+        value: 3,
+        writable: true
+    }));
+
+    // It should have performed exactly these operations on the proxy, in this
+    // order. See ES6 rev 24 (2014 April 27) 6.2.4.5 ToPropertyDescriptor.
+    assertDeepEq(log, [
+        "has enumerable", "get enumerable",
+        "has configurable", "get configurable",
+        "has value", "get value",
+        "has writable", "get writable",
+        "has get",
+        "has set"
+    ]);
+}
+
+reportCompare(0, 0);
new file mode 100644