Bug 706301 - Don't cache own properties on XrayProxy. r=mrbkap
authorBobby Holley <bobbyholley@gmail.com>
Tue, 06 Dec 2011 11:05:26 -0800
changeset 82873 1e14abc06ad782ef80c28278f1ab7cea81fb526f
parent 82872 0a5f66d5d8e47c24c1e4988c2d25d571650c7a63
child 82874 d6049bb1770de30273c455a10d1a08ff5af00930
push idunknown
push userunknown
push dateunknown
reviewersmrbkap
bugs706301
milestone11.0a1
Bug 706301 - Don't cache own properties on XrayProxy. r=mrbkap
js/xpconnect/wrappers/WrapperFactory.cpp
js/xpconnect/wrappers/XrayWrapper.cpp
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -392,18 +392,16 @@ WrapperFactory::Rewrap(JSContext *cx, JS
             }
         }
     }
 
     JSObject *wrapperObj = Wrapper::New(cx, obj, wrappedProto, parent, wrapper);
     if (!wrapperObj || !xrayHolder)
         return wrapperObj;
 
-    // NB: The fact that the only wrappers to use ProxyExtra are XrayWrappers
-    // is relied on by XPCNativeWrapper.unwrap.
     js::SetProxyExtra(wrapperObj, 0, js::ObjectValue(*xrayHolder));
     return wrapperObj;
 }
 
 typedef FilteringWrapper<XrayWrapper<SameCompartmentSecurityWrapper>,
                          SameOriginOrCrossOriginAccessiblePropertiesOnly> LW;
 
 bool
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -1078,16 +1078,19 @@ XrayProxy::XrayProxy(uintN flags)
   : XrayWrapper<CrossCompartmentWrapper>(flags)
 {
 }
 
 XrayProxy::~XrayProxy()
 {
 }
 
+// The 'holder' here isn't actually of [[Class]] HolderClass like those used by
+// XrayWrapper. Instead, it's a funny hybrid of the 'expando' and 'holder'
+// properties. However, we store it in the same slot. Exercise caution.
 static JSObject *
 GetHolderObject(JSContext *cx, JSObject *wrapper, bool createHolder = true)
 {
     if (!js::GetProxyExtra(wrapper, 0).isUndefined())
         return &js::GetProxyExtra(wrapper, 0).toObject();
 
     JSObject *obj = JS_NewObjectWithGivenProto(cx, nsnull, nsnull,
                                                JS_GetGlobalForObject(cx, wrapper));
@@ -1125,30 +1128,36 @@ XrayProxy::getPropertyDescriptor(JSConte
                 return false;
         }
 
         if (desc->obj)
             desc->obj = wrapper;
         return JS_WrapPropertyDescriptor(cx, desc);
     }
 
+    // We don't want to cache own properties on our holder. So we first do this
+    // call, and return if we find it (without caching). If we don't find it,
+    // we check the cache and do a full resolve (caching any result).
+    if (!js::GetProxyHandler(obj)->getOwnPropertyDescriptor(cx, wrapper, id, set, desc))
+        return false;
+    if (desc->obj) {
+        desc->obj = wrapper;
+        return true;
+    }
+
+    // Now that we're sure this isn't an own property, look up cached non-own
+    // properties before calling all the way through.
     if (!JS_GetPropertyDescriptorById(cx, holder, id, JSRESOLVE_QUALIFIED, desc))
         return false;
     if (desc->obj) {
         desc->obj = wrapper;
         return true;
     }
 
-    if (!js::GetProxyHandler(obj)->getOwnPropertyDescriptor(cx, wrapper, id, set, desc))
-        return false;
-    if (desc->obj) {
-        desc->obj = wrapper;
-        return true;
-    }
-
+    // Nothing in the cache. Call through, and cache the result.
     if (!js::GetProxyHandler(obj)->getPropertyDescriptor(cx, wrapper, id, set, desc))
         return false;
 
     if (!desc->obj) {
         if (id != nsXPConnect::GetRuntimeInstance()->GetStringID(XPCJSRuntime::IDX_TO_STRING))
             return true;
 
         JSFunction *toString = JS_NewFunction(cx, XrayToString, 0, 0, holder, "toString");
@@ -1206,20 +1215,22 @@ XrayProxy::getOwnPropertyDescriptor(JSCo
     if (desc->obj) {
         desc->obj = wrapper;
         return true;
     }
 
     if (!js::GetProxyHandler(obj)->getOwnPropertyDescriptor(cx, wrapper, id, set, desc))
         return false;
 
-    desc->obj = wrapper;
+    // The 'not found' property descriptor has obj == NULL.
+    if (desc->obj)
+      desc->obj = wrapper;
 
-    return JS_DefinePropertyById(cx, holder, id, desc->value, desc->getter, desc->setter,
-                                 desc->attrs);
+    // Own properties don't get cached on the holder. Just return.
+    return true;
 }
 
 bool
 XrayProxy::defineProperty(JSContext *cx, JSObject *wrapper, jsid id,
                           js::PropertyDescriptor *desc)
 {
     JSObject *holder = GetHolderObject(cx, wrapper);
     if (!holder)