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.

--HG--
extra : rebase_source : bb4860a0eb6f32ae64c170dfe1c8d716d2a25bac

js/src/jsapi.h

@@ -3031,6 +3031,13 @@ class PropertyDescriptorOperations
     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);
     }

js/src/proxy/BaseProxyHandler.cpp

@@ -78,20 +78,71 @@ BaseProxyHandler::set(JSContext *cx, HandleObject proxy, HandleObject receiver,
 {
     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/src/proxy/ScriptedIndirectProxyHandler.cpp

@@ -287,10 +287,30 @@ ScriptedIndirectProxyHandler::set(JSContext *cx, HandleObject proxy, HandleObjec
     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

js/src/proxy/ScriptedIndirectProxyHandler.h

@@ -52,6 +52,10 @@ class ScriptedIndirectProxyHandler : public BaseProxyHandler
 
     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() */