523082 - nsXULTemplateBuilder should unlink more

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Split from bug 335998.

Comment 1

[Peter Van der Beken [:peterv]]

Comment on attachment 406999 [details] [diff] [review]
patch

>diff --git a/content/xul/templates/src/nsXULTemplateBuilder.cpp b/content/xul/templates/src/nsXULTemplateBuilder.cpp

>+      tmp->mMatchMap.EnumerateRead(DestroyMatchList, &(tmp->mPool));
>+      tmp->mMatchMap.Clear();

Can we make DestroyMatchList return PL_DHASH_REMOVE instead of making every caller call Clear and effectively enumerating twice?

>diff --git a/content/xul/templates/src/nsXULTemplateQueryProcessorXML.cpp b/content/xul/templates/src/nsXULTemplateQueryProcessorXML.cpp

>     NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mTemplateBuilder)
>     NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mRequest)
>+    NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mRoot)
>+    NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mEvaluator)

Maybe use the same order that these are declared in.

Looks like we also need to hook up nsXMLBindingSet and nsXMLBinding, and mRuleToBindingsMap (both keys and values).

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 406999 [details] [diff] [review]
patch

New patch coming

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Unlink even more.
I took the nsDocument.cpp part from Bug 335998.
I could file a new bug for that if really needed.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

I posted the patch to tryserver, though I did run
template tests locally.

Comment 5

[Peter Van der Beken [:peterv]]

Comment on attachment 438744 [details] [diff] [review]
patch

>diff --git a/content/base/src/nsDocument.cpp b/content/base/src/nsDocument.cpp

>+  if (tmp->mBoxObjectTable) {
>+   tmp->mBoxObjectTable->EnumerateRead(ClearAllBoxObjects, nsnull);
>+   delete tmp->mBoxObjectTable;
>+   tmp->mBoxObjectTable = nsnull;
>+ }

My worry here was/is that maybe something needs their boxobject when they are unlinked or being destroyed from unlinking.

>diff --git a/content/xul/templates/src/nsXULTemplateQueryProcessorXML.cpp b/content/xul/templates/src/nsXULTemplateQueryProcessorXML.cpp

>+TraverseRuleToBindingsMap(nsISupports* aKey, nsXMLBindingSet* aMatch, void* aContext)

>+    cb->NoteXPCOMChild(aKey);
>+    cb->NoteNativeChild(aMatch, &NS_CYCLE_COLLECTION_NAME(nsXMLBindingSet));

Please add NS_CYCLE_COLLECTION_NOTE_EDGE_NAME for both.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

I need to remember to land this

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Attachment: up-to-date

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/b99eafbd3644