Closed Bug 716006 Opened 8 years ago Closed 8 years ago

Don't traverse NodeInfoManager so much


(Core :: DOM: Core & HTML, defect)

12 Branch
Not set





(Reporter: smaug, Assigned: smaug)


(Blocks 1 open bug)



(1 file)

Attached patch patchSplinter Review
If NodeInfoManager has mDocument and mDocument is in CC generation, it certainly
keeps NodeInfoManager alive, since mDocument owns the manager, and the manager
will be deleted only when document is deleted.
Attachment #586510 - Flags: review?(jst)
Attachment #586510 - Flags: review?(continuation)
Blocks: 698919
OS: Linux → All
Hardware: x86_64 → All
What is the path from a document to the nsNodeInfoManager?  Can we statically check that path, or is it just from elements in the document?
Comment on attachment 586510 [details] [diff] [review]


The path from the document to the nodeinfo manager is a direct strong reference (from nsIDocument::mNodeInfoManager).
Attachment #586510 - Flags: review?(jst) → review+
Comment on attachment 586510 [details] [diff] [review]

Review of attachment 586510 [details] [diff] [review]:

Looks good to me. It might be a good idea to add an assertion that checks the document really holds onto the infomanager, but unlike some of the patches that seems like a very fundamental thing and is unlikely to change.
Attachment #586510 - Flags: review?(continuation) → review+
If nodeinfomanager has a document, it is created when document::init is called, and 
document does not ever release it before dtor.
Note, nodeinfomanager does not own document, if document doesn't have any child nodes connected to it.
This way nodes in the document own their ownerDocument, but just having document alive doesn't
create a cycle to itself.

If document is marked to be in generation, it is either in some docshell/contentviewer or it is 
an XBL document hold by xul cache.
Closed: 8 years ago
Resolution: --- → FIXED
Blocks: 716598
No longer blocks: 698919
You need to log in before you can comment on or make changes to this bug.