399994 - Crash [@ nsPropertyTable::PropertyList::Equals] on closing print preview with page-break-after: always, position: fixed

Status: RESOLVED FIXED

(Core :: Layout, defect, P4)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, which crashes when print previewing it, and then closing the print preview window.

This seems to have regressed between 2007-10-01 and 2007-10-02, so I think a regression of bug 154892, somehow.

http://crash-stats.mozilla.com/report/index/5bc73961-7be2-11dc-8288-001a4bd43e5c
0  	nsPropertyTable::PropertyList::Equals(unsigned short, nsIAtom*)  	 mozilla/content/base/src/nsPropertyTable.cpp:85
1 	nsPropertyTable::GetPropertyListFor(unsigned short, nsIAtom*) 	mozilla/content/base/src/nsPropertyTable.cpp:295
2 	nsPropertyTable::GetPropertyInternal(nsPropertyOwner, unsigned short, nsIAtom*, int, unsigned int*) 	mozilla/content/base/src/nsPropertyTable.cpp:190
3 	nsPropertyTable::UnsetProperty(nsPropertyOwner, nsIAtom*, unsigned int*) 	mozilla/content/base/src/nsPropertyTable.h:212
4 	nsContainerFrame::GetOverflowFrames(nsPresContext*, int) 	mozilla/layout/generic/nsContainerFrame.cpp:1077
5 	nsContainerFrame::Destroy() 	mozilla/layout/generic/nsContainerFrame.cpp:264
6 	nsFrameList::DestroyFrames() 	mozilla/layout/generic/nsFrameList.cpp:67
7 	nsContainerFrame::Destroy() 	mozilla/layout/generic/nsContainerFrame.cpp:259
8 	nsFrameList::DestroyFrames() 	mozilla/layout/generic/nsFrameList.cpp:67
9 	nsFrameList::Destroy() 	mozilla/layout/generic/nsFrameList.cpp:57
10 	DestroyPropertyEnumerator 	mozilla/content/base/src/nsPropertyTable.cpp:336
11 	PL_DHashTableEnumerate 	pldhash.c:724
12 	nsPropertyTable::PropertyList::Destroy() 	mozilla/content/base/src/nsPropertyTable.cpp:346
13 	nsPropertyTable::DeleteAllProperties() 	mozilla/content/base/src/nsPropertyTable.cpp:106
14 	PresShell::Destroy() 	mozilla/layout/base/nsPresShell.cpp:1675
15 	nsPrintObject::DestroyPresentation() 	mozilla/layout/printing/nsPrintObject.cpp:96
16 	nsPrintObject::~nsPrintObject() 	mozilla/layout/printing/nsPrintObject.cpp:61
17 	nsPrintData::~nsPrintData() 	mozilla/layout/printing/nsPrintData.cpp:124
18 	nsPrintEngine::Destroy() 	mozilla/layout/printing/nsPrintEngine.cpp:259
19 	DocumentViewerImpl::ReturnToGalleyPresentation() 	mozilla/layout/base/nsDocumentViewer.cpp:3930
20 	DocumentViewerImpl::ExitPrintPreview() 	mozilla/layout/base/nsDocumentViewer.cpp:3701
21 	NS_InvokeByIndex_P 	mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:101
22 	XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) 	mozilla/js/src/xpconnect/src/xpcwrappednative.cpp:2326
23 	XPC_WN_CallMethod(JSContext*, JSObject*, unsigned int, long*, long*) 	mozilla/js/src/xpconnect/src/xpcwrappednativejsop

Comment 1

[AndrewM]

Also crashes on Windows Vista.

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a9pre) Gecko/2007101605 Minefield/3.0a9pre ID:2007101605

Comment 2

[fantasai]

This one also isn't crashing for me anymore. See bug 400190 comment 2

Comment 3

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

I still crash when I do "Print Preview" on Mac.

Comment 4

[fantasai]

Attachment: more straightforward testcase

Forgot to run print preview on that. Here's a simpler test case.

This bug triggers several assertions before it crashes,

###!!! ASSERTION: must have doc root as pageContentFrame's only child: 'overflow && !overflow->GetNextSibling()', file /home/fantasai/moz/mozilla/layout/generic/nsPageContentFrame.cpp, line 77
###!!! ASSERTION: null child frame pointer: 'aChildFrame', file /home/fantasai/moz/mozilla/layout/generic/nsHTMLContainerFrame.cpp, line 421
###!!! ASSERTION: null ptr: 'nsnull != aFrameList', file /home/fantasai/moz/mozilla/layout/generic/nsFrameList.cpp, line 203

The first one shows a pretty major problem: the 'overflow' frame is null, and it's not expected to be null. I think what's happening is that the page-break-after code is requesting a new nsPageContentFrame, but that frame is empty because the root frame and all its content fit entirely on the first page. Normally a fixed-positioned box won't split: it's told it has infinite availableHeight to lay itself out and it gets cropped if it doesn't fit. But our page-break code is wacky, and doesn't work through the same pathway.

Comment 5

[Mike Schroepfer]

Moving to wanted list...

Comment 6

[Martijn Wargers (dead)]

Still crashing, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041807 Minefield/3.0pre

Comment 7

[Martijn Wargers (dead)]

Attachment: testcase2

This is a testcase that crashes on reload. It uses -moz-column-count. It seems to be about the same issue, because it has the same stacktrace.

Comment 8

[Martijn Wargers (dead)]

Comment on attachment 318009 [details]
testcase2

This one doesn't crash anymore in current trunk build.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

bug 430569 comment 5 through 7 could be helpful.

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: valgrind's explanation for crash

This is using DEBUG_TRACEMALLOC_FRAMEARENA, attachment 290918 [details].  I wonder about the ordering of the calls within PresShell::Destroy.

Comment 11

[Samuel Sidler (old account; do not CC)]

I'm not sure why we'd block on this now? Martijn?

Comment 12

[Martijn Wargers (dead)]

I guess not, but it would be nice to get it fixed within the next 2 years or so.

Comment 13

[Samuel Sidler (old account; do not CC)]

Not blocking then, but wanted.

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

So I think we should destroy the property table before we destroy the style set.  (I would have thought destroying the frame tree would have destroyed all the properties of those frames -- but perhaps we suppress that?)

Comment 15

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

Yes, we don't destroy frame properties while we're tearing down the entire frame tree.

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

So the obvious patch here works.  I used attachment 290918 [details] for a crashtest; it crashes in the reftest harness without the patch, and doesn't crash with the patch.

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

http://hg.mozilla.org/mozilla-central/rev/16120b84f2a3

Comment 18

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 390138 [details] [diff] [review]
patch

We probably want to think about landing this on 1.9.1 after it's baked a bit on mozilla-central.

Comment 19

[Daniel Veditz [:dveditz]]

Comment on attachment 390138 [details] [diff] [review]
patch

Approved for 1.9.1.4, a=dveditz for release-drivers

Comment 20

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/53e5ec69d3fb

Comment 21

[Al Billings [:abillings - ex-MoCo]]

Verified fixed for 1.9.1.4 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.4pre) Gecko/20090914 Shiretoko/3.5.4pre after crashing with 1.9.1.3.