[FIX]Leak with XUL in iframe

VERIFIED FIXED in mozilla1.9.1b1

Status

()

Core
XUL
P2
normal
VERIFIED FIXED
11 years ago
9 years ago

People

(Reporter: Jesse Ruderman, Assigned: bz)

Tracking

(Blocks: 2 bugs, {assertion, memory-leak, testcase})

Trunk
mozilla1.9.1b1
x86
Mac OS X
assertion, memory-leak, testcase
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

11 years ago
Created attachment 289463 [details]
testcase

On about 80% of loads, this testcase triggers a bunch of assertions and makes DOMWindows leak.

###!!! ASSERTION: initial containing block already created: 'nsnull == mInitialContainingBlock', file /Users/jruderman/trunk/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 9000

###!!! ASSERTION: Popup set is already defined! Only 1 allowed.: 'Not Reached', file /Users/jruderman/trunk/mozilla/layout/xul/base/src/nsRootBoxFrame.cpp, line 298

###!!! ASSERTION: Unexpected PopupSetFrame: 'rootBox->GetPopupSetFrame() == newFrame', file /Users/jruderman/trunk/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 6204

###!!! ASSERTION: Unexpected child of document element containing block: 'mDocElementContainingBlock->GetFirstChild(nsnull) == nsnull', file /Users/jruderman/trunk/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 9028

###!!! ASSERTION: already have a child frame: 'mFrames.IsEmpty()', file /Users/jruderman/trunk/mozilla/layout/xul/base/src/nsRootBoxFrame.cpp, line 165
JavaScript error: , line 0: uncaught exception: Permission denied to get property XPCComponents.classes

###!!! ASSERTION: Some objects allocated with AllocateFrame were not freed: 'mFrameCount == 0', file /Users/jruderman/trunk/mozilla/layout/base/nsPresShell.cpp, line 673
we'd better get this fixed
Assignee: nobody → roc
Flags: blocking1.9?
Priority: -- → P2
The issue is most likely that nsXULDocument::StartLayout calls InitialReflow on the presshell unconditionally.  If it's already been called (e.g. by nsSubDocumentFrame::Init calling nsSubDocumentFrame::ShowDocShell calling nsDocShell::SetVisibility calling DocumentViewerImpl::Show which does InitPresentationStuff(PR_TRUE)), then we'll get these asserts.

So it's a timing issue: if the style change that causes a frame reconstruct (the body display change) comes through after the XUL has started loading but before it's finished loading (e.g. while it's waiting for the stylesheet), we'll get this problem.

I suppose we could skip the InitialReflow call if it's already happened (as the HTML sink does).  But we should double-check the BeginObservingDocument() mess.  I _think_ it would always have been called in that case and the presshell would be observing the document, but worth checking.

Updated

11 years ago
Flags: blocking1.9? → blocking1.9+
I have reproduced this with the testcase but *very* infrequently. I let it run for hundreds of reloads and I've only seen the assertions fire twice.

I also got one of these once:
###!!! ASSERTION: Shallow unbind won't clear document and binding parent on kids!: 'aDeep || (!GetCurrentDoc() && !GetBindingParent())', file /Users/roc/mozilla-trunk/content/base/src/nsGenericElement.cpp, line 2086

Boris, do you want to take this bug?
Created attachment 290621 [details]
Testcase that should reliably reproduce this

I can try to look at this, but I'm pretty crunched for time right now...  I'm also not sure there's a sane way to fix this while giving correct behavior in all cases without digging deep into the tree-building code that the XUL prototype walk does.  :(

I'm pretty sure I can get to this for beta3.
I think it's pretty clear you're the right person to fix this if you possibly can. Send some bugs my way in return :-)
Assignee: roc → bzbarsky
(Reporter)

Updated

11 years ago
Blocks: 334514
(Reporter)

Comment 6

11 years ago
Sweet, this is the last known bug that triggers the assertion added in bug 334514.
Created attachment 296858 [details] [diff] [review]
Proposed fix

The major change here is that when doing Show() on a viewer for an already-existing document we'll only do the initial reflow if the document is ready for it.  Documents start our ready by default, and become unready when StartDocumentLoad is called.  After that, either the sink or the caller of StartDocumentLoad() sets the document as ready again.

The existing BeginObservingDocument() calls are all not needed, since document viewer calls it.
Attachment #296858 - Flags: superreview?(jst)
Attachment #296858 - Flags: review?(jst)
Summary: Leak with XUL in iframe → [FIX]Leak with XUL in iframe

Updated

11 years ago
Attachment #296858 - Flags: superreview?(jst)
Attachment #296858 - Flags: superreview+
Attachment #296858 - Flags: review?(jst)
Attachment #296858 - Flags: review+
Fixed 2008-01-20 10:02.  Checked in my testcase as a crashtest.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
verified fixed using  Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008050621 Firefox/3.0pre - no leak/assertion on testcase

--> Verified fixed
Status: RESOLVED → VERIFIED

Updated

10 years ago
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
Target Milestone: --- → mozilla1.9.1b1
You need to log in before you can comment on or make changes to this bug.