424291 - Crash [@ nsCellMap::SetNextSibling] while trying to print

Status: VERIFIED FIXED

(Core :: Layout: Tables, defect, P2)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, when clicking on the print button and then printing something, current trunk builds of Mozilla crash.

I get to see a few assertions before the crash:
###!!! ASSERTION: data loss - incomplete row needed more height than available, on top of page: 'rowMetrics.height <= rowReflowState.availableHeight', file c:/mozilla-build/mozilla/layout/tables/nsTableRowGroupFrame.cpp, line 1137
nsBlockReflowContext: TableOuter(table)(3)@05A20434 metrics=54336,41688905!
nsBlockReflowContext: Block(body)(1)@04CCFA1C metrics=54336,41688905!
###!!! ASSERTION: GetMapFor called with continuation: '!aRowGroup->GetPrevInFlow()', file c:/mozilla-build/mozilla/layout/tables/nsCellMap.cpp, line 278
###!!! ASSERTION: invalid array index: 'i < Length()', file c:\mozilla-build\mozilla\_firefox\dist\include\xpcom\nsTArray.h, line 317

Then the crash:
>	gklayout.dll!nsCellMap::SetNextSibling(nsCellMap * aSibling=0x00000000)  Line 688 + 0x6 bytes	C++
 	gklayout.dll!nsTableCellMap::Synchronize(nsTableFrame * aTableFrame=0x04ccfd0c)  Line 339	C++
 	gklayout.dll!nsTableFrame::RemoveFrame(nsIAtom * aListName=0x00000000, nsIFrame * aOldFrame=0x04ccec34)  Line 2412	C++
 	gklayout.dll!nsFrameManager::RemoveFrame(nsIFrame * aParentFrame=0x04ccfd0c, nsIAtom * aListName=0x00000000, nsIFrame * aOldFrame=0x04ccec34)  Line 695	C++
 	gklayout.dll!nsCSSFrameConstructor::ContentRemoved(nsIContent * aContainer=0x05688bd0, nsIContent * aChild=0x020263f8, int aIndexInContainer=3, int * aDidReconstruct=0x0012e844)  Line 9648 + 0x12 bytes	C++
 	gklayout.dll!PresShell::ContentRemoved(nsIDocument * aDocument=0x04cc1150, nsIContent * aContainer=0x05688bd0, nsIContent * aChild=0x020263f8, int aIndexInContainer=3)  Line 4789	C++
 	gklayout.dll!nsNodeUtils::ContentRemoved(nsINode * aContainer=0x05688bd0, nsIContent * aChild=0x020263f8, int aIndexInContainer=3)  Line 167 + 0xe6 bytes	C++
 	gklayout.dll!nsGenericElement::doRemoveChildAt(unsigned int aIndex=3, int aNotify=1, nsIContent * aKid=0x020263f8, nsIContent * aParent=0x05688bd0, nsIDocument * aDocument=0x04cc1150, nsAttrAndChildArray & aChildArray={...})  Line 2850 + 0x11 bytes	C++
 	gklayout.dll!nsGenericElement::RemoveChildAt(unsigned int aIndex=3, int aNotify=1)  Line 2780 + 0x2a bytes	C++
 	gklayout.dll!nsGenericElement::doRemoveChild(nsIDOMNode * aOldChild=0x02026414, nsIContent * aParent=0x05688bd0, nsIDocument * aDocument=0x04cc1150, nsIDOMNode * * aReturn=0x0012eb3c)  Line 3433 + 0x13 bytes	C++
 	gklayout.dll!nsGenericElement::RemoveChild(nsIDOMNode * aOldChild=0x02026414, nsIDOMNode * * aReturn=0x0012eb3c)  Line 2999 + 0x1a bytes	C++
 	gklayout.dll!nsHTMLTableElement::RemoveChild(nsIDOMNode * oldChild=0x02026414, nsIDOMNode * * _retval=0x0012eb3c)  Line 72 + 0x14 bytes	C++
 	xpcom_core.dll!NS_InvokeByIndex_P(nsISupports * that=0x00000011, unsigned int methodIndex=2, unsigned int paramCount=1239852, nsXPTCVariant * params=0x0012e9f8)  Line 102	C++
etc...

Also, what is strange is, that the testcase has a scrollbar that indicates the page is very large. I don't think that is expected.

Comment 1

[Bernd]

Attachment: patch

I have seen this on breakpad before but I couldn't imagine how this goes wrong. Thanks for the test case (is this a way to do js  during printing?). And many thanks to Chris Karnaze for the assert that make the patch a nobrainer.

Comment 2

[Bernd]

This is a regression that I introduced in the Synchronize Cellmap code when I wrote it initially in bug 339130. So this should also crash on previous branches (1.8.0 and 1.8.1)

Comment 3

[Boris Zbarsky [:bzbarsky]]

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

Yech.  We really need to fix the cellmap stuff when splitting happens... :(

Comment 4

[Boris Zbarsky [:bzbarsky]]

And yeah, the fact that this timeout seems to run during printing seems odd.  If it does, can you please file a separate bug so we stop doing that?  That would be a pretty major issue, imo.

Comment 5

[Martijn Wargers (dead)]

I filed bug 424377 the "timers running while printing" issue.

Comment 6

[Mike Schroepfer]

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

Let's land this + the crashtest.

Comment 7

[Bernd]

I did land the patch but I am reluctant to land the crashtest as it still asserts ###!!! ASSERTION: data loss - incomplete row needed more height than available,
on top of page: 'rowMetrics.height <= rowReflowState.availableHeight', file

I would not like to own a active assert  on the crash tests.

Comment 8

[Damon Sicore (:damons)]

Can we close this out?  Going ahead and +'ing this to keep the record straight.

Comment 9

[Martijn Wargers (dead)]

Ok, I filed bug 424434 for the very large height issue.
And I added a comment in bug 362084 for the assertion on print preview.
So marking this bug FIXED.

Comment 10

[Bernd]

Attachment: branch patch (1.8)

Comment 11

[Bernd]

Comment on attachment 311119 [details] [diff] [review]
branch patch (1.8)

this should go on branches after some baking on trunk

Comment 12

[Daniel Veditz [:dveditz]]

Comment on attachment 311119 [details] [diff] [review]
branch patch (1.8)

approved for 1.8.1.14, a=dveditz for release-drivers

Comment 13

[Carsten Book [:Tomcat]]

verified fixed for trunk on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008032614 Minefield/3.0b5pre ID:2008032614 with the testcase.

No Crash on testcase -> Verified fixed

Comment 14

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

Verified for 1.8.1.15 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.15pre) Gecko/2008061203 BonEcho/2.0.0.15pre.