398733 - Crash [@ BuildTextRunsScanner::ScanFrame] with position: absolute, rtl text and changing styles

Status: VERIFIED FIXED

(Core :: Layout, defect, P3)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, which crashes current trunk build after 100ms.

It doesn't crash in a 2006-10-18 and 2006-10-19:
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=2006-10-18+04&maxdate=2006-10-19+09&cvsroot=%2Fcvsroot
I guess a regression from bug 343445, somehow?

http://crash-stats.mozilla.com/report/index/8b027545-7353-11dc-95e8-001a4bd43e5c
0  	@0x0  	
1 	BuildTextRunsScanner::ScanFrame(nsIFrame*) 	mozilla/layout/generic/nsTextFrameThebes.cpp:1163
2 	BuildTextRunsScanner::ScanFrame(nsIFrame*) 	mozilla/layout/generic/nsTextFrameThebes.cpp:1163
3 	BuildTextRuns 	mozilla/layout/generic/nsTextFrameThebes.cpp:941
4 	nsTextFrame::EnsureTextRun(nsIRenderingContext*, nsIFrame*, nsLineList_iterator const*, unsigned int*) 	mozilla/layout/generic/nsTextFrameThebes.cpp:1717
5 	nsTextFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) 	mozilla/layout/generic/nsTextFrameThebes.cpp:5088
6 	nsLineLayout::ReflowFrame(nsIFrame*, unsigned int&, nsHTMLReflowMetrics*, int&) 	mozilla/layout/generic/nsLineLayout.cpp:882
7 	nsInlineFrame::ReflowInlineFrame(nsPresContext*, nsHTMLReflowState const&, nsInlineFrame::InlineReflowState&, nsIFrame*, unsigned int&) 	mozilla/layout/generic/nsInlineFrame.cpp:603
8 	nsInlineFrame::ReflowFrames(nsPresContext*, nsHTMLReflowState const&, nsInlineFrame::InlineReflowState&, nsHTMLReflowMetrics&, unsigned int&) 	mozilla/layout/generic/nsInlineFrame.cpp:470
9 	nsInlineFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) 	mozilla/layout/generic/nsInlineFrame.cpp:385
10 	nsLineLayout::ReflowFrame(nsIFrame*, unsigned int&, nsHTMLReflowMetrics*, int&) 	mozilla/layout/generic/nsLineLayout.cpp:882
etc..

Comment 1

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

The important thing here is the assertions earlier:

###!!! ASSERTION: Deleting out of flow without tearing down placeholder relationship; see comments in nsFrame.h: '!(mState & NS_FRAME_OUT_OF_FLOW) || !shell->FrameManager()->GetPlaceholderFrameFor(this)', file /Users/roc/mozilla-checkin/mozilla/layout/generic/nsFrame.cpp, line 487
###!!! ASSERTION: frame was not removed from primary frame map before destruction or was readded to map after being removed: 'Not Reached', file /Users/roc/mozilla-checkin/mozilla/layout/base/nsFrameManager.cpp, line 708

I suspect this is related to other out-of-flow destruction issues.

Comment 2

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

Attachment: simpler testcase

I simplified that testcase a bit.

Comment 3

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

Attachment: fix

This is fairly simple and obvious, I think. We should be using GetFirstContinuation here instead of GetNextInFlow.

Comment 4

[Boris Zbarsky [:bzbarsky]]

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

Looks good, but I wonder whether it's worth eliminating this function (AdjustAbsoluteContainingBlock) altogether...  In any case, get rid of the unused aPresContext arg?

Comment 5

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

Sure, I'll eliminate the arg.

I think having this function just proved useful since I only had to change this in one place :-)

Comment 6

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

Attachment: updated for checkin

Comment 7

[Reed Loden [:reed]]

Checking in layout/base/nsCSSFrameConstructor.cpp;
/cvsroot/mozilla/layout/base/nsCSSFrameConstructor.cpp,v  <--  nsCSSFrameConstructor.cpp
new revision: 1.1421; previous revision: 1.1420
done

Comment 8

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007110805 Minefield/3.0b2pre

Comment 9

[(no longer active)]

Mass-assigning the new rtl keyword to RTL-related (see bug 349193).

Comment 10

[Mats Palmgren (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e259367d81df

Comment 11

[Gregory Szorc [:gps]]

https://hg.mozilla.org/mozilla-central/rev/e259367d81df