8975 - Circular ref counting between nsHTMLFragmentContentSink and nsParser

Status: VERIFIED FIXED

(Core :: DOM: HTML Parser, defect, P3)

Description

[Akkana Peck]

nsHTMLFragmentContentSink::SetParser addrefs the parser from the
nsHTMLFragmentContentSink; but nsParser::SetContentSink addrefs the content
sink.  We have a circular reference and neither the parser nor the content sink
is ever deleted.  This is a problem because nsHTMLFragmentContentSink maintains
a buffer which may need to be flushed, and it never gets the signal to flush it,
so the end of the fragment is lost.  (I'll work on fixing that problem
separately).

What's the policy on parser and sink ownership?  Neither creates the other -- so
which is master and should addref, which is slave and should not?

A good way to demonstrate this (to developers, I don't expect QA to do this
though if you have a debugger you're welcome to join in the fun) is to run
apprunner -edit, set breakpoints in both addref and release, and also in the
destructors (to see that they're never called) for both nsParser and
nsHTMLFragmentContentSink, then click somewhere in the editor window and type
alt-I (a debug key sequence hardwired to insert a small html fragment).  Then
count the addrefs and releases.

Comment 1

[rickg]

Vidur -- since you create the parser and the sink, I think you own this problem.
I can fix it if you don't have time -- but my preference is to switch over to
core layout code.

Comment 2

[vidur (gone)]

Yup. There's code in the HTMLContentSink to break the circular reference and I
forgot to include it. Fix will be checked in when the tree opens.

Comment 3

[vidur (gone)]

Fix checked in on 8/4. The circular reference is broken in DidBuildModel().