Bug 975277 - Rewrite Proxy::set logic. r=efaust
authorBobby Holley <bobbyholley@gmail.com>
Fri, 21 Feb 2014 15:55:31 -0800
changeset 170336 5db44a9eece27d6aa7345fcd97a3e2b33fa77629
parent 170335 65192746bdee99ded19f577058d4b66d00545d3c
child 170337 fd04a8b8ccc489af9a2a9ad10f639f35dd0857ba
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewersefaust
bugs975277
milestone30.0a1
Bug 975277 - Rewrite Proxy::set logic. r=efaust The current logic ends up invoking BaseProxyHandler::set in various cases that will cause it to invoke handler->getPropertyDescriptor, which is verboten for mHasPrototype proxies.
js/src/jsproxy.cpp
--- a/js/src/jsproxy.cpp
+++ b/js/src/jsproxy.cpp
@@ -2538,38 +2538,35 @@ bool
 Proxy::set(JSContext *cx, HandleObject proxy, HandleObject receiver, HandleId id, bool strict,
            MutableHandleValue vp)
 {
     JS_CHECK_RECURSION(cx, return false);
     BaseProxyHandler *handler = proxy->as<ProxyObject>().handler();
     AutoEnterPolicy policy(cx, handler, proxy, id, BaseProxyHandler::SET, true);
     if (!policy.allowed())
         return policy.returnValue();
-    if (handler->hasPrototype()) {
-        // If we're using a prototype, we still want to use the proxy trap unless
-        // we have a non-own property with a setter.
-        bool hasOwn;
-        if (!handler->hasOwn(cx, proxy, id, &hasOwn))
-            return false;
-        if (!hasOwn) {
-            RootedObject proto(cx);
-            // Proxies might still use the normal prototype mechanism, rather than
-            // a hook, so query the engine proper
-            if (!JSObject::getProto(cx, proxy, &proto))
-                return false;
-            if (proto) {
-                Rooted<PropertyDescriptor> desc(cx);
-                if (!JS_GetPropertyDescriptorById(cx, proto, id, 0, &desc))
-                    return false;
-                if (desc.object() && desc.setter())
-                    return JSObject::setGeneric(cx, proto, receiver, id, vp, strict);
-            }
-        }
-    }
-    return handler->set(cx, proxy, receiver, id, strict, vp);
+
+    // If the proxy doesn't require that we consult its prototype for the
+    // non-own cases, we can sink to the |set| trap.
+    if (!handler->hasPrototype())
+        return handler->set(cx, proxy, receiver, id, strict, vp);
+
+    // If we have an existing (own or non-own) property with a setter, we want
+    // to invoke that.
+    Rooted<PropertyDescriptor> desc(cx);
+    if (!Proxy::getPropertyDescriptor(cx, proxy, id, &desc, JSRESOLVE_ASSIGNING))
+        return false;
+    if (desc.object() && desc.setter() && desc.setter() != JS_StrictPropertyStub)
+        return CallSetter(cx, receiver, id, desc.setter(), desc.attributes(), desc.shortid(), strict, vp);
+
+    // Ok. Either there was no pre-existing property, or it was a value prop
+    // that we're going to shadow. Make a property descriptor and define it.
+    Rooted<PropertyDescriptor> newDesc(cx);
+    newDesc.value().set(vp);
+    return handler->defineProperty(cx, receiver, id, &newDesc);
 }
 
 bool
 Proxy::keys(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);