Circular ref counting between nsHTMLFragmentContentSink and nsParser

VERIFIED FIXED in M9

Status

()

Core
HTML: Parser
P3
major
VERIFIED FIXED
19 years ago
19 years ago

People

(Reporter: Akkana Peck, Assigned: vidur (gone))

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

19 years ago
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.

Updated

19 years ago
Assignee: rickg → vidur

Comment 1

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

Updated

19 years ago
Status: NEW → ASSIGNED
Target Milestone: M9
(Assignee)

Comment 2

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

Updated

19 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED
(Assignee)

Comment 3

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

Updated

19 years ago
Whiteboard: 8/4 need response from reporter to verify
(Reporter)

Updated

19 years ago
Status: RESOLVED → VERIFIED
Whiteboard: 8/4 need response from reporter to verify
You need to log in before you can comment on or make changes to this bug.