404146 - "ASSERTION: comparing iterators over different lists" and "ASSERTION: running past end" with inline-block next to wrapping text in -moz-column

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Jesse Ruderman]

Attachment: testcase

Loading the testcase triggers a whole bunch of:

###!!! ASSERTION: comparing iterators over different lists: 'mListLink == aOther.mListLink', file /Users/jruderman/trunk/mozilla/layout/base/../generic/nsLineBox.h, line 699

###!!! ASSERTION: running past end: 'mCurrent != mListLink', file /Users/jruderman/trunk/mozilla/layout/base/../generic/nsLineBox.h, line 620

Comment 1

[Jesse Ruderman]

I think the "running past end" assertion is bad.  I've seen it precede scary crashes.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Relevant part of the stack:

#0  nsLineList_iterator::operator!= (this=0xbfff8f00, aOther={mCurrent = 0x34b8d20, mListLink = 0x34b8d88}) at ../../../mozilla/layout/base/../generic/nsLineBox.h:700
#1  0x1beea1e1 in nsBlockInFlowLineIterator::Prev (this=0xbfff8efc) at ../../../mozilla/layout/generic/nsBlockFrame.cpp:5168
#2  0x1bf7bdd9 in BuildTextRuns (aContext=0x3b20baa0, aForFrame=0x34b8538, aLineContainer=0x34b8d48, aForFrameLine=0xbfff9274) at ../../../mozilla/layout/generic/nsTextFrameThebes.cpp:914
#3  0x1bf7c302 in nsTextFrame::EnsureTextRun (this=0x34b8538, aReferenceContext=0x3b20baa0, aLineContainer=0x0, aLine=0xbfff9274, aFlowEndInTextRun=0xbfff9138) at ../../../mozilla/layout/generic/nsTextFrameThebes.cpp:1785
#4  0x1bf7ea8d in nsTextFrame::AddInlineMinWidthForFlow (this=0x34b8538, aRenderingContext=0x3a99eaf0, aData=0xbfff9248) at ../../../mozilla/layout/generic/nsTextFrameThebes.cpp:4878
#5  0x1bf7ee25 in nsTextFrame::AddInlineMinWidth (this=0x34b8538, aRenderingContext=0x3a99eaf0, aData=0xbfff9248) at ../../../mozilla/layout/generic/nsTextFrameThebes.cpp:4965

In Prev(), mInOverflowLines is null.  It looks like we'll keep doing Prev() here until we walk off the list.

The relevant part of the frame tree looks like this:

Float-list<
  ColumnSet(div)(1)@0x34b81e8 {0,0,6960,7440} [state=00001120] [content=0x34b5ed8] [sc=0x34b8104]<
    Area(div)(1)@0x34b82d0 next=0x34b8d48 next-in-flow=0x34b8d48 {120,120,3000,7200} [state=00d01000] sc=0x34b83c4(i=3,b=0) pst=:-moz-column-content<
      line 0x34b8790: count=1 state=inline,clean,prevmarginclean,not impacted,wrapped,before:nobr,after:nobr[0x4120] {0,0,1296,780} <
        Text(0)@0x34b84f8[0,4,F]  next=0x34b8c20 next-continuation=0x34b8c20 {0,0,1296,780} [state=11600000] [content=0x34b5f20] sc=0x34b84a8 pst=:-moz-non-element<
          "The "
        >
      >
      line 0x34b8c64: count=1 state=inline,dirty,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x4141] {0,780,2160,780} <
        Text(0)@0x34b8c20[4,6,T]  next=0x34b85c8 prev-continuation=0x34b84f8 {0,780,2160,780} [state=10600004] [content=0x34b5f20] sc=0x34b84a8 pst=:-moz-non-element<
          "quick "
        >
      >
      line 0x34b8cc0: count=1 state=inline,dirty,prevmarginclean,not impacted,wrapped,before:nobr,after:nobr[0x4161] {0,1560,1740,5040} <
        Inline(span)(1)@0x34b85c8 next-continuation=0x34b8ce8 {0,6000,1740,600} [state=00001008] [content=0x34b5f58] [overflow=0,-4440,1740,5040] [sc=0x34b8578]<
          Block(div)(0)@0x34b86e8 {0,-4440,1740,5040} [state=00d00400] sc=0x34b527c(i=0,b=0)<
          >
        >
      >
    >
    Area(div)(1)@0x34b8d48 prev-in-flow=0x34b82d0 {3840,120,3000,0} [state=00d0000c] [overflow=0,0,2160,780] sc=0x34b83c4(i=1,b=0) pst=:-moz-column-content<
      line 0x34b8d20: count=1 state=inline,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x4100] {0,0,2160,780} <
        Inline(span)(1)@0x34b8ce8 prev-continuation=0x34b85c8 {0,0,2160,780} [state=00000004] [content=0x34b5f58] [sc=0x34b8578]<
          Text(1)@0x34b8538[0,5,T]  {0,0,2160,780} [state=10600000] [content=0x34b5fe8] sc=0x34b8740 pst=:-moz-non-element<
            "brown"
          >
        >
      >
    >
  >
>

When we call nsBlockInFlowLineIterator::Prev, mFrame is 0x34b8d48 (which is the block in the second column there), but mLine is the last line of the block in the second column (the block with address 0x34b82d0).

So as we walk backwards, we'll never hit mFrame->begin_lines(), which is bad.

Comment 3

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

Attachment: fix

The bug is actually because InlineData::line gets out of sync in nsContainerFrame as we advance through the next-in-flows of the frame. We can avoid this by just nulling it out when we're not sure what it should be.

This could potentially hurt performance, but it probably won't because textrun construction runs ahead constructing textruns for the text in the block that follows the first textframe of interest.

Comment 4

[David Baron :dbaron:]

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

If it's ok if line is null even if the inlines are associated with a block, could you fix the documentation in nsIFrame.h to reflect that (i.e.., that setting it only speeds things up and doesn't change behavior).  I assume you verified that's actually the case...

r+sr=dbaron

Comment 5

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

Good idea.

checked in with crashtest