664452 - Unlink content/xul more

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

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

Attachment: patch

Comment 1

[Peter Van der Beken [:peterv]]

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

Review of attachment 539552 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great, except for nsXULDocument::Destroy.

::: content/xul/document/src/nsXULDocument.cpp
@@ +636,5 @@
>      OnPrototypeLoadDone(PR_TRUE);
>  }
>  
> +void
> +nsXULDocument::Destroy()

We should be removing Destroy, not add more to it. Why doesn't the existing unlinking code work here?

::: content/xul/document/src/nsXULPrototypeDocument.cpp
@@ +198,5 @@
>                                                      nsXULPrototypeElement)
>      cb.NoteXPCOMChild(static_cast<nsIScriptGlobalObject*>(tmp->mGlobalObject));
>      NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NATIVE_MEMBER(mNodeInfoManager,
>                                                      nsNodeInfoManager)
> +    for (PRUint32 i = 0; i < tmp->mPrototypeWaiters.Length(); ++i) {

You need to add NS_CYCLE_COLLECTION_NOTE_EDGE_NAME here.

Comment 2

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

(In reply to comment #1)
> > +void
> > +nsXULDocument::Destroy()
> 
> We should be removing Destroy, not add more to it. Why doesn't the existing
> unlinking code work here?
Actually, I think that is a leftover from some earlier state of the
patch. I'll re-test.

Comment 3

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

Attachment: patch

Comment 4

[Peter Van der Beken [:peterv]]

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

Review of attachment 542798 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/xul/document/src/nsXULPrototypeDocument.cpp
@@ +200,1 @@
>      cb.NoteXPCOMChild(static_cast<nsIScriptGlobalObject*>(tmp->mGlobalObject));

Might be able to use NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR_AMBIGUOUS here?

@@ +201,5 @@
>      NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NATIVE_MEMBER(mNodeInfoManager,
>                                                      nsNodeInfoManager)
> +    for (PRUint32 i = 0; i < tmp->mPrototypeWaiters.Length(); ++i) {
> +        NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mPrototypeWaiters[i]");
> +        cb.NoteXPCOMChild(static_cast<nsINode*>(tmp->mPrototypeWaiters[i].get()));

Same here? Not sure, so no big deal if you can't.

Comment 5

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

http://hg.mozilla.org/mozilla-central/rev/6cf983334e7a