268575 - Crash in [@ nsCachedStyleData::GetStyleData] when closing tab on this site (a lot of nested fonts, ordered list, animated gif) (BuildFloatList removal)

Status: RESOLVED FIXED

(Core :: Layout: Floats, defect)

Description

[Martijn Wargers (dead)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a5) Gecko/20041107 Firefox/0.9.1+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a5) Gecko/20041107 Firefox/0.9.1+

This was mentioned on:
http://forums.mozillazine.org/viewtopic.php?t=157851&sid=c93a1e33e5a5441dd9837fa03b3846bb

Every time I open that site in a different tab(and let it load till the end) and
then close that tab, Mozilla crashes.
Happens with the latest nightly build, but I see it also happening with
Mozilla1.0, so this is not a (recent) regression.

I have a fairly minimal testcase coming up.
The fairly minimal testcase consists of a lot of nested font an ordered list and
an animated gif image and some </p> end tags. Almost all of the tags are not
properly closed.

Reproducible: Always
Steps to Reproduce:
1. Load URL in tab
2. Wait for the site to finish loading
3. Close tab

Actual Results:  
Mozilla crashes

Expected Results:  
No crash.

Some talkback ID's: 
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB1822034K
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB1823343X
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB1823457Q
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB1823557H
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB1825081G

The "nsStyleContext::GetStyleData" crash I got sometimes while loading the
testcase , when it was in the stage when it was not minimal yet.

The "nsImageFrame::FrameChanged" crash I got when closing the tab.

Comment 1

[Martijn Wargers (dead)]

Attachment: animated gif image for the testcase

Comment 2

[Martijn Wargers (dead)]

Attachment: Testcase (crashes when closing tab)

Comment 3

[Martijn Wargers (dead)]

Might be worth knowing, that the animated gif is not showing in this testcase. 
While minimizing the testcase, I noticed that when the animated gif did show,
the testcase would not crash anymore when I closed the tab.

Comment 4

[Paul van Schayck]

Confirmed for this build: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.5)
Gecko/20041109 Firefox/1.0RC2

Comment 5

[Michali Sarris]

Confirmed for Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5)
Gecko/20041107 Firefox/1.0

Comment 6

[Alessio]

(In reply to comment #5)
> Confirmed for Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5)
> Gecko/20041107 Firefox/1.0

confirmed on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3)
Gecko/20040910 Win XP Pro

talkback id: TB1833413W

Comment 7

[Boris Zbarsky [:bzbarsky]]

Oh, this is a fun one...

The ultimate cause of the crash is that the floated image is completely dropped
from the frame tree, so we never Destroy() it before deallocating the PLArena. 
Then after the PLArena is gone the frame gets a framechanged notification from
the animated image, we dereference bogus memory, and crash.

The reason we lose the float is that the number of inlines here exceeds
MAX_FRAME_DEPTH, we bail out of reflow, never end up hitting the float
placeholde, so never put the float in the float cache, and HasFloats() on the
line returns false.  As a result BuildFloatList() doesn't see the float, and
ends up setting the float list to null.

The "right" thing to do is to move away from BuildFloatList() and work out some
other solution for dealing with floats in blocks that have next-in-flows... 
Past that, I'm not quite sure what we can do here.

Comment 8

[Martijn Wargers (dead)]

Testcase and url don't crash anymore, probably because bug 58917 is fixed.

Comment 9

[Boris Zbarsky [:bzbarsky]]

One could still cause this by building a deep tree via DOM manipulation.

Comment 10

[Martijn Wargers (dead)]

Attachment: Testcase2 based on comment 9

Yes, you're right. This testcase still crashes on close.

Comment 11

[Adam Guthrie]

It crashes in a slightly different place in trunk:

#9  0xb680c780 in nsCachedStyleData::GetStyleData (this=0xdadadaf6, aSID=@0xbfc2eb00) at nsRuleNode.h:210
#10 0xb680d788 in nsStyleContext::GetStyleData (this=0xdadadada, aSID=eStyleStruct_Visibility) at /moz/trunk/mozilla/layout/style/nsStyleContext.cpp:248
#11 0xb665b01d in nsIFrame::GetStyleData (this=0x88fb0e0, aSID=eStyleStruct_Visibility) at nsIFrame.h:608
#12 0xb665b04d in nsIFrame::GetStyleVisibility (this=0x88fb0e0) at nsStyleStructList.h:98
#13 0xb6717732 in nsImageFrame::FrameChanged (this=0x88fb0e0, aContainer=0x888d400, aNewFrame=0x888ad98, aDirtyRect=0xbfc2ecd4) at /moz/trunk/mozilla/layout/generic/nsImageFrame.cpp:658
#14 0xb67177d8 in nsImageListener::FrameChanged (this=0x88f7f38, aContainer=0x888d400, newframe=0x888ad98, dirtyRect=0xbfc2ecd4) at /moz/trunk/mozilla/layout/generic/nsImageFrame.cpp:2046
#15 0xb690fac1 in nsImageLoadingContent::FrameChanged (this=0x88f364c, aContainer=0x888d400, aFrame=0x888ad98, aDirtyRect=0xbfc2ecd4) at /moz/trunk/mozilla/content/base/src/nsImageLoadingContent.cpp:147
#16 0xb5919b45 in imgRequestProxy::FrameChanged (this=0x88f8030, container=0x888d400, newframe=0x888ad98, dirtyRect=0xbfc2ecd4) at /moz/trunk/mozilla/modules/libpr0n/src/imgRequestProxy.cpp:392
#17 0xb5914013 in imgRequest::FrameChanged (this=0x88f47a0, container=0x888d400, newframe=0x888ad98, dirtyRect=0xbfc2ecd4) at /moz/trunk/mozilla/modules/libpr0n/src/imgRequest.cpp:401
#18 0xb5921715 in imgContainerGIF::Notify (this=0x888d400, timer=0x87d49a0) at /moz/trunk/mozilla/modules/libpr0n/decoders/gif/imgContainerGIF.cpp:455

Comment 12

[Jesse Ruderman]

2006-04-08 mac trunk build: crashes
2006-04-10 mac trunk build: does not crash

-> FIXED by BuildFloatList removal.