Closed Bug 363370 Opened 14 years ago Closed 14 years ago
Crash [@ ns
TArray _base::Shift Data] with testcase that appends table cells
See upcoming testcase, which crashes on a 2006-12-10 build. It doesn't crash in a 2006-12-06 build, so very likely a regression from the reflow branch landing. Talkback ID: TB27142781X MSVCR80.dll + 0x150aa (0x781450aa) nsTArray_base::ShiftData [mozilla\xpcom\build\nstarray.cpp, line 176] nsTArray_base::InsertSlotsAt [mozilla\xpcom\build\nstarray.cpp, line 196] nsTArray<nsTPtrArray<CellData> >::InsertElementsAt<int> [mozilla\dist\include\xpcom\nstarray.h, line 641] nsCellMap::Grow [mozilla\layout\tables\nscellmap.cpp, line 1235] nsCellMap::ExpandWithRows [mozilla\layout\tables\nscellmap.cpp, line 1696] nsCellMap::InsertRows [mozilla\layout\tables\nscellmap.cpp, line 1284] nsTableCellMap::InsertRows [mozilla\layout\tables\nscellmap.cpp, line 558] nsTableFrame::InsertRows [mozilla\layout\tables\nstableframe.cpp, line 1078] nsTableFrame::AppendRows [mozilla\layout\tables\nstableframe.cpp, line 1044] nsCSSFrameConstructor::AppendFrames [mozilla\layout\base\nscssframeconstructor.cpp, line 8149] nsCSSFrameConstructor::ContentAppended [mozilla\layout\base\nscssframeconstructor.cpp, line 8878]
Its unlikely that is a reflow branch issue, its probably a regression from bug 356335. Looks like we need a complete run of bug 339128 again :-(
Summary: [reflow branch] Crash [@ nsTArray_base::ShiftData] with testcase that appends table cells → Crash [@ nsTArray_base::ShiftData] with testcase that appends table cells
So the issue here is that mRows is empty (has length 0) and mRowCount is 1. Then nsTableCellMap::InsertRows is called with aFirstRowIndex=3. This calls nsCellMap::InsertRows with aFirstRowIndex=3. Since nsCellMap::InsertRows grows mRows based on mRowCount, not on mRows.Length(), we run into issues... Should nsCellMap::InsertRows just use mRows.Length()?
Both RebuildConsideringRows and ExpandWithRows have issues with the growing. We got more picky with the cellmap changes
Assignee: nobody → bernd_mozilla
We could just go ahead and condition all appends/inserts to arrays to grow first as needed if we want... that should make things safe.
I will rename mRowCount into mContentRowCount so that the mess that is written in those functions becomes obvious. I should have done so for a long time. Further I propose to put an assert at http://lxr.mozilla.org/seamonkey/source/layout/tables/nsCellMap.cpp#1234 that startRowIndex <= mRows.Length(); and after that to put a PR_MIN(startRowIndex,mRows.Length()) to prevent these conditions to crash.
Comment on attachment 249086 [details] [diff] [review] patch >Index: nsCellMap.cpp > nsCellMap::InsertRows(nsTableCellMap& aMap, > nsVoidArray& aRows, > PRInt32 aFirstRowIndex, ... >+ // update mContentRowCount, since non-empty rows will be added >+ mContentRowCount = PR_MAX(aFirstRowIndex, mContentRowCount); Why doesn't this depend on aRows.Count()? I guess we'll increment for those rows when we actually expand, so this is equivalent to what the old code did, eh? > nsCellMap::ExpandWithRows(nsTableCellMap& aMap, >+ NS_ASSERTION(PRUint32(startRowIndex) <= mRows.Length(), "caller should have grown cellmap before"); >+ >+ Take out one of the blank lines? >@@ -1750,17 +1758,21 @@ void nsCellMap::ExpandWithCells(nsTableC This hunk is the fix for bug 363239, right? So needs to get sr from roc there... I can't really sr it, since I wrote it. ;) r+sr=bzbarsky with that. Again, thanks for handling this...
fix checked in
Assignee: bernd_mozilla → nobody
fix checked in
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Crash Signature: [@ nsTArray_base::ShiftData]
You need to log in before you can comment on or make changes to this bug.