322185 - Crash [@ nsBox::DoesNeedRecalc] with <svg:g style="display: -moz-grid-line; overflow: hidden;">

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jesse Ruderman]

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

To crash, just load the testcase.

Stack trace:

nsBox::DoesNeedRecalc(nsSize const&) + 0
nsFrame::GetPrefSize(nsBoxLayoutState&, nsSize&) + 48
nsSprocketLayout::GetPrefSize(nsIFrame*, nsBoxLayoutState&, nsSize&) + 268
nsGridRowLeafLayout::GetPrefSize(nsIFrame*, nsBoxLayoutState&, nsSize&) + 116
nsBoxFrame::GetPrefSize(nsBoxLayoutState&, nsSize&) + 200
nsXULScrollFrame::GetPrefSize(nsBoxLayoutState&, nsSize&) + 240
nsBoxFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned&) + 204
nsXULScrollFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned&) + 56
nsSVGOuterSVGFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned&) + 256
nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned, unsigned&) + 148
CanvasFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned&) + 356
nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned, unsigned&) + 148
nsHTMLScrollFrame::ReflowScrolledFrame(ScrollReflowState const&, int, int, nsHTMLReflowMetrics*, int) + 500
nsHTMLScrollFrame::ReflowContents(ScrollReflowState*, nsHTMLReflowMetrics const&) + 160
nsHTMLScrollFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned&) + 848
nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned, unsigned&) + 148
ViewportFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned&) + 300
IncrementalReflow::Dispatch(nsPresContext*, nsHTMLReflowMetrics&, nsSize const&, nsIRenderingContext&) + 280
PresShell::ProcessReflowCommands(int) + 524
PresShell::WillPaint() + 88
nsViewManager::FlushPendingInvalidates() + 164
nsViewManager::EnableRefresh(unsigned) + 156
nsViewManager::EndUpdateViewBatch(unsigned) + 132
PresShell::InitialReflow(int, int) + 748
nsContentSink::StartLayout(int) + 208
nsXMLContentSink::StartLayout() + 144
nsXMLContentSink::DidBuildModel() + 456
nsExpatDriver::DidBuildModel(unsigned, int, nsIParser*, nsIContentSink*) + 56
nsParser::DidBuildModel(unsigned) + 120
nsParser::ResumeParse(int, int, int) + 592
nsParser::OnStopRequest(nsIRequest*, nsISupports*, unsigned) + 192
nsDocumentOpenInfo::OnStopRequest(nsIRequest*, nsISupports*, unsigned) + 124
nsBaseChannel::OnStopRequest(nsIRequest*, nsISupports*, unsigned) + 92
nsInputStreamPump::OnStateStop() + 160
nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) + 128
nsAStreamCopier::PostContinuationEvent_Locked() + 1240
PL_HandleEvent + 36
PL_ProcessPendingEvents + 128
...

Comment 1

[Jesse Ruderman]

Attachment: testcase - crashes Firefox

Comment 2

[Bernd]

Attachment: patch

The problem here is that xul creates frames based on display type regardless whether they are special content or not. This patch assumes that we want to keep it, otherwise we need to change

// Display types for XUL start here
    // First is BOX
    if (!newFrame && isXULDisplay) {
it to look up IsSpecialContent.

Comment 3

[Bernd]

Attachment: patch

Comment 4

[Boris Zbarsky [:bzbarsky]]

Hmmm...  I guess this is OK for now pending us having a saner frame construction arch.  :(

Comment 5

[Bernd]

Attachment: alternative patch

alternative patch,

does slicing CreateXULFrame into three pieces
one for tag based frame creation
the second one the display based frame moving to createframesbydisplay type
the third, cleanup after frame creation called by both of them count as a sane architecture or do you have something other in mind.
The rearch is(should be) equivalent to the one liner attached as a patch.

Comment 6

[Boris Zbarsky [:bzbarsky]]

I think I prefer the alternative patch, if there's not too much perf impact.  And the rearch I want to do would be a lot more drastic than just slicing up CreateXULFrame.  ;)

Comment 7

[Bernd]

Attachment: rev. patch

I hope this miminimizes the performance issue

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 207737 [details] [diff] [review]
rev. patch

Hmm....  r+sr=bzbarsky; let's see how this goes.

Comment 9

[Bernd]

fixed on trunk, I did not see a tp txul or ts change due to the bug.

Martijn this patch touches  bugs where xul display types are assigned to mathml or svg tags it will get ignored now ;-)

Comment 10

[Martijn Wargers (dead)]

You're spoiling the fun ;-)
Will this patch also fix bug 314244?

Comment 11

[Bernd]

yes that seems to be now wfm and even bug 322656 - wfm, I had my fun today ;-)

Comment 12

[Jesse Ruderman]

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

Comment 13

[Jesse Ruderman]

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

Comment 14

[Bernd]

Comment on attachment 207737 [details] [diff] [review]
rev. patch

Giving the number of bugs which block bug  	306939 that got fixed/or wfm'ed by this it might go with some more baking on branch.

Comment 15

[Daniel Veditz [:dveditz]]

Comment on attachment 207737 [details] [diff] [review]
rev. patch

approved for 1.8.0 branch, a=dveditz

Comment 16

[Boris Zbarsky [:bzbarsky]]

Bernd, you're going to land this on the 1.8.1 branch, right?

Comment 17

[Bernd]

yep, saturday is checkin day, only emergency checkins on weekdays

Comment 18

[Bernd]

fixed on 1.8.1

Comment 19

[Marcia Knous [:marcia]]

verified on the 1.8.0 branch using Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.1) Gecko/20060302 Firefox/1.5.0.1. No testcase crash, adding keyword.

Comment 20

[Marcia Knous [:marcia]]

verified on the 1.8.1 branch using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1b1) Gecko/20060810 BonEcho/2.0b1. The testcase cited in the bug (https://bugzilla.mozilla.org/attachment.cgi?id=207407) does not crash. Adding keyword.

Comment 21

[Jesse Ruderman]

Crashtest checked in.