Bug 733984 - Stop specializing createHolder, and simplify holder creation in WrapperFactory::Rewrap. r=mrbkap

js/xpconnect/wrappers/WrapperFactory.cpp

@@ -287,7 +287,7 @@ WrapperFactory::Rewrap(JSContext *cx, JSObject *obj, JSObject *wrappedProto, JSO
 
     JSCompartment *origin = js::GetObjectCompartment(obj);
     JSCompartment *target = js::GetContextCompartment(cx);
-    JSObject *xrayHolder = nsnull;
+    bool usingXray = false;
 
     Wrapper *wrapper;
     CompartmentPrivate *targetdata =
@@ -320,10 +320,8 @@ WrapperFactory::Rewrap(JSContext *cx, JSObject *obj, JSObject *wrappedProto, JSO
                         wrapper = &XrayProxy::singleton;
                     } else {
                         typedef XrayWrapper<CrossCompartmentSecurityWrapper> Xray;
+                        usingXray = true;
                         wrapper = &Xray::singleton;
-                        xrayHolder = Xray::createHolder(cx, obj, parent);
-                        if (!xrayHolder)
-                            return nsnull;
                     }
                 } else {
                     wrapper = &NoWaiverWrapper::singleton;
@@ -344,11 +342,9 @@ WrapperFactory::Rewrap(JSContext *cx, JSObject *obj, JSObject *wrappedProto, JSO
             (wn = GetWrappedNative(cx, obj)) &&
             wn->HasProto() && wn->GetProto()->ClassIsDOMObject()) {
             typedef XrayWrapper<CrossCompartmentSecurityWrapper> Xray;
+            usingXray = true;
             wrapper = &FilteringWrapper<Xray,
                                         CrossOriginAccessiblePropertiesOnly>::singleton;
-            xrayHolder = Xray::createHolder(cx, obj, parent);
-            if (!xrayHolder)
-                return nsnull;
         } else {
             wrapper = &FilteringWrapper<CrossCompartmentSecurityWrapper,
                                         ExposedPropertiesOnly>::singleton;
@@ -365,10 +361,8 @@ WrapperFactory::Rewrap(JSContext *cx, JSObject *obj, JSObject *wrappedProto, JSO
                 wrapper = &XrayProxy::singleton;
             } else {
                 typedef XrayWrapper<CrossCompartmentWrapper> Xray;
+                usingXray = true;
                 wrapper = &Xray::singleton;
-                xrayHolder = Xray::createHolder(cx, obj, parent);
-                if (!xrayHolder)
-                    return nsnull;
             }
         } else {
             wrapper = &CrossCompartmentWrapper::singleton;
@@ -391,6 +385,7 @@ WrapperFactory::Rewrap(JSContext *cx, JSObject *obj, JSObject *wrappedProto, JSO
                                             CrossOriginAccessiblePropertiesOnly>::singleton;
             } else {
                 typedef XrayWrapper<CrossCompartmentSecurityWrapper> Xray;
+                usingXray = true;
 
                 // Location objects can become same origin after navigation, so we might
                 // have to grant transparent access later on.
@@ -401,18 +396,17 @@ WrapperFactory::Rewrap(JSContext *cx, JSObject *obj, JSObject *wrappedProto, JSO
                     wrapper = &FilteringWrapper<Xray,
                         CrossOriginAccessiblePropertiesOnly>::singleton;
                 }
-
-                xrayHolder = Xray::createHolder(cx, obj, parent);
-                if (!xrayHolder)
-                    return nsnull;
             }
         }
     }
 
     JSObject *wrapperObj = Wrapper::New(cx, obj, wrappedProto, parent, wrapper);
-    if (!wrapperObj || !xrayHolder)
+    if (!wrapperObj || !usingXray)
         return wrapperObj;
 
+    JSObject *xrayHolder = XrayUtils::createHolder(cx, obj, parent);
+    if (!xrayHolder)
+        return nsnull;
     js::SetProxyExtra(wrapperObj, 0, js::ObjectValue(*xrayHolder));
     return wrapperObj;
 }
@@ -430,7 +424,7 @@ WrapperFactory::IsLocationObject(JSObject *obj)
 JSObject *
 WrapperFactory::WrapLocationObject(JSContext *cx, JSObject *obj)
 {
-    JSObject *xrayHolder = LW::createHolder(cx, obj, js::GetObjectParent(obj));
+    JSObject *xrayHolder = XrayUtils::createHolder(cx, obj, js::GetObjectParent(obj));
     if (!xrayHolder)
         return nsnull;
     JSObject *wrapperObj = Wrapper::New(cx, obj, js::GetObjectProto(obj), js::GetObjectParent(obj),

js/xpconnect/wrappers/XrayWrapper.cpp

@@ -103,6 +103,9 @@ holder_get(JSContext *cx, JSObject *holder, jsid id, jsval *vp);
 static JSBool
 holder_set(JSContext *cx, JSObject *holder, jsid id, JSBool strict, jsval *vp);
 
+
+static XPCWrappedNative *GetWrappedNative(JSObject *obj);
+
 namespace XrayUtils {
 
 JSClass HolderClass = {
@@ -112,6 +115,35 @@ JSClass HolderClass = {
     JS_EnumerateStub,       JS_ResolveStub,  JS_ConvertStub
 };
 
+JSObject *
+createHolder(JSContext *cx, JSObject *wrappedNative, JSObject *parent)
+{
+    JSObject *holder = JS_NewObjectWithGivenProto(cx, &HolderClass, nsnull, parent);
+    if (!holder)
+        return nsnull;
+
+    CompartmentPrivate *priv =
+        (CompartmentPrivate *)JS_GetCompartmentPrivate(js::GetObjectCompartment(holder));
+    JSObject *inner = JS_ObjectToInnerObject(cx, wrappedNative);
+    XPCWrappedNative *wn = GetWrappedNative(inner);
+    Value expando = ObjectOrNullValue(priv->LookupExpandoObject(wn));
+
+    // A note about ownership: the holder has a direct pointer to the wrapped
+    // native that we're wrapping. Normally, we'd have to AddRef the pointer
+    // so that it doesn't have to be collected, but then we'd have to tell the
+    // cycle collector. Fortunately for us, we know that the Xray wrapper
+    // itself has a reference to the flat JS object which will hold the
+    // wrapped native alive. Furthermore, the reachability of that object and
+    // the associated holder are exactly the same, so we can use that for our
+    // strong reference.
+    JS_ASSERT(IS_WN_WRAPPER(wrappedNative) ||
+              js::GetObjectClass(wrappedNative)->ext.innerObject);
+    js::SetReservedSlot(holder, JSSLOT_WN, PrivateValue(wn));
+    js::SetReservedSlot(holder, JSSLOT_RESOLVING, PrivateValue(NULL));
+    js::SetReservedSlot(holder, JSSLOT_EXPANDO, expando);
+    return holder;
+}
+
 }
 
 using namespace XrayUtils;
@@ -1042,35 +1074,6 @@ XrayWrapper<Base>::construct(JSContext *cx, JSObject *wrapper, unsigned argc,
     return true;
 }
 
-template <typename Base>
-JSObject *
-XrayWrapper<Base>::createHolder(JSContext *cx, JSObject *wrappedNative, JSObject *parent)
-{
-    JSObject *holder = JS_NewObjectWithGivenProto(cx, &HolderClass, nsnull, parent);
-    if (!holder)
-        return nsnull;
-
-    CompartmentPrivate *priv =
-        (CompartmentPrivate *)JS_GetCompartmentPrivate(js::GetObjectCompartment(holder));
-    JSObject *inner = JS_ObjectToInnerObject(cx, wrappedNative);
-    XPCWrappedNative *wn = GetWrappedNative(inner);
-    Value expando = ObjectOrNullValue(priv->LookupExpandoObject(wn));
-
-    // A note about ownership: the holder has a direct pointer to the wrapped
-    // native that we're wrapping. Normally, we'd have to AddRef the pointer
-    // so that it doesn't have to be collected, but then we'd have to tell the
-    // cycle collector. Fortunately for us, we know that the Xray wrapper
-    // itself has a reference to the flat JS object which will hold the
-    // wrapped native alive. Furthermore, the reachability of that object and
-    // the associated holder are exactly the same, so we can use that for our
-    // strong reference.
-    JS_ASSERT(IS_WN_WRAPPER(wrappedNative) ||
-              js::GetObjectClass(wrappedNative)->ext.innerObject);
-    js::SetReservedSlot(holder, JSSLOT_WN, PrivateValue(wn));
-    js::SetReservedSlot(holder, JSSLOT_RESOLVING, PrivateValue(NULL));
-    js::SetReservedSlot(holder, JSSLOT_EXPANDO, expando);
-    return holder;
-}
 
 XrayProxy::XrayProxy(unsigned flags)
   : XrayWrapper<CrossCompartmentWrapper>(flags)

js/xpconnect/wrappers/XrayWrapper.h

@@ -52,6 +52,8 @@ namespace XrayUtils {
 
 extern JSClass HolderClass;
 
+JSObject *createHolder(JSContext *cx, JSObject *wrappedNative, JSObject *parent);
+
 bool
 IsTransparent(JSContext *cx, JSObject *wrapper);
 
@@ -94,8 +96,6 @@ class XrayWrapper : public Base {
     virtual bool construct(JSContext *cx, JSObject *wrapper,
                            unsigned argc, js::Value *argv, js::Value *rval);
 
-    static JSObject *createHolder(JSContext *cx, JSObject *wrappedNative, JSObject *parent);
-
     static XrayWrapper singleton;
 
   private: