381502 - Crash [@ nsFrame::GetBoxAscent] with tree stuff, display: table-row and display: inline

Status: VERIFIED FIXED

(Core :: XUL, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, when loading in current trunk build it crashes.

Talkback ID: TB32369804K
nsFrame::GetBoxAscent  [mozilla/layout/generic/nsframe.cpp, line 6038]
nsBoxFrame::GetBoxAscent  [mozilla/layout/xul/base/src/nsboxframe.cpp, line 823]

This regressed between 2007-05-18 and 2007-05-20, so I think a regression from bug 380217, somehow.

Comment 1

[Eli Friedman]

This is a frame constructor bug; apparently the root box frame somehow thinks it has a table cell as a direct child.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Attachment: fixes the crash

So the return value of the Init() isn't handled properly always :(
Is this patch too must like a hack? TreeBodyFrame should be used as a
nsITreeBoxObject only if it has the owning reference to the real
box object.
Fixing all Init()s in the CSSFC is a different bug, IMO.

Comment 3

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

where are we doing the QI? Does that need to be fixed? How many Init() calls in the frame constructor would need to be fixed?

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

We are QIing in nsTreeBoxObject and once in presshell so that
it can call ClearStyleAndImageCaches()

return value of CSSFM::InitAndRestoreFrame isn't usually handled;
it is used ~40 times and return value is handled properly only ~5 times.
And there are also other calls to Init.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Ofc the number when Init can be called on treebodyframe is smaller.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Hmm, or, is there after all something else wrong. looking

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

bernd, any comments on this one?

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Is this a dup of that other one where boxmetrics is also null,
Bug 380853?

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Haven't yet found the right fix. Someone more familiar with
frame constructor might find the bug easier.
The crash is gone if I replace
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/base/nsCSSFrameConstructor.cpp&rev=1.1352&mark=12614#12614
with |continue;|, but that makes presshell to assert "Some frame destructors were not called".
Also, the crash is gone if any frame (for example inlineframe) is created for treechildren.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 265656 [details] [diff] [review]
fixes the crash

Because treebodyframe is currently pretty much the only frame which returns error code (when there isn't OOM or such)
I'd like to use the patch until the handling of ::Init can be resolved. I mean, what should actually happen when ::Init fails? Should the frame be marked display:none or leave as it is, but then  computedstyle lies.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 265656 [details] [diff] [review]
fixes the crash

I'll have fix for this in bug 381502

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

er, in Bug 381862

Comment 13

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070529 Minefield/3.0a5pre