Testcase from bug 362275 crashes [@ nsCellMapColumnIterator::GetNextFrame] in latest trunk build

VERIFIED FIXED

Status

()

Core
Layout: Tables
--
critical
VERIFIED FIXED
12 years ago
6 years ago

People

(Reporter: Martijn Wargers (zombie), Assigned: bz)

Tracking

({crash, regression, testcase})

Trunk
x86
Windows XP
crash, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(crash signature, URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

12 years ago
https://bugzilla.mozilla.org/attachment.cgi?id=246977 is crashing on load with 2007-01-22 trunk build. It doesn't crash with 2007-01-21 trunk build, so I think a regression from bug 366892 or bug 352461.
Talkback doesn't come up with a useful stacktrace right now.

Also https://bugzilla.mozilla.org/attachment.cgi?id=247409 is crashing (testcase from bug 362724).
Created attachment 252355 [details] [diff] [review]
Proposed fix

Fixes this bug and bug 367755.

The patch has (in addition to some assertion stuff) the following changes:

1)  We need to skip over nsCellMaps for rowgroups with no rows when advancing
    rowgroups and when initializing the iterator.
2)  We need to report the correct row to the caller.  When multiple rowgroups are
    involved, reporting the row in the current rowgroup is wrong.  So we need to
    accumulate our total current row.
3)  When advancing by a rowspan past the end of a rowgroup, we need to only
    advance mRow to the end of the non-dead rows in the rowgroup.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #252355 - Flags: superreview?(roc)
Attachment #252355 - Flags: review?(bernd_mozilla)
Duplicate of this bug: 367785
Attachment #252355 - Flags: superreview?(roc) → superreview+

Comment 3

12 years ago
Comment on attachment 252355 [details] [diff] [review]
Proposed fix

Arg sorry for the delay, I thought that you will revise the patch after our talk at irc.

In short there are no dead rowspans, we have effective rowspans that might be short than the rowspan if the number of content based rows is not enough. This happens pretty frequently during incr. reflow.
There are however dead rows due to rowspans
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/tables/nsCellMap.h&rev=3.62&mark=617-621#610


// mRow is the row in the entire nsTableCellMap where we are right now.
I would prefer
mRow is the content based row.... 

please rename
mCurMapRowCount --> mCurMapContentRowCount

+ // It's possible that we're dealing with a dead rowspan here.

what are the other possibilities, either the comment is wrong and we are dealing here with dead rows unconditional or the other possibilities needs to be handled

r+ on the code, the comments however need to be improved before checkin, no need for additional review, I am sure that you got the idea when discussed it.
Attachment #252355 - Flags: review?(bernd_mozilla) → review+
(Reporter)

Updated

12 years ago
Summary: Testcase from bug 362275 crashes in latest trunk build → Testcase from bug 362275 crashes [@ nsCellMapColumnIterator::GetNextFrame] in latest trunk build
(Reporter)

Updated

12 years ago
Blocks: 368013
Created attachment 252654 [details] [diff] [review]
With updated comments

Sorry about the miscommunication there...
Attachment #252355 - Attachment is obsolete: true
Created attachment 252655 [details] [diff] [review]
Er, and rename mCurMapRowCount to mCurMapContentRowCount

To be applied on top of previous patch.
Fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Duplicate of this bug: 367755
Duplicate of this bug: 368013
Duplicate of this bug: 368165
No longer blocks: 368013
No longer blocks: 367755
Duplicate of this bug: 368169
(Reporter)

Comment 11

12 years ago
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070131 Minefield/3.0a2pre
Status: RESOLVED → VERIFIED
Crash Signature: [@ nsCellMapColumnIterator::GetNextFrame]
Crash test:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5df73ae6fc5
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.