Bug 978238 - Part 2: Implement Proxy.[[GetOwnProperty]] to new ES6 standard. (r=jorendorff)
☠☠ backed out by 51dd4f1ba186 ☠ ☠
authorEric Faust <efaustbmo@gmail.com>
Tue, 03 Jun 2014 13:23:03 -0700
changeset 206755 ea412568c4be152f88f17745c5fbbe8a83a3ac1e
parent 206754 c25abea181d7960baf96261f3db4124d591fa642
child 206756 e79c9dba2792a39706178dea722ddfdca38508b2
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
bugs978238
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 978238 - Part 2: Implement Proxy.[[GetOwnProperty]] to new ES6 standard. (r=jorendorff)
js/src/jit-test/tests/asm.js/testGlobals.js
js/src/jit-test/tests/proxy/testDirectProxyGetOwnPropertyDescriptor10.js
js/src/js.msg
js/src/jsapi.h
js/src/jsobj.cpp
js/src/jsproxy.cpp
--- a/js/src/jit-test/tests/asm.js/testGlobals.js
+++ b/js/src/jit-test/tests/asm.js/testGlobals.js
@@ -118,16 +118,16 @@ assertEq(asmLink(asmCompile('global', 'i
 assertEq(asmLink(asmCompile('global', 'imp', USE_ASM + "const i=+imp.i; function f() { return +i } return f")(this, {i:1.4})), 1.4);
 assertEq(asmLink(asmCompile(USE_ASM + "var g=0; function f() { var i=42; while (1) { break; } g = i; return g|0 } return f"))(), 42);
 
 var f1 = asmCompile('global', 'foreign', 'heap', USE_ASM + 'var i32 = new global.Int32Array(heap); function g() { return i32[4]|0 } return g');
 var global = this;
 var ab = new ArrayBuffer(4096);
 var p = new Proxy(global,
                   {has:function(name) { f1(global, null, ab); return true},
-                   getOwnPropertyDescriptor:function(name) { return {value:Int32Array}}});
+                   getOwnPropertyDescriptor:function(name) { return {configurable:true, value:Int32Array}}});
 new Int32Array(ab)[4] = 42;
 assertEq(f1(p, null, ab)(), 42);
 
 // GVN checks
 assertEq(asmLink(asmCompile(USE_ASM + "var g=0; function f() { var x = 0; g = 1; x = g; return (x+g)|0 } return f"))(), 2);
 assertEq(asmLink(asmCompile(USE_ASM + "var g=0; function f() { var x = 0; g = 1; x = g; g = 2; return (x+g)|0 } return f"))(), 3);
 assertEq(asmLink(asmCompile(USE_ASM + "var g=0; var h=0; function f() { var x = 0; g = 1; x = g; h = 3; return (x+g)|0 } return f"))(), 2);
--- a/js/src/jit-test/tests/proxy/testDirectProxyGetOwnPropertyDescriptor10.js
+++ b/js/src/jit-test/tests/proxy/testDirectProxyGetOwnPropertyDescriptor10.js
@@ -18,19 +18,20 @@ var desc1 = Object.getOwnPropertyDescrip
     }
 }), 'foo');
 assertEq(desc1 == desc, false);
 assertEq(desc1.value, 'baz');
 assertEq(desc1.writable, false);
 assertEq(desc1.enumerable, true);
 assertEq(desc1.configurable, true);
 
-var desc = {};
+// The returned descriptor must agree in configurability.
+var desc = { configurable : true };
 var desc1 = Object.getOwnPropertyDescriptor(new Proxy(target, {
     getOwnPropertyDescriptor: function (target, name) {
         return desc;
     }
 }), 'foo');
 assertEq(desc1 == desc, false);
 assertEq(desc1.value, undefined);
 assertEq(desc1.writable, false);
 assertEq(desc1.enumerable, false);
-assertEq(desc1.configurable, false);
+assertEq(desc1.configurable, true);
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -434,8 +434,10 @@ MSG_DEF(JSMSG_DECLARATION_AFTER_EXPORT, 
 MSG_DEF(JSMSG_INVALID_PROTOTYPE,        380, 0, JSEXN_TYPEERR, "prototype field is not an object")
 MSG_DEF(JSMSG_TYPEDOBJECT_HANDLE_TO_UNSIZED, 381, 0, JSEXN_TYPEERR, "cannot create a handle to an unsized type")
 MSG_DEF(JSMSG_SETPROTOTYPEOF_FAIL,      382, 1, JSEXN_TYPEERR, "[[SetPrototypeOf]] failed on {0}")
 MSG_DEF(JSMSG_INVALID_ARG_TYPE,         383, 3, JSEXN_TYPEERR, "Invalid type: {0} can't be a{1} {2}")
 MSG_DEF(JSMSG_TERMINATED,               384, 1, JSEXN_ERR, "Script terminated by timeout at:\n{0}")
 MSG_DEF(JSMSG_NO_SUCH_SELF_HOSTED_PROP, 385, 1, JSEXN_ERR, "No such property on self-hosted object: {0}")
 MSG_DEF(JSMSG_PROXY_EXTENSIBILITY,      386, 0, JSEXN_TYPEERR, "proxy must report same extensiblitity as target")
 MSG_DEF(JSMSG_PROXY_CONSTRUCT_OBJECT,   387, 0, JSEXN_TYPEERR, "proxy [[Construct]] must return an object")
+MSG_DEF(JSMSG_PROXY_GETOWN_OBJORUNDEF,  388, 0, JSEXN_TYPEERR, "proxy [[GetOwnProperty]] must return an object or undefined")
+MSG_DEF(JSMSG_CANT_REPORT_C_AS_NC,      389, 0, JSEXN_TYPEERR, "proxy can't report existing configurable property as non-configurable")
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -2886,16 +2886,17 @@ template <typename Outer>
 class PropertyDescriptorOperations
 {
     const JSPropertyDescriptor * desc() const { return static_cast<const Outer*>(this)->extract(); }
 
   public:
     bool isEnumerable() const { return desc()->attrs & JSPROP_ENUMERATE; }
     bool isReadonly() const { return desc()->attrs & JSPROP_READONLY; }
     bool isPermanent() const { return desc()->attrs & JSPROP_PERMANENT; }
+    bool isConfigurable() const { return !isPermanent(); }
     bool hasNativeAccessors() const { return desc()->attrs & JSPROP_NATIVE_ACCESSORS; }
     bool hasGetterObject() const { return desc()->attrs & JSPROP_GETTER; }
     bool hasSetterObject() const { return desc()->attrs & JSPROP_SETTER; }
     bool hasGetterOrSetterObject() const { return desc()->attrs & (JSPROP_GETTER | JSPROP_SETTER); }
     bool isShared() const { return desc()->attrs & JSPROP_SHARED; }
     bool isIndex() const { return desc()->attrs & JSPROP_INDEX; }
     bool hasAttributes(unsigned attrs) const { return desc()->attrs & attrs; }
 
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -356,17 +356,16 @@ PropDesc::makeObject(JSContext *cx)
     descObj_ = obj;
     return true;
 }
 
 bool
 js::GetOwnPropertyDescriptor(JSContext *cx, HandleObject obj, HandleId id,
                              MutableHandle<PropertyDescriptor> desc)
 {
-    // FIXME: Call TrapGetOwnProperty directly once ScriptedIndirectProxies is removed
     if (obj->is<ProxyObject>())
         return Proxy::getOwnPropertyDescriptor(cx, obj, id, desc);
 
     RootedObject pobj(cx);
     RootedShape shape(cx);
     if (!HasOwnProperty<CanGC>(cx, obj->getOps()->lookupGeneric, obj, id, &pobj, &shape))
         return false;
     if (!shape) {
--- a/js/src/jsproxy.cpp
+++ b/js/src/jsproxy.cpp
@@ -1194,24 +1194,25 @@ IsAccessorDescriptor(const PropertyDescr
 {
     return desc.obj && desc.attrs & (JSPROP_GETTER | JSPROP_SETTER);
 }
 
 // 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
-ValidatePropertyDescriptor(JSContext *cx, Handle<PropDesc> desc, Handle<PropertyDescriptor> current,
-                           bool *bp)
+ValidatePropertyDescriptor(JSContext *cx, bool extensible, Handle<PropDesc> desc,
+                           Handle<PropertyDescriptor> current, bool *bp)
 {
-    /*
-     * step 2 is redundant since ValidatePropertyDescriptor is never called unless
-     * target.[[HasOwn]](P) is true
-     */
-    JS_ASSERT(current.object());
+    // step 2
+    if (!current.object()) {
+        // Since |O| is always undefined, substeps c and d fall away.
+        *bp = extensible;
+        return true;
+    }
 
     // step 3
     if (!desc.hasValue() && !desc.hasWritable() && !desc.hasGet() && !desc.hasSet() &&
         !desc.hasEnumerable() && !desc.hasConfigurable())
     {
         *bp = true;
         return true;
     }
@@ -1291,28 +1292,16 @@ ValidatePropertyDescriptor(JSContext *cx
     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;
@@ -1348,123 +1337,16 @@ IdToExposableValue(JSContext *cx, Handle
 // NB: This *must* stay synched with proxy().
 static JSObject *
 GetDirectProxyHandlerObject(JSObject *proxy)
 {
     JS_ASSERT(proxy->as<ProxyObject>().handler() == &ScriptedDirectProxyHandler::singleton);
     return proxy->as<ProxyObject>().extra(0).toObjectOrNull();
 }
 
-// TrapGetOwnProperty(O, P)
-static bool
-TrapGetOwnProperty(JSContext *cx, HandleObject proxy, HandleId id, MutableHandleValue rval)
-{
-    // 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().getOwnPropertyDescriptor, &trap))
-        return false;
-
-    // step 4
-    if (trap.isUndefined()) {
-        Rooted<PropertyDescriptor> desc(cx);
-        if (!GetOwnPropertyDescriptor(cx, target, id, &desc))
-            return false;
-        return NewPropertyDescriptorObject(cx, desc, rval);
-    }
-
-    // step 5
-    RootedValue value(cx);
-    if (!IdToExposableValue(cx, id, &value))
-        return false;
-    Value argv[] = {
-        ObjectValue(*target),
-        value
-    };
-    RootedValue trapResult(cx);
-    if (!Invoke(cx, ObjectValue(*handler), trap, ArrayLength(argv), argv, &trapResult))
-        return false;
-
-    // step 6
-    if (!NormalizeAndCompletePropertyDescriptor(cx, &trapResult))
-        return false;
-
-    // step 7
-    if (trapResult.isUndefined()) {
-        bool sealed;
-        if (!IsSealed(cx, target, id, &sealed))
-            return false;
-        if (sealed) {
-            JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_REPORT_NC_AS_NE);
-            return false;
-        }
-
-        bool extensible;
-        if (!JSObject::isExtensible(cx, target, &extensible))
-            return false;
-        if (!extensible) {
-            bool found;
-            if (!HasOwn(cx, target, id, &found))
-                return false;
-            if (found) {
-                JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_REPORT_E_AS_NE);
-                return false;
-            }
-        }
-
-        rval.set(UndefinedValue());
-        return true;
-    }
-
-    // step 8
-    bool isFixed;
-    if (!HasOwn(cx, target, id, &isFixed))
-        return false;
-
-    // step 9
-    bool extensible;
-    if (!JSObject::isExtensible(cx, target, &extensible))
-        return false;
-    if (!extensible && !isFixed) {
-        JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_REPORT_NEW);
-        return false;
-    }
-
-    Rooted<PropDesc> desc(cx);
-    if (!desc.initialize(cx, trapResult))
-        return false;
-
-    /* step 10 */
-    if (isFixed) {
-        bool valid;
-        if (!ValidatePropertyDescriptor(cx, target, id, desc, &valid))
-            return false;
-
-        if (!valid) {
-            JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_REPORT_INVALID);
-            return false;
-        }
-    }
-
-    // step 11
-    if (!desc.configurable() && !isFixed) {
-        JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_REPORT_NE_AS_NC);
-        return false;
-    }
-
-    // step 12
-    rval.set(trapResult);
-    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,
@@ -1641,33 +1523,131 @@ ScriptedDirectProxyHandler::getPropertyD
         return false;
     if (!proto) {
         JS_ASSERT(!desc.object());
         return true;
     }
     return JS_GetPropertyDescriptorById(cx, proto, id, desc);
 }
 
+// ES6 (5 April 2014) Proxy.[[GetOwnProperty]](P)
 bool
 ScriptedDirectProxyHandler::getOwnPropertyDescriptor(JSContext *cx, HandleObject proxy, HandleId id,
                                                      MutableHandle<PropertyDescriptor> desc)
 {
-    // step 1
-    RootedValue v(cx);
-    if (!TrapGetOwnProperty(cx, proxy, id, &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().getOwnPropertyDescriptor, &trap))
+        return false;
+
+    // step 7
+    if (trap.isUndefined())
+        return DirectProxyHandler::getOwnPropertyDescriptor(cx, proxy, id, desc);
+
+    // step 8-9
+    RootedValue propKey(cx);
+    if (!IdToExposableValue(cx, id, &propKey))
+        return false;
+
+    Value argv[] = {
+        ObjectValue(*target),
+        propKey
+    };
+    RootedValue trapResult(cx);
+    if (!Invoke(cx, ObjectValue(*handler), trap, ArrayLength(argv), argv, &trapResult))
         return false;
 
-    // step 2
-    if (v.isUndefined()) {
+    // step 10
+    if (!trapResult.isUndefined() && !trapResult.isObject()) {
+        JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_PROXY_GETOWN_OBJORUNDEF);
+        return false;
+    }
+
+    //step 11-12
+    Rooted<PropertyDescriptor> targetDesc(cx);
+    if (!GetOwnPropertyDescriptor(cx, target, id, &targetDesc))
+        return false;
+
+    // step 13
+    if (trapResult.isUndefined()) {
+        // substep a
+        if (!targetDesc.object()) {
+            desc.object().set(nullptr);
+            return true;
+        }
+
+        // substep b
+        if (targetDesc.isPermanent()) {
+            JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_REPORT_NC_AS_NE);
+            return false;
+        }
+
+        // substep c-e
+        bool extensibleTarget;
+        if (!JSObject::isExtensible(cx, target, &extensibleTarget))
+            return false;
+        if (!extensibleTarget) {
+            JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_REPORT_E_AS_NE);
+            return false;
+        }
+
+        // substep f
         desc.object().set(nullptr);
         return true;
     }
 
-    // steps 3-4
-    return ParsePropertyDescriptorObject(cx, proxy, v, desc, true);
+    // step 14-15
+    bool extensibleTarget;
+    if (!JSObject::isExtensible(cx, target, &extensibleTarget))
+        return false;
+
+    // step 16-17
+    Rooted<PropDesc> resultDesc(cx);
+    if (!resultDesc.initialize(cx, trapResult))
+        return false;
+
+    // step 18
+    resultDesc.complete();
+
+    // step 19
+    bool valid;
+    if (!ValidatePropertyDescriptor(cx, extensibleTarget, resultDesc, targetDesc, &valid))
+        return false;
+
+    // step 20
+    if (!valid) {
+        JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_REPORT_INVALID);
+        return false;
+    }
+
+    // step 21
+    if (!resultDesc.configurable()) {
+        if (!targetDesc.object()) {
+            JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_REPORT_NE_AS_NC);
+            return false;
+        }
+
+        if (!targetDesc.isPermanent()) {
+            JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_REPORT_C_AS_NC);
+            return false;
+        }
+    }
+
+    // step 22
+    // FIXME: This is incorrect with respect to [[Origin]]. See bug 999156.
+    resultDesc.populatePropertyDescriptor(proxy, desc);
+    return true;
 }
 
 // ES6 (5 April 2014) Proxy.[[DefineOwnProperty]](O,P)
 bool
 ScriptedDirectProxyHandler::defineProperty(JSContext *cx, HandleObject proxy, HandleId id,
                                            MutableHandle<PropertyDescriptor> desc)
 {
     // step 2
@@ -1732,17 +1712,17 @@ ScriptedDirectProxyHandler::defineProper
                 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))
+            if (!ValidatePropertyDescriptor(cx, extensibleTarget, pd, targetDesc, &valid))
                 return false;
             if (!valid || (settingConfigFalse && !targetDesc.isPermanent())) {
                 JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_DEFINE_INVALID);
                 return false;
             }
         }
     }