474362 - Merge nsNodeInfoManager and nsDocument?

Status: NEW

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

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Right now nsINode::GetOwnerDocument/GetCurrentDocument traverse mNodeInfo->mOwnerManager->mDocument. The relationship between nsNodeInfoManager and nsDocument is 1-to-1 so we could merge them, and then going from a node to its owner document would be reduced from 3 to 2 loads. They could be merged by having nsDocument include nsNodeInfoManager as a direct member and then make nsNodeInfoManager::GetDocument an inline method that does a pointer adjustment using offsetof.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

There's currently two cases where the lifetime of nsDocument and nsNodeInfoManager is different:

1) hen calling DOMImplementation.createDocType we use an "anonymous" nodeinfomanager since the returned doctype node doesn't have an ownerDocument.

2) If a node is leaked, the node only holds on to the nodeinfomanager which doesn't really hold on to anything, we don't hold on to the document


For 1 we could possibly create a special document and use that to create nodeinfos. Just have to ensure that GetOwnerDoc returns null.

2 might be changing soon anyway in bug 335998

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to Jonas Sicking (:sicking) from comment #1)
> There's currently two cases where the lifetime of nsDocument and
> nsNodeInfoManager is different:
> 
> 1) hen calling DOMImplementation.createDocType we use an "anonymous"
> nodeinfomanager since the returned doctype node doesn't have an
> ownerDocument.
> 
> 2) If a node is leaked, the node only holds on to the nodeinfomanager which
> doesn't really hold on to anything, we don't hold on to the document
> 
> 
> For 1 we could possibly create a special document and use that to create
> nodeinfos. Just have to ensure that GetOwnerDoc returns null.
> 
> 2 might be changing soon anyway in bug 335998

And 1 was changed in bug 675166. Should we fix this now?

Comment 3

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

Yes. That wouldn't probably speed up anything, since nodeinfos do have mDocument nowadays, but
it could clean up the code.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Doesn't seem like a high priority item since we've already reduced the number of loads to get to the owner document.

But yes, it would be a nice code cleanup. We just need to be careful about the nodeinfo for the document node itself.