387460 - [FIX]Crash [@ nsCellMapColumnIterator::GetNextFrame] with DOMAttrModified and changing rowspan

Status: VERIFIED FIXED

(Core :: Layout: Tables, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase which crashes current trunk build after 200ms.

This regressed between 2007-01-21 and 2007-01-22:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-01-21+04&maxdate=2007-01-22+09&cvsroot=%2Fcvsroothttp://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-01-21+04&maxdate=2007-01-22+09&cvsroot=%2Fcvsroot
I guess a regression from bug 352461.

http://crash-stats.mozilla.com/report/index/6a2e1307-2e54-11dc-92f0-001a4bd43ef6
0  	nsCellMapColumnIterator::GetNextFrame(int *,int *)
1 	BasicTableLayoutStrategy::ComputeColumnIntrinsicWidths(nsIRenderingContext *)
2 	BasicTableLayoutStrategy::ComputeIntrinsicWidths(nsIRenderingContext *)
3 	BasicTableLayoutStrategy::GetMinWidth(nsIRenderingContext *)
4 	nsTableFrame::TableShrinkWidthToFit(nsIRenderingContext *,int)
5 	nsTableFrame::ComputeAutoSize(nsIRenderingContext *,nsSize,int,nsSize,nsSize,nsSize,int)
6 	nsFrame::ComputeSize(nsIRenderingContext *,nsSize,int,nsSize,nsSize,nsSize,int)
7 	nsTableFrame::ComputeSize(nsIRenderingContext *,nsSize,int,nsSize,nsSize,nsSize,int)
8 	nsHTMLReflowState::InitConstraints(nsPresContext *,int,int,nsMargin const *,nsMargin const *)
9 	nsHTMLReflowState::Init(nsPresContext *,int,int,nsMargin const *,nsMargin const *)

Comment 1

[Bernd]

Bug 352461 is seldom the cause but rather exposing a underlying problem in the cellmap. Why does the testcase need  
window.addEventListener('DOMAttrModified', function(e){document.getElementById('a').focus();}, true); 
Is that a good old layout breakpoint? Or can it be replaced by one?

Comment 2

[Boris Zbarsky [:bzbarsky]]

Anything that triggers a layout flush in there will cause a crash.  document.body.offsetWidth does.

I should have a patch soon; doing some CVS archeology.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Trunk fix

This has been broken ever since mutation events landed.  Basically, we're firing the mutation event after changing the attr value but before calling AttributeChanged, so some data structures (in particular the cellmap) are not getting updated as they should before the mutation event runs arbitrary script.

This might well be exploitable if one tries hard enough...

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Branch patch

Comment 5

[Boris Zbarsky [:bzbarsky]]

Fixed on trunk.

Comment 6

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007071905 Minefield/3.0a7pre

Comment 7

[Daniel Veditz [:dveditz]]

Comment on attachment 271622 [details] [diff] [review]
Branch patch

approved for 1.8.1.7 and 1.8.0.14, a=dveditz for release-drivers

Comment 8

[Boris Zbarsky [:bzbarsky]]

Fixed on branches.

Comment 9

[Carsten Book [:Tomcat]]

verified fixed 1.8.1.7 using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.7pre) Gecko/2007083003 BonEcho/2.0.0.7pre and Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.7pre) Gecko/20070830 BonEcho/2.0.0.7pre ID:2007083003 and the testcase this bug.

No crash on testcase - adding verified keyword.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Filed bug 394418 on a similar issue.

Comment 11

[Al Billings [:abillings - ex-MoCo]]

I can't repro the crash with the released Firefox 1.5.0.12 (Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.12) Gecko/20070508 Firefox/1.5.0.12).

Because of this, I can't verify that the fix works in the current 1.8.0 branch builds. Is there an alternative test case?

Comment 12

[Al Billings [:abillings - ex-MoCo]]

It doesn't crash in Windows FF 1.5.0.12 either: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.12) Gecko/20070508 Firefox/1.5.0.12.

Comment 13

[Bob Clary [:bc] (inactive)]

crash test landed
http://hg.mozilla.org/mozilla-central/rev/9fa246f8f121