Bug 978236 - Implement Proxy.[[DefineProperty]] to ES6 standard. (r=jorendorff)
☠☠ backed out by cbc90d9dafdb ☠ ☠
authorEric Faust <efaustbmo@gmail.com>
Tue, 03 Jun 2014 13:00:59 -0700
changeset 206749 e89d2416585f3c8ed09845f655c65b721551253d
parent 206748 efbad7dfa8706b46226f5d05f23680aebbba5961
child 206750 68ba56dc7149cf62ff4c69cadaadd371b33b62c8
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)
reviewersjorendorff
bugs978236
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 978236 - Implement Proxy.[[DefineProperty]] to ES6 standard. (r=jorendorff)
js/src/jit-test/tests/proxy/bug897403.js
js/src/jsobj.cpp
js/src/jsproxy.cpp
--- a/js/src/jit-test/tests/proxy/bug897403.js
+++ b/js/src/jit-test/tests/proxy/bug897403.js
@@ -1,3 +1,5 @@
+// |jit-test| error: TypeError
+
 var f = (function () {}).bind({});
 var p = new Proxy(f, {});
 Object.defineProperty(p, "caller", {get: function(){}});
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -1044,20 +1044,16 @@ js::DefineProperty(JSContext *cx, Handle
                    bool throwError, bool *rval)
 {
     if (obj->is<ArrayObject>()) {
         Rooted<ArrayObject*> arr(cx, &obj->as<ArrayObject>());
         return DefinePropertyOnArray(cx, arr, id, desc, throwError, rval);
     }
 
     if (obj->getOps()->lookupGeneric) {
-        /*
-         * FIXME: Once ScriptedIndirectProxies are removed, this code should call
-         * TrapDefineOwnProperty directly
-         */
         if (obj->is<ProxyObject>()) {
             RootedValue pd(cx, desc.descriptorValue());
             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);
@@ -1130,20 +1126,16 @@ js::DefineProperties(JSContext *cx, Hand
         for (size_t i = 0, len = ids.length(); i < len; i++) {
             if (!DefinePropertyOnArray(cx, arr, ids[i], descs[i], true, &dummy))
                 return false;
         }
         return true;
     }
 
     if (obj->getOps()->lookupGeneric) {
-        /*
-         * FIXME: Once ScriptedIndirectProxies are removed, this code should call
-         * TrapDefineOwnProperty directly
-         */
         if (obj->is<ProxyObject>()) {
             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))
                     return false;
             }
             return true;
         }
--- a/js/src/jsproxy.cpp
+++ b/js/src/jsproxy.cpp
@@ -1194,40 +1194,38 @@ IsDataDescriptor(const PropertyDescripto
 }
 
 static inline bool
 IsAccessorDescriptor(const PropertyDescriptor &desc)
 {
     return desc.obj && desc.attrs & (JSPROP_GETTER | JSPROP_SETTER);
 }
 
-// Aux.5 ValidateProperty(O, P, Desc)
+// ES6 (5 April 2014) ValidateAndApplyPropertyDescriptor(O, P, Extensible, Desc, Current)
+// Since we are actually performing 9.1.6.2 IsCompatiblePropertyDescriptor(Extensible, Desc,
+// Current), some parameters are omitted.
 static bool
-ValidateProperty(JSContext *cx, HandleObject obj, HandleId id, Handle<PropDesc> desc, bool *bp)
+ValidatePropertyDescriptor(JSContext *cx, Handle<PropDesc> desc, Handle<PropertyDescriptor> current,
+                           bool *bp)
 {
-    // step 1
-    Rooted<PropertyDescriptor> current(cx);
-    if (!GetOwnPropertyDescriptor(cx, obj, id, &current))
-        return false;
-
     /*
-     * steps 2-4 are redundant since ValidateProperty is never called unless
+     * step 2 is redundant since ValidatePropertyDescriptor is never called unless
      * target.[[HasOwn]](P) is true
      */
     JS_ASSERT(current.object());
 
-    // step 5
+    // step 3
     if (!desc.hasValue() && !desc.hasWritable() && !desc.hasGet() && !desc.hasSet() &&
         !desc.hasEnumerable() && !desc.hasConfigurable())
     {
         *bp = true;
         return true;
     }
 
-    // step 6
+    // step 4
     if ((!desc.hasWritable() || desc.writable() == !current.isReadonly()) &&
         (!desc.hasGet() || desc.getter() == current.getter()) &&
         (!desc.hasSet() || desc.setter() == current.setter()) &&
         (!desc.hasEnumerable() || desc.enumerable() == current.isEnumerable()) &&
         (!desc.hasConfigurable() || desc.configurable() == !current.isPermanent()))
     {
         if (!desc.hasValue()) {
             *bp = true;
@@ -1237,46 +1235,46 @@ ValidateProperty(JSContext *cx, HandleOb
         if (!SameValue(cx, desc.value(), current.value(), &same))
             return false;
         if (same) {
             *bp = true;
             return true;
         }
     }
 
-    // step 7
+    // step 5
     if (current.isPermanent()) {
         if (desc.hasConfigurable() && desc.configurable()) {
             *bp = false;
             return true;
         }
 
         if (desc.hasEnumerable() &&
             desc.enumerable() != current.isEnumerable())
         {
             *bp = false;
             return true;
         }
     }
 
-    // step 8
+    // step 6
     if (desc.isGenericDescriptor()) {
         *bp = true;
         return true;
     }
 
-    // step 9
+    // step 7a
     if (IsDataDescriptor(current) != desc.isDataDescriptor()) {
         *bp = !current.isPermanent();
         return true;
     }
 
-    // step 10
+    // step 8
     if (IsDataDescriptor(current)) {
-        JS_ASSERT(desc.isDataDescriptor()); // by step 9
+        JS_ASSERT(desc.isDataDescriptor()); // by step 7a
         if (current.isPermanent() && current.isReadonly()) {
             if (desc.hasWritable() && desc.writable()) {
                 *bp = false;
                 return true;
             }
 
             if (desc.hasValue()) {
                 bool same;
@@ -1288,25 +1286,37 @@ ValidateProperty(JSContext *cx, HandleOb
                 }
             }
         }
 
         *bp = true;
         return true;
     }
 
-    // steps 11-12
-    JS_ASSERT(IsAccessorDescriptor(current)); // by step 10
-    JS_ASSERT(desc.isAccessorDescriptor()); // by step 9
+    // step 9
+    JS_ASSERT(IsAccessorDescriptor(current)); // by step 8
+    JS_ASSERT(desc.isAccessorDescriptor()); // by step 7a
     *bp = (!current.isPermanent() ||
            ((!desc.hasSet() || desc.setter() == current.setter()) &&
             (!desc.hasGet() || desc.getter() == current.getter())));
     return true;
 }
 
+static bool
+ValidatePropertyDescriptor(JSContext *cx, HandleObject obj, HandleId id, Handle<PropDesc> desc, bool *bp)
+{
+    // step 1
+    Rooted<PropertyDescriptor> current(cx);
+    if (!GetOwnPropertyDescriptor(cx, obj, id, &current))
+        return false;
+
+    return ValidatePropertyDescriptor(cx, desc, current, bp);
+}
+
+
 // Aux.6 IsSealed(O, P)
 static bool
 IsSealed(JSContext* cx, HandleObject obj, HandleId id, bool *bp)
 {
     // step 1
     Rooted<PropertyDescriptor> desc(cx);
     if (!GetOwnPropertyDescriptor(cx, obj, id, &desc))
         return false;
@@ -1429,17 +1439,17 @@ TrapGetOwnProperty(JSContext *cx, Handle
 
     Rooted<PropDesc> desc(cx);
     if (!desc.initialize(cx, trapResult))
         return false;
 
     /* step 10 */
     if (isFixed) {
         bool valid;
-        if (!ValidateProperty(cx, target, id, desc, &valid))
+        if (!ValidatePropertyDescriptor(cx, target, id, desc, &valid))
             return false;
 
         if (!valid) {
             JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_REPORT_INVALID);
             return false;
         }
     }
 
@@ -1449,101 +1459,16 @@ TrapGetOwnProperty(JSContext *cx, Handle
         return false;
     }
 
     // step 12
     rval.set(trapResult);
     return true;
 }
 
-// TrapDefineOwnProperty(O, P, DescObj, Throw)
-static bool
-TrapDefineOwnProperty(JSContext *cx, HandleObject proxy, HandleId id, MutableHandleValue vp)
-{
-    // step 1
-    RootedObject handler(cx, GetDirectProxyHandlerObject(proxy));
-
-    // step 2
-    RootedObject target(cx, proxy->as<ProxyObject>().target());
-
-    // step 3
-    RootedValue trap(cx);
-    if (!JSObject::getProperty(cx, handler, handler, cx->names().defineProperty, &trap))
-        return false;
-
-    // step 4
-    if (trap.isUndefined()) {
-        Rooted<PropertyDescriptor> desc(cx);
-        if (!ParsePropertyDescriptorObject(cx, proxy, vp, &desc))
-            return false;
-        return JS_DefinePropertyById(cx, target, id, desc.value(), desc.attributes(),
-                                     desc.getter(), desc.setter());
-    }
-
-    // step 5
-    RootedValue normalizedDesc(cx, vp);
-    if (!NormalizePropertyDescriptor(cx, &normalizedDesc))
-        return false;
-
-    // step 6
-    RootedValue value(cx);
-    if (!IdToExposableValue(cx, id, &value))
-        return false;
-    Value argv[] = {
-        ObjectValue(*target),
-        value,
-        normalizedDesc
-    };
-    RootedValue trapResult(cx);
-    if (!Invoke(cx, ObjectValue(*handler), trap, ArrayLength(argv), argv, &trapResult))
-        return false;
-
-    // steps 7-8
-    if (ToBoolean(trapResult)) {
-        bool isFixed;
-        if (!HasOwn(cx, target, id, &isFixed))
-            return false;
-
-        bool extensible;
-        if (!JSObject::isExtensible(cx, target, &extensible))
-            return false;
-        if (!extensible && !isFixed) {
-            JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_DEFINE_NEW);
-            return false;
-        }
-
-        Rooted<PropDesc> desc(cx);
-        if (!desc.initialize(cx, normalizedDesc))
-            return false;
-
-        if (isFixed) {
-            bool valid;
-            if (!ValidateProperty(cx, target, id, desc, &valid))
-                return false;
-            if (!valid) {
-                JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_DEFINE_INVALID);
-                return false;
-            }
-        }
-
-        if (!desc.configurable() && !isFixed) {
-            JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_DEFINE_NE_AS_NC);
-            return false;
-        }
-
-        vp.set(BooleanValue(true));
-        return true;
-    }
-
-    // step 9
-    // FIXME: API does not include a Throw parameter
-    vp.set(BooleanValue(false));
-    return true;
-}
-
 static inline void
 ReportInvalidTrapResult(JSContext *cx, JSObject *proxy, JSAtom *atom)
 {
     RootedValue v(cx, ObjectOrNullValue(proxy));
     JSAutoByteString bytes;
     if (!AtomToPrintableString(cx, atom, &bytes))
         return;
     js_ReportValueError2(cx, JSMSG_INVALID_TRAP_RESULT, JSDVG_IGNORE_STACK, v,
@@ -1739,29 +1664,100 @@ ScriptedDirectProxyHandler::getOwnProper
         desc.object().set(nullptr);
         return true;
     }
 
     // steps 3-4
     return ParsePropertyDescriptorObject(cx, proxy, v, desc, true);
 }
 
+// ES6 (5 April 2014) Proxy.[[DefineOwnProperty]](O,P)
 bool
 ScriptedDirectProxyHandler::defineProperty(JSContext *cx, HandleObject proxy, HandleId id,
                                            MutableHandle<PropertyDescriptor> desc)
 {
-    // step 1
-    Rooted<PropDesc> d(cx);
-    d.initFromPropertyDescriptor(desc);
-    RootedValue v(cx);
-    if (!FromGenericPropertyDescriptor(cx, &d, &v))
+    // step 2
+    RootedObject handler(cx, GetDirectProxyHandlerObject(proxy));
+
+    // TODO: step 3: Implement revocation semantics. See bug 978279.
+
+    // step 4
+    RootedObject target(cx, proxy->as<ProxyObject>().target());
+
+    // step 5-6
+    RootedValue trap(cx);
+    if (!JSObject::getProperty(cx, handler, handler, cx->names().defineProperty, &trap))
+         return false;
+
+    // step 7
+    if (trap.isUndefined())
+        return DirectProxyHandler::defineProperty(cx, proxy, id, desc);
+
+    // step 8-9, with 9 blatantly defied.
+    // FIXME: This is incorrect with respect to [[Origin]]. See bug 601379.
+    RootedValue descObj(cx);
+    if (!NewPropertyDescriptorObject(cx, desc, &descObj))
+        return false;
+
+    // step 10, 12
+    RootedValue propKey(cx);
+    if (!IdToExposableValue(cx, id, &propKey))
+        return false;
+
+    Value argv[] = {
+        ObjectValue(*target),
+        propKey,
+        descObj
+    };
+    RootedValue trapResult(cx);
+    if (!Invoke(cx, ObjectValue(*handler), trap, ArrayLength(argv), argv, &trapResult))
         return false;
 
-    // step 2
-    return TrapDefineOwnProperty(cx, proxy, id, &v);
+    // step 11, 13
+    if (ToBoolean(trapResult)) {
+        // step 14-15
+        Rooted<PropertyDescriptor> targetDesc(cx);
+        if (!GetOwnPropertyDescriptor(cx, target, id, &targetDesc))
+            return false;
+
+        // step 16-17
+        bool extensibleTarget;
+        if (!JSObject::isExtensible(cx, target, &extensibleTarget))
+            return false;
+
+        // step 18-19
+        bool settingConfigFalse = desc.isPermanent();
+        if (!targetDesc.object()) {
+            // step 20a
+            if (!extensibleTarget) {
+                JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_DEFINE_NEW);
+                return false;
+            }
+            // step 20b
+            if (settingConfigFalse) {
+                JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_DEFINE_NE_AS_NC);
+                return false;
+            }
+        } else {
+            // step 21
+            bool valid;
+            Rooted<PropDesc> pd(cx);
+            pd.initFromPropertyDescriptor(desc);
+            if (!ValidatePropertyDescriptor(cx, pd, targetDesc, &valid))
+                return false;
+            if (!valid || (settingConfigFalse && !targetDesc.isPermanent())) {
+                JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_DEFINE_INVALID);
+                return false;
+            }
+        }
+    }
+
+    // [[DefineProperty]] should return a boolean value, which is used to do things like
+    // strict-mode throwing. At present, the engine is not prepared to do that. See bug 826587.
+    return true;
 }
 
 bool
 ScriptedDirectProxyHandler::getOwnPropertyNames(JSContext *cx, HandleObject proxy,
                                                 AutoIdVector &props)
 {
     // step a
     RootedObject handler(cx, GetDirectProxyHandlerObject(proxy));