Closed Bug 372237 Opened 17 years ago Closed 17 years ago

Crash [@ GetChildListNameFor] with -moz-box, float, position: fixed, block-in-inline

Categories

(Core :: Layout, defect)

x86
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: jruderman, Assigned: roc)

References

Details

(4 keywords, Whiteboard: [sg:critical?])

Crash Data

Attachments

(4 files)

###!!! 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?
Similar assertion and stack in bug 364427 (fixed).
Whiteboard: [sg:critical?]
Jesse suggested in irc that we ask BZ to look at this next week.
No, I'll take it
Assignee: nobody → roc
Mats, you interested in this?  You've been in this code a lot recently...
I'm already mostly done here.
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.
Attached patch fix problem #3Splinter Review
As described in the comment above
Attachment #257910 - Flags: superreview?(dbaron)
Attachment #257910 - Flags: review?(dbaron)
as described in the comment above
Attachment #257911 - Flags: superreview?(dbaron)
Attachment #257911 - Flags: review?(dbaron)
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 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+
Both patches checked in. Filed bug 373566 on problem #1.
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"?
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
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?
"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.
Flags: in-testsuite?
Flags: blocking1.8.1.4?
Flags: blocking1.8.0.12?
Flags: blocking1.8.1.4?
Flags: blocking1.8.1.4+
Flags: blocking1.8.0.12?
Flags: blocking1.8.0.12+
Comment on attachment 257911 [details] [diff] [review]
fix problem #2 (NS_FRAME_SET_TRUNCATION)

Do we not want this part on the branches?
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+
(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.
Fixed on branches.
Flags: blocking1.9?
Blocks: 374356
Group: security
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+
Crash Signature: [@ GetChildListNameFor]
You need to log in before you can comment on or make changes to this bug.