Closed Bug 585598 Opened 9 years ago Closed 9 years ago
Crash [@ ns
Container Frame::Destroy From] on print preview with select, float and page-break
153 bytes, application/xhtml+xml
61.78 KB, text/plain; charset=UTF-8
2.02 KB, patch
|Details | Diff | Splinter Review|
2.26 KB, patch
|Details | Diff | Splinter Review|
See testcase, which crashes current trunk build on print preview. It doesn't crash in a 2010-08-04 trunk build, so I guess it's a regression from bug 563584. http://crash-stats.mozilla.com/report/index/bp-4755f15f-5db0-4293-9ad7-882a32100809 0 xul.dll nsContainerFrame::DestroyFrom layout/generic/nsContainerFrame.cpp:268 1 xul.dll nsListControlFrame::DestroyFrom layout/forms/nsListControlFrame.cpp:219 2 xul.dll nsFrameList::DestroyFramesFrom layout/generic/nsFrameList.cpp:98 3 xul.dll nsComboboxControlFrame::DestroyFrom layout/forms/nsComboboxControlFrame.cpp:1218 4 xul.dll nsIFrame::Destroy layout/generic/nsIFrame.h:531 5 xul.dll nsFrameList::DestroyFrames layout/generic/nsFrameList.cpp:87 6 xul.dll nsFrameList::Destroy layout/generic/nsFrameList.cpp:70 7 xul.dll nsContainerFrame::DestroyFrameList layout/generic/nsContainerFrame.h:334 8 xul.dll mozilla::FramePropertyTable::DeleteEnumerator layout/base/FramePropertyTable.cpp:248 9 xul.dll nsTHashtable<nsIdentifierMapEntry>::s_EnumStub obj-firefox/dist/include/nsTHashtable.h:420 10 xul.dll PL_DHashTableEnumerate obj-firefox/xpcom/build/pldhash.c:754 11 xul.dll PresShell::Destroy layout/base/nsPresShell.cpp:1968 12 xul.dll DocumentViewerImpl::DestroyPresShell layout/base/nsDocumentViewer.cpp:4319 13 xul.dll DocumentViewerImpl::SetIsPrintPreview layout/base/nsDocumentViewer.cpp:4145 14 xul.dll DocumentViewerImpl::ReturnToGalleyPresentation layout/base/nsDocumentViewer.cpp:4178 15 xul.dll DocumentViewerImpl::ExitPrintPreview layout/base/nsDocumentViewer.cpp:3939 16 xul.dll NS_InvokeByIndex_P xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102
(In reply to comment #0) > See testcase, which crashes current trunk build on print preview. It doesn't Actually, it's crashing when closing print preview.
So the basic problem here is that we have a view on something inside the float continuations list, but nsBlockFrame doesn't destroy its float continuations list in its DestroyFrom method; it instead waits for the property table destruction to happen. But by this point the frame for the view's parent view has been destroyed... and, along with it, the view itself. So when we try to destroy the property table we delete the float continuations list and its contents and end up trying to destroy the view a second time. This actually isn't a problem in the dynamic case since PresShell::NotifyDestroyingFrame destroys the properties at the same time as the frame destruction, via its call to mPresContext->NotifyDestroyingFrame(aFrame). But when mIgnoreFrameDestruction is true, as it is during teardown, it's a problem. The obvious fix is to make sure nsBlockFrame deletes its float continuations list in its destroy method. I should figure out whether the reordering done in bug 399994 was also done because of the float continuations list not being destroyed, or whether there is another list we need to fix.
That said, we also really shouldn't have a pushed floats list lying around after reflow is complete.
Additional bad things happening: * the outer float tries to fit on the first page, despite the page break frame before it, since it's zero height. This might be fixed if we made the page break frames take up one extra appunit. * the outer float doesn't split despite that the inner float is truncated and pushed to the float continuations list -- this despite returning a reflow status of NS_FRAME_REFLOW_NEXTINFLOW | NS_FRAME_OVERFLOW_INCOMPLETE (this would be the cause of having a PushedFloats list still lying around)
This fixes the crash, but not the layout bug. We should take it anyway; it's good crash defense even if there shouldn't really be any of these lists hanging around at frame destruction time. (We already do the same for overflow lines, which are the same way.) The crashtest doesn't crash in the harness even without the patch; I'm not sure why, but I'm not going to bother investigating.
Attachment #467060 - Flags: review?(roc)
This also (without patch 1) fixes the crash. It also fixes the layout of the testcase. We should split a float when it's overflow-incomplete.
Attachment #467061 - Flags: review?(roc)
Attachment #467060 - Flags: review?(roc) → review+
Attachment #467061 - Flags: review?(roc) → review+
Status: NEW → RESOLVED
Closed: 9 years ago
OS: Windows 7 → All
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b5
Verified fixed, using: Mozilla/5.0 (Windows NT 6.1; rv:2.0b5pre) Gecko/20100822 Minefield/4.0b5pre
Status: RESOLVED → VERIFIED
Crash Signature: [@ nsContainerFrame::DestroyFrom]
You need to log in before you can comment on or make changes to this bug.