Fix for bug 461563 (Allow WrapNative to return a jsval without the wrapper). r/sr=jst.
authorPeter Van der Beken <peterv@propagandism.org>
Tue, 02 Dec 2008 11:07:34 +0100
changeset 22147 5edef3a148a5f91b2b9b4b5bf536fb845f46750b
parent 22146 03b31906e2893f2b5515e50652bb7552c8eb1c7a
child 22148 def8ca28ac75c62451916c3ea0332d098f82fcc1
push id3825
push userpvanderbeken@mozilla.com
push dateTue, 02 Dec 2008 10:07:47 +0000
treeherdermozilla-central@5edef3a148a5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs461563
milestone1.9.2a1pre
Fix for bug 461563 (Allow WrapNative to return a jsval without the wrapper). r/sr=jst.
dom/src/base/nsDOMClassInfo.cpp
dom/src/base/nsDOMClassInfo.h
js/src/xpconnect/idl/nsIXPConnect.idl
js/src/xpconnect/src/nsXPConnect.cpp
js/src/xpconnect/src/xpcconvert.cpp
js/src/xpconnect/src/xpcprivate.h
js/src/xpconnect/src/xpcquickstubs.cpp
js/src/xpconnect/src/xpcwrappedjsclass.cpp
--- a/dom/src/base/nsDOMClassInfo.cpp
+++ b/dom/src/base/nsDOMClassInfo.cpp
@@ -1631,39 +1631,28 @@ nsDOMClassInfo::DefineStaticJSVals(JSCon
 }
 
 // static
 nsresult
 nsDOMClassInfo::WrapNative(JSContext *cx, JSObject *scope, nsISupports *native,
                            const nsIID& aIID, jsval *vp,
                            nsIXPConnectJSObjectHolder **aHolder)
 {
-  *aHolder = nsnull;
-  
   if (!native) {
+    NS_ASSERTION(!aHolder || !*aHolder, "*aHolder should be null!");
+
     *vp = JSVAL_NULL;
 
     return NS_OK;
   }
 
   NS_ENSURE_TRUE(sXPConnect, NS_ERROR_UNEXPECTED);
 
-  nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
-  nsresult rv = sXPConnect->WrapNative(cx, ::JS_GetGlobalForObject(cx, scope),
-                                       native, aIID, getter_AddRefs(holder));
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  JSObject* obj = nsnull;
-  rv = holder->GetJSObject(&obj);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  *vp = OBJECT_TO_JSVAL(obj);
-  holder.swap(*aHolder);
-
-  return rv;
+  return sXPConnect->WrapNativeToJSVal(cx, ::JS_GetGlobalForObject(cx, scope),
+                                       native, aIID, vp, aHolder);
 }
 
 // static
 nsresult
 nsDOMClassInfo::ThrowJSException(JSContext *cx, nsresult aResult)
 {
   JSAutoRequest ar(cx);
 
@@ -7719,19 +7708,17 @@ nsArraySH::GetProperty(nsIXPConnectWrapp
 
     // Make sure rv == NS_OK here, so GetItemAt implementations that never fail
     // don't have to set rv.
     rv = NS_OK;
     nsISupports* array_item = GetItemAt(wrapper->Native(), n, &rv);
     NS_ENSURE_SUCCESS(rv, rv);
 
     if (array_item) {
-      nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
-      rv = WrapNative(cx, obj, array_item, NS_GET_IID(nsISupports), vp,
-                      getter_AddRefs(holder));
+      rv = WrapNative(cx, obj, array_item, NS_GET_IID(nsISupports), vp);
       NS_ENSURE_SUCCESS(rv, rv);
 
       rv = NS_SUCCESS_I_DID_SOMETHING;
     }
   }
 
   return rv;
 }
@@ -7856,19 +7843,17 @@ nsNamedArraySH::GetProperty(nsIXPConnect
 {
   if (JSVAL_IS_STRING(id) && !ObjectIsNativeWrapper(cx, obj)) {
     nsresult rv = NS_OK;
     nsISupports* item = GetNamedItem(wrapper->Native(), nsDependentJSString(id),
                                      &rv);
     NS_ENSURE_SUCCESS(rv, rv);
 
     if (item) {
-      nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
-      rv = WrapNative(cx, obj, item, NS_GET_IID(nsISupports), vp,
-                      getter_AddRefs(holder));
+      rv = WrapNative(cx, obj, item, NS_GET_IID(nsISupports), vp);
       NS_ENSURE_SUCCESS(rv, rv);
 
       rv = NS_SUCCESS_I_DID_SOMETHING;
     }
 
     // Don't fall through to nsArraySH::GetProperty() here
     return rv;
   }
@@ -8503,19 +8488,17 @@ nsHTMLDocumentSH::DocumentAllGetProperty
 
     nsCOMPtr<nsIDOMNode> node;
     nodeList->Item(JSVAL_TO_INT(id), getter_AddRefs(node));
 
     result = node;
   }
 
   if (result) {
-    nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
-    rv = nsDOMClassInfo::WrapNative(cx, obj, result, NS_GET_IID(nsISupports),
-                                    vp, getter_AddRefs(holder));
+    rv = WrapNative(cx, obj, result, NS_GET_IID(nsISupports), vp);
     if (NS_FAILED(rv)) {
       nsDOMClassInfo::ThrowJSException(cx, rv);
 
       return JS_FALSE;
     }
   } else {
     *vp = JSVAL_VOID;
   }
@@ -8937,19 +8920,17 @@ nsHTMLDocumentSH::GetProperty(nsIXPConne
     nsCOMPtr<nsISupports> result;
 
     JSAutoRequest ar(cx);
 
     nsresult rv = ResolveImpl(cx, wrapper, id, getter_AddRefs(result));
     NS_ENSURE_SUCCESS(rv, rv);
 
     if (result) {
-      nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
-      rv = WrapNative(cx, obj, result, NS_GET_IID(nsISupports), vp,
-                      getter_AddRefs(holder));
+      rv = WrapNative(cx, obj, result, NS_GET_IID(nsISupports), vp);
       if (NS_SUCCEEDED(rv)) {
         rv = NS_SUCCESS_I_DID_SOMETHING;
       }
       return rv;
     }
   }
 
   return nsDocumentSH::GetProperty(wrapper, cx, obj, id, vp, _retval);
@@ -9079,33 +9060,29 @@ nsHTMLFormElementSH::GetProperty(nsIXPCo
       nsCOMPtr<nsISupports> result;
 
       JSString *str = JSVAL_TO_STRING(id);
       FindNamedItem(form, str, getter_AddRefs(result));
 
       if (result) {
         // Wrap result, result can be either an element or a list of
         // elements
-        nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
-        nsresult rv = WrapNative(cx, obj, result, NS_GET_IID(nsISupports), vp,
-                                 getter_AddRefs(holder));
+        nsresult rv = WrapNative(cx, obj, result, NS_GET_IID(nsISupports), vp);
         return NS_FAILED(rv) ? rv : NS_SUCCESS_I_DID_SOMETHING;
       }
     }
   } else {
     PRInt32 n = GetArrayIndexFromId(cx, id);
 
     if (n >= 0) {
       nsCOMPtr<nsIFormControl> control;
       form->GetElementAt(n, getter_AddRefs(control));
 
       if (control) {
-        nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
-        nsresult rv = WrapNative(cx, obj, control, NS_GET_IID(nsISupports), vp,
-                                 getter_AddRefs(holder));
+        nsresult rv = WrapNative(cx, obj, control, NS_GET_IID(nsISupports), vp);
         return NS_FAILED(rv) ? rv : NS_SUCCESS_I_DID_SOMETHING;
       }
     }
   }
 
   return nsHTMLElementSH::GetProperty(wrapper, cx, obj, id, vp, _retval);;
 }
 
@@ -9202,19 +9179,17 @@ nsHTMLSelectElementSH::GetProperty(nsIXP
     nsCOMPtr<nsIDOMHTMLOptionsCollection> options;
     s->GetOptions(getter_AddRefs(options));
 
     if (options) {
       nsCOMPtr<nsIDOMNode> node;
 
       options->Item(n, getter_AddRefs(node));
 
-      nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
-      rv = WrapNative(cx, obj, node, NS_GET_IID(nsIDOMNode), vp,
-                      getter_AddRefs(holder));
+      rv = WrapNative(cx, obj, node, NS_GET_IID(nsIDOMNode), vp);
       if (NS_SUCCEEDED(rv)) {
         rv = NS_SUCCESS_I_DID_SOMETHING;
       }
       return rv;
     }
   }
 
   return nsHTMLElementSH::GetProperty(wrapper, cx, obj, id, vp, _retval);;
--- a/dom/src/base/nsDOMClassInfo.h
+++ b/dom/src/base/nsDOMClassInfo.h
@@ -127,19 +127,19 @@ public:
   static nsIClassInfo *doCreate(nsDOMClassInfoData* aData)
   {
     return new nsDOMClassInfo(aData);
   }
 
   static nsresult WrapNative(JSContext *cx, JSObject *scope,
                              nsISupports *native, const nsIID& aIID,
                              jsval *vp,
-                             // aHolder keeps the jsval alive while
-                             // there's a ref to it
-                             nsIXPConnectJSObjectHolder** aHolder);
+                             // If non-null aHolder will keep the jsval alive
+                             // while there's a ref to it
+                             nsIXPConnectJSObjectHolder** aHolder = nsnull);
   static nsresult ThrowJSException(JSContext *cx, nsresult aResult);
 
   static nsresult InitDOMJSClass(JSContext *cx, JSObject *obj);
 
   static JSClass sDOMJSClass;
 
   /**
    * Get our JSClass pointer for the XPCNativeWrapper class
--- a/js/src/xpconnect/idl/nsIXPConnect.idl
+++ b/js/src/xpconnect/idl/nsIXPConnect.idl
@@ -400,17 +400,17 @@ interface nsIXPCFunctionThisTranslator :
 %{ C++
 // For use with the service manager
 // {CB6593E0-F9B2-11d2-BDD6-000064657374}
 #define NS_XPCONNECT_CID \
 { 0xcb6593e0, 0xf9b2, 0x11d2, \
     { 0xbd, 0xd6, 0x0, 0x0, 0x64, 0x65, 0x73, 0x74 } }
 %}
 
-[uuid(d4c6bc06-2a4f-4315-90ec-d12904aca046)]
+[uuid(f8bf005e-3700-411c-ba0c-e018075f22a4)]
 interface nsIXPConnect : nsISupports
 {
 %{ C++
   NS_DEFINE_STATIC_CID_ACCESSOR(NS_XPCONNECT_CID)
 %}
 
     void
     initClasses(in JSContextPtr aJSContext,
@@ -463,16 +463,29 @@ interface nsIXPConnect : nsISupports
     */
     nsIXPConnectJSObjectHolder
     wrapNative(in JSContextPtr aJSContext,
                in JSObjectPtr  aScope,
                in nsISupports  aCOMObj,
                in nsIIDRef     aIID);
 
     /**
+     * Same as wrapNative, but also returns the JSObject in aVal. C++ callers
+     * can pass in null for the aHolder argument, but in that case they must
+     * ensure that aVal is rooted.
+     */
+    void
+    wrapNativeToJSVal(in JSContextPtr aJSContext,
+                      in JSObjectPtr  aScope,
+                      in nsISupports  aCOMObj,
+                      in nsIIDRef     aIID,
+                      out JSVal       aVal,
+                      out nsIXPConnectJSObjectHolder aHolder);
+
+    /**
     * wrapJS will yield a new or previously existing xpcom interface pointer
     * to represent the JSObject passed in.
     *
     * This method now correctly deals with cases where the passed in JSObject
     * already has an associated xpcom interface for the cases:
     *  1) The JSObject has already been wrapped as a nsIXPConnectWrappedJS.
     *  2) The JSObject is in fact a nsIXPConnectWrappedNative and thus already
     *     has an underlying xpcom object.
--- a/js/src/xpconnect/src/nsXPConnect.cpp
+++ b/js/src/xpconnect/src/nsXPConnect.cpp
@@ -1096,36 +1096,38 @@ nsXPConnect::InitClassesWithNewWrappedGl
     JSObject* tempGlobal = JS_NewSystemObject(aJSContext, &xpcTempGlobalClass,
                                               nsnull, nsnull, system);
 
     if(!tempGlobal ||
        !JS_SetParent(aJSContext, tempGlobal, nsnull) ||
        !JS_SetPrototype(aJSContext, tempGlobal, nsnull))
         return UnexpectedFailure(NS_ERROR_FAILURE);
 
+    jsval v;
     nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
     {
         // Scope for our auto-marker; it just needs to keep tempGlobal alive
         // long enough for InitClasses and WrapNative to do their work
         AUTO_MARK_JSVAL(ccx, OBJECT_TO_JSVAL(tempGlobal));
 
         if(NS_FAILED(InitClasses(aJSContext, tempGlobal)))
             return UnexpectedFailure(NS_ERROR_FAILURE);
 
         nsresult rv;
-        if(!XPCConvert::NativeInterface2JSObject(ccx, getter_AddRefs(holder),
+        if(!XPCConvert::NativeInterface2JSObject(ccx, &v,
+                                                 getter_AddRefs(holder),
                                                  aCOMObj, &aIID, tempGlobal,
                                                  PR_FALSE, OBJ_IS_GLOBAL, &rv))
             return UnexpectedFailure(rv);
 
         NS_ASSERTION(NS_SUCCEEDED(rv) && holder, "Didn't wrap properly");
     }
 
-    JSObject* globalJSObj;
-    if(NS_FAILED(holder->GetJSObject(&globalJSObj)) || !globalJSObj)
+    JSObject* globalJSObj = JSVAL_TO_OBJECT(v);
+    if(!globalJSObj)
         return UnexpectedFailure(NS_ERROR_FAILURE);
 
     if(aFlags & nsIXPConnect::FLAG_SYSTEM_GLOBAL_OBJECT)
         NS_ASSERTION(JS_IsSystemObject(aJSContext, globalJSObj), "huh?!");
 
     // voodoo to fixup scoping and parenting...
 
     JS_SetParent(aJSContext, globalJSObj, nsnull);
@@ -1175,39 +1177,52 @@ nsXPConnect::InitClassesWithNewWrappedGl
 }
 
 /* nsIXPConnectJSObjectHolder wrapNative (in JSContextPtr aJSContext, in JSObjectPtr aScope, in nsISupports aCOMObj, in nsIIDRef aIID); */
 NS_IMETHODIMP
 nsXPConnect::WrapNative(JSContext * aJSContext,
                         JSObject * aScope,
                         nsISupports *aCOMObj,
                         const nsIID & aIID,
-                        nsIXPConnectJSObjectHolder **_retval)
+                        nsIXPConnectJSObjectHolder **aHolder)
+{
+    NS_ASSERTION(aHolder, "bad param");
+
+    jsval v;
+    return WrapNativeToJSVal(aJSContext, aScope, aCOMObj, aIID, &v, aHolder);
+}
+
+/* void wrapNativeToJSVal (in JSContextPtr aJSContext, in JSObjectPtr aScope, in nsISupports aCOMObj, in nsIIDRef aIID, out JSVal aVal, out nsIXPConnectJSObjectHolder aHolder); */
+NS_IMETHODIMP
+nsXPConnect::WrapNativeToJSVal(JSContext * aJSContext,
+                               JSObject * aScope,
+                               nsISupports *aCOMObj,
+                               const nsIID & aIID,
+                               jsval *aVal,
+                               nsIXPConnectJSObjectHolder **aHolder)
 {
     NS_ASSERTION(aJSContext, "bad param");
     NS_ASSERTION(aScope, "bad param");
     NS_ASSERTION(aCOMObj, "bad param");
-    NS_ASSERTION(_retval, "bad param");
 
-    *_retval = nsnull;
+    if(aHolder)
+        *aHolder = nsnull;
 
     XPCCallContext ccx(NATIVE_CALLER, aJSContext);
     if(!ccx.IsValid())
         return UnexpectedFailure(NS_ERROR_FAILURE);
 
     nsresult rv;
-    if(!XPCConvert::NativeInterface2JSObject(ccx, _retval, aCOMObj, &aIID,
+    if(!XPCConvert::NativeInterface2JSObject(ccx, aVal, aHolder, aCOMObj, &aIID,
                                              aScope, PR_FALSE,
                                              OBJ_IS_NOT_GLOBAL, &rv))
         return rv;
 
 #ifdef DEBUG
-    JSObject* returnObj;
-    (*_retval)->GetJSObject(&returnObj);
-    NS_ASSERTION(!XPCNativeWrapper::IsNativeWrapper(returnObj),
+    NS_ASSERTION(!XPCNativeWrapper::IsNativeWrapper(JSVAL_TO_OBJECT(*aVal)),
                  "Shouldn't be returning a native wrapper here");
 #endif
     
     return NS_OK;
 }
 
 /* void wrapJS (in JSContextPtr aJSContext, in JSObjectPtr aJSObj, in nsIIDRef aIID, [iid_is (aIID), retval] out nsQIResult result); */
 NS_IMETHODIMP
--- a/js/src/xpconnect/src/xpcconvert.cpp
+++ b/js/src/xpconnect/src/xpcconvert.cpp
@@ -462,34 +462,27 @@ XPCConvert::NativeData2JS(XPCCallContext
                     }
                     // else...
                     
                     // XXX The OBJ_IS_NOT_GLOBAL here is not really right. In
                     // fact, this code is depending on the fact that the
                     // global object will not have been collected, and
                     // therefore this NativeInterface2JSObject will not end up
                     // creating a new XPCNativeScriptableShared.
-                    nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
-                    if(!NativeInterface2JSObject(ccx, getter_AddRefs(holder),
-                                                 iface, iid, scope, PR_TRUE,
+                    if(!NativeInterface2JSObject(ccx, d, nsnull, iface, iid,
+                                                 scope, PR_TRUE,
                                                  OBJ_IS_NOT_GLOBAL, pErr))
                         return JS_FALSE;
 
-                    if(holder)
-                    {
-                        JSObject* jsobj;
-                        if(NS_FAILED(holder->GetJSObject(&jsobj)))
-                            return JS_FALSE;
 #ifdef DEBUG
-                        if(!STOBJ_GET_PARENT(jsobj))
-                            NS_ASSERTION(STOBJ_GET_CLASS(jsobj)->flags & JSCLASS_IS_GLOBAL,
-                                         "Why did we recreate this wrapper?");
+                    JSObject* jsobj = JSVAL_TO_OBJECT(*d);
+                    if(jsobj && !STOBJ_GET_PARENT(jsobj))
+                        NS_ASSERTION(STOBJ_GET_CLASS(jsobj)->flags & JSCLASS_IS_GLOBAL,
+                                     "Why did we recreate this wrapper?");
 #endif
-                        *d = OBJECT_TO_JSVAL(jsobj);
-                    }
                 }
                 break;
             }
         default:
             NS_ASSERTION(0, "bad type");
             return JS_FALSE;
         }
     }
@@ -1028,33 +1021,53 @@ XPCConvert::JSData2Native(XPCCallContext
         default:
             NS_ASSERTION(0, "bad type");
             return JS_FALSE;
         }
     }
     return JS_TRUE;
 }
 
+JSBool
+CreateHolderIfNeeded(XPCCallContext& ccx, JSObject* obj, jsval* d,
+                     nsIXPConnectJSObjectHolder** dest)
+{
+    if(dest)
+    {
+        XPCJSObjectHolder* objHolder = XPCJSObjectHolder::newHolder(ccx, obj);
+        if(!objHolder)
+            return JS_FALSE;
+        
+        NS_ADDREF(*dest = objHolder);
+    }
+
+    *d = OBJECT_TO_JSVAL(obj);
+
+    return JS_TRUE;
+}
+
 /***************************************************************************/
 // static
 JSBool
 XPCConvert::NativeInterface2JSObject(XPCCallContext& ccx,
+                                     jsval* d,
                                      nsIXPConnectJSObjectHolder** dest,
                                      nsISupports* src,
                                      const nsID* iid,
                                      JSObject* scope,
                                      PRBool allowNativeWrapper,
                                      PRBool isGlobal,
                                      nsresult* pErr)
 {
-    NS_ASSERTION(dest, "bad param");
     NS_ASSERTION(iid, "bad param");
     NS_ASSERTION(scope, "bad param");
 
-    *dest = nsnull;
+    *d = JSVAL_NULL;
+    if(dest)
+        *dest = nsnull;
     if(!src)
         return JS_TRUE;
     if(pErr)
         *pErr = NS_ERROR_XPC_BAD_CONVERT_NATIVE;
 
 // #define this if we want to 'double wrap' of JSObjects.
 // This is for the case where we have a JSObject wrapped for native use
 // which needs to be converted to a JSObject. Originally, we were unwrapping
@@ -1068,59 +1081,72 @@ XPCConvert::NativeInterface2JSObject(XPC
 #ifndef XPC_DO_DOUBLE_WRAP
     // is this a wrapped JS object?
     if(nsXPCWrappedJSClass::IsWrappedJS(src))
     {
         NS_ASSERTION(!isGlobal, "The global object must be native");
 
         // verify that this wrapper is for the right interface
         nsCOMPtr<nsISupports> wrapper;
-        if(NS_FAILED(src->QueryInterface(*iid,(void**)getter_AddRefs(wrapper))))
+        src->QueryInterface(*iid, (void**)getter_AddRefs(wrapper));
+        nsCOMPtr<nsIXPConnectJSObjectHolder> holder =
+            do_QueryInterface(wrapper);
+        JSObject* flat;
+        if(!holder || !(flat = holder->GetFlatJSObject()))
             return JS_FALSE;
-        return NS_SUCCEEDED(wrapper->QueryInterface(
-                                        NS_GET_IID(nsIXPConnectJSObjectHolder),
-                                        (void**) dest));
+
+        *d = OBJECT_TO_JSVAL(flat);
+        if(dest)
+            holder.swap(*dest);
+        return JS_TRUE;
     }
     else
 #endif /* XPC_DO_DOUBLE_WRAP */
     {
         XPCWrappedNativeScope* xpcscope =
             XPCWrappedNativeScope::FindInJSObjectScope(ccx, scope);
         if(!xpcscope)
             return JS_FALSE;
 
         AutoMarkingNativeInterfacePtr iface(ccx);
         iface = XPCNativeInterface::GetNewOrUsed(ccx, iid);
         if(!iface)
             return JS_FALSE;
 
         nsresult rv;
         XPCWrappedNative* wrapper;
+        nsRefPtr<XPCWrappedNative> strongWrapper;
         nsWrapperCache* cache = nsnull;
         CallQueryInterface(src, &cache);
         if(cache &&
            (wrapper = static_cast<XPCWrappedNative*>(cache->GetWrapper())))
         {
-            NS_ADDREF(wrapper);
+            // If asked to return the wrapper we'll return a strong reference,
+            // otherwise we'll just return its JSObject in rval (which should be
+            // rooted in that case).
+            if(dest)
+                strongWrapper = wrapper;
             wrapper->FindTearOff(ccx, iface, JS_FALSE, &rv);
-            if(NS_FAILED(rv))
-                NS_RELEASE(wrapper);
         }
         else
         {
             rv = XPCWrappedNative::GetNewOrUsed(ccx, src, xpcscope, iface,
-                                                isGlobal, &wrapper);
+                                                isGlobal,
+                                                getter_AddRefs(strongWrapper));
+
+            wrapper = strongWrapper;
         }
 
         if(pErr)
             *pErr = rv;
         if(NS_SUCCEEDED(rv) && wrapper)
         {
             uint32 flags = 0;
             JSObject *flat = wrapper->GetFlatJSObject();
+            jsval v = OBJECT_TO_JSVAL(flat);
             if (allowNativeWrapper && wrapper->GetScope() != xpcscope)
             {
                 // Cross scope access detected. Check if chrome code
                 // is accessing non-chrome objects, and if so, wrap
                 // the XPCWrappedNative with an XPCNativeWrapper to
                 // prevent user-defined properties from shadowing DOM
                 // properties from chrome code.
 
@@ -1171,16 +1197,22 @@ XPCConvert::NativeInterface2JSObject(XPC
                 // else don't create XPCNativeWrappers, since we have
                 // no idea what's calling what here.
 
                 flags = script ? JS_GetScriptFilenameFlags(script) : 0;
                 NS_ASSERTION(flags != JSFILENAME_NULL, "null script filename");
 
                 if(!JS_IsSystemObject(ccx, flat))
                 {
+                    // From here on we might create new JSObjects, so we need to
+                    // make sure that wrapper stays alive.
+                    if(!strongWrapper)
+                        strongWrapper = wrapper;
+
+                    JSObject *destObj = nsnull;
                     if(flags & JSFILENAME_PROTECTED)
                     {
 #ifdef DEBUG_XPCNativeWrapper
                         {
                             char *s = wrapper->ToString(ccx);
                             printf("Content accessed from chrome, wrapping "
                                    "wrapper (%s) in XPCNativeWrapper\n", s);
                             if (s)
@@ -1206,107 +1238,61 @@ XPCConvert::NativeInterface2JSObject(XPC
                             if(scriptPrincipal)
                             {
                                 nsJSPrincipals *nsjsp =
                                     static_cast<nsJSPrincipals *>(scriptPrincipal);
                                 objPrincipal = nsjsp->nsIPrincipalPtr;
                             }
                         }
 
-                        JSObject *nativeWrapper =
+                        destObj =
                             XPCNativeWrapper::GetNewOrUsed(ccx, wrapper,
                                                            objPrincipal);
-
-                        if(nativeWrapper)
-                        {
-                            XPCJSObjectHolder *objHolder =
-                                XPCJSObjectHolder::newHolder(ccx, nativeWrapper);
-
-                            if (objHolder)
-                            {
-                                NS_ADDREF(objHolder);
-                                NS_RELEASE(wrapper);
-
-                                *dest = objHolder;
-                                return JS_TRUE;
-                            }
-                        }
-
-                        // Out of memory or other failure that already
-                        // threw a JS exception.
-                        NS_RELEASE(wrapper);
-                        return JS_FALSE;
                     }
-
-                    if (flags & JSFILENAME_SYSTEM)
+                    else if (flags & JSFILENAME_SYSTEM)
                     {
 #ifdef DEBUG_mrbkap
                         printf("Content accessed from chrome, wrapping in an "
                                "XPCSafeJSObjectWrapper\n");
 #endif
 
-                        jsval v = OBJECT_TO_JSVAL(wrapper->GetFlatJSObject());
-                        XPCJSObjectHolder *objHolder;
-                        if(!XPC_SJOW_Construct(ccx, nsnull, 1, &v, &v) ||
-                           !(objHolder = XPCJSObjectHolder::newHolder(ccx,
-                                                         JSVAL_TO_OBJECT(v))))
-                        {
-                            NS_RELEASE(wrapper);
-                            return JS_FALSE;
-                        }
-
-                        NS_ADDREF(objHolder);
-                        NS_RELEASE(wrapper);
-
-                        *dest = objHolder;
-                        return JS_TRUE;
+                        if(XPC_SJOW_Construct(ccx, nsnull, 1, &v, &v))
+                            destObj = JSVAL_TO_OBJECT(v);
+                    }
+                    else
+                    {
+                        // Reaching across scopes from content code. Wrap
+                        // the new object in a XOW.
+                        if (XPC_XOW_WrapObject(ccx, scope, &v))
+                            destObj = JSVAL_TO_OBJECT(v);
                     }
 
-                    // Reaching across scopes from content code. Wrap
-                    // the new object in a XOW.
-                    jsval v = OBJECT_TO_JSVAL(flat);
-                    XPCJSObjectHolder *objHolder = nsnull;
-                    if (!XPC_XOW_WrapObject(ccx, scope, &v) ||
-                        !(objHolder =
-                          XPCJSObjectHolder::newHolder(ccx,
-                                                       JSVAL_TO_OBJECT(v))))
-                    {
-                        NS_RELEASE(wrapper);
-                        return JS_FALSE;
-                    }
-
-                    NS_ADDREF(objHolder);
-                    NS_RELEASE(wrapper);
-                    *dest = objHolder;
-                    return JS_TRUE;
+                    return destObj &&
+                           CreateHolderIfNeeded(ccx, destObj, d, dest);
                 }
             }
 
             const char *name = STOBJ_GET_CLASS(flat)->name;
             if(allowNativeWrapper &&
                !(flags & JSFILENAME_SYSTEM) &&
                !JS_IsSystemObject(ccx, flat) &&
                XPC_XOW_ClassNeedsXOW(name))
             {
-                jsval v = OBJECT_TO_JSVAL(flat);
-                XPCJSObjectHolder *objHolder = nsnull;
-                if (!XPC_XOW_WrapObject(ccx, scope, &v) ||
-                    !(objHolder = XPCJSObjectHolder::newHolder(ccx, JSVAL_TO_OBJECT(v))))
-                {
-                    NS_RELEASE(wrapper);
-                    return JS_FALSE;
-                }
+                // From here on we might create new JSObjects, so we need to
+                // make sure that wrapper stays alive.
+                if(!strongWrapper)
+                    strongWrapper = wrapper;
 
-                NS_ADDREF(objHolder);
-                NS_RELEASE(wrapper);
-                *dest = objHolder;
-                return JS_TRUE;
+                return XPC_XOW_WrapObject(ccx, scope, &v) &&
+                       CreateHolderIfNeeded(ccx, JSVAL_TO_OBJECT(v), d, dest);
             }
 
-            *dest = static_cast<nsIXPConnectJSObjectHolder*>(wrapper);
+            *d = v;
+            if(dest)
+                *dest = strongWrapper.forget().get();
             return JS_TRUE;
         }
     }
     return JS_FALSE;
 }
 
 /***************************************************************************/
 
--- a/js/src/xpconnect/src/xpcprivate.h
+++ b/js/src/xpconnect/src/xpcprivate.h
@@ -2756,16 +2756,17 @@ public:
      * @param iid the interface of src that we want
      * @param scope the default scope to put on the new JSObject's __parent__
      *        chain
      * @param allowNativeWrapper if true, this method may wrap the resulting
      *        JSObject in an XPCNativeWrapper and return that, as needed.
      * @param pErr [out] relevant error code, if any.
      */
     static JSBool NativeInterface2JSObject(XPCCallContext& ccx,
+                                           jsval* d,
                                            nsIXPConnectJSObjectHolder** dest,
                                            nsISupports* src,
                                            const nsID* iid,
                                            JSObject* scope,
                                            PRBool allowNativeWrapper,
                                            PRBool isGlobal,
                                            nsresult* pErr);
 
--- a/js/src/xpconnect/src/xpcquickstubs.cpp
+++ b/js/src/xpconnect/src/xpcquickstubs.cpp
@@ -751,46 +751,35 @@ xpc_qsXPCOMObjectToJsval(XPCCallContext 
     JSObject *scope = ccx.GetCurrentJSObject();
     NS_ASSERTION(scope, "bad ccx");
 
     // XXX The OBJ_IS_NOT_GLOBAL here is not really right. In
     // fact, this code is depending on the fact that the
     // global object will not have been collected, and
     // therefore this NativeInterface2JSObject will not end up
     // creating a new XPCNativeScriptableShared.
-    nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
     nsresult rv;
-    if(!XPCConvert::NativeInterface2JSObject(ccx, getter_AddRefs(holder),
-                                              p, &iid, scope, PR_TRUE,
-                                              OBJ_IS_NOT_GLOBAL, &rv))
+    if(!XPCConvert::NativeInterface2JSObject(ccx, rval, nsnull, p, &iid, scope,
+                                             PR_TRUE, OBJ_IS_NOT_GLOBAL, &rv))
     {
         // I can't tell if NativeInterface2JSObject throws JS exceptions
         // or not.  This is a sloppy stab at the right semantics; the
         // method really ought to be fixed to behave consistently.
         if(!JS_IsExceptionPending(ccx))
             xpc_qsThrow(ccx, NS_FAILED(rv) ? rv : NS_ERROR_UNEXPECTED);
         return JS_FALSE;
     }
 
-    if(holder)
-    {
-        JSObject* jsobj;
-        if(NS_FAILED(holder->GetJSObject(&jsobj)))
-            return JS_FALSE;
 #ifdef DEBUG
-        if(!STOBJ_GET_PARENT(jsobj))
-            NS_ASSERTION(STOBJ_GET_CLASS(jsobj)->flags & JSCLASS_IS_GLOBAL,
-                         "Why did we recreate this wrapper?");
+    JSObject* jsobj = JSVAL_TO_OBJECT(*rval);
+    if(jsobj && !STOBJ_GET_PARENT(jsobj))
+        NS_ASSERTION(STOBJ_GET_CLASS(jsobj)->flags & JSCLASS_IS_GLOBAL,
+                     "Why did we recreate this wrapper?");
 #endif
-        *rval = OBJECT_TO_JSVAL(jsobj);
-    }
-    else
-    {
-        *rval = JSVAL_NULL;
-    }
+
     return JS_TRUE;
 }
 
 JSBool
 xpc_qsVariantToJsval(XPCCallContext &ccx,
                      nsIVariant *p,
                      uintN paramNum,
                      jsval *rval)
--- a/js/src/xpconnect/src/xpcwrappedjsclass.cpp
+++ b/js/src/xpconnect/src/xpcwrappedjsclass.cpp
@@ -1341,29 +1341,28 @@ nsXPCWrappedJSClass::CallMethod(nsXPCWra
                                 goto pre_call_clean_up;
                             }
                             if(newThis)
                             {
                                 if(!newWrapperIID)
                                     newWrapperIID =
                                         const_cast<nsIID*>
                                                   (&NS_GET_IID(nsISupports));
-                                nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
+                                jsval v;
                                 JSBool ok =
                                   XPCConvert::NativeInterface2JSObject(ccx,
-                                        getter_AddRefs(holder), newThis,
-                                        newWrapperIID, obj, PR_FALSE, PR_FALSE,
-                                        nsnull);
+                                        &v, nsnull, newThis, newWrapperIID, obj,
+                                        PR_FALSE, PR_FALSE, nsnull);
                                 if(newWrapperIID != &NS_GET_IID(nsISupports))
                                     nsMemory::Free(newWrapperIID);
-                                if(!ok ||
-                                    NS_FAILED(holder->GetJSObject(&thisObj)))
+                                if(!ok)
                                 {
                                     goto pre_call_clean_up;
                                 }
+                                thisObj = JSVAL_TO_OBJECT(v);
                             }
                         }
                     }
                 }
             }
         }
         else if(!JS_GetMethod(cx, obj, name, &thisObj, &fval))
         {