DESCRIPTION: We leak a parser and content sink and everything they own (i.e., the document) when we load a multi-mixed page and leave that page by going to another page rather than exiting the browser. We leak because the parser leaks when DidBuildModel is not called. STEPS TO REPRODUCE: 1. load http://bugzilla.mozilla.org/ 2. click on "View Bugs Already Reported Today" 3. load about:blank I will attach leak logs showing the case where we leak (the above steps), and the same parser where I replace (3) above with immediately exiting the browser. Exiting the browser causes DidBuildModel to be called by the following chain: nsDocShell::Destroy nsDocShell::Stop DocumentViewerImpl::Stop nsHTMLDocument::StopDocumentLoad nsParser::Terminate nsParser::DidBuildModel This doesn't happen when moving to another page. It's not clear to me where the problem is, but this is a big leak. I wonder if some of the Stop methods should normally be called. I also wonder if there's something strange about the multi-mixed converter.

Nominating for nsbeta3 since this is a big leak and assigning to harish since rickg is away.

someone else pointed out a case in which DidBuildModel was not being called. This is big.

there we go: 25368 . Sounds like DidBuildModel() call semantics need to change.

Harish, what do you think of these changes? I do need to test them a good bit more. They make it so that rather than silently leak if DidBuildModel is called, we assert instead (and don't leak). The previous ownership model was (as I understand it): document owns parser parser owns content sink content sink owns parser content sink owns document I made the latter two weak references.

would this patch solve 25368 as well?

Probably. How does one hand the parser a non-existant URL?

Your patch looks good to me. Weak reference would definitely break the circularity but rememeber that it's a big HACK by itself. Post release we should find a better way to breakup the circularity ( IMO we should avoid using weak references as much as possible ). BTW, Good work David in fixing this leak and the 1M (!!) leak.

Harish, give a yell if "Your patch looks good to me." is not "r=harishd"... See also bug 37434.

"Your patch looks good to me." is "r=harishd" :-) Go ahead.

Actually, I won't check it in because I discovered my patch causes http://www.netscape.com/ to crash. I'll add more details to bug 37434 (which I think is the appropriate bug).

Setting priority to P1.

PDT agrees P1

Putting [pdtp1] in whiteboard

Rick Potts recent checkin to the docshell fixed this leak. Marking FIXED.

updated qa contact.

Verified on: build: 2001-03-29-09-Mtrunk Platform: Win NT Marked verified as per developer comments, since cannot verify the leaks in the parser.