388058 - Crash [@ nsGfxScrollFrameInner::AsyncScrollPortEvent::Run()] on print preview with xbl, table elements and float

Status: RESOLVED WORKSFORME

(Core :: Layout, defect, P4)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, which crashes on print preview for me with current trunk build.

http://crash-stats.mozilla.com/report/index/def09530-315b-11dc-9b88-001a4bd43ed6
0  	nsGfxScrollFrameInner::AsyncScrollPortEvent::Run()
1 	nsThread::ProcessNextEvent(int,int *)
2 	NS_ProcessNextEvent_P(nsIThread *,int)
3 	nsBaseAppShell::Run()
4 	nsAppStartup::Run()
5 	XRE_main
6 	main
7 	WinMain
8 	__tmainCRTStartup
9 	BaseProcessStart

The xbl consists of this:
<bindings xmlns="http://www.mozilla.org/xbl">
<binding id="randomxbl" inheritstyle="false">
<content>
<children/>

<iframe xmlns="http://www.w3.org/1999/xhtml"></iframe>
<div xmlns="http://www.w3.org/1999/xhtml" style="overflow: scroll; display: block;"></div>
<div xmlns="http://www.w3.org/1999/xhtml" style="overflow: scroll; display: table-footer-group;"></div>
<div xmlns="http://www.w3.org/1999/xhtml" style="overflow: scroll; display: block;"></div>

</content>
<implementation>
<constructor>
</constructor>
</implementation>
</binding>
</bindings>

This seems to have regressed somehow between 2006-01-26 and 2006-01-28:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-01-26+04&maxdate=2006-01-28+09&cvsroot=%2Fcvsroot
I have no idea what might have caused it.

I tried to minimize it as far as I could, but it seems I'm stuck with this, which is still quite complicate, unfortunately.

Comment 1

[Boris Zbarsky [:bzbarsky]]

This seems to worksforme on Linux... :(

Comment 2

[Mats Palmgren (inactive)]

Attachment: Frame dumps

###!!! ASSERTION: Creating a circular frame list, this is very bad.: 'this != aNextSibling', file ./../generic/nsIFrame.h, line 751

The problem is that we add a next-in-flow here:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/tables/nsTableFrame.cpp&rev=3.697&root=/cvsroot&mark=2990#2984
that is already in |rowGroups|

Here are three frame dumps at various stages of reflow.
When we reflow the inner table (blue) it creates an overflow list,
that table itself gets a continuation (lime), but this continuation is not
reflowed for some reason (so it does not pick up the overflowed row-groups)
before we get a new reflow for the outer table, leading to a reflow of the
inner table again.

The error is created when the blue frame picks up its own overflow at the
start of Reflow() through MoveOverflowToChildList().  It never resets the
next-in-flow chain for the frames it picks up.

Comment 3

[Mats Palmgren (inactive)]

Attachment: Like so?

This fixes it for me...

Comment 4

[Mats Palmgren (inactive)]

I'm a little bit curious about that 'else'-part (which was added in
bug 347367 (first patch)).  Why are we pushing an existing next-in-flow
as if it were a child of this table?  Shouldn't an existing next-in-flow
be on a frame list somewhere already?
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/tables/nsTableFrame.cpp&rev=3.697&root=/cvsroot&mark=2990#2984

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 272296 [details] [diff] [review]
Like so?

I'll look, but roc or Eli should also look -- they know the printing code better than I do.

Comment 6

[Mats Palmgren (inactive)]

The testcase doesn't seem to work for branch:
###!!! ASSERTION: Binding load from a non-URL URI not allowed.: 'Error', file nsXBLService.cpp, line 853

Martijn, is it possible to make a testcase for branch that works around
that somehow?

Comment 7

[Martijn Wargers (dead)]

Attachment: testcase2

Yeah, sorry, I should have used the inline binding stuff in the first place.
This should work also work fine on branch.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 272296 [details] [diff] [review]
Like so?

I;m definitely the wrong reviewer here.  You want bernd and roc for r and sr respectively.

Comment 9

[Martijn Wargers (dead)]

The testcases don't crash anymore with current trunk build.
Can this bug be resolved?

Comment 10

[Daniel Holbert [:dholbert]]

Works for me on current trunk build, except for this assertion:

###!!! ASSERTION: Adding child where we already have a child?  This will likely misbehave: 'Error', file /scratch/work/builds/trunk.07-09-28.10-58/mozilla/docshell/shistory/src/nsSHEntry.cpp, line 586

I get that assertion only on reloading the testcases - not on the initial load.

I also get a different assertion the first time I navigate away from the testcase (e.g. by entering another URL in address bar) and then when I click back and forward going from the testcase to other pages:

###!!! ASSERTION: cannot call on a dirty frame not currently being reflowed: '!NS_SUBTREE_DIRTY(this) || (GetStateBits() & NS_FRAME_IN_REFLOW)', file /scratch/work/builds/trunk.07-09-28.10-58/mozilla/layout/generic/nsFrame.cpp, line 556

Comment 11

[Daniel Holbert [:dholbert]]

(last comment applies to both Linux and Windows -- tried current trunk builds on both)

Comment 12

[Daniel Holbert [:dholbert]]

The "Adding Child" assertion is already filed as bug 307241.
The "Cannot call on a dirty frame" assertion is already filed as bug 387213.

 --> Resolving this bug as WFM, because its crash is fixed and the remaining assertions have other bugs filed already.

Comment 13

[Mats Palmgren (inactive)]

I still get a few nasty assertions in an up-to-date Linux build:

###!!! ASSERTION: Creating a circular frame list, this is very bad.: 'this != aNextSibling', file ./../generic/nsIFrame.h, line 802
###!!! ASSERTION: Placeholder relationship should have been torn down; see comments in nsPlaceholderFrame.h: '!shell->FrameManager()->GetPlaceholderFrameFor(mOutOfFlowFrame)', file nsPlaceholderFrame.cpp, line 132
###!!! ASSERTION: Some objects allocated with AllocateFrame were not freed: 'mFrameCount == 0', file nsPresShell.cpp, line 673

Comment 14

[Martijn Wargers (dead)]

Well, there is already a bug for the first assertion, at least, bug 363120.

Comment 15

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

So can anyone reproduce this crash?

Comment 16

[Martijn Wargers (dead)]

I can't reproduce it anymore.

Comment 17

[Martijn Wargers (dead)]

I'm marking this worksforme, I'm only seeing this assertion anymore:
###!!! ASSERTION: Can't split negative heights: 'aSpanningRowBottom >= 0', file
c:/mozilla-build-1.3/src/layout/tables/nsTableRowGroupFrame.cpp, line 960
But no crash.

I don't know if the patch is still useful for something? I guess not.