348510 - [FIX]Crash with iExploder test 243244 [@ nsHTMLReflowState::ComputePadding]

Status: VERIFIED FIXED

(Core :: Layout, defect, P1)

Description

[Joonas]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060812 BonEcho/2.0b1

Bon Echo (Firefox 2.0) branch builds crash with iExploder test 243244.
http://toadstool.se/software/iexploder/demo/iexploder.cgi?lookup=1&test=243244&subtest=

Found using http://toadstool.se/software/iexploder/

Comment 1

[Joonas]

Attachment: testcase (crashes Bon Echo)

The source is:

<marquee>
<a>
<object>
<dd>
<form>
</object>
aaaaaaa

Comment 2

[Joonas]

Linux builds don't crash. Could someone test with a Windows Bon Echo build and provide a Talkback ID?

Comment 3

[Ria Klaassen (not reading all bugmail)]

TB22051287Q
Looks similar to bug 345740.

Comment 4

[Adam Guthrie]

Incident ID: 22051287
Stack Signature	nsHTMLReflowState::ComputePadding e2cc62d3
Product ID	Firefox2
Build ID	2006081205
Trigger Time	2006-08-13 10:06:52.0
Platform	Win32
Operating System	Windows NT 5.1 build 2600
Module	firefox.exe + (00254dbd)
URL visited	
User Comments	
Since Last Crash	3 sec
Total Uptime	1590 sec
Trigger Reason	Access violation
Source File, Line No.	c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/generic/nsHTMLReflowState.cpp, line 2432
Stack Trace 	
nsHTMLReflowState::ComputePadding  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/generic/nsHTMLReflowState.cpp, line 2432]
nsHTMLReflowState::InitConstraints  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/generic/nsHTMLReflowState.cpp, line 1739]
nsHTMLReflowState::Init  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/generic/nsHTMLReflowState.cpp, line 337]
nsHTMLReflowState::nsHTMLReflowState  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/generic/nsHTMLReflowState.cpp, line 208]
nsLineLayout::ReflowFrame  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/generic/nsLineLayout.cpp, line 1145]
nsInlineFrame::ReflowFrames  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/generic/nsInlineFrame.cpp, line 587]
nsInlineFrame::Reflow  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/generic/nsInlineFrame.cpp, line 426]
nsInlineFrame::Paint  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/generic/nsInlineFrame.cpp, line 326]
nsLineLayout::ApplyStartMargin  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/generic/nsLineLayout.cpp, line 1243]
nsBlockFrame::DoReflowInlineFrames  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/generic/nsBlockFrame.cpp, line 3992]
nsBlockFrame::DoReflowInlineFrames  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/generic/nsBlockFrame.cpp, line 3907]
nsBlockFrame::ReflowBlockFrame  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/generic/nsBlockFrame.cpp, line 3704]
nsBlockFrame::ReflowLine  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/generic/nsBlockFrame.cpp, line 2684]
nsBlockFrame::ReflowDirtyLines  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/generic/nsBlockFrame.cpp, line 2241]
nsBlockFrame::Reflow  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/generic/nsBlockFrame.cpp, line 885]
nsFrame::DoLayout  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/generic/nsFrame.cpp, line 5196]
nsIFrame::IsFocusable  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/generic/nsFrame.cpp, line 4777]
nsFrame::RefreshSizeCache  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/generic/nsFrame.cpp, line 4955]
nsSprocketLayout::PopulateBoxSizes  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsSprocketLayout.cpp, line 859]
nsSprocketLayout::Layout  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsSprocketLayout.cpp, line 348]
nsBoxFrame::GetPrefSize  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp, line 955]
nsLineLayout::ApplyStartMargin  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/generic/nsLineLayout.cpp, line 1243]
...

Comment 5

[Martijn Wargers (dead)]

Confirmed, with the latest nightly 1.8.1 build on windows.
Doesn't crash in trunk builds for me.

Comment 6

[Joonas]

This crash shows up frequently in iExploder testing and makes it difficult to tell whether there are other bugs iExploder can find on this branch.

Comment 7

[Mike Schroepfer]

Martjin/Adam can we get a fixed window on the trunk for this?

Comment 8

[Mats Palmgren (inactive)]

WFM, Firefox 20060823 BonEcho/2.0b2 and fresh SeaMonkey debug build on Linux

Comment 9

[Hermann Schwab]

Crash TB22576183H
Testcase is crashing both Firefox and Seamonkey, URL: is running on both.
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1b2) Gecko/20060827 BonEcho/2.0b2
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1b2) Gecko/20060827 SeaMonkey/1.1b

Comment 10

[Martijn Wargers (dead)]

(In reply to comment #7)
> Martjin/Adam can we get a fixed window on the trunk for this?

This was at least fixed before 2005-10-01, it is probably not useful to find a smaller regression range. This was likely fixed by bug 1156 or one of the related patches, and those can't be added in the 1.8.1 tree.

Comment 11

[Boris Zbarsky [:bzbarsky]]

So I'm seeing crashes on the original testcase (not on the reduced one) with a Linux build.  Are there bugs on those?

For this bug, could someone who can reproduce look into _why_ we're crashing in nsHTMLReflowState::ComputePadding?  Is something null?  Is a frame deleted?

The talkbacks are being of little use, because either the top few frames are completely lying about function names or the line numbers are way off.

Comment 12

[Joonas]

Attachment: another crashing testcase

This testcase looks a bit similar, and it crashes like this: TB22970125Q.

The source is:

<listing>
<marquee>
<aa>
<object>
<fieldset>
</object>
a

Comment 13

[Joonas]

The talkback incidents (TB22051287Q, TB22576183H, TB22970125Q) all seem to have nsHTMLReflowState::InitConstraints() and nsHTMLReflowState::nsHTMLReflowState() near the top.

Comment 14

[Joonas]

Forgot to mention: attachment 236986 [details] crashes on Linux too.
 
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1b2) Gecko/20060906 BonEcho/2.0b2

Comment 15

[Boris Zbarsky [:bzbarsky]]

OK, the "another crashing testcase" crashes on Linux, after some asserts like:

###!!! ASSERTION: unexpected flow: 'mFrames.ContainsFrame(nextInFlow)', file ../../../mozilla/layout/generic/nsInlineFrame.cpp, line 514
###!!! ASSERTION: bad geometric parent: 'mFrames.ContainsFrame(aNextInFlow)', file ../../../mozilla/layout/generic/nsContainerFrame.cpp, line 1037
###!!! ASSERTION: failed to remove frame: 'result', file ../../../mozilla/layout/generic/nsContainerFrame.cpp, line 1076

Are people seeing those on windows as well, with the original testcase?  Or do we have two separate bugs here?

I'm seeing the next-in-flow's parent be some random frame, with us being part of an {ib} split...  Presumably the usual issue with SplitToContainingBlock not doing things quite right, but I'm not sure what it's screwing up.

Comment 16

[Martijn Wargers (dead)]

(In reply to comment #15)
> Are people seeing those on windows as well, with the original testcase?  Or do
> we have two separate bugs here?

Yes, I'm seeing those asserts on windows, with the 2 attached testcases. With the url testcase, I get different assertions, the 'frame was not removed from the primary map' assertion.

Comment 17

[Hermann Schwab]

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1b2) Gecko/20060906 BonEcho/2.0b2
I don't crash on the original testcase, but I'm crashing on the two reduced testcases: TB23154270Q, TB23154189X

Comment 18

[Mike Schroepfer]

Moving out to 1.8.1.1..

Comment 19

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

Awesome. I can totally reproduce the crash with "another crashing testcase", and this patch fixes both that and the original site.  Could someone who crashes on the first testcase check whether that's getting fixed too?

Comment 20

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

Awesome. I can totally reproduce the crash with "another crashing testcase", and this patch fixes both that and the original site.  Could someone who crashes on the first testcase check whether that's getting fixed too?

Comment 21

[Daniel Veditz [:dveditz]]

Comment on attachment 238734 [details] [diff] [review]
Fix

approved 1.8/1.8.0 branches, a=dveditz for drivers

Comment 22

[Boris Zbarsky [:bzbarsky]]

Fixed on both branches.  Resolving, since this is not a problem on trunk (all this code doesn't exist).

Comment 23

[Boris Zbarsky [:bzbarsky]]

Note that I still have no confirmation that the first testcase is fixed... but the original site is.

Comment 24

[Boris Zbarsky [:bzbarsky]]

Not quite sure how to add this to a testsuite...  Probably want to add both testcases, but I'm worried about the other test suite content hiding the problem.

Comment 25

[Joonas]

(In reply to comment #23)
> Note that I still have no confirmation that the first testcase is fixed... but
> the original site is.

The first testcase is fixed too.

Win32 Firefox 2 crashed with both testcases (and the original iExploder test 243244). New 1.8.1 builds don't crash anymore.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061111 BonEcho/2.0

Comment 26

[Boris Zbarsky [:bzbarsky]]

Joonas, thanks!

Comment 27

[Boris Zbarsky [:bzbarsky]]

I meant 1.8.1.1.

Comment 28

[Carsten Book [:Tomcat]]

Verified Fixed for 1.8.0.9 and 1.8.1.1 with :

Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.1pre) Gecko/20061127 BonEcho/2.0.0.1pre (Windows XP x64)
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.1pre) Gecko/20061127 BonEcho/2.0.0.1pre

and

Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.0.9pre) Gecko/20061127 Firefox/1.5.0.9pre (Windows XP x64)
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.0.9pre) Gecko/20061127 Firefox/1.5.0.9pre

No crash on Testcases

Comment 29

[Mats Palmgren (inactive)]

Crash tests:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a72c01c68b9

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7a72c01c68b9