Closed
Bug 372237
Opened 18 years ago
Closed 18 years ago
Crash [@ GetChildListNameFor] with -moz-box, float, position: fixed, block-in-inline
Categories
(Core :: Layout, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jruderman, Assigned: roc)
References
Details
(4 keywords, Whiteboard: [sg:critical?])
Crash Data
Attachments
(4 files)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
dbaron
:
review+
dbaron
:
superreview+
dveditz
:
approval1.8.1.4+
dveditz
:
approval1.8.0.12+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
dbaron
:
review+
dbaron
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
###!!! ASSERTION: out-of-flow is already in the destroy queue: 'aDestroyQueue.IndexOf(outOfFlowFrame) == kNotFound', file /Users/jruderman/trunk/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 9233
Exception: EXC_BAD_ACCESS (0x0001)
Codes: KERN_INVALID_ADDRESS (0x0001) at 0xddddddfd
Thread 0 Crashed:
0 libgklayout.dylib 0x18a84b94 nsCachedStyleData::GetStyleDisplay() + 12 (nsStyleStructList.h:95)
1 libgklayout.dylib 0x1851dfdc nsStyleContext::GetStyleDisplay() + 20 (nsStyleStructList.h:95)
2 libgklayout.dylib 0x18a3765b nsIFrame::GetStyleDisplay() const + 83 (nsStyleStructList.h:95)
3 libgklayout.dylib 0x1838c60e GetChildListNameFor(nsIFrame*) + 52 (nsCSSFrameConstructor.cpp:1716)
4 libgklayout.dylib 0x1838e116 DeletingFrameSubtree(nsFrameManager*, nsIFrame*) + 296 (nsCSSFrameConstructor.cpp:9303)
5 libgklayout.dylib 0x183a68ba nsCSSFrameConstructor::ContentRemoved(nsIContent*, nsIContent*, int, int) + 1008 (nsCSSFrameConstructor.cpp:9453)
6 libgklayout.dylib 0x183a6208 nsCSSFrameConstructor::ReinsertContent(nsIContent*, nsIContent*) + 74 (nsCSSFrameConstructor.cpp:9161)
7 libgklayout.dylib 0x183a63c6 nsCSSFrameConstructor::ReframeContainingBlock(nsIFrame*) + 374 (nsCSSFrameConstructor.cpp:12698)
8 libgklayout.dylib 0x183a64b3 nsCSSFrameConstructor::MaybeRecreateContainerForIBSplitterFrame(nsIFrame*, unsigned*) + 141 (nsCSSFrameConstructor.cpp:10995)
9 libgklayout.dylib 0x183a43a5 nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*) + 173 (nsCSSFrameConstructor.cpp:11026)
10 libgklayout.dylib 0x183a4a3e nsCSSFrameConstructor::RestyleElement(nsIContent*, nsIFrame*, nsChangeHint) + 192 (nsCSSFrameConstructor.cpp:9924)
11 libgklayout.dylib 0x183a4c4d nsCSSFrameConstructor::ProcessOneRestyle(nsIContent*, nsReStyleHint, nsChangeHint) + 215 (nsCSSFrameConstructor.cpp:12787)
12 libgklayout.dylib 0x183a4e88 nsCSSFrameConstructor::ProcessPendingRestyles() + 420 (nsCSSFrameConstructor.cpp:12834)
13 libgklayout.dylib 0x183a4fd2 nsCSSFrameConstructor::RestyleEvent::Run() + 200 (nsCSSFrameConstructor.cpp:12904)
14 libxpcom_core.dylib 0x013499ea nsThread::ProcessNextEvent(int, int*) + 556 (nsThread.cpp:483)
Flags: blocking1.9?
Reporter | ||
Comment 1•18 years ago
|
||
Similar assertion and stack in bug 364427 (fixed).
Reporter | ||
Updated•18 years ago
|
Whiteboard: [sg:critical?]
Comment 2•18 years ago
|
||
Jesse suggested in irc that we ask BZ to look at this next week.
Comment 4•18 years ago
|
||
Mats, you interested in this? You've been in this code a lot recently...
Assignee | ||
Comment 5•18 years ago
|
||
I'm already mostly done here.
Assignee | ||
Comment 6•18 years ago
|
||
There are three bugs here. Here's what happens:
1) The -moz-box DIV with overflow:hidden containing an IMG goes haywire and the image (which is box-wrapped, of course) gets a crazy ascent (NS_INTRINSICSIZE to be precise). This appears to be ultimately sourced from the NS_INTRINSICSIZE passed in as the height here:
http://lxr.mozilla.org/seamonkey/source/layout/generic/nsFrame.cpp#5829
I'm not really sure what's going on. I'll file a followup bug with a nice reduced testcase. Anyway, this leads to some frames getting an insane height.
2) NS_FRAME_SET_TRUNCATION then decides that frames have exceeded the available height (which was meant to be unconstrained) and pushes them to overflow lists. I have a patch that fixes this by never setting truncation if the available height is NS_INTRINSICSIZE, even if absurd heights are running around.
This is bad but still shouldn't be fatal, we're supposed to not crash even when there are frames hanging around in overflow lists.
3) Later we crash deleting some frames. This problem is quite simple, nsCSSFrameConstructor::DoDeletingFrameSubtree wants to skip iterating through child lists which contain out-of-flow-frames, preferring to delete frames through placeholders instead. The problem is just that it needs skip the overflowOutOfFlow list and doesn't. The simple patch for that fixes the crash.
Assignee | ||
Comment 7•18 years ago
|
||
As described in the comment above
Attachment #257910 -
Flags: superreview?(dbaron)
Attachment #257910 -
Flags: review?(dbaron)
Assignee | ||
Comment 8•18 years ago
|
||
as described in the comment above
Attachment #257911 -
Flags: superreview?(dbaron)
Attachment #257911 -
Flags: review?(dbaron)
Comment 9•18 years ago
|
||
Comment on attachment 257910 [details] [diff] [review]
fix problem #3
r+sr=dbaron
Attachment #257910 -
Flags: superreview?(dbaron)
Attachment #257910 -
Flags: superreview+
Attachment #257910 -
Flags: review?(dbaron)
Attachment #257910 -
Flags: review+
Comment 10•18 years ago
|
||
Comment on attachment 257911 [details] [diff] [review]
fix problem #2 (NS_FRAME_SET_TRUNCATION)
r+sr=dbaron. Please get that other bug filed, though, if you have a testcase. :-)
Attachment #257911 -
Flags: superreview?(dbaron)
Attachment #257911 -
Flags: superreview+
Attachment #257911 -
Flags: review?(dbaron)
Attachment #257911 -
Flags: review+
Assignee | ||
Comment 11•18 years ago
|
||
Both patches checked in. Filed bug 373566 on problem #1.
Assignee | ||
Comment 12•18 years ago
|
||
I'm not really sure how to write a reftest for this. Maybe we should just have a collection of "pages that should not crash the browser"?
Assignee | ||
Updated•18 years ago
|
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 13•18 years ago
|
||
Comment on attachment 257910 [details] [diff] [review]
fix problem #3
I think we should take this on branches. It probably fixes some dangerous crashes, even if this particular crash is not present on branch. It's very small, obviously correct, and I think very low risk.
Attachment #257910 -
Flags: approval1.8.1.4?
Attachment #257910 -
Flags: approval1.8.0.12?
Reporter | ||
Comment 14•18 years ago
|
||
"Pages that should not crash the browser" become mochitests, right? But see bug 368573 for an issue with assertions that might result in a split.
I'm guessing this page shouldn't go into a public test suite until the bug is fixed on branches, though.
Updated•18 years ago
|
Flags: in-testsuite?
Updated•18 years ago
|
Flags: blocking1.8.1.4?
Flags: blocking1.8.0.12?
Updated•18 years ago
|
Flags: blocking1.8.1.4?
Flags: blocking1.8.1.4+
Flags: blocking1.8.0.12?
Flags: blocking1.8.0.12+
Comment 15•18 years ago
|
||
Comment on attachment 257911 [details] [diff] [review]
fix problem #2 (NS_FRAME_SET_TRUNCATION)
Do we not want this part on the branches?
Comment 16•18 years ago
|
||
Comment on attachment 257910 [details] [diff] [review]
fix problem #3
approved for 1.8.0.12 and 1.8.1.4, a=dveditz for release-drivers
Attachment #257910 -
Flags: approval1.8.1.4?
Attachment #257910 -
Flags: approval1.8.1.4+
Attachment #257910 -
Flags: approval1.8.0.12?
Attachment #257910 -
Flags: approval1.8.0.12+
Assignee | ||
Comment 17•18 years ago
|
||
(In reply to comment #15)
> (From update of attachment 257911 [details] [diff] [review])
> Do we not want this part on the branches?
It might help, but I didn't nominate it because I don't know of any crashes that it fixes other than this one, and we have a better fix for this crash.
Assignee | ||
Comment 18•18 years ago
|
||
Assignee | ||
Comment 19•18 years ago
|
||
Fixed on branches.
Flags: blocking1.9?
Keywords: fixed1.8.0.12,
fixed1.8.1.4
Updated•17 years ago
|
Group: security
Reporter | ||
Comment 20•17 years ago
|
||
I checked in the testcase as a crashtest, but since there were multiple patches, it might be good to have more tests.
Flags: in-testsuite? → in-testsuite+
Updated•16 years ago
|
Blocks: CVE-2008-2811
Updated•13 years ago
|
Crash Signature: [@ GetChildListNameFor]
You need to log in
before you can comment on or make changes to this bug.
Description
•