Last Comment Bug 542136 - "ASSERTION: Incorrect aState.mPrevChild before inserting line at end"
: "ASSERTION: Incorrect aState.mPrevChild before inserting line at end"
: assertion, testcase, verified1.9.2
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: x86 Mac OS X
-- normal (vote)
: mozilla1.9.3a1
Assigned To: Timothy Nikkel (:tnikkel)
: Jet Villegas (:jet)
Depends on:
Blocks: randomstyles 336383
  Show dependency treegraph
Reported: 2010-01-25 17:32 PST by Jesse Ruderman
Modified: 2010-07-25 16:43 PDT (History)
7 users (show)
dveditz: wanted1.9.0.x-
tnikkel: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (402 bytes, text/html)
2010-01-25 17:32 PST, Jesse Ruderman
no flags Details
patch (1.25 KB, patch)
2010-01-26 13:39 PST, Timothy Nikkel (:tnikkel)
roc: review+
mbeltzner: approval1.9.2.2+
mbeltzner: approval1.9.1.9+
Details | Diff | Splinter Review

Description User image Jesse Ruderman 2010-01-25 17:32:53 PST
Created attachment 423454 [details]

###!!! ASSERTION: Incorrect aState.mPrevChild before inserting line at end: 'aState.mPrevChild == mFrames.LastChild()', file /Users/jruderman/mozilla-central/layout/generic/nsBlockFrame.cpp, line 2162

bz marked a previous bug with this assertion as security-sensitive (bug 522170), so I'm filing this one as security-sensitive.
Comment 1 User image Timothy Nikkel (:tnikkel) 2010-01-25 21:58:40 PST
On trunk this assertion shouldn't itself lead to a security problem. On 3.6 and earlier it is likely to cause a messed up frame tree, so a security problem is likely just around the corner.
Comment 2 User image Timothy Nikkel (:tnikkel) 2010-01-25 22:56:14 PST
So I think the problem lies with this bit of code in nsBlockFrame::ReflowInlineFrame

    // If we just ended a first-letter frame or reflowed a placeholder then 
    // don't split the line and don't stop the line reflow...
    if (!(frameReflowStatus & NS_INLINE_BREAK_FIRST_LETTER_COMPLETE) && 
        nsGkAtoms::placeholderFrame != frameType) {
      // Split line after the current frame
      *aLineReflowStatus = LINE_REFLOW_STOP;
      rv = SplitLine(aState, aLineLayout, aLine, aFrame->GetNextSibling(), aLineReflowStatus);
      NS_ENSURE_SUCCESS(rv, rv);

At this point *aLineReflowStatus is already LINE_REFLOW_STOP because the reflow status was break after for the frame we just reflowed, but we don't split the line because of the first letter complete check. And so we stop reflow after 1 of 2 frames on the line and when we get back to nsBlockFrame::ReflowDirtyLines mPrevChild is not the last frame on the line. Adding "|| *aLineReflowStatus == LINE_REFLOW_STOP" to the if condition fixes the asserts for me.
Comment 3 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2010-01-26 01:24:51 PST
Sounds right.
Comment 4 User image Timothy Nikkel (:tnikkel) 2010-01-26 13:39:18 PST
Created attachment 423615 [details] [diff] [review]
Comment 5 User image Timothy Nikkel (:tnikkel) 2010-02-03 22:28:18 PST
Comment 6 User image Timothy Nikkel (:tnikkel) 2010-02-03 22:31:43 PST
After some bake time on trunk this is something that would probably be good to have on the branches.
Comment 7 User image Mike Beltzner [:beltzner, not reading bugmail] 2010-03-05 10:53:21 PST
Comment on attachment 423615 [details] [diff] [review]

a=beltzner for and
Comment 8 User image Timothy Nikkel (:tnikkel) 2010-03-05 15:13:11 PST
Comment 9 User image Timothy Nikkel (:tnikkel) 2010-03-05 15:15:06 PST
Comment 10 User image Al Billings [:abillings] 2010-03-15 17:00:09 PDT
I notice with the testcase post-fix that I'm not seeing the assert above anymore on OS X but I do get:

###!!! ASSERTION: overflow list w/o frames: 'mFrames.NotEmpty()', file /Users/albill/mozilla-192/layout/generic/nsInlineFrame.cpp, line 374

Is this a non-issue?
Comment 11 User image Timothy Nikkel (:tnikkel) 2010-03-15 17:17:10 PDT
I don't get that assertion on trunk, I would guess because of bug 533379 which is fixed on trunk but not 1.9.2.
Comment 12 User image Al Billings [:abillings] 2010-03-15 17:21:26 PDT
Well, that isn't *this* assertion so I will call this verified1.9.2.
Comment 13 User image Carsten Book [:Tomcat] 2010-03-22 12:39:23 PDT
verified 1.9.2-2 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv: Gecko/20100321 Firefox/3.6.2 ID:20100321170417 (debug build) - the testcase didn't caused the assertion from comment #0 - for comment 10 i have made a comment in bug 533379 - maybe that bug need to land on 1.9.2 too
Comment 14 User image Timothy Nikkel (:tnikkel) 2010-07-25 16:43:32 PDT
Added crashtest

Note You need to log in before you can comment on or make changes to this bug.