Bug 751995 - Handle orphaned wrappers. r=peterv
authorBobby Holley <bobbyholley@gmail.com>
Mon, 04 Jun 2012 10:13:18 +0200
changeset 95745 a6cee80d4fde9442df8bf9a5b5d4cad6656ed5fa
parent 95744 030fa99e5bdef64a1c4a1a55d076945cc2036c41
child 95746 1fa953e7d4c15b19a763b888e1f22ecb1ff81d05
push idunknown
push userunknown
push dateunknown
reviewerspeterv
bugs751995
milestone15.0a1
Bug 751995 - Handle orphaned wrappers. r=peterv
js/src/jswrapper.h
js/xpconnect/crashtests/751995.html
js/xpconnect/crashtests/crashtests.list
js/xpconnect/src/XPCWrappedNative.cpp
js/xpconnect/src/nsXPConnect.cpp
js/xpconnect/src/xpcprivate.h
--- a/js/src/jswrapper.h
+++ b/js/src/jswrapper.h
@@ -233,17 +233,17 @@ UnwrapObject(JSObject *obj, bool stopAtO
 
 // Given a JSObject, returns that object stripped of wrappers. At each stage,
 // the security wrapper has the opportunity to veto the unwrap. Since checked
 // code should never be unwrapping outer window wrappers, we always stop at
 // outer windows.
 JS_FRIEND_API(JSObject *)
 UnwrapObjectChecked(JSContext *cx, JSObject *obj);
 
-bool
+JS_FRIEND_API(bool)
 IsCrossCompartmentWrapper(const JSObject *obj);
 
 void
 NukeCrossCompartmentWrapper(JSObject *wrapper);
 
 } /* namespace js */
 
 #endif
new file mode 100644
--- /dev/null
+++ b/js/xpconnect/crashtests/751995.html
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<head>
+<script>
+
+function frameDoc() { return document.getElementById("f").contentDocument; }
+
+function arm() {
+  // Create an element in the iframe.
+  var div = frameDoc().createElement("div");
+
+  // Force a wrapper to be created for .style.
+  var style = div.style;
+  style.color = "green";
+
+  // Adopt the element out of the iframe, leaving the |style| behind.
+  document.adoptNode(div);
+}
+
+function boom()
+{
+  // Create an orphan.
+  arm();
+
+  // Force an iteration over all the wrappers in frameDoc's scope, causing
+  // us to notice the orphan.
+  frameDoc().write("2");
+
+  // All done.
+  document.documentElement.removeAttribute("class");
+}
+
+</script>
+</head>
+<body onload="boom();"><iframe id="f" src="data:text/html,1"></iframe></body>
+</html>
--- a/js/xpconnect/crashtests/crashtests.list
+++ b/js/xpconnect/crashtests/crashtests.list
@@ -32,10 +32,11 @@ load 603146-1.html
 load 603858-1.html
 load 608963.html
 load 616930-1.html
 load 639737-1.html
 load 648206-1.html
 load 705875.html
 load 720305-1.html
 load 723465.html
+load 751995.html
 asserts(0-1) load 752038.html # We may hit bug 645229 here.
 load 754311.html
--- a/js/xpconnect/src/XPCWrappedNative.cpp
+++ b/js/xpconnect/src/XPCWrappedNative.cpp
@@ -1724,16 +1724,90 @@ XPCWrappedNative::ReparentWrapperIfFound
     }
 
     *aWrapper = nsnull;
     wrapper.swap(*aWrapper);
 
     return NS_OK;
 }
 
+XPCWrappedNative*
+XPCWrappedNative::GetParentWrapper()
+{
+    XPCWrappedNative *wrapper = nsnull;
+    JSObject *parent = js::GetObjectParent(mFlatJSObject);
+    if (parent && IS_WN_WRAPPER(parent)) {
+        wrapper = static_cast<XPCWrappedNative*>(js::GetObjectPrivate(parent));
+    }
+    return wrapper;
+}
+
+// Orphans are sad little things - If only we could treat them better. :-(
+//
+// When a wrapper gets reparented to another scope (for example, when calling
+// adoptNode), it's entirely possible that it previously served as the parent for
+// other wrappers (via PreCreate hooks). When it moves, the old mFlatJSObject is
+// replaced by a cross-compartment wrapper. Its descendants really _should_ move
+// too, but we have no way of locating them short of a compartment-wide sweep
+// (which we believe to be prohibitively expensive).
+//
+// So we just leave them behind. In practice, the only time this turns out to
+// be a problem is during subsequent wrapper reparenting. When this happens, we
+// call into the below fixup code at the last minute and straighten things out
+// before proceeding.
+//
+// See bug 751995 for more information.
+
+bool
+XPCWrappedNative::IsOrphan()
+{
+    JSObject *parent = js::GetObjectParent(mFlatJSObject);
+
+    // If there's no parent, we've presumably got a global, which can't be an
+    // orphan by definition.
+    if (!parent)
+        return false;
+
+    // If our parent is a cross-compartment wrapper, it has left us behind.
+    return js::IsCrossCompartmentWrapper(parent);
+}
+
+// Recursively fix up orphans on the parent chain of a wrapper. Note that this
+// can cause a wrapper to move even if IsOrphan() is false, since its parent
+// might be an orphan, and fixing the parent causes this wrapper to become an
+// orphan.
+nsresult
+XPCWrappedNative::RescueOrphans(XPCCallContext& ccx)
+{
+    // Even if we're not an orphan at the moment, one of our ancestors might
+    // be. If so, we need to recursively rescue up the parent chain.
+    nsresult rv;
+    XPCWrappedNative *parentWrapper = GetParentWrapper();
+    if (parentWrapper && parentWrapper->IsOrphan()) {
+        rv = parentWrapper->RescueOrphans(ccx);
+        NS_ENSURE_SUCCESS(rv, rv);
+    }
+
+    // Now that we know our parent is in the right place, determine if we've
+    // been orphaned. If not, we have nothing to do.
+    if (!IsOrphan())
+        return NS_OK;
+
+    // We've been orphaned. Find where our parent went, and follow it.
+    JSObject *parentGhost = js::GetObjectParent(mFlatJSObject);
+    JSObject *realParent = js::UnwrapObject(parentGhost);
+    nsRefPtr<XPCWrappedNative> ignored;
+    return ReparentWrapperIfFound(ccx,
+                                  XPCWrappedNativeScope::
+                                    FindInJSObjectScope(ccx, parentGhost),
+                                  XPCWrappedNativeScope::
+                                    FindInJSObjectScope(ccx, realParent),
+                                  realParent, mIdentity, getter_AddRefs(ignored));
+}
+
 #define IS_TEAROFF_CLASS(clazz)                                               \
           ((clazz) == &XPC_WN_Tearoff_JSClass)
 
 // static
 XPCWrappedNative*
 XPCWrappedNative::GetWrappedNativeOfJSObject(JSContext* cx,
                                              JSObject* obj,
                                              JSObject* funobj,
--- a/js/xpconnect/src/nsXPConnect.cpp
+++ b/js/xpconnect/src/nsXPConnect.cpp
@@ -1542,16 +1542,29 @@ MoveWrapper(XPCCallContext& ccx, XPCWrap
     // First, check to see if this wrapper really needs to be
     // reparented.
 
     if (wrapper->GetScope() == newScope) {
         // The wrapper already got moved, nothing to do here.
         return NS_OK;
     }
 
+    // For performance reasons, we wait to fix up orphaned wrappers (wrappers
+    // whose parents have moved to another scope) until right before they
+    // threaten to confuse us.
+    //
+    // If this wrapper is an orphan, reunite it with its parent. If, following
+    // that, the wrapper is no longer in the old scope, then we don't need to
+    // reparent it.
+    MOZ_ASSERT(wrapper->GetScope() == oldScope);
+    nsresult rv = wrapper->RescueOrphans(ccx);
+    NS_ENSURE_SUCCESS(rv, rv);
+    if (wrapper->GetScope() != oldScope)
+        return NS_OK;
+
     nsISupports *identity = wrapper->GetIdentityObject();
     nsCOMPtr<nsIClassInfo> info(do_QueryInterface(identity));
 
     // ClassInfo is implemented as singleton objects. If the identity
     // object here is the same object as returned by the QI, then it
     // is the singleton classinfo, so we don't need to reparent it.
     if (SameCOMIdentity(identity, info))
         info = nsnull;
@@ -1566,19 +1579,19 @@ MoveWrapper(XPCCallContext& ccx, XPCWrap
                                                      sciProto, sci);
 
     // If the wrapper doesn't want precreate, then we don't need to
     // worry about reparenting it.
     if (!sciWrapper.GetFlags().WantPreCreate())
         return NS_OK;
 
     JSObject *newParent = oldScope->GetGlobalJSObject();
-    nsresult rv = sciWrapper.GetCallback()->PreCreate(identity, ccx,
-                                                      newParent,
-                                                      &newParent);
+    rv = sciWrapper.GetCallback()->PreCreate(identity, ccx,
+                                             newParent,
+                                             &newParent);
     if (NS_FAILED(rv))
         return rv;
 
     if (newParent == oldScope->GetGlobalJSObject()) {
         // The old scope still works for this wrapper. We have to
         // assume that the wrapper will continue to return the old
         // scope from PreCreate, so don't move it.
         return NS_OK;
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -2699,16 +2699,25 @@ public:
     static nsresult
     ReparentWrapperIfFound(XPCCallContext& ccx,
                            XPCWrappedNativeScope* aOldScope,
                            XPCWrappedNativeScope* aNewScope,
                            JSObject* aNewParent,
                            nsISupports* aCOMObj,
                            XPCWrappedNative** aWrapper);
 
+    // Returns the wrapper corresponding to the parent of our mFlatJSObject.
+    //
+    // If the parent does not have a WN, or if there is no parent, null is
+    // returned.
+    XPCWrappedNative *GetParentWrapper();
+
+    bool IsOrphan();
+    nsresult RescueOrphans(XPCCallContext& ccx);
+
     void FlatJSObjectFinalized();
 
     void SystemIsBeingShutDown();
 
     enum CallMode {CALL_METHOD, CALL_GETTER, CALL_SETTER};
 
     static JSBool CallMethod(XPCCallContext& ccx,
                              CallMode mode = CALL_METHOD);