Bug 348156: Fix leaks by relying on cycle collector rather than calling UnbindFromTree on all nodes. r/sr=jst

This commit is contained in:
jonas@sicking.cc 2007-11-29 00:41:25 -08:00
parent 450727effa
commit a6bf2b9522
13 changed files with 93 additions and 31 deletions

View File

@ -62,8 +62,8 @@ class nsIDocShell;
// IID for the nsIContent interface
#define NS_ICONTENT_IID \
{ 0x36b375cb, 0xf01e, 0x4c18, \
{ 0xbf, 0x9e, 0xba, 0xad, 0x77, 0x1d, 0xce, 0x22 } }
{ 0xe0c5d967, 0x2c15, 0x4097, \
{ 0xb0, 0xdc, 0x75, 0xa3, 0xa7, 0xfc, 0xcd, 0x1a } }
// hack to make egcs / gcc 2.95.2 happy
class nsIContent_base : public nsINode {
@ -792,6 +792,12 @@ public:
*/
virtual void UpdateEditableState();
/**
* Destroy this node and it's children. Ideally this shouldn't be needed
* but for now we need to do it to break cycles.
*/
virtual void DestroyContent() = 0;
#ifdef DEBUG
/**
* List the content (and anything it contains) out to the given

View File

@ -1059,6 +1059,11 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsDocument)
// Tear down linkmap. This is a performance optimization so that we
// don't waste time removing links one by one as they are removed
// from the doc.
DestroyLinkMap();
// Unlink the mChildren nsAttrAndChildArray.
for (PRInt32 indx = PRInt32(tmp->mChildren.ChildCount()) - 1;
indx >= 0; --indx) {
@ -2057,7 +2062,10 @@ SubDocClearEntry(PLDHashTable *table, PLDHashEntryHdr *entry)
SubDocMapEntry *e = static_cast<SubDocMapEntry *>(entry);
NS_RELEASE(e->mKey);
NS_IF_RELEASE(e->mSubDocument);
if (e->mSubDocument) {
e->mSubDocument->SetParentDocument(nsnull);
NS_RELEASE(e->mSubDocument);
}
}
PR_STATIC_CALLBACK(PRBool)
@ -2090,8 +2098,6 @@ nsDocument::SetSubDocumentFor(nsIContent *aContent, nsIDocument* aSubDoc)
PL_DHASH_LOOKUP));
if (PL_DHASH_ENTRY_IS_BUSY(entry)) {
entry->mSubDocument->SetParentDocument(nsnull);
PL_DHashTableRawRemove(mSubDocuments, entry);
}
}
@ -5580,26 +5586,16 @@ 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) {
mChildren.ChildAt(i)->DestroyContent();
}
mLayoutHistoryState = nsnull;
nsContentList::OnDocumentDestroy(this);
delete mContentWrapperHash;
mContentWrapperHash = nsnull;
}
already_AddRefed<nsILayoutHistoryState>

View File

@ -811,6 +811,10 @@ nsGenericDOMDataNode::IsNodeOfType(PRUint32 aFlags) const
return !(aFlags & ~(eCONTENT | eDATA_NODE));
}
void
nsGenericDOMDataNode::DestroyContent()
{
}
#ifdef DEBUG
void

View File

@ -225,6 +225,7 @@ public:
PRBool aNotify);
virtual PRBool TextIsOnlyWhitespace();
virtual void AppendTextTo(nsAString& aResult);
virtual void DestroyContent();
#ifdef DEBUG
virtual void List(FILE* out, PRInt32 aIndent) const;
virtual void DumpContent(FILE* out, PRInt32 aIndent, PRBool aDumpAll) const;

View File

@ -2857,6 +2857,22 @@ nsGenericElement::GetPrimaryFrameFor(nsIContent* aContent,
return presShell->GetPrimaryFrameFor(aContent);
}
void
nsGenericElement::DestroyContent()
{
nsIDocument *document = GetOwnerDoc();
if (document) {
document->BindingManager()->ChangeDocumentFor(this, document, nsnull);
document->ClearBoxObjectFor(this);
}
PRUint32 i, count = mAttrsAndChildren.ChildCount();
for (i = 0; i < count; ++i) {
// The child can remove itself from the parent in BindToTree.
mAttrsAndChildren.ChildAt(i)->DestroyContent();
}
}
//----------------------------------------------------------------------
// Generic DOMNode implementations
@ -3340,7 +3356,7 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsGenericElement)
PRUint32 i;
PRUint32 kids = tmp->mAttrsAndChildren.ChildCount();
for (i = kids; i > 0; i--) {
tmp->mAttrsAndChildren.ChildAt(i-1)->UnbindFromTree();
tmp->mAttrsAndChildren.ChildAt(i-1)->UnbindFromTree(PR_FALSE);
tmp->mAttrsAndChildren.RemoveChildAt(i-1);
}
}
@ -3348,8 +3364,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

View File

@ -434,6 +434,7 @@ public:
virtual PRUint32 GetScriptTypeID() const;
virtual nsresult SetScriptTypeID(PRUint32 aLang);
virtual void DestroyContent();
#ifdef DEBUG
virtual void List(FILE* out, PRInt32 aIndent) const
{

View File

@ -3066,6 +3066,17 @@ nsGenericHTMLFrameElement::SetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
return rv;
}
void
nsGenericHTMLFrameElement::DestroyContent()
{
if (mFrameLoader) {
mFrameLoader->Destroy();
mFrameLoader = nsnull;
}
nsGenericHTMLElement::DestroyContent();
}
//----------------------------------------------------------------------
void

View File

@ -933,6 +933,7 @@ public:
virtual nsresult SetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
nsIAtom* aPrefix, const nsAString& aValue,
PRBool aNotify);
virtual void DestroyContent();
// nsIDOMNSHTMLElement
NS_IMETHOD GetTabIndex(PRInt32 *aTabIndex);

View File

@ -323,7 +323,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;
@ -1103,10 +1103,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();
}
@ -1723,10 +1725,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

View File

@ -104,8 +104,15 @@ nsXTFElementWrapper::Init()
//----------------------------------------------------------------------
// nsISupports implementation
NS_IMPL_ADDREF_INHERITED(nsXTFElementWrapper,nsXTFElementWrapperBase)
NS_IMPL_RELEASE_INHERITED(nsXTFElementWrapper,nsXTFElementWrapperBase)
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)

View File

@ -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

View File

@ -1471,6 +1471,16 @@ nsXULElement::GetAttrCount() const
return count;
}
void
nsXULElement::DestroyContent()
{
nsDOMSlots* slots = GetExistingDOMSlots();
if (slots) {
NS_IF_RELEASE(slots->mControllers);
}
nsGenericElement::DestroyContent();
}
#ifdef DEBUG
void

View File

@ -568,6 +568,7 @@ public:
PRBool aNotify);
virtual const nsAttrName* GetAttrNameAt(PRUint32 aIndex) const;
virtual PRUint32 GetAttrCount() const;
virtual void DestroyContent();
#ifdef DEBUG
virtual void List(FILE* out, PRInt32 aIndent) const;