307979 - testcase crashes at [@ nsHTMLFramesetFrame::Reflow]

Status: RESOLVED FIXED

(Core :: Layout: Images, Video, and HTML Frames, defect, P1)

Description

[Jesse Ruderman]

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20050910
Firefox/1.6a1

Comment 1

[Jesse Ruderman]

Attachment: testcase

Comment 2

[Jesse Ruderman]

TB9232787K

Comment 3

[Adam Guthrie]

Seeing this on Linux and Windows as well.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

I was poking at this for fun. The problem boils down to the fact that
nsBoxFrame::Reflow is causing whatever frames under it to receive multiple
initial reflows, which, in the case of nsFrameFrame, causes us to deref null
(see also bug 101746, comment 34). If you replace 'frameset' with 'table' in the
testcase, you get similar asserts (though we don't crash).

More directly, the problem lines are found at:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/xul/base/src/nsBoxFrame.cpp&rev=1.275&mark=812-814#812
. They cause us to try to reflow the children of the box frame with the reason
given in |state| (which happens to be eReflowReason_Initial). I'm not really
sure where the best place to fix this is. It seems like the easiest place to fix
it would be to make nsFrame::HandleIncrementalReflow return eReflowReason_Resize
if the frame already had had an initial reflow, but that feels like a hack.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Waterson's suggestion in bug 101746, comment 36 does fix this crash, though. Is
that sufficient to fix this bug, or is a more general solution lurking?

Comment 6

[Boris Zbarsky [:bzbarsky]]

We have other issues with (and existing bugs on) the multiple initial reflows
boxes do....  So just fixing this in frame(set) is not the way to go.

Comment 7

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

The reflow branch should fix this, or at least should get rid of the fact that
there are multiple "initial" reflows, since it gets rid of reflow reasons and types.

Comment 8

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

Attachment: patch

There's nothing *incorrect* about this patch.  In fact, it's been on the reflow branch(es) for months.  And I don't see anyone cleaning up the ex-BoxToBlockAdaptor mess in nsFrame.cpp anytime before that lands, either, so we may as well just get this in to fix the crash.

Comment 9

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

Checked in to trunk, 2005-12-20 19:32 -0800.

Comment 10

[Jesse Ruderman]

Crashtest checked in.