Last Comment Bug 424291 - Crash [@ nsCellMap::SetNextSibling] while trying to print
: Crash [@ nsCellMap::SetNextSibling] while trying to print
: crash, regression, testcase, verified1.8.1.15
Product: Core
Classification: Components
Component: Layout: Tables (show other bugs)
: Trunk
: x86 Windows XP
P2 critical (vote)
: ---
Assigned To: Bernd
Depends on:
Blocks: 339130
  Show dependency treegraph
Reported: 2008-03-20 21:49 PDT by Martijn Wargers [:mwargers]
Modified: 2011-06-09 14:58 PDT (History)
6 users (show)
dsicore: blocking1.9+
martijn.martijn: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (447 bytes, text/html)
2008-03-20 21:49 PDT, Martijn Wargers [:mwargers]
no flags Details
patch (1008 bytes, patch)
2008-03-21 01:01 PDT, Bernd
bzbarsky: review+
bzbarsky: superreview+
mtschrep: approval1.9b5+
Details | Diff | Splinter Review
branch patch (1.8) (915 bytes, patch)
2008-03-22 00:39 PDT, Bernd
bzbarsky: review+
dveditz: approval1.8.1.15+
Details | Diff | Splinter Review

Description User image Martijn Wargers [:mwargers] 2008-03-20 21:49:10 PDT
Created attachment 310925 [details]

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++

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 User image Bernd 2008-03-21 01:01:50 PDT
Created attachment 310949 [details] [diff] [review]

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 User image Bernd 2008-03-21 01:06:10 PDT
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 User image Boris Zbarsky [:bz] (still a bit busy) 2008-03-21 09:32:33 PDT
Comment on attachment 310949 [details] [diff] [review]

Yech.  We really need to fix the cellmap stuff when splitting happens... :(
Comment 4 User image Boris Zbarsky [:bz] (still a bit busy) 2008-03-21 09:34:24 PDT
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 User image Martijn Wargers [:mwargers] 2008-03-21 09:52:16 PDT
I filed bug 424377 the "timers running while printing" issue.
Comment 6 User image Mike Schroepfer 2008-03-21 13:32:14 PDT
Comment on attachment 310949 [details] [diff] [review]

Let's land this + the crashtest.
Comment 7 User image Bernd 2008-03-21 14:03:09 PDT
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 User image Damon Sicore (:damons) 2008-03-21 14:38:17 PDT
Can we close this out?  Going ahead and +'ing this to keep the record straight.
Comment 9 User image Martijn Wargers [:mwargers] 2008-03-21 15:03:28 PDT
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 User image Bernd 2008-03-22 00:39:10 PDT
Created attachment 311119 [details] [diff] [review]
branch patch (1.8)
Comment 11 User image Bernd 2008-03-22 13:03:28 PDT
Comment on attachment 311119 [details] [diff] [review]
branch patch (1.8)

this should go on branches after some baking on trunk
Comment 12 User image Daniel Veditz [:dveditz] 2008-03-26 11:42:43 PDT
Comment on attachment 311119 [details] [diff] [review]
branch patch (1.8)

approved for, a=dveditz for release-drivers
Comment 13 User image Carsten Book [:Tomcat] 2008-03-26 13:51:28 PDT
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 User image Al Billings [:abillings] 2008-06-12 18:11:14 PDT
Verified for with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/2008061203 BonEcho/

Note You need to log in before you can comment on or make changes to this bug.