Bug 1083211 - Reimplement BaseProxyHandler::set from scratch to follow ES6 draft rev 27 9.1.9. r=bholley.
☠☠ backed out by 3670435aed44 ☠ ☠
authorJason Orendorff <jorendorff@mozilla.com>
Mon, 13 Oct 2014 16:46:04 -0500
changeset 241599 c69e27e865656ccd9180a82e5ca761e32140860c
parent 241598 ff97ac763705dd01700fc605500047a1bd374297
child 241600 b3f742c00ba1395e499d859b6374ca6864d4f522
push id4311
push userraliiev@mozilla.com
push dateMon, 12 Jan 2015 19:37:41 +0000
treeherdermozilla-beta@150c9fed433b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs1083211
milestone36.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 1083211 - Reimplement BaseProxyHandler::set from scratch to follow ES6 draft rev 27 9.1.9. r=bholley. The handlers affected by this change are: SandboxProxyHandler XrayWrapper DeadObjectProxy (but not really) In the near future, I will change Proxy::set() to use this code when mHasPrototype is true. Handlers that do not override set() but nonetheless are not affected: * WindowNamedPropertiesHandler. Not affected yet because hasPrototype=true, so set() is never called. However it's worth thinking about this one. It will be changing to use this code soon. * ScriptedIndirectProxyHandler. This class was the original motivation for the old bad code; its old bad behavior has been preserved (by changing it to override set() with the old code). This is necessary, alas -- there's in-tree code depending on these details of Proxy.create()'s behavior.
js/src/jsapi.h
js/src/proxy/BaseProxyHandler.cpp
js/src/proxy/ScriptedIndirectProxyHandler.cpp
js/src/proxy/ScriptedIndirectProxyHandler.h
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -3026,16 +3026,23 @@ class PropertyDescriptorOperations
     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 hasGetterOrSetter() const { return desc()->getter || desc()->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; }
 
+    // Descriptors with JSPropertyOps are considered data descriptors. It's
+    // complicated.
+    bool isAccessorDescriptor() const { return hasGetterOrSetterObject(); }
+    bool isDataDescriptor() const { return !isAccessorDescriptor(); }
+
+    bool isWritable() const { MOZ_ASSERT(isDataDescriptor()); return !isReadonly(); }
+
     JS::HandleObject object() const {
         return JS::HandleObject::fromMarkedLocation(&desc()->obj);
     }
     unsigned attributes() const { return desc()->attrs; }
     JSPropertyOp getter() const { return desc()->getter; }
     JSStrictPropertyOp setter() const { return desc()->setter; }
     JS::HandleObject getterObject() const {
         MOZ_ASSERT(hasGetterObject());
--- a/js/src/proxy/BaseProxyHandler.cpp
+++ b/js/src/proxy/BaseProxyHandler.cpp
@@ -73,30 +73,81 @@ BaseProxyHandler::get(JSContext *cx, Han
 }
 
 bool
 BaseProxyHandler::set(JSContext *cx, HandleObject proxy, HandleObject receiver,
                       HandleId id, bool strict, MutableHandleValue vp) const
 {
     assertEnteredPolicy(cx, proxy, id, SET);
 
-    // Find an own or inherited property. The code here is strange for maximum
-    // backward compatibility with earlier code written before ES6 and before
-    // SetPropertyIgnoringNamedGetter.
-    Rooted<PropertyDescriptor> desc(cx);
-    if (!getOwnPropertyDescriptor(cx, proxy, id, &desc))
+    // This method is not covered by any spec, but we follow ES6 draft rev 28
+    // (2014 Oct 14) 9.1.9 fairly closely, adapting it slightly for
+    // SpiderMonkey's particular foibles.
+
+    // Steps 2-3.  (Step 1 is a superfluous assertion.)
+    Rooted<PropertyDescriptor> ownDesc(cx);
+    if (!getOwnPropertyDescriptor(cx, proxy, id, &ownDesc))
         return false;
-    bool descIsOwn = desc.object() != nullptr;
-    if (!descIsOwn) {
-        if (!getPropertyDescriptor(cx, proxy, id, &desc))
+
+    // Step 4.
+    if (!ownDesc.object()) {
+        // The spec calls this variable "parent", but that word has weird
+        // connotations in SpiderMonkey, so let's go with "proto".
+        RootedObject proto(cx);
+        if (!JSObject::getProto(cx, proxy, &proto))
             return false;
+        if (proto)
+            return JSObject::setGeneric(cx, proto, receiver, id, vp, strict);
+
+        // Change ownDesc to be a complete descriptor for a configurable,
+        // writable, enumerable data property. Then fall through to step 5.
+        ownDesc.clear();
+        ownDesc.setAttributes(JSPROP_ENUMERATE);
     }
 
-    return SetPropertyIgnoringNamedGetter(cx, this, proxy, receiver, id, &desc, descIsOwn, strict,
-                                          vp);
+    // Step 5.
+    if (ownDesc.isDataDescriptor()) {
+        // Steps 5.a-b, adapted to our nonstandard implementation of ES6
+        // [[Set]] return values.
+        if (!ownDesc.isWritable()) {
+            if (strict)
+                return JSObject::reportReadOnly(cx, id, JSREPORT_ERROR);
+            if (cx->compartment()->options().extraWarnings(cx))
+                return JSObject::reportReadOnly(cx, id, JSREPORT_STRICT | JSREPORT_WARNING);
+            return true;
+        }
+
+        // Nonstandard SpiderMonkey special case: setter ops.
+        StrictPropertyOp setter = ownDesc.setter();
+        if (setter && setter != JS_StrictPropertyStub)
+            return CallSetter(cx, receiver, id, setter, ownDesc.attributes(), strict, vp);
+
+        // Steps 5.c-d. Adapt for SpiderMonkey by using HasOwnProperty instead
+        // of the standard [[GetOwnProperty]].
+        bool existingDescriptor;
+        if (!HasOwnProperty(cx, receiver, id, &existingDescriptor))
+            return false;
+
+        // Steps 5.e-f.
+        unsigned attrs =
+            existingDescriptor
+            ? JSPROP_IGNORE_ENUMERATE | JSPROP_IGNORE_READONLY | JSPROP_IGNORE_PERMANENT
+            : JSPROP_ENUMERATE;
+        return JSObject::defineGeneric(cx, receiver, id, vp, nullptr, nullptr, attrs);
+    }
+
+    // Step 6.
+    MOZ_ASSERT(ownDesc.isAccessorDescriptor());
+    RootedObject setter(cx);
+    if (ownDesc.hasSetterObject())
+        setter = ownDesc.setterObject();
+    if (!setter)
+        return js_ReportGetterOnlyAssignment(cx, strict);
+    RootedValue setterValue(cx, ObjectValue(*setter));
+    return InvokeGetterOrSetter(cx, receiver, setterValue, 1, vp.address(), vp);
 }
 
 bool
 js::SetPropertyIgnoringNamedGetter(JSContext *cx, const BaseProxyHandler *handler,
                                    HandleObject proxy, HandleObject receiver,
                                    HandleId id, MutableHandle<PropertyDescriptor> desc,
                                    bool descIsOwn, bool strict, MutableHandleValue vp)
 {
--- a/js/src/proxy/ScriptedIndirectProxyHandler.cpp
+++ b/js/src/proxy/ScriptedIndirectProxyHandler.cpp
@@ -282,21 +282,41 @@ ScriptedIndirectProxyHandler::set(JSCont
     JS::AutoValueArray<3> argv(cx);
     argv[0].setObjectOrNull(receiver);
     argv[1].set(idv);
     argv[2].set(vp);
     RootedValue fval(cx);
     if (!GetDerivedTrap(cx, handler, cx->names().set, &fval))
         return false;
     if (!IsCallable(fval))
-        return BaseProxyHandler::set(cx, proxy, receiver, id, strict, vp);
+        return derivedSet(cx, proxy, receiver, id, strict, vp);
     return Trap(cx, handler, fval, 3, argv.begin(), &idv);
 }
 
 bool
+ScriptedIndirectProxyHandler::derivedSet(JSContext *cx, HandleObject proxy, HandleObject receiver,
+                                         HandleId id, bool strict, MutableHandleValue vp) const
+{
+    // Find an own or inherited property. The code here is strange for maximum
+    // backward compatibility with earlier code written before ES6 and before
+    // SetPropertyIgnoringNamedGetter.
+    Rooted<PropertyDescriptor> desc(cx);
+    if (!getOwnPropertyDescriptor(cx, proxy, id, &desc))
+        return false;
+    bool descIsOwn = desc.object() != nullptr;
+    if (!descIsOwn) {
+        if (!getPropertyDescriptor(cx, proxy, id, &desc))
+            return false;
+    }
+
+    return SetPropertyIgnoringNamedGetter(cx, this, proxy, receiver, id, &desc, descIsOwn, strict,
+                                          vp);
+}
+
+bool
 ScriptedIndirectProxyHandler::getOwnEnumerablePropertyKeys(JSContext *cx, HandleObject proxy,
                                                            AutoIdVector &props) const
 {
     RootedObject handler(cx, GetIndirectProxyHandlerObject(proxy));
     RootedValue value(cx);
     if (!GetDerivedTrap(cx, handler, cx->names().keys, &value))
         return false;
     if (!IsCallable(value))
--- a/js/src/proxy/ScriptedIndirectProxyHandler.h
+++ b/js/src/proxy/ScriptedIndirectProxyHandler.h
@@ -47,16 +47,20 @@ class ScriptedIndirectProxyHandler : pub
                          MutableHandleObject objp) const MOZ_OVERRIDE;
     virtual bool nativeCall(JSContext *cx, IsAcceptableThis test, NativeImpl impl,
                             CallArgs args) const MOZ_OVERRIDE;
     virtual JSString *fun_toString(JSContext *cx, HandleObject proxy, unsigned indent) const MOZ_OVERRIDE;
     virtual bool isScripted() const MOZ_OVERRIDE { return true; }
 
     static const char family;
     static const ScriptedIndirectProxyHandler singleton;
+
+private:
+    bool derivedSet(JSContext *cx, HandleObject proxy, HandleObject receiver, HandleId id,
+                    bool strict, MutableHandleValue vp) const;
 };
 
 /* Derived class to handle Proxy.createFunction() */
 class CallableScriptedIndirectProxyHandler : public ScriptedIndirectProxyHandler
 {
   public:
     CallableScriptedIndirectProxyHandler() : ScriptedIndirectProxyHandler() { }
     virtual bool call(JSContext *cx, HandleObject proxy, const CallArgs &args) const MOZ_OVERRIDE;