Status

()

Core
XUL
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: smaug, Assigned: smaug)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
Created attachment 539552 [details] [diff] [review]
patch
Attachment #539552 - Flags: review?(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.
(Assignee)

Comment 2

6 years ago
(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.
(Assignee)

Comment 3

6 years ago
Created attachment 542798 [details] [diff] [review]
patch
Attachment #539552 - Attachment is obsolete: true
Attachment #539552 - Flags: review?(peterv)
Attachment #542798 - Flags: review?(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.
Attachment #542798 - Flags: review?(peterv) → review+
(Assignee)

Comment 5

6 years ago
http://hg.mozilla.org/mozilla-central/rev/6cf983334e7a
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.