Bug 348156: Don't rely on UnbindFromTree to break cycles since that puts us in an inconsistent state. r/sr=jst

content/base/src/nsDocument.cpp

@@ -5433,26 +5433,17 @@ nsDocument::Destroy()
   if (mIsGoingAway)
     return;
 
-  PRInt32 count = mChildren.ChildCount();
 
   mIsGoingAway = PR_TRUE;
-  DestroyLinkMap();
-  for (PRInt32 indx = 0; indx < count; ++indx) {
-    // XXXbz what we _should_ do here is to clear mChildren and null out
-    // mRootContent.  If we did this (or at least the latter), we could remove
-    // the silly null-checks in nsHTMLDocument::MatchLinks.  Unfortunately,
-    // doing that introduces several problems:
-    // 1) Focus issues (see bug 341730).  The fix for bug 303260 may fix these.
-    // 2) Crashes in OnPageHide if it fires after Destroy.  See bug 303260
-    //    comments 9 and 10.
-    // So we're just creating an inconsistent DOM for now and hoping.  :(
-    mChildren.ChildAt(indx)->UnbindFromTree();
+
+  PRUint32 i, count = mChildren.ChildCount();
+  for (i = 0; i < count; ++i) {
+    nsNodeUtils::DestroySubtree(mChildren.ChildAt(i));
   }
+
   mLayoutHistoryState = nsnull;
 
   nsContentList::OnDocumentDestroy(this);
-  delete mContentWrapperHash;
-  mContentWrapperHash = nsnull;
 }
 
 already_AddRefed<nsILayoutHistoryState>

content/base/src/nsGenericElement.cpp

@@ -3271,8 +3271,11 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsGenericElement)
   // Unlink any DOM slots of interest.
   {
     nsDOMSlots *slots = tmp->GetExistingDOMSlots();
-    if (slots)
+    if (slots) {
       slots->mAttributeMap = nsnull;
+      if (tmp->IsNodeOfType(nsINode::eXUL))
+        NS_IF_RELEASE(slots->mControllers);
+    }
   }
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 

content/base/src/nsNodeUtils.cpp

@@ -677,3 +677,27 @@ nsNodeUtils::UnlinkUserData(nsINode *aNode)
                                                       DOM_USER_DATA_HANDLER);
   }
 }
+
+/* static */
+void
+nsNodeUtils::DestroySubtree(nsIContent* aRoot)
+{
+  nsXULElement* xul = nsXULElement::FromContent(aRoot);
+  if (xul) {
+    nsGenericElement::nsDOMSlots* slots = xul->GetExistingDOMSlots();
+    if (slots) {
+      NS_IF_RELEASE(slots->mControllers);
+    }
+  }
+
+  nsIDocument *document = aRoot->GetOwnerDoc();
+  if (document) {
+    document->BindingManager()->ChangeDocumentFor(aRoot, document, nsnull);
+    document->ClearBoxObjectFor(aRoot);
+  }
+
+  PRUint32 i, count = aRoot->GetChildCount();
+  for (i = 0; i < count; ++i) {
+    DestroySubtree(aRoot->GetChildAt(i));
+  }
+}

content/base/src/nsNodeUtils.h

@@ -258,6 +258,14 @@ public:
    */
   static void UnlinkUserData(nsINode *aNode);
 
+  /**
+   * Remove neccesary components of all nodes in a subtree to avoid leaking.
+   * So far this removes XBL bindings and XUL controllers.
+   *
+   * @param aRoot the node that is the root of the subtree to clear.
+   */
+  static void DestroySubtree(nsIContent* aRoot);
+
 private:
   friend PLDHashOperator PR_CALLBACK
     AdoptFunc(nsAttrHashKey::KeyType aKey, nsIDOMNode *aData, void* aUserArg);

content/html/content/src/nsHTMLFormElement.cpp

@@ -363,7 +363,7 @@ protected:
   /** The request currently being submitted */
   nsCOMPtr<nsIRequest> mSubmittingRequest;
   /** The web progress object we are currently listening to */
-  nsCOMPtr<nsIWebProgress> mWebProgress;
+  nsWeakPtr mWebProgress;
 
   /** The default submit element -- WEAK */
   nsIFormControl* mDefaultSubmitElement;
@@ -1057,10 +1057,12 @@ nsHTMLFormElement::SubmitSubmission(nsIFormSubmission* aFormSubmission)
     PRBool pending = PR_FALSE;
     mSubmittingRequest->IsPending(&pending);
     if (pending && !schemeIsJavaScript) {
-      mWebProgress = do_GetInterface(docShell);
-      NS_ASSERTION(mWebProgress, "nsIDocShell not converted to nsIWebProgress!");
-      rv = mWebProgress->AddProgressListener(this, nsIWebProgress::NOTIFY_STATE_ALL);
+      nsCOMPtr<nsIWebProgress> webProgress = do_GetInterface(docShell);
+      NS_ASSERTION(webProgress, "nsIDocShell not converted to nsIWebProgress!");
+      rv = webProgress->AddProgressListener(this, nsIWebProgress::NOTIFY_STATE_ALL);
       NS_ENSURE_SUBMIT_SUCCESS(rv);
+      mWebProgress = do_GetWeakReference(webProgress);
+      NS_ASSERTION(mWebProgress, "can't hold weak ref to webprogress!");
     } else {
       ForgetCurrentSubmission();
     }
@@ -1632,10 +1634,11 @@ nsHTMLFormElement::ForgetCurrentSubmission()
   mNotifiedObservers = PR_FALSE;
   mIsSubmitting = PR_FALSE;
   mSubmittingRequest = nsnull;
-  if (mWebProgress) {
-    mWebProgress->RemoveProgressListener(this);
-    mWebProgress = nsnull;
+  nsCOMPtr<nsIWebProgress> webProgress = do_QueryReferent(mWebProgress);
+  if (webProgress) {
+    webProgress->RemoveProgressListener(this);
   }
+  mWebProgress = nsnull;
 }
 
 // nsIWebProgressListener

content/xtf/src/nsXTFElementWrapper.cpp

@@ -107,6 +107,13 @@ nsXTFElementWrapper::Init()
 NS_IMPL_ADDREF_INHERITED(nsXTFElementWrapper, nsXTFElementWrapperBase)
 NS_IMPL_RELEASE_INHERITED(nsXTFElementWrapper, nsXTFElementWrapperBase)
 
+NS_IMPL_CYCLE_COLLECTION_CLASS(nsXTFElementWrapper)
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(nsXTFElementWrapper,
+                                                  nsXTFElementWrapperBase)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mXTFElement)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mAttributeHandler)
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
+
 NS_IMETHODIMP
 nsXTFElementWrapper::QueryInterface(REFNSIID aIID, void** aInstancePtr)
 {

content/xtf/src/nsXTFElementWrapper.h

@@ -65,6 +65,8 @@ public:
 
   // nsISupports interface
   NS_DECL_ISUPPORTS_INHERITED
+  NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED_NO_UNLINK(nsXTFElementWrapper,
+                                                     nsXTFElementWrapperBase)
 
   // nsIXTFElementWrapper
   NS_DECL_NSIXTFELEMENTWRAPPER