testcase crashes at [@ nsHTMLFramesetFrame::Reflow]

RESOLVED FIXED in mozilla1.9alpha1

Status

()

P1
critical
RESOLVED FIXED
13 years ago
7 years ago

People

(Reporter: jruderman, Assigned: dbaron)

Tracking

(Blocks: 1 bug, {crash, testcase})

Trunk
mozilla1.9alpha1
crash, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:nse][patch] null deref, crash signature)

Attachments

(2 attachments)

(Reporter)

Description

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

Comment 1

13 years ago
Created attachment 195601 [details]
testcase
(Reporter)

Comment 2

13 years ago
TB9232787K

Comment 3

13 years ago
Seeing this on Linux and Windows as well.
OS: MacOS X → All
Hardware: Macintosh → All
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.
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?
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.
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.
Whiteboard: [sg:nse] null deref
Assignee: nobody → dbaron
Created attachment 206363 [details] [diff] [review]
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.
Attachment #206363 - Flags: superreview?(roc)
Attachment #206363 - Flags: review?(roc)
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [sg:nse] null deref → [sg:nse][patch] null deref
Target Milestone: --- → mozilla1.9alpha
Attachment #206363 - Flags: superreview?(roc)
Attachment #206363 - Flags: superreview+
Attachment #206363 - Flags: review?(roc)
Attachment #206363 - Flags: review+
Checked in to trunk, 2005-12-20 19:32 -0800.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
(Reporter)

Comment 10

11 years ago
Crashtest checked in.
Flags: in-testsuite+
Crash Signature: [@ nsHTMLFramesetFrame::Reflow]
You need to log in before you can comment on or make changes to this bug.