400779 - "ASSERTION: post-reflow queues not empty" with xul:listboxbody and xul:menulist

Status: VERIFIED FIXED

(Core :: XUL, defect, P2)

Description

[Jesse Ruderman]

Attachment: testcase (reload to see the assertions)

Reloading the testcase triggers:

###!!! ASSERTION: post-reflow queues not empty.  This means we're leaking: 'mFirstCallbackEventRequest == nsnull && mLastCallbackEventRequest == nsnull', file /Users/jruderman/trunk/mozilla/layout/base/nsPresShell.cpp, line 1406

###!!! ASSERTION: Some objects allocated with AllocateFrame were not freed: 'mFrameCount == 0', file /Users/jruderman/trunk/mozilla/layout/base/nsPresShell.cpp, line 673

Comment 1

[Neil Deakin]

Similar to bug 377592 and may be the same bug. Seems like there are cases where a reflow isn't finishing up properly for some reason.

Comment 2

[Jesse Ruderman]

Moving from P3 to P2 because this bug gets in the way of testing for an important assertion.  The sooner this is fixed, the sooner we will find out whether there are other ways to trigger this assertion.

Comment 3

[Mike Schroepfer]

Neil - can you take this?

Comment 4

[Neil Deakin]

nsMenuFrame::Init posts a reflow callback, but in this case, reflow is done via nsPresShell::ContentAppended which uses a WillCauseReflow/DidCauseReflow pair and doesn't process the reflow callbacks. However, other reflows are done via a WillDoReflow/DidDoReflow pair which does process the callbacks.

Don't know what the distinction with these is though. Maybe someone can enlighten us?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Huh?  ContentAppended doesn't do a reflow.  It just posts an event to do the reflow.  So like every other function that does that it's using WillCauseReflow/DidCauseReflow (which are somewhat misnamed, but that's a detail).

So what's actually going on here?  What data is comment 4 based on?  Specifically, the "reflow is done via nsPresShell::ContentAppended" part.

Comment 6

[Neil Deakin]

(In reply to comment #5)
> Huh?  ContentAppended doesn't do a reflow.  It just posts an event to do the
> reflow.  So like every other function that does that it's using
> WillCauseReflow/DidCauseReflow (which are somewhat misnamed, but that's a
> detail).
> 

OK, the misnamed methods made me assume a reflow was occuring. Further investigation...

Within nsMenuFrame::Init, we set some attributes without notification, so no reflowing is added. Then, we're trying to post a reflow callback with PostReflowCallback from nsMenuFrame::Init when no reflow commands are available to be performed. So the callback never gets done or removed.

Olli, this used to be a nsIRunnable callback, why was this changed?

Comment 7

[Mats Palmgren (inactive)]

This is the same underlying issue as bug 395628, no? 
I already had r=Olli sr=dbaron but I had to make a couple of
additions for new consumers added after the first review...

Comment 8

[Boris Zbarsky [:bzbarsky]]

> from nsMenuFrame::Init when no reflow commands are available to be performed

That shouldn't matter.  We're in a frame Init() method; we should post a reflow command for whatever new frames we've created once we finish creating them.

Comment 9

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

(In reply to comment #6)
> Olli, this used to be a nsIRunnable callback, why was this changed?
Because of Bug 368501. nsIRunnable run too late.

(In reply to comment #7)
> This is the same underlying issue as bug 395628, no? 
I think so. Or the patch there possibly fixes this on too.

Comment 10

[Jesse Ruderman]

I don't see this any more.

--> FIXED by Mats' patch in bug 395628.

Comment 11

[Carsten Book [:Tomcat]]

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 assertion on testcase

--> Verified fixed

Comment 12

[Jesse Ruderman]

Crashtest: http://hg.mozilla.org/mozilla-central/rev/43339764467b