Bug 731845 - XPCWrappedNative::ReparentWrapperIfFound should handle preserved wrappers. r=mrbkap
☠☠ backed out by a9e38eff9ea6 ☠ ☠
authorBobby Holley <bobbyholley@gmail.com>
Mon, 05 Mar 2012 16:58:59 -0800
changeset 88340 2fbc7d9ac670dfa2dcfb1d9386356d41bfcaf69c
parent 88339 54a7b61b95355a30bc79f2ed511a77710e854f27
child 88341 003ffd0621306000d7bc59acb809696e70e86a46
push id22194
push usermak77@bonardo.net
push dateWed, 07 Mar 2012 09:33:54 +0000
treeherdermozilla-central@8ef88a69f861 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap
bugs731845
milestone13.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 731845 - XPCWrappedNative::ReparentWrapperIfFound should handle preserved wrappers. r=mrbkap
content/base/src/nsNodeUtils.cpp
js/xpconnect/src/XPCWrappedNative.cpp
--- a/content/base/src/nsNodeUtils.cpp
+++ b/content/base/src/nsNodeUtils.cpp
@@ -592,39 +592,20 @@ nsNodeUtils::CloneAndAdopt(nsINode *aNod
 
     if (elem) {
       elem->RecompileScriptEventListeners();
     }
 
     if (aCx && wrapper) {
       nsIXPConnect *xpc = nsContentUtils::XPConnect();
       if (xpc) {
-        JSObject *preservedWrapper = nsnull;
-
-        // If reparenting moves us to a new compartment, preserving causes
-        // problems. In that case, we release ourselves and re-preserve after
-        // reparenting so we're sure to have the right JS object preserved.
-        // We use a JSObject stack copy of the wrapper to protect it from GC
-        // under ReparentWrappedNativeIfFound.
-        if (aNode->PreservingWrapper()) {
-          preservedWrapper = wrapper;
-          nsContentUtils::ReleaseWrapper(aNode, aNode);
-          NS_ASSERTION(aNode->GetWrapper(),
-                       "ReleaseWrapper cleared our wrapper, this code needs to "
-                       "be changed to deal with that!");
-        }
-
         nsCOMPtr<nsIXPConnectJSObjectHolder> oldWrapper;
         rv = xpc->ReparentWrappedNativeIfFound(aCx, wrapper, aNewScope, aNode,
                                                getter_AddRefs(oldWrapper));
 
-        if (preservedWrapper) {
-          nsContentUtils::PreserveWrapper(aNode, aNode);
-        }
-
         if (NS_FAILED(rv)) {
           aNode->mNodeInfo.swap(nodeInfo);
 
           return rv;
         }
       }
     }
   }
--- a/js/xpconnect/src/XPCWrappedNative.cpp
+++ b/js/xpconnect/src/XPCWrappedNative.cpp
@@ -48,16 +48,18 @@
 #include "XPCLog.h"
 #include "nsINode.h"
 #include "XPCQuickStubs.h"
 #include "jsproxy.h"
 #include "AccessCheck.h"
 #include "WrapperFactory.h"
 #include "dombindings.h"
 
+#include "nsContentUtils.h"
+
 #include "mozilla/Util.h"
 
 bool
 xpc_OkToHandOutWrapper(nsWrapperCache *cache)
 {
     NS_ABORT_IF_FALSE(cache->GetWrapper(), "Must have wrapper");
     NS_ABORT_IF_FALSE(cache->IsProxy() || IS_WN_WRAPPER(cache->GetWrapper()),
                       "Must have proxy or XPCWrappedNative wrapper");
@@ -1474,16 +1476,53 @@ XPCWrappedNative::SystemIsBeingShutDown(
     if (mFirstChunk.mNextChunk) {
         delete mFirstChunk.mNextChunk;
         mFirstChunk.mNextChunk = nsnull;
     }
 }
 
 /***************************************************************************/
 
+// If we have to transplant an object across compartments, we need to be
+// careful if the underlying object implements nsWrapperCache and is preserving
+// the wrapper.
+//
+// The class brackets a pair of Unpreserve/Preserve calls in the given scope.
+//
+// This class _must_ live on the stack, in part so that mPreservedWrapper is
+// visible to the stack scanner. The caller wants the wrapper to be preserved,
+// so we don't want it to get accidentally GCed.
+class AutoWrapperChanger NS_STACK_CLASS {
+public:
+    AutoWrapperChanger() : mCache(nsnull)
+                         , mCOMObj(nsnull)
+                         , mPreservedWrapper(nsnull)
+    {}
+
+    void init(nsISupports* aCOMObj, nsWrapperCache* aWrapperCache) {
+        mCOMObj = aCOMObj;
+        mCache = aWrapperCache;
+        if (mCache->PreservingWrapper()) {
+            mPreservedWrapper = mCache->GetWrapper();
+            MOZ_ASSERT(mPreservedWrapper);
+            nsContentUtils::ReleaseWrapper(mCOMObj, mCache);
+        }
+    }
+
+    ~AutoWrapperChanger() {
+        if (mPreservedWrapper)
+            nsContentUtils::PreserveWrapper(mCOMObj, mCache);
+    }
+
+private:
+    nsWrapperCache* mCache;
+    nsISupports* mCOMObj;
+    JSObject* mPreservedWrapper;
+};
+
 // static
 nsresult
 XPCWrappedNative::ReparentWrapperIfFound(XPCCallContext& ccx,
                                          XPCWrappedNativeScope* aOldScope,
                                          XPCWrappedNativeScope* aNewScope,
                                          JSObject* aNewParent,
                                          nsISupports* aCOMObj,
                                          XPCWrappedNative** aWrapper)
@@ -1492,20 +1531,26 @@ XPCWrappedNative::ReparentWrapperIfFound
         XPCNativeInterface::GetISupports(ccx);
 
     if (!iface)
         return NS_ERROR_FAILURE;
 
     nsresult rv;
 
     nsRefPtr<XPCWrappedNative> wrapper;
+    AutoWrapperChanger wrapperChanger;
     JSObject *flat;
     nsWrapperCache* cache = nsnull;
     CallQueryInterface(aCOMObj, &cache);
     if (cache) {
+
+        // There's a wrapper cache. Make sure we keep it sane no matter what
+        // happens.
+        wrapperChanger.init(aCOMObj, cache);
+
         flat = cache->GetWrapper();
         if (flat && !IS_SLIM_WRAPPER_OBJECT(flat)) {
             wrapper = static_cast<XPCWrappedNative*>(xpc_GetJSPrivate(flat));
             NS_ASSERTION(wrapper->GetScope() == aOldScope,
                          "Incorrect scope passed");
         }
     } else {
         rv = XPCWrappedNative::GetUsedOnly(ccx, aCOMObj, aOldScope, iface,