467487 - "ASSERTION: Bad justifiable character count" with pre-line

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect, P3)

Description

[Jesse Ruderman]

Attachment: testcase

This testcase triggers a number of assertions:

###!!! ASSERTION: Attempting to allocate excessively large array: 'Error', file nsTArray.cpp, line 69

###!!! ASSERTION: Bad justifiable character count: 'numJustifiableCharacters <= charsFit', file layout/generic/nsTextFrameThebes.cpp, line 6047

###!!! ASSERTION: Bad offset looking for glyphrun: 'aOffset <= mCharacterCount', file gfx/thebes/src/gfxFont.cpp, line 2154

###!!! ASSERTION: Invalid offset: 'aOffset <= mSkipChars->mCharCount', file gfx/thebes/src/gfxSkipChars.cpp, line 92

###!!! ASSERTION: Negative flow length?: 'flowEndInTextRun >= transformedOffset', file layout/generic/nsTextFrameThebes.cpp, line 5827

###!!! ASSERTION: No characters in run (initial length too large?): 'mRunLength > 0', file layout/generic/nsTextFrameUtils.cpp, line 229

###!!! ASSERTION: Range out of bounds: 'IsInBounds(mStart, mLength, aStart, aLength)', file layout/generic/nsTextFrameThebes.cpp, line 2304

###!!! ASSERTION: Text run does not map enough text for our reflow: 'gfxSkipCharsIterator(iter).ConvertOriginalToSkipped(offset + length) <= mTextRun->GetLength()', file layout/generic/nsTextFrameThebes.cpp, line 5784

Security-sensitive because a few of these assertions make me worry there might be integer overflows going on.

Comment 1

[Jesse Ruderman]

Nominating for blocking: pre-line (new feature) + common stuff = bad asserts.

Comment 2

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

What happens here is, in the first reflow we split the line into three text frames, "AB ", "DEF\n" and " ". Frames #1 and #2 share one text run for "AB DEF\n", frame #3 gets another text run for " ". Then we reflow again with increased width. Frame #1 gets the text "AB DEF\n" now. Frame #2 gets the following " ", but its mTextRun is still set to the "AB DEF\n" textrun. We haven't cleared it/adjusted it when the offsets changed we should have.

Comment 3

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

I need to figure out why pre-wrap doesn't have this problem. But the solution is probably going to involve modifying nsTextFrame::SetLength.

Comment 4

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

Attachment: fix

Just need to tweak the test that detects when the textrun currently assigned to the frame is too short. We need to drop the condition that length > 0; in this testcase, length is 0, but we've advanced 'offset'.

Comment 5

[Simon Montagu :smontagu]

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

makes sense

Comment 6

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

Attachment: fix v2

Actually I've got a better idea. We can directly ask the textrun how much DOM text it maps.

Comment 7

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

Pushed 62c3545aabad to trunk.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d890ad91b262