314502 - Crash [@ nsGridRowGroupLayout::CountRowsColumns] with evil xul testcase, using grid, float:left and overflow:scroll

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Martijn Wargers (dead)]

Upcoming testcase crashes 2005-10-28 trunk Mozilla build when clicking on the button. It doesn't crash Mozilla1.7.

Talkback ID: TB11282963Q

nsGridRowGroupLayout::CountRowsColumns  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/grid/nsGridRowGroupLayout.cpp, line 242]
nsGrid::CountRowsColumns  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/grid/nsGrid.cpp, line 346]
nsGrid::RebuildIfNeeded  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/grid/nsGrid.cpp, line 226]
nsGrid::GetExtraRowCount  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/grid/nsGrid.cpp, line 572]
nsBoxFrame::GetPrefSize  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp, line 933]
nsStackLayout::GetPrefSize  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsStackLayout.cpp, line 95]
nsGridLayout2::GetPrefSize  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/grid/nsGridLayout2.cpp, line 151]
nsBoxFrame::GetPrefSize  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp, line 933]
nsSprocketLayout::PopulateBoxSizes  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsSprocketLayout.cpp, line 822]
nsSprocketLayout::Layout  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsSprocketLayout.cpp, line 265]
nsBoxFrame::DoLayout  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp, line 1091]
nsBoxFrame::DoLayout  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp, line 1091]
nsRootBoxFrame::Reflow  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsRootBoxFrame.cpp, line 226]
nsContainerFrame::ReflowChild  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsContainerFrame.cpp, line 891]
ViewportFrame::Reflow  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsViewportFrame.cpp, line 239]
IncrementalReflow::Dispatch  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp, line 860]
PresShell::ProcessReflowCommands  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp, line 6506]
PresShell::WillPaint  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp, line 6143]

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase

Comment 2

[Martijn Wargers (dead)]

Ok, this regressed between 2005-03-17 and 2005-03-18:
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=2005-03-17+07%3A00%3A00&maxdate=2005-03-18+08%3A00%3A00&cvsroot=%2Fcvsroot
Maybe regression from bug 283385?

Comment 3

[Boris Zbarsky [:bzbarsky]]

Quite possible.  I'll look into this tonight.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: This could work

This is basically the same issue as bug 307809 and has the same question -- should GetScrolledBox actually return the scrollbox for cases when the scrolled thing is not a box?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 201465 [details] [diff] [review]
This could work

See question in comment 4.

Comment 6

[Bernd]

at least http://lxr.mozilla.org/seamonkey/source/layout/xul/base/src/nsScrollBoxObject.cpp#151

expects the function to return a null pointer.

Comment 7

[Boris Zbarsky [:bzbarsky]]

That's not the same GetScrolledBox.  In this bug we care about nsGrid::GetScrolledBox...

Comment 8

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

Comment on attachment 201465 [details] [diff] [review]
This could work

r+sr=dbaron, but please move the |deepChild| variables into the loop and remove the bogus assignment to them near the end of the loop.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Updated to comments

Comment 10

[Boris Zbarsky [:bzbarsky]]

Fixed.

Comment 11

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED on trunk SeaMonkey 1.5a;Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051109 using https://bugzilla.mozilla.org/attachment.cgi?id=201419&action=view as a testcase.  No crash.

Comment 12

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

Comment on attachment 201465 [details] [diff] [review]
This could work

This is just like bug 307809; a null-check that at least makes some sense.

Comment 13

[Mike Schroepfer]

You don't want the version updated to comments?

Comment 14

[Mike Schroepfer]

Comment on attachment 202327 [details] [diff] [review]
Updated to comments

a=schrep for crash patch...

Comment 15

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

Checked in to MOZILLA_1_8_BRANCH.