Bug 975277 - Rewrite Proxy::set logic. r=efaust
authorBobby Holley <bobbyholley@gmail.com>
Fri, 21 Feb 2014 15:55:31 -0800
changeset 169974 5db44a9eece2
parent 169973 65192746bdee
child 169975 fd04a8b8ccc4
push id40118
push userbobbyholley@gmail.com
push dateSat, 22 Feb 2014 00:03:42 +0000
treeherdermozilla-inbound@84904662e2d5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersefaust
bugs975277
milestone30.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 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);