390050 - vlad's blog hangs Firefox due to use of -moz-column

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007072820 Minefield/3.0a7pre ID:2007072820

repro:
open FF
open url

result:
FF is dead and you can only kill the process.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Er, you'll have to give some more info, because it works for a lot of people across all platforms...

Comment 2

[Peter van der Woude [:Peter6]]

Most if not all people on the moz forum say it locks up.
It doesn't matter if loaded in background or foreground.
With a fresh install/profile I get as far as that the page is shown.
The statusbar says "done" but the page is still loading (blue bar in statusbar).
At that point FF is unresponsive and remains like that with high processorload untill you kill the process.

Comment 3

[Peter van der Woude [:Peter6]]

Attachment: screenshot of progress at lock up

Comment 4

[Bill Gianopoulos [:WG9s]]

(In reply to comment #1)
> Er, you'll have to give some more info, because it works for a lot of people
> across all platforms...
> 
The issue is not with the demo page itself.  It is with the page on your blog that links to it.  From my experience if you wait for the page to load completely before trying to interact with it, there is no issue.  If, however, you try to scroll before it is finished loading then bad things seem to happen.

I will try to do more testing on this.  So far I have only seen the issue
under windows.  Seems to work OK under Linux.

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Oh, on my actual blog page.. yes, known issue with trunk, working on it...

Comment 6

[Peter van der Woude [:Peter6]]

works in 20070703_2037_firefox-3.0a7pre.en-US.win32
fails in 20070703_2113_firefox-3.0a7pre.en-US.win32

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=1183520220&maxdate=1183522379&cvsroot=%2Fcvsroot

Bug 385423

(I'm not authorised to make it block bug 385423)

Comment 7

[[:Aleksej]]

Attachment: A minimized testcase to hang on Linux.

Hangs with 100% CPU usage

on (almost) Debian Lenny
when increasing font size to a certain point
when viewing with DejaVu Sans font.

The regression range matches the one from comment #6: 2007070304-2007070404.

Comment 8

[[:Aleksej]]

Attachment: A more minimized version of attachment 274375: A minimized testcase to hang on Linux.

The former testcase also hung on me with Bitstream Vera Sans and Bitstream Vera Mono, but not Bitstream Vera Serif.

This one hangs with DejaVu Sans, Bitstream Vera Sans and Bitstream Vera Serif, Nimbus Sans L, Doulos SIL, but not Bitstream Vera Sans Mono, DejaVu Sans Mono or Linux Libertine.

Comment 9

[[:Aleksej]]

PS: as I've removed the first font-size, I have to decrease the font size to hang with attachment 274384 [details].

Comment 10

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

In my blog, I get in an infinite loop here:

>	xul.dll!nsBlockFrame::ReflowInlineFrames(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012c8c8)  Line 3169	C++
 	xul.dll!nsBlockFrame::ReflowLine(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012c8c8)  Line 2224	C++
 	xul.dll!nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & aState={...})  Line 2050	C++
 	xul.dll!nsBlockFrame::Reflow(nsPresContext * aPresContext=0x0a5beaa8, nsHTMLReflowMetrics & aMetrics={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=0)  Line 925	C++
 	xul.dll!nsBlockReflowContext::ReflowBlock(const nsRect & aSpace={...}, int aApplyTopMargin=1, nsCollapsingMargin & aPrevMargin={...}, int aClearance=13684, int aIsAdjacentWithTop=0, nsMargin & aComputedOffsets={...}, nsHTMLReflowState & aFrameRS={...}, unsigned int & aFrameReflowStatus=0)  Line 370 + 0x19 bytes	C++
 	xul.dll!nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012cf4c)  Line 2930	C++
 	xul.dll!nsBlockFrame::ReflowLine(nsBlockReflowState & aState={...}, nsLineList_iterator aLine={...}, int * aKeepReflowGoing=0x0012cf4c)  Line 2167 + 0xf bytes	C++
 	xul.dll!nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & aState={...})  Line 1828	C++
 	xul.dll!nsBlockFrame::Reflow(nsPresContext * aPresContext=0x0a5beaa8, nsHTMLReflowMetrics & aMetrics={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=3)  Line 925	C++
 	xul.dll!nsContainerFrame::ReflowChild(nsIFrame * aKidFrame=0x0a588b74, nsPresContext * aPresContext=0x0a5beaa8, nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, int aX=0, int aY=0, unsigned int aFlags=0, unsigned int & aStatus=3, nsOverflowContinuationTracker * aTracker=0x00000000)  Line 715 + 0x16 bytes	C++
 	xul.dll!nsColumnSetFrame::ReflowChildren(nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=3, const nsColumnSetFrame::ReflowConfig & aConfig={...}, int aUnboundedLastColumn=0, nsCollapsingMargin * aBottomMarginCarriedOut=0x0012d3b4)  Line 523	C++
 	xul.dll!nsColumnSetFrame::Reflow(nsPresContext * aPresContext=0x0a5beaa8, nsHTMLReflowMetrics & aDesiredSize={...}, const nsHTMLReflowState & aReflowState={...}, unsigned int & aStatus=3)  Line 857 + 0x1a bytes	C++
 	xul.dll!nsBlockReflowContext::ReflowBlock(const nsRect & aSpace={...}, int aApplyTopMargin=1, nsCollapsingMargin & aPrevMargin={...}, int aClearance=3588, int aIsAdjacentWithTop=0, nsMargin & aComputedOffsets={...}, nsHTMLReflowState & aFrameRS={...}, unsigned int & aFrameReflowStatus=3)  Line 370 + 0x19 bytes	C++

Basically, lineReflowStatus always ends up as LINE_REFLOW_REDO_NEXT_BAND.

Comment 11

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

But I think the Linux hangs are a different issue.

Comment 12

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Just before a crash:

###!!! ASSERTION: overflow containers out of order or bad parent: '!(aOverflowCont->GetStateBits() & NS_FRAME_IS_OVERFLOW_CONTAINER)', file c:/Users/vladimir/proj/mozilla-cvs/firefox/layout/generic/../../../mozilla/layout/generic/nsContainerFrame.cpp, line 1331
###!!! ASSERTION: existing framelist: 'rv != NS_PROPTABLE_PROP_OVERWRITTEN', file c:/Users/vladimir/proj/mozilla-cvs/firefox/layout/generic/../../../mozilla/layout/generic/nsContainerFrame.cpp, line 1141

We're trying to poke an already-freed parent frame (0xdddddddd).  Need some purify action here.

Comment 13

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

Attachment: reduced testcase

This causes assertions and crashes for me. I reduced it from Vlad's blog.

Comment 14

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

Attachment: fix

This fixes Vlad's blog. The core issue is textrun reconstruction when a block has been broken vertically: we have to scan all the lines in the block's prev-in-flows and next-in-flows, plus their overflow lists. Right now we're just searching the current block frame which means we break textruns at places where content can actually be pulled across from one frame to another.

nsBlockInFlowLineIterator could be used to replace TryAllLines (and should), but I won't do it here, to reduce the risk.

Unfortunately this still doesn't fix bug 389603 --- although it must help --- so I'll keep looking for a fix for that.

Comment 15

[Damon Sicore (:damons)]

I'm thinking this should block M7.  Roc, thoughts?

Comment 16

[[:Aleksej]]

Attachment: (the real one) A more minimized version of attachment 274375: A minimized testcase to hang on Linux.

Sorry, the wrong version.

Comment 17

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

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

this is definitely right, so seeking review

Comment 18

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

Attachment: improved fix

The concept was right. The patch, however, was wrong. We need to make sure the second line iterator has all the right state from the first.

Comment 19

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

Comment on attachment 275709 [details] [diff] [review]
improved fix

Simon ought to look at the textframe changes.

Comment 20

[Mats Palmgren (inactive)]

Comment on attachment 275709 [details] [diff] [review]
improved fix

Style nit: please put the opening brace for nsBlockInFlowLineIterator's
methods on a separate line.
r+sr=mats

Comment 21

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 275709 [details] [diff] [review]
improved fix

a=bzbarsky

Comment 22

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

checked in.

Comment 23

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007082210 Minefield/3.0a8pre ID:2007082210

VERIFIED

Comment 24

[Jesse Ruderman]

The crashtests checked in.