Closed Bug 339130 Opened 19 years ago Closed 18 years ago

Crash in [@ nsVoidArray::Count] called by nsCellMap::RebuildConsideringRows

Categories

(Core :: Layout: Tables, defect)

Product:
Core
Component:
Layout: Tables
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.8.1

People

(Reporter: MatsPalmgren_bugz, Assigned: bernd_mozilla)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [sg:critical] fixed rolled into 346980)

Crash Data

Attachments

(4 files, 1 obsolete file)

stack
(deleted), text/plain
Details
stack #2
(deleted), text/plain
Details
testcase
(deleted), text/html
Details
revised patch
(deleted), patch
bzbarsky
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review
Attached file stack (deleted) —
A few of the assertions leading up to the crash, the stack and some data.
Attached file stack #2 (deleted) —
The same call stack as above, but 'row' = 0xdadadada
Summary: Crash in [@ nsVoidArray::Count] → Crash in [@ nsVoidArray::Count] called by nsCellMap::RebuildConsideringRows
I'm about to attach a testcase that (on Mac) sometimes crashes [@ nsVoidArray::Count], sometimes crashes [@ nsCellMap::GetRowSpanForNewCell], and sometimes doesn't crash. It also triggers the following assertion: ###!!! ASSERTION: SetDataAt called with row index > num rows: 'PR_FALSE', file /Users/admin/trunk/mozilla/layout/tables/nsCellMap.cpp, line 2401
Blocks: 339131
Keywords: testcase
OS: Linux → All
Hardware: PC → All
Attached file testcase (deleted) —
<table id="table"> <thead></thead> <tfoot id="A"><tr><td>td</td></tr></tfoot> <tfoot id="B"><tr><td>td</td></tr></tfoot> <thead></thead> </table> produces the following cellmap ***** START TABLE CELL MAP DUMP ***** 02FEBF00 cols array orig/span-> 02FEBF000=2/0 cols in cache 1 ***** START GROUP CELL MAP DUMP ***** 02FF5C58 thead mapRowCount=0 tableRowCount=0 ***** END GROUP CELL MAP DUMP ***** ***** START GROUP CELL MAP DUMP ***** 02FF5CC0 tfoot mapRowCount=1 tableRowCount=1 row 0 : C0,0 C0,0=02FF51C4(0) ***** END GROUP CELL MAP DUMP ***** ***** START GROUP CELL MAP DUMP ***** 02FF5E38 thead mapRowCount=0 tableRowCount=0 ***** END GROUP CELL MAP DUMP ***** ***** START GROUP CELL MAP DUMP ***** 02FF5EA0 tfoot mapRowCount=1 tableRowCount=1 row 0 : C0,0 C0,0=02FF4EB4(0) ***** END GROUP CELL MAP DUMP ***** ***** END TABLE CELL MAP DUMP ***** ***END TABLE DUMP*** the dynamic case: function boom() { var table = document.getElementById("table"); table.insertBefore(document.createElement("thead"), document.getElementById("A")); } </script> </head> <body onload="boom();"> <table id="table"> <tfoot id="A"><tr><td>td</td></tr></tfoot> <tfoot id="B"><tr><td>td</td></tr></tfoot> <thead></thead> </table> produces the following cellmap: Please notice the difference between the second maps. ***** START TABLE CELL MAP DUMP ***** 02B91FD8 cols array orig/span-> 02B91FD80=2/0 cols in cache 1 ***** START GROUP CELL MAP DUMP ***** 03017028 thead mapRowCount=0 tableRowCount=0 ***** END GROUP CELL MAP DUMP ***** ***** START GROUP CELL MAP DUMP ***** 03004348 thead mapRowCount=0 tableRowCount=0 ***** END GROUP CELL MAP DUMP ***** ***** START GROUP CELL MAP DUMP ***** 030043B0 tfoot mapRowCount=1 tableRowCount=1 row 0 : C0,0 C0,0=03003374(0) ***** END GROUP CELL MAP DUMP ***** ***** START GROUP CELL MAP DUMP ***** 02B92190 tfoot mapRowCount=1 tableRowCount=1 row 0 : C0,0 C0,0=030CAA54(0) ***** END GROUP CELL MAP DUMP ***** ***** END TABLE CELL MAP DUMP ***** ***END TABLE DUMP*** as I stated in https://bugzilla.mozilla.org/show_bug.cgi?id=317554#c7 > Once the cellmap is out of sync with the row > group frames, its only a matter of dom transitions till we crash And this is exactly what is happening again here. The cure for this is to remove OrderRowGroups see https://bugzilla.mozilla.org/show_bug.cgi?id=328017#c6 This however implies a major surgery at table printing. To test this one needs to get the printing regression tests running again. They got removed from tree together with the viewer removal and need to be reimplemented in the layout debugger. Which is exactly the plan that I had to follow the next weeks.
Whiteboard: [sg:critical?]
I will develop the patch open inside bug 328017. The print rtest work is bug 339500 and bug 339521.
Depends on: 328017
Attached patch patch (obsolete) (deleted) — — Splinter Review
Assignee: nobody → bernd_mozilla
Status: NEW → ASSIGNED
Comment on attachment 224446 [details] [diff] [review] patch I did not go for the big solution. This fixes the synchronization between rowgroups and cellmaps. The sad news is that it does not fix any other of the stirtable crashes. Its rather frustrating. The good thing is due to the small size the patch is branchable.
Attachment #224446 - Flags: superreview?(bzbarsky)
Attachment #224446 - Flags: review?(bzbarsky)
aTableFrame.OrderRowGroups(orderedRowGroups, numRowGroups); - NS_ASSERTION(orderedRowGroups.Count() == (PRInt32) numRowGroups,"problem in OrderRowGroups"); the assertion is bogus as the table could have non rowgroup children which would then be after the rowgroups (see nsTableFrame::OrderRowGroups)
Attached patch revised patch (deleted) — — Splinter Review
Attachment #224446 - Attachment is obsolete: true
Attachment #224446 - Flags: superreview?(bzbarsky)
Attachment #224446 - Flags: review?(bzbarsky)
Bernd, do you still want review on the "revised patch"?
Comment on attachment 224723 [details] [diff] [review] revised patch Yes, I need a review for this. I am only a little bit scared by the low quality that the patches currently have where I ask for review. (see bug 339315 for another example). The second thing is that due to the various crashes its difficult to test that one does not introduce new crashes. Hopefully the patch for bug 339246 will help, but currently, whichever seed I take, the stirtable crashes within 250 steps :-( .
Attachment #224723 - Flags: superreview?(bzbarsky)
Attachment #224723 - Flags: review?(bzbarsky)
Comment on attachment 224723 [details] [diff] [review] revised patch >Index: nsCellMap.cpp >+nsTableCellMap::Synchronize(nsTableFrame* aTableFrame) >+ maps.Clear(); This isn't needed, right? Why do it? >+ aTableFrame->OrderRowGroups(orderedRowGroups, numRowGroups); If this fails due to OOM, is that an issue? >+ maps.AppendElement(map); If this fails due to OOM, is that an issue? r+sr=bzbarsky
Attachment #224723 - Flags: superreview?(bzbarsky)
Attachment #224723 - Flags: superreview+
Attachment #224723 - Flags: review?(bzbarsky)
Attachment #224723 - Flags: review+
fixed on trunk, the whole bunch of fixes for 339128 needs serious baking as the changed code is pretty old.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
No longer depends on: 328017
Resolution: --- → FIXED
Depends on: 342559
No longer depends on: 342559
This crash happens on the 1.8 branches also, nominating.
Flags: blocking1.8.1?
Flags: blocking1.8.0.6?
Whiteboard: [sg:critical?] → [sg:critical]
Flags: blocking1.8.1? → blocking1.8.1+
Flags: blocking1.8.0.6? → blocking1.8.0.6+
Blocks: 346980
Is this patch suitable for the branch? Can we get a branch-suitable patch up for approval?
Target Milestone: --- → mozilla1.8.1
I think the patch in bug 346980 is the branch-suitable version (and includes this fix). I'm not sure, though, and bernd is away. :(
fixed by the patch for bug 346980 on branch
Keywords: fixed1.8.1
the only bug that should block 1.8.0.7 is bug 346980 which is blocked by the regression 347725 asking for 1.8.0.7- on this
https://bugzilla.mozilla.org/attachment.cgi?id=223437&action=view ff2b2 windows, linux, macppc no crash; macppc shows 2 tds while windows/linux show 3. ff2b2 debug windows, linux no crash verified fixed 1.8
Keywords: fixed1.8.1 → verified1.8.1
Whiteboard: [sg:critical] → [sg:critical] fixed rolled into 346980
Moving to 1.8.0.8 at Bernd's request (regression worries)
Flags: blocking1.8.0.8+
Flags: blocking1.8.0.7-
Flags: blocking1.8.0.7+
fixed by the cellmap branch patch
Keywords: fixed1.8.0.7
https://bugzilla.mozilla.org/attachment.cgi?id=223437&action=view should not crash browser. Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.7pre) Gecko/20060829 Firefox/1.5.0.7pre verified 1.8.0.7
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.0.7 → verified1.8.0.7
Flags: blocking1.8.0.7- → blocking1.8.0.7+
Group: security
Flags: in-testsuite?
Depends on: 424291
crash test landed http://hg.mozilla.org/mozilla-central/rev/bc81c570de7e
Flags: in-testsuite? → in-testsuite+
Crash Signature: [@ nsVoidArray::Count]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: