425253 - stack overflow Marquee testcase from bug 239840 is crashing [@ ntdll.dll] Mozilla again

Status: VERIFIED FIXED

(Core :: Layout: Block and Inline, defect)

Description

[Martijn Wargers (dead)]

Attachment: stack from debug build

I filed this once as bug 363722, but that bug became worksforme, somewhere in current trunk development cycle.
However, that testcase started crashing again. Hence, this new bug.

Part of the stack from the debug build:
 	ntdll.dll!__SEH_prolog()  + 0x1a bytes	
 	msvcr80d.dll!_heap_alloc_base(unsigned int size=1079)  Line 105 + 0x28 bytes	C
 	msvcr80d.dll!_heap_alloc_dbg(unsigned int nSize=1043, int nBlockUse=1, const char * szFileName=0x00000000, int nLine=0)  Line 409 + 0x9 bytes	C++
 	msvcr80d.dll!_nh_malloc_dbg(unsigned int nSize=1043, int nhFlag=0, int nBlockUse=1, const char * szFileName=0x00000000, int nLine=0)  Line 266 + 0x15 bytes	C++
 	msvcr80d.dll!malloc(unsigned int nSize=1043)  Line 152 + 0x15 bytes	C++
>	nspr4.dll!PR_Malloc(unsigned int size=1043)  Line 508 + 0xa bytes	C
 	plds4.dll!PL_ArenaAllocate(PLArenaPool * pool=0x00033384, unsigned int nb=60)  Line 228 + 0xa bytes	C
 	gklayout.dll!nsLineLayout::NewPerSpanData(nsLineLayout::PerSpanData * * aResult=0x0003321c)  Line 385 + 0x43 bytes	C++
 	gklayout.dll!nsLineLayout::BeginLineReflow(int aX=0, int aY=0, int aWidth=0, int aHeight=1073741824, int aImpactedByFloats=0, int aIsTopOfPage=0)  Line 217	C++
 	gklayout.dll!nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState & aState={...}, nsLineLayout & aLineLayout={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x00033648, LineReflowStatus * aLineReflowStatus=0x000333b8, int aAllowPullUp=1)  Line 3344	C++
 	gklayout.dll!nsBlockFrame::ReflowInlineFrames(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x00033648)  Line 3223 + 0x2a bytes	C++
 	gklayout.dll!nsBlockFrame::ReflowLine(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x00033648)  Line 2289 + 0x1b bytes	C++

Comment 1

[Martijn Wargers (dead)]

Breakpad ID's (they are not available yet):
http://crash-stats.mozilla.com/report/index/0e0347fd-fb74-11dc-a3b5-001a4bd43e5c
http://crash-stats.mozilla.com/report/index/069d156b-fb74-11dc-93d4-001a4bd43ef6

Comment 2

[Daniel Holbert [:dholbert]]

Clickable link to testcase: attachment 148963 [details]

Comment 3

[Ria Klaassen (not reading all bugmail)]

Regression window (when crash began):
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1205535420&maxdate=1205541239

Comment 4

[Kurt Weinschenker]

FWIW, this also gives a SPWOD to Camino 1.6b4pre (1.8.1.14pre 2008032504) on branch.  Test machine was running Mac OS X 10.4.11, as I don't have a Leopard machine around to test it on.  

The bug also appears to hang Mac OS X's Activity Monitor, when I attempted to obtain a sample.  I'll try to test with a Camino trunk build within a week or so.

Comment 5

[timeless]

Signature	nsSprocketLayout::PopulateBoxSizes(nsIFrame*, nsBoxLayoutState&, nsBoxSize*&, nsComputedBoxSize*&, int&, int&, int&)
UUID	069d156b-fb74-11dc-93d4-001a4bd43ef6
Time	2008-03-26 13:27:33-07:00
Uptime	83
Product	Firefox
Version	3.0b5pre
Build ID	2008032605
OS	Windows NT
OS Version	5.1.2600 Service Pack 2
CPU	x86
CPU Info	GenuineIntel family 6 model 13 stepping 8
Crash Reason	EXCEPTION_STACK_OVERFLOW
Crash Address	0x60547c00
Comments	
Crashing Thread
Frame 	Signature 	Source
0 	nsSprocketLayout::PopulateBoxSizes(nsIFrame*, nsBoxLayoutState&, nsBoxSize*&, nsComputedBoxSize*&, int&, int&, int&) 	mozilla/layout/xul/base/src/nsSprocketLayout.cpp:704
1 	nsSprocketLayout::Layout(nsIFrame*, nsBoxLayoutState&) 	mozilla/layout/xul/base/src/nsSprocketLayout.cpp:249
2 	nsBoxFrame::DoLayout(nsBoxLayoutState&) 	mozilla/layout/xul/base/src/nsBoxFrame.cpp:945
3 	nsIFrame::Layout(nsBoxLayoutState&) 	mozilla/layout/xul/base/src/nsBox.cpp:561
4 	nsXULScrollFrame::LayoutScrollArea(nsBoxLayoutState&, nsRect const&) 	mozilla/layout/generic/nsGfxScrollFrame.cpp:2070
5 	nsXULScrollFrame::Layout(nsBoxLayoutState&) 	mozilla/layout/generic/nsGfxScrollFrame.cpp:2224
6 	nsXULScrollFrame::DoLayout(nsBoxLayoutState&) 	mozilla/layout/generic/nsGfxScrollFrame.cpp:1222
7 	nsIFrame::Layout(nsBoxLayoutState&) 	mozilla/layout/xul/base/src/nsBox.cpp:561
8 	nsBoxFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) 	mozilla/layout/xul/base/src/nsBoxFrame.cpp:756
9 	nsLineLayout::ReflowFrame(nsIFrame*, unsigned int&, nsHTMLReflowMetrics*, int&) 	mozilla/layout/generic/nsLineLayout.cpp:859
10 	nsBlockFrame::ReflowInlineFrame(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) 	mozilla/layout/generic/nsBlockFrame.cpp:3552
11 	nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, int*, LineReflowStatus*, int) 	mozilla/layout/generic/nsBlockFrame.cpp:3374
12 	nsBlockFrame::ReflowInlineFrames(nsBlockReflowState&, nsLineList_iterator, int*) 	mozilla/layout/generic/nsBlockFrame.cpp:3223
13 	nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, int*) 	mozilla/layout/generic/nsBlockFrame.cpp:2289
14 	nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) 	mozilla/layout/generic/nsBlockFrame.cpp:1870
15 	nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) 	mozilla/layout/generic/nsBlockFrame.cpp:936
16 	nsLineLayout::ReflowFrame(nsIFrame*, unsigned int&, nsHTMLReflowMetrics*, int&) 	mozilla/layout/generic/nsLineLayout.cpp:859
17 	nsBlockFrame::ReflowInlineFrame(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) 	mozilla/layout/generic/nsBlockFrame.cpp:3552
18 	nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, int*, LineReflowStatus*, int) 	mozilla/layout/generic/nsBlockFrame.cpp:3374
19 	nsBlockFrame::ReflowInlineFrames(nsBlockReflowState&, nsLineList_iterator, int*) 	mozilla/layout/generic/nsBlockFrame.cpp:3223
20 	nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, int*) 	mozilla/layout/generic/nsBlockFrame.cpp:2289
21 	nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) 	mozilla/layout/generic/nsBlockFrame.cpp:1870
22 	nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) 	mozilla/layout/generic/nsBlockFrame.cpp:936
23 	nsFrame::BoxReflow(nsBoxLayoutState&, nsPresContext*, nsHTMLReflowMetrics&, nsIRenderingContext*, int, int, int, int, int) 	mozilla/layout/generic/nsFrame.cpp:6242
24 	nsFrame::DoLayout(nsBoxLayoutState&) 	mozilla/layout/generic/nsFrame.cpp:6032
25 	nsIFrame::Layout(nsBoxLayoutState&) 	mozilla/layout/xul/base/src/nsBox.cpp:561
26 	nsSprocketLayout::Layout(nsIFrame*, nsBoxLayoutState&) 	mozilla/layout/xul/base/src/nsSprocketLayout.cpp:523
27 	nsBoxFrame::DoLayout(nsBoxLayoutState&) 	mozilla/layout/xul/base/src/nsBoxFrame.cpp:945
28 	nsIFrame::Layout(nsBoxLayoutState&) 	mozilla/layout/xul/base/src/nsBox.cpp:561
29 	nsSprocketLayout::Layout(nsIFrame*, nsBoxLayoutState&) 	mozilla/layout/xul/base/src/nsSprocketLayout.cpp:523
30 	nsBoxFrame::DoLayout(nsBoxLayoutState&) 	mozilla/layout/xul/base/src/nsBoxFrame.cpp:945
31 	nsIFrame::Layout(nsBoxLayoutState&) 	mozilla/layout/xul/base/src/nsBox.cpp:561
32 	nsXULScrollFrame::LayoutScrollArea(nsBoxLayoutState&, nsRect const&) 	mozilla/layout/generic/nsGfxScrollFrame.cpp:2070
33 	nsXULScrollFrame::Layout(nsBoxLayoutState&) 	mozilla/layout/generic/nsGfxScrollFrame.cpp:2224
34 	nsXULScrollFrame::DoLayout(nsBoxLayoutState&) 	mozilla/layout/generic/nsGfxScrollFrame.cpp:1222
35 	nsIFrame::Layout(nsBoxLayoutState&) 	mozilla/layout/xul/base/src/nsBox.cpp:561
36 	nsBoxFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) 	mozilla/layout/xul/base/src/nsBoxFrame.cpp:756
37 	nsLineLayout::ReflowFrame(nsIFrame*, unsigned int&, nsHTMLReflowMetrics*, int&) 	mozilla/layout/generic/nsLineLayout.cpp:859
38 	nsBlockFrame::ReflowInlineFrame(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) 	mozilla/layout/generic/nsBlockFrame.cpp:3552
39 	nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, int*, LineReflowStatus*, int) 	mozilla/layout/generic/nsBlockFrame.cpp:3374
40 	nsBlockFrame::ReflowInlineFrames(nsBlockReflowState&, nsLineList_iterator, int*) 	mozilla/layout/generic/nsBlockFrame.cpp:3223
41 	nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, int*) 	mozilla/layout/generic/nsBlockFrame.cpp:2289
42 	nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) 	mozilla/layout/generic/nsBlockFrame.cpp:1870
43 	nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) 	mozilla/layout/generic/nsBlockFrame.cpp:936
44 	nsLineLayout::ReflowFrame(nsIFrame*, unsigned int&, nsHTMLReflowMetrics*, int&) 	mozilla/layout/generic/nsLineLayout.cpp:859
45 	nsBlockFrame::ReflowInlineFrame(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) 	mozilla/layout/generic/nsBlockFrame.cpp:3552
46 	nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, int*, LineReflowStatus*, int) 	mozilla/layout/generic/nsBlockFrame.cpp:3374
47 	nsBlockFrame::ReflowInlineFrames(nsBlockReflowState&, nsLineList_iterator, int*) 	mozilla/layout/generic/nsBlockFrame.cpp:3223
48 	nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, int*) 	mozilla/layout/generic/nsBlockFrame.cpp:2289
49 	nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) 	mozilla/layout/generic/nsBlockFrame.cpp:1870
50 	nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) 	mozilla/layout/generic/nsBlockFrame.cpp:936
51 	nsFrame::BoxReflow(nsBoxLayoutState&, nsPresContext*, nsHTMLReflowMetrics&, nsIRenderingContext*, int, int, int, int, int) 	mozilla/layout/generic/nsFrame.cpp:6242
52 	nsFrame::DoLayout(nsBoxLayoutState&) 	mozilla/layout/generic/nsFrame.cpp:6032

Comment 6

[Kurt Weinschenker]

Tested as promised, with a recent Camino trunk build: Version 2.0a1pre (1.9pre 2008033119) running under Mac OS 10.4.11.

Camino trunk does not appear to hang on testcase in comment #2, unlike the branch build I tested last week.

Comment 7

[Damon Sicore (:damons)]

+'ing for investigation as it seems we regressed this again.

Comment 8

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

Is MAX_REFLOW_DEPTH or MAX_FRAME_DEPTH too high for OSX or Windows?
http://lxr.mozilla.org/seamonkey/source/parser/htmlparser/public/nsIHTMLContentSink.h#91
http://lxr.mozilla.org/seamonkey/source/layout/generic/nsFrame.cpp#3724
I don't get the crash on 64bit nor 32bit Linux. The browser becomes slow
when loading the page, but after closing the tab/window, things are ok again.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Doesn't crash for me on Mac debug. I'll test Windows in a jiffy.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yeah, crashes on Windows. It probably is just a stack depth issue with the stack getting deeper, and could probably be fixed by changing MAX_FRAME_DEPTH.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Just for fun I worked out the stack frame sizes for the repeating part of the call stack in this testcase:

1216	gklayout.dll!nsBlockFrame::Reflow(nsPresContext * aPresContext=0x0571f028, nsHTMLReflowMetrics & aMetrics={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0)  Line 936 + 0xf bytes	C++
528	gklayout.dll!nsFrame::BoxReflow(nsBoxLayoutState & aState={...}, nsPresContext * aPresContext=0x0571f028, nsHTMLReflowMetrics & aDesiredSize={...}, nsIRenderingContext * aRenderingContext=0x04649280, int aX=0, int aY=0, int aWidth=0, int aHeight=0, int aMoveFrame=1)  Line 6244	C++
152	gklayout.dll!nsFrame::DoLayout(nsBoxLayoutState & aState={...})  Line 6032 + 0x2a bytes	C++
36	gklayout.dll!nsIFrame::Layout(nsBoxLayoutState & aState={...})  Line 563	C++
408	gklayout.dll!nsSprocketLayout::Layout(nsIFrame * aBox=0x04ad4fa8, nsBoxLayoutState & aState={...})  Line 527	C++
32	gklayout.dll!nsBoxFrame::DoLayout(nsBoxLayoutState & aState={...})  Line 945 + 0x24 bytes	C++
36	gklayout.dll!nsIFrame::Layout(nsBoxLayoutState & aState={...})  Line 563	C++
408	gklayout.dll!nsSprocketLayout::Layout(nsIFrame * aBox=0x04ad4ce4, nsBoxLayoutState & aState={...})  Line 527	C++
32	gklayout.dll!nsBoxFrame::DoLayout(nsBoxLayoutState & aState={...})  Line 945 + 0x24 bytes	C++
36	gklayout.dll!nsIFrame::Layout(nsBoxLayoutState & aState={...})  Line 563	C++
132	gklayout.dll!nsXULScrollFrame::LayoutScrollArea(nsBoxLayoutState & aState={...}, const nsRect & aRect={...})  Line 2072	C++
216	gklayout.dll!nsXULScrollFrame::Layout(nsBoxLayoutState & aState={...})  Line 2227	C++
20	gklayout.dll!nsXULScrollFrame::DoLayout(nsBoxLayoutState & aState={...})  Line 1222 + 0xc bytes	C++
36	gklayout.dll!nsIFrame::Layout(nsBoxLayoutState & aState={...})  Line 563	C++
156	gklayout.dll!nsBoxFrame::Reflow(nsPresContext * aPresContext=0x0571f028, nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0)  Line 761	C++
408	gklayout.dll!nsLineLayout::ReflowFrame(nsIFrame * aFrame=0x04ad4d4c, unsigned int & aReflowStatus=0, nsHTMLReflowMetrics * aMetrics=0x00000000, int & aPushedFrame=0)  Line 859 + 0x2d bytes	C++
192	gklayout.dll!nsBlockFrame::ReflowInlineFrame(nsBlockReflowState & aState={...}, nsLineLayout & aLineLayout={...}, nsLineList_iterator aLine={...}, nsIFrame * aFrame=0x04ad4d4c, LineReflowStatus * aLineReflowStatus=0x00034c24)  Line 3552 + 0x16 bytes	C++
156	gklayout.dll!nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState & aState={...}, nsLineLayout & aLineLayout={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x00034fe8, LineReflowStatus * aLineReflowStatus=0x00034d58, int aAllowPullUp=1)  Line 3374 + 0x20 bytes	C++
260	gklayout.dll!nsBlockFrame::ReflowInlineFrames(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x00034fe8)  Line 3223 + 0x2a bytes	C++
256	gklayout.dll!nsBlockFrame::ReflowLine(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x00034fe8)  Line 2289 + 0x1b bytes	C++
440	gklayout.dll!nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & aState={...})  Line 1870 + 0x1b bytes	C++
1216	gklayout.dll!nsBlockFrame::Reflow(nsPresContext * aPresContext=0x0571f028, nsHTMLReflowMetrics & aMetrics={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=78465984)  Line 936 + 0xf bytes	C++
408	gklayout.dll!nsLineLayout::ReflowFrame(nsIFrame * aFrame=0x04ad4bc0, unsigned int & aReflowStatus=78465984, nsHTMLReflowMetrics * aMetrics=0x00000000, int & aPushedFrame=0)  Line 859 + 0x2d bytes	C++
192	gklayout.dll!nsBlockFrame::ReflowInlineFrame(nsBlockReflowState & aState={...}, nsLineLayout & aLineLayout={...}, nsLineList_iterator aLine={...}, nsIFrame * aFrame=0x04ad4bc0, LineReflowStatus * aLineReflowStatus=0x00035794)  Line 3552 + 0x16 bytes	C++
156	gklayout.dll!nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState & aState={...}, nsLineLayout & aLineLayout={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x00035b58, LineReflowStatus * aLineReflowStatus=0x000358c8, int aAllowPullUp=1)  Line 3374 + 0x20 bytes	C++
260	gklayout.dll!nsBlockFrame::ReflowInlineFrames(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x00035b58)  Line 3223 + 0x2a bytes	C++
256	gklayout.dll!nsBlockFrame::ReflowLine(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x00035b58)  Line 2289 + 0x1b bytes	C++
440	gklayout.dll!nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & aState={...})  Line 1870 + 0x1b bytes	C++
1216	gklayout.dll!nsBlockFrame::Reflow(nsPresContext * aPresContext=0x0571f028, nsHTMLReflowMetrics & aMetrics={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0)  Line 936 + 0xf bytes	C++

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

By far the biggest offender is nsBlockFrame::Reflow. I can't see why, though. The only obvious big thing there is "nsBlockReflowState state", which only 396 bytes apparently.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I really need to repeat this with an opt build, since the compiler will probably try a lot harder to reuse stack space there. But in the meantime I'll see if I can break some code out of nsBlockFrame::Reflow to fix the problem.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I tried that, but it didn't have much effect. Looking at the disassembly, the stack frame generated for nsBlockFrame::Reflow seems to have a bunch of unused space in it. Maybe stack usage in debug builds is just not very sane.

Did anyone reproduce this in an opt build?

Comment 15

[Martijn Wargers (dead)]

(In reply to comment #14) 
> Did anyone reproduce this in an opt build?

What do you mean? Yes, for me, the testcase is crashing with a normal trunk build.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

In a debug build, there's a 400-byte char array used as a buffer for sprintf debug output. Making that static, and changing NS_BLOCK_BAND_DATA_TRAPS from 6 to 2, plus my other changes, has reduced the size of the nsBlockFrame::Reflow stack frame from 1216 to 516 bytes. But the crash still happens in my debug build.

Comment 17

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Hmm, when we crash, aReflowState.mReflowDepth is 0 so it's probably not being propagated through XUL layout...

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

The right fix is to propagate mReflowDepth through box layout. Right now <marquee> effectively resets the reflow depth, making it trivial to subvert. This fixes the bug and it seems pretty safe. I guess someone out there might have a really deep frame tree...

Comment 19

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 314446 [details] [diff] [review]
fix

+  virtual nsMargin GetDesiredScrollbarSizes(nsPresContext* aPresContext,
>+          nsIRenderingContext* aRC) {
>+    nsBoxLayoutState bls(aPresContext, aRC, 0);
>+    return GetDesiredScrollbarSizes(&bls);
>+  }

Could this just go on nsIScrollableFrame, and be non-virtual?

>+  nsBoxLayoutState(nsPresContext* aPresContext, nsIRenderingContext* aRenderingContext = nsnull,
>+                   PRUint16 aReflowDepth = 0) NS_HIDDEN;

Having this as a default parameter seems like it could encourage leaving it off.  Maybe file a followup bug on making it not be a default parameter (or if it's trivial, doing it now)?

r+sr=dbaron

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

> Could this just go on nsIScrollableFrame, and be non-virtual?

Yes it could, although that means multiple inheritance of implementation. If you're OK with that, I'll do it.

I did start making that parameter non-optional, but I ended up changing a lot of code which made the patch look bigger than it really was. And it does actually need to be zero in a lot of places --- we use nsBoxLayoutState as a generic tuple in a lot of places where we're not actually doing reflow. So I could go either way.

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 314446 [details] [diff] [review]
fix

crash regression, fairly low risk.

Comment 22

[Damon Sicore (:damons)]

Per discussion with dbaron and roc, wouldn't hold the release for this if it were the last bug on the list (we're at that point now).  Moving to wanted1.9.0.x+.

We should take this patch ASAP, however.  Approval granted.

Comment 23

[Damon Sicore (:damons)]

Comment on attachment 314446 [details] [diff] [review]
fix

a1.9+=damons

Comment 24

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in

Comment 25

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I checked the test in as a crashtest.

Comment 26

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041005 Minefield/3.0pre