Closed Bug 390050 Opened 13 years ago Closed 13 years ago

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

Categories

(Core :: General, defect)

x86
Windows XP
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: Peter6, Assigned: roc)

References

()

Details

(4 keywords)

Attachments

(5 files, 2 obsolete files)

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.
Er, you'll have to give some more info, because it works for a lot of people across all platforms...
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.
(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.


Oh, on my actual blog page.. yes, known issue with trunk, working on it...
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.
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.
PS: as I've removed the first font-size, I have to decrease the font size to hang with attachment 274384 [details].
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.
Summary: svg page locks Firefox → vlad's blog locks Firefox
But I think the Linux hangs are a different issue.
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.
Attached file reduced testcase
This causes assertions and crashes for me. I reduced it from Vlad's blog.
Assignee: nobody → roc
Status: NEW → ASSIGNED
Attached patch fix (obsolete) — Splinter Review
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.
I'm thinking this should block M7.  Roc, thoughts?
Flags: blocking1.9?
Comment on attachment 274424 [details] [diff] [review]
fix

this is definitely right, so seeking review
Attachment #274424 - Flags: superreview?(mats.palmgren)
Attachment #274424 - Flags: review?(mats.palmgren)
Attached patch improved fixSplinter Review
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.
Attachment #274424 - Attachment is obsolete: true
Attachment #275709 - Flags: superreview?(mats.palmgren)
Attachment #275709 - Flags: review?(mats.palmgren)
Attachment #274424 - Flags: superreview?(mats.palmgren)
Attachment #274424 - Flags: review?(smontagu)
Attachment #274424 - Flags: review?(mats.palmgren)
Comment on attachment 275709 [details] [diff] [review]
improved fix

Simon ought to look at the textframe changes.
Attachment #275709 - Flags: review?(smontagu)
Attachment #275709 - Flags: review?(smontagu) → review+
Severity: normal → critical
Summary: vlad's blog locks Firefox → vlad's blog hangs Firefox due to use of -moz-column
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
Attachment #275709 - Flags: superreview?(mats.palmgren)
Attachment #275709 - Flags: superreview+
Attachment #275709 - Flags: review?(mats.palmgren)
Attachment #275709 - Flags: review+
Comment on attachment 275709 [details] [diff] [review]
improved fix

a=bzbarsky
Attachment #275709 - Flags: approval1.9? → approval1.9+
checked in.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Flags: in-testsuite?
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007082210 Minefield/3.0a8pre ID:2007082210

VERIFIED
The crashtests checked in.
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.