bug 580128 - The rewrap hook needs to know what parent to use when creating wrappers. r=gal

js/src/jscompartment.cpp

@@ -189,19 +189,6 @@ JSCompartment::wrap(JSContext *cx, Value *vp)
     if (!wrap(cx, &proto))
         return false;
 
-    /*
-     * We hand in the original wrapped object into the wrap hook to allow
-     * the wrap hook to reason over what wrappers are currently applied
-     * to the object.
-     */
-    JSObject *wrapper = cx->runtime->wrapObjectCallback(cx, obj, proto, flags);
-    if (!wrapper)
-        return false;
-    wrapper->setProto(proto);
-    vp->setObject(*wrapper);
-    if (!crossCompartmentWrappers.put(wrapper->getProxyPrivate(), *vp))
-        return false;
-
     /*
      * Wrappers should really be parented to the wrapped parent of the wrapped
      * object, but in that case a wrapped global object would have a NULL
@@ -219,6 +206,29 @@ JSCompartment::wrap(JSContext *cx, Value *vp)
             return false;
     }
 
+    /*
+     * We hand in the original wrapped object into the wrap hook to allow
+     * the wrap hook to reason over what wrappers are currently applied
+     * to the object.
+     */
+    JSObject *wrapper = cx->runtime->wrapObjectCallback(cx, obj, proto, global, flags);
+    if (!wrapper)
+        return false;
+
+    vp->setObject(*wrapper);
+
+    /*
+     * If the returned "wrapper" is not a proxy, then we were attempting to
+     * wrap an XPConnect "holder" object and the actual wrapped object was
+     * in our compartment.
+     */
+    if (!wrapper->isProxy())
+        return true;
+
+    wrapper->setProto(proto);
+    if (!crossCompartmentWrappers.put(wrapper->getProxyPrivate(), *vp))
+        return false;
+
     wrapper->setParent(global);
     return true;
 }

js/src/jspubtd.h

@@ -559,7 +559,8 @@ typedef JSBool
  * destination compartment.
  */
 typedef JSObject *
-(* JSWrapObjectCallback)(JSContext *cx, JSObject *obj, JSObject *proto, uintN flags);
+(* JSWrapObjectCallback)(JSContext *cx, JSObject *obj, JSObject *proto, JSObject *parent,
+                         uintN flags);
 
 typedef enum {
     JSCOMPARTMENT_NEW, /* XXX Does it make sense to have a NEW? */

js/src/jswrapper.cpp

@@ -287,7 +287,8 @@ JSWrapper::New(JSContext *cx, JSObject *obj, JSObject *proto, JSObject *parent,
 namespace js {
 
 extern JSObject *
-TransparentObjectWrapper(JSContext *cx, JSObject *obj, JSObject *wrappedProto, uintN flags)
+TransparentObjectWrapper(JSContext *cx, JSObject *obj, JSObject *wrappedProto, JSObject *parent,
+                         uintN flags)
 {
     // Allow wrapping outer window proxies.
     JS_ASSERT(!obj->isWrapper() || obj->getClass()->ext.innerObject);

js/src/jswrapper.h

@@ -173,7 +173,8 @@ class AutoCompartment
 };
 
 extern JSObject *
-TransparentObjectWrapper(JSContext *cx, JSObject *obj, JSObject *wrappedProto, uintN flags);
+TransparentObjectWrapper(JSContext *cx, JSObject *obj, JSObject *wrappedProto, JSObject *parent,
+                         uintN flags);
 
 }
 

js/src/xpconnect/wrappers/WrapperFactory.cpp

@@ -65,7 +65,8 @@ JSWrapper WaiveXrayWrapperWrapper(WrapperFactory::WAIVE_XRAY_WRAPPER_FLAG);
 JSCrossCompartmentWrapper XrayWrapperWaivedWrapper(WrapperFactory::WAIVE_XRAY_WRAPPER_FLAG);
 
 JSObject *
-WrapperFactory::Rewrap(JSContext *cx, JSObject *obj, JSObject *wrappedProto, uintN flags)
+WrapperFactory::Rewrap(JSContext *cx, JSObject *obj, JSObject *wrappedProto, JSObject *parent,
+                       uintN flags)
 {
     NS_ASSERTION(!obj->isWrapper() || obj->getClass()->ext.innerObject,
                  "wrapped object passed to rewrap");

js/src/xpconnect/wrappers/WrapperFactory.h

@@ -57,6 +57,7 @@ class WrapperFactory {
     static JSObject *Rewrap(JSContext *cx,
                             JSObject *obj,
                             JSObject *wrappedProto,
+                            JSObject *parent,
                             uintN flags);
 };