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

VERIFIED FIXED

Status

()

Core
Layout: Tables
P2
critical
VERIFIED FIXED
9 years ago
6 years ago

People

(Reporter: Martijn Wargers (dead), Assigned: Bernd)

Tracking

(4 keywords)

Trunk
x86
Windows XP
crash, regression, testcase, verified1.8.1.15
Points:
---
Bug Flags:
blocking1.9 +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(3 attachments)

(Reporter)

Description

9 years ago
Created attachment 310925 [details]
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.
(Assignee)

Comment 1

9 years ago
Created attachment 310949 [details] [diff] [review]
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.
Attachment #310949 - Flags: superreview?(bzbarsky)
Attachment #310949 - Flags: review?(bzbarsky)
(Assignee)

Updated

9 years ago
Attachment #310949 - Attachment is patch: true
(Assignee)

Comment 2

9 years ago
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)
Flags: blocking1.9?
Keywords: regression
(Assignee)

Updated

9 years ago
Assignee: nobody → bernd_mozilla
Comment on attachment 310949 [details] [diff] [review]
patch

Yech.  We really need to fix the cellmap stuff when splitting happens... :(
Attachment #310949 - Flags: superreview?(bzbarsky)
Attachment #310949 - Flags: superreview+
Attachment #310949 - Flags: review?(bzbarsky)
Attachment #310949 - Flags: review+
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.
(Reporter)

Comment 5

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

Comment 6

9 years ago
Comment on attachment 310949 [details] [diff] [review]
patch

Let's land this + the crashtest.
Attachment #310949 - Flags: approval1.9b5+
(Assignee)

Comment 7

9 years ago
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.
Can we close this out?  Going ahead and +'ing this to keep the record straight.
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2
(Reporter)

Comment 9

9 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
(Assignee)

Comment 10

9 years ago
Created attachment 311119 [details] [diff] [review]
branch patch (1.8)
Attachment #311119 - Flags: review?(bzbarsky)
Attachment #311119 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 11

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

this should go on branches after some baking on trunk
Attachment #311119 - Flags: approval1.8.1.14?
Blocks: 339130
Comment on attachment 311119 [details] [diff] [review]
branch patch (1.8)

approved for 1.8.1.14, a=dveditz for release-drivers
Attachment #311119 - Flags: approval1.8.1.14? → approval1.8.1.14+
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
Status: RESOLVED → VERIFIED
(Assignee)

Updated

9 years ago
Keywords: fixed1.8.1.14
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.
Keywords: fixed1.8.1.15 → verified1.8.1.15
Crash Signature: [@ nsCellMap::SetNextSibling]
You need to log in before you can comment on or make changes to this bug.