28434 - crash clicking link on www.nytimes.com

Status: VERIFIED DUPLICATE of bug 28084

(Core :: Layout, defect, P3)

Description

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

DESCRIPTION:  I crash clicking the first line of the first AP dispatch link on
http://www.nytimes.com/ after scrolling down the page.  I suspect the conditions
to reproduce are a bit more general.

STEPS TO REPRODUCE:
 * load http://www.nytimes.com/
 * drag the scrollbar slider down so that the AP dispatches are visible on the
right side (about a page down)
 * click on the first line of the first dispatch listed

ACTUAL RESULTS:
 * crash.  Stack trace is below.  I reproduced this crash twice

EXPECTED RESULTS:
 * no crash

DOES NOT WORK CORRECTLY ON:
 * Linux, mozilla, pull of 2000-02-18 10:53-11:00 PST.

WORKS CORRECTLY ON:
 * older builds

ADDITIONAL INFORMATION:
Asserts, debugging info, and stack trace:

Gdk-WARNING **: Missing charsets in FontSet creation


Gdk-WARNING **:     ISO8859-1


Gdk-WARNING **:     ISO8859-1

###!!! ASSERTION: failed to remove frame: 'result', file nsContainerFrame.cpp,
line 777
###!!! Break: at file nsContainerFrame.cpp, line 777
###!!! ASSERTION: failed to remove frame: 'result', file nsContainerFrame.cpp,
line 777
###!!! Break: at file nsContainerFrame.cpp, line 777

Program received signal SIGILL, Illegal instruction.
0x413bee21 in ?? ()
   from /home/david/mozilla/src/mozilla/dist/bin/components/libraptorhtml.so

[loaded shared libraries snipped...]

(gdb) bt
#0  0x413bee21 in nsInlineFrame::nsIFrameDebug virtual table ()
#1  0x4128b4fc in nsFrameList::DestroyFrames (this=0xbfff9cb0, 
    aPresContext=0x8590d90) at nsFrameList.cpp:34
#2  0x41039066 in DestroyOverflowFrames (aPresContext=0x8590d90, 
    aFrame=0x86dfce8, aPropertyName=0x8203f68, aPropertyValue=0x889db10)
    at nsContainerFrame.cpp:825
#3  0x410486e0 in FrameManager::PropertyList::RemovePropertyForFrame (
    this=0x8714db8, aPresContext=0x8590d90, aFrame=0x86dfce8)
    at nsFrameManager.cpp:1934
#4  0x41047c49 in FrameManager::RemoveAllPropertiesFor (this=0x859b9d0, 
    aPresContext=0x8590d90, aFrame=0x86dfce8) at nsFrameManager.cpp:1638
#5  0x41044ce2 in FrameManager::NotifyDestroyingFrame (this=0x859b9d0, 
    aFrame=0x86dfce8) at nsFrameManager.cpp:658
#6  0x4106d289 in PresShell::NotifyDestroyingFrame (this=0x8620158, 
    aFrame=0x86dfce8) at nsPresShell.cpp:1408
#7  0x4103c2ee in nsFrame::Destroy (this=0x86dfce8, aPresContext=0x8590d90)
    at nsFrame.cpp:386
#8  0x41037aea in nsContainerFrame::Destroy (this=0x86dfce8, 
    aPresContext=0x8590d90) at nsContainerFrame.cpp:98
#9  0x41038e2e in nsContainerFrame::DeleteChildsNextInFlow (this=0x86dfe40, 
    aPresContext=0x8590d90, aChild=0x889dbb8) at nsContainerFrame.cpp:780
#10 0x41038dc5 in nsContainerFrame::DeleteChildsNextInFlow (this=0x8725fe8, 
    aPresContext=0x8590d90, aChild=0x88af260) at nsContainerFrame.cpp:769
#11 0x41060048 in nsLineLayout::ReflowFrame (this=0xbfffaa34, 
    aFrame=0x88af260, aNextRCFrame=0xbfffa128, aReflowStatus=@0xbfffa998, 
    aMetrics=0x0, aPushedFrame=@0xbfffa04c) at nsLineLayout.cpp:1155
#12 0x4105a5b1 in nsInlineFrame::ReflowInlineFrame (this=0x8725fe8, 
    aPresContext=0x8590d90, aReflowState=@0xbfffa254, irs=@0xbfffa128, 
    aFrame=0x88af260, aStatus=@0xbfffa998) at nsInlineFrame.cpp:503
#13 0x4105a0ce in nsInlineFrame::ReflowFrames (this=0x8725fe8, 
    aPresContext=0x8590d90, aReflowState=@0xbfffa254, irs=@0xbfffa128, 
    aMetrics=@0xbfffa1fc, aStatus=@0xbfffa998) at nsInlineFrame.cpp:356
#14 0x41059dd5 in nsInlineFrame::Reflow (this=0x8725fe8, 
    aPresContext=0x8590d90, aMetrics=@0xbfffa1fc, aReflowState=@0xbfffa254, 
    aStatus=@0xbfffa998) at nsInlineFrame.cpp:259
#15 0x4105f81d in nsLineLayout::ReflowFrame (this=0xbfffaa34, 
    aFrame=0x8725fe8, aNextRCFrame=0xbfffa43c, aReflowStatus=@0xbfffa998, 
    aMetrics=0x0, aPushedFrame=@0xbfffa360) at nsLineLayout.cpp:987
#16 0x4105a5b1 in nsInlineFrame::ReflowInlineFrame (this=0x88ba720, 
    aPresContext=0x8590d90, aReflowState=@0xbfffa568, irs=@0xbfffa43c, 
    aFrame=0x8725fe8, aStatus=@0xbfffa998) at nsInlineFrame.cpp:503
#17 0x4105a0ce in nsInlineFrame::ReflowFrames (this=0x88ba720, 
    aPresContext=0x8590d90, aReflowState=@0xbfffa568, irs=@0xbfffa43c, 
    aMetrics=@0xbfffa510, aStatus=@0xbfffa998) at nsInlineFrame.cpp:356
#18 0x41059dd5 in nsInlineFrame::Reflow (this=0x88ba720, 
    aPresContext=0x8590d90, aMetrics=@0xbfffa510, aReflowState=@0xbfffa568, 
    aStatus=@0xbfffa998) at nsInlineFrame.cpp:259
#19 0x4105f81d in nsLineLayout::ReflowFrame (this=0xbfffaa34, 
    aFrame=0x88ba720, aNextRCFrame=0xbfffa750, aReflowStatus=@0xbfffa998, 
    aMetrics=0x0, aPushedFrame=@0xbfffa674) at nsLineLayout.cpp:987
#20 0x4105a5b1 in nsInlineFrame::ReflowInlineFrame (this=0x88ba758, 
    aPresContext=0x8590d90, aReflowState=@0xbfffa87c, irs=@0xbfffa750, 
    aFrame=0x88ba720, aStatus=@0xbfffa998) at nsInlineFrame.cpp:503
#21 0x4105a0ce in nsInlineFrame::ReflowFrames (this=0x88ba758, 
    aPresContext=0x8590d90, aReflowState=@0xbfffa87c, irs=@0xbfffa750, 
    aMetrics=@0xbfffa824, aStatus=@0xbfffa998) at nsInlineFrame.cpp:356
#22 0x41059dd5 in nsInlineFrame::Reflow (this=0x88ba758, 
    aPresContext=0x8590d90, aMetrics=@0xbfffa824, aReflowState=@0xbfffa87c, 
    aStatus=@0xbfffa998) at nsInlineFrame.cpp:259
#23 0x4105f81d in nsLineLayout::ReflowFrame (this=0xbfffaa34, 
    aFrame=0x88ba758, aNextRCFrame=0xbfffb4f4, aReflowStatus=@0xbfffa998, 
    aMetrics=0x0, aPushedFrame=@0xbfffa994) at nsLineLayout.cpp:987
#24 0x4102e181 in nsBlockFrame::ReflowInlineFrame (this=0x8788b68, 
    aState=@0xbfffb474, aLineLayout=@0xbfffaa34, aLine=0x8875908, 
    aFrame=0x88ba758, aLineReflowStatus=0xbfffa9e7 "") at nsBlockFrame.cpp:3969
#25 0x4102de72 in nsBlockFrame::DoReflowInlineFrames (this=0x8788b68, 
    aState=@0xbfffb474, aLineLayout=@0xbfffaa34, aLine=0x8875908, 
    aKeepReflowGoing=0xbfffb250, aLineReflowStatus=0xbfffb10b "\002", 
    aUpdateMaximumWidth=1) at nsBlockFrame.cpp:3854

etc...

Comment 1

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

This also crashes in today's release build.

Comment 2

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

Attachment: simplified testcase

Comment 3

[jst]

I noticed a similar crash on mozillazine.org, mozilla crashed when clicking
on the "the verification-a-rama page" link, I managed to make mozilla not
crash on that link but I doubt my fix is the real fix for this problem.

The mozillazine crash was due to frames being deleted in
nsLineLayout::ReflowFrame() (nsLineLayout.cpp:1155) while still being the value
of the overflowProperty of it's parent frame. I fixed this by checking if the
frame that is about to be deleted is the overflowProperty of it's parent and if
so I removed the property. Withou this mozilla crashed when trying to destroy
the overflowProperty of the parent node when the parentNode was destroyed (I
think, it's getting late here).

From lookin at the nytimes.com crash in the debugger it looks like that one is
due to a similar problem but there a frame is deleted while still on the
mReflowTextRuns list in an nsLineLayout object.

Also, I noticed that this is very sensitive to how things are layd out on a
line, I was able to get the mozillazine link work by resizeing the window so
that the line where the link was changed. This bug shows up on different places
on different OS's, I've seen it on bot Linux and on Windows but because of
de different font sizes and such things might end up on different lines on
different computers and thus this is rather compilcated, for instance the
simplified testcase I'm about to attach works for me on linux but crashes on
windows, the existing testcase works for me on both linux and windows, go
figure...

If you want patches let me know, but as I said I don't think they fix the
actual problem.

Because of the weirdness of this bug I'm marking this as beta1, it might not
be that common but since this bug can make just about any site a mozilla crasher
(any time) I think it would be great to get this one nailed down before beta1.

Comment 4

[jst]

Attachment: One more simplified testcase.

Comment 5

[jst]

Attachment: Simplified testcase of the mozillaZine case...

Comment 6

[jst]

Adding myself to CC, sorry about the spam...

Comment 7

[troy]

Pierre, why does clicking on a link cause us to do an incremental reflow?

Comment 8

[troy]

Steve, if you define OLD_BLOCK_INCREMENTAL_REFLOW in nsBlockFrame.cpp, then we 
no longer crash. At least using the first of the test cases (id=5435). I haven't 
tried the newer test cases

My guess is that because we just reflow the one line we're not reflowing the 
continuing line and so the overflow list gets messed up. Either that or the 
continuing frames get messed up

Comment 9

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

Remarking beta1 and All/All since jst's changes got wiped out...

Comment 10

[jst]

troy, I think the incremental reflow is caused by the "link state" change that
happends when you click on a link, all the testcases have vlink="..." and
link="..." on the body tag so clicking on a link would change the style (color)
of the link, thus the reflow (right?).

I thought I marked this M14, was that removed intentionally?

Comment 11

[troy]

Remarking M14. There was a bugzilla collision and I probably wiped out some of 
your changes by mistake

Comment 12

[troy]

No, changing the color of a link shouldn't cause us to reflow. At least it 
shouldn't. We should just repaint, that's all.

Pierre, we need to fix this. This has got to be hurting performance if this 
happens on all pages.

Comment 13

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

Could the reflow have something to do with the outline property?  I seem to
remember someone proposing a hack to fix outline painting involving causing a
reflow, but I don't know if that was actually done...

Comment 14

[jst]

I'm unable to look into bug 18321 because of this bug, clicking on the links on
the left side of the of the url in that bug crashes due to this same proble.

Comment 15

[troy]

It looks like it is vlink="#444464" link="#000066" that is causing us to do an 
incremental reflow when the link is clicked.

That's bad for performance and I filed bug #28522

Comment 16

[Cyril Bortolato]

Looks similar to bug #28337

Comment 17

[jst]

*** Bug 28337 has been marked as a duplicate of this bug. ***

Comment 18

[buster]

*** This bug has been marked as a duplicate of 28084 ***

Comment 19

[Joseph Elwell]

[bugday] this bug does not occur anymore - and troy's 2/28 comment points out 
the stack traces are the same. marking verified - this is a dupe, and worksforme 
on winNT build 2000022908.

Comment 20

[buster]

*** Bug 28337 has been marked as a duplicate of this bug. ***