675470 - Interpolating between already-interpolated transforms crashes Firefox

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Mathieu Merdy]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:8.0a1) Gecko/20110730 Firefox/8.0a1
Build ID: 20110730030836

Steps to reproduce:

I was playing with css transition property in javascript and at some point I had to make two 2s transform with a 1s delay between each. Each transition cleans up the style it modifies once finished. At some point, I get the value of the element.style -moz-transform property, split it into an array, splice some part out of it and this is where the browser crashes.

See http://jsfiddle.net/xSMmW/ (may crash your Firefox Nightly), http://pastebin.com/KpzwbewZ or attached file for code, line 33 being the trigger.

OS: Mac OS 10.6.8
windows and linux untested

Crashes on:
Firefox Nightly 8.0a1 (2011-07-30)

Do not crash on:
Firefox 5.0.1
Firefox Aurora 7.0a2 (2011-07-30)




Actual results:

The browser failed to remove the string from the array and crashes *every time* the code is executed.


Expected results:

It should have removed the string from the array.

Comment 1

[Kevin Brosnan [Ex-Mozilla]]

Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:8.0a1) Gecko/20110729 Firefox/8.0a1

Comment 2

[Kevin Brosnan [Ex-Mozilla]]

bp-3f16f131-4d66-4876-a6af-8414d2110730

0 	xul.dll 	nsStyleTransformMatrix::ReadTransforms 	layout/style/nsStyleTransformMatrix.cpp:519
1 	xul.dll 	nsNativeTheme::GetContentState 	widget/src/xpwidgets/nsNativeTheme.cpp:130
2 	xul.dll 	SearchTable 	obj-firefox/xpcom/build/pldhash.c:472
3 	xul.dll 	PL_DHashTableOperate 	obj-firefox/xpcom/build/pldhash.c:625
4 	xul.dll 	nsStyleTransformMatrix::ProcessInterpolateMatrix 	layout/style/nsStyleTransformMatrix.cpp:194
5 	xul.dll 	nsCSSKeywords::LookupKeyword 	layout/style/nsCSSKeywords.cpp:111
6 	xul.dll 	nsContainerFrame::BuildDisplayListForNonBlockChildren 	layout/generic/nsContainerFrame.cpp:370
7 	xul.dll 	nsStyleTransformMatrix::TransformFunctionOf 	layout/style/nsStyleTransformMatrix.cpp:442
8 	xul.dll 	nsStyleTransformMatrix::MatrixForTransformFunction 	
9 	xul.dll 	nsStyleTransformMatrix::ReadTransforms 	layout/style/nsStyleTransformMatrix.cpp:519
10 	xul.dll 	GetDeltaToMozTransformOrigin 	
11 	xul.dll 	nsDisplayTransform::GetResultingTransformMatrix 	layout/base/nsDisplayList.cpp:2387
12 	xul.dll 	nsDisplayTransform::UntransformRect 	layout/base/nsDisplayList.cpp:2693
13 	xul.dll 	`vector destructor iterator' 	
14 	xul.dll 	DisplayLine 	layout/generic/nsBlockFrame.cpp:6226
15 	xul.dll 	nsIFrame::BuildDisplayListForStackingContext

Comment 3

[Thomas Ahlblom]

Reproduced:
Mozilla/5.0 (X11; Linux x86_64; rv:8.0a1) Gecko/20110730 Firefox/8.0a1


Last good nightly: 2011-07-25
First bad nightly: 2011-07-26

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=4f38df646524&tochange=58c04967ac5b


bp-48df1533-1883-451f-8c24-16b802110730
bp-c0a98e01-0ccb-4f76-a7c1-cd3c02110730
bp-ca1d5eae-9364-4aa9-973c-3d5b12110730

Comment 4

[Thomas Ahlblom]

Local track down:

Due to skipped revisions, the first bad revision could be any of:
changeset:   73261:ed32cfcfd3f0
user:        Hernan Rodriguez Colmeiro <[e-mail]>
date:        Fri Jul 22 15:15:12 2011 -0700
summary:     Bug 564667: Allow bootstrapped add-ons to have chrome URLs. r=dtownsend, sr=bsmedberg

changeset:   73262:6c423d80fe27
user:        Luke Wagner <[e-mail]>
date:        Fri Jul 22 15:22:05 2011 -0700
summary:     Bug 672026 - JSObject::principals should return the compartment's principals if there is no object-principals-finder (r=mrbkap)

changeset:   73263:7e16ec834b15
user:        Matt Woodrow <[e-mail]>
date:        Sat Jul 23 10:28:07 2011 +1200
summary:     Bug 505115 - Part 3 - Convert nsStyleTransformMatrix to be backed by a 4x4 matrix. r=dbaron

changeset:   73264:92bd75756f43
user:        Matt Woodrow <[e-mail]>
date:        Sat Jul 23 10:28:33 2011 +1200
summary:     Bug 505115 - Part 4 - Add a lot of new functionality to gfx3DMatrix. r=jrmuizel

changeset:   73265:89f90f9fac80
user:        Matt Woodrow <[e-mail]>
date:        Sat Jul 23 10:28:51 2011 +1200
summary:     Bug 505115 - Part 5 - Use gfx3DMatrix in layout. r=roc

changeset:   73266:0a532134fdd6
user:        Matt Woodrow <[e-mail]>
date:        Sat Jul 23 10:29:04 2011 +1200
summary:     Bug 673572 - Temporarily disable failing test for bug 568683 on mac. r=roc

changeset:   73267:0017163dc003
user:        Ehsan Akhgari <[e-mail]>
date:        Fri Jul 22 19:02:47 2011 -0400
summary:     Backout changeset ed32cfcfd3f0 (bug 564667) because it breaks the build

Comment 5

[Matt Woodrow (:mattwoodrow)]

This crash happens because we are getting the pseudo-transform function eCSSKeyword_interpolatematrix passed into nsStyleAnimation.cpp:AddTransformLists.

This should only ever be created as the result of interpolating two specified transforms. How does this end up as an input for another interpolation? Is this expected behaviour?

We can probably just pass these through the same code path as eCSSKeyword_matrix if necessary.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #5)
> This should only ever be created as the result of interpolating two
> specified transforms. How does this end up as an input for another
> interpolation? Is this expected behaviour?

It's expected behavior if a CSS transition is reversed halfway through, I think.

Comment 7

[Mathieu Merdy]

To be more specific if that helps, a timelime of the animation (speaking only about the transform) would look like :

t = 0s:
    style.setProperty('-moz-transition-property', '-moz-transform', '');
    style.setProperty('-moz-transition-duration', '2s', '');
    style.setProperty('-moz-transform', 'translate(-100px)', '');

t = 1s:
    style.setProperty('-moz-transition-property', '-moz-transform', '');
    style.setProperty('-moz-transition-duration', '2s', '');
    style.setProperty('-moz-transform', 'translate(-100px) rotate(-15deg)', '');

t = 2s:
    /* now that I think of it, the -moz-transition-property and -moz-transition-duration are set to an empty string (I think) instead of being kept until the end of the rotation or removed via style.removeProperty. */
    style.setProperty('-moz-transition-property', '', '');
    style.setProperty('-moz-transition-duration', '', '');
    style.setProperty('-moz-transform', 'rotate(-15deg)', '');

t = 3s:
    /* Not sure what happends here now, but it crashes at some point. I split and splice the values of the 3 css properties, 2 of them being already empty. array.remove is has seen on http://pastebin.com/KpzwbewZ */
    style.setProperty('-moz-transition-property', (style.getPropertyValue('-moz-transition-property') || '').split(' ').remove('').remove('-moz-transform').join(' '), '');
    style.setProperty('-moz-transition-duration', (style.getPropertyValue('-moz-transition-duration') || '').split(' ').remove('').remove('2s').join(' '), '');
    style.setProperty('-moz-transform', (style.getPropertyValue('-moz-transform') || '').split(' ').remove('').remove('rotate(-15deg)').join(' '), '');

Comment 8

[Matt Woodrow (:mattwoodrow)]

Attachment: Handle eCSSKeyword_interpolatematrix in AddTransformLists

Comment 9

[Boris Zbarsky [:bzbarsky]]

Matt, do we need this on aurora or beta?

Comment 10

[Matt Woodrow (:mattwoodrow)]

No, this only landed on central last week

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 549695 [details] [diff] [review]
Handle eCSSKeyword_interpolatematrix in AddTransformLists

r=dbaron

Comment 12

[Matt Woodrow (:mattwoodrow)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/92fb925e1735

Comment 13

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/92fb925e1735

Comment 14

[Ioana (away)]

Verified as fixed on:
Mozilla/5.0 (Windows NT 5.1; rv:9.0) Gecko/20100101 Firefox/9.0 (20111206234556)
Mozilla/5.0 (Windows NT 5.1; rv:10.0a2) Gecko/20111207 Firefox/10.0a2
Mozilla/5.0 (Windows NT 5.1; rv:11.0a1) Gecko/20111208 Firefox/11.0a1

Mozilla/5.0 (Windows NT 6.1; rv:9.0) Gecko/20100101 Firefox/9.0 (20111206234556)
Mozilla/5.0 (Windows NT 6.1; rv:10.0a2) Gecko/20111207 Firefox/10.0a2
Mozilla/5.0 (Windows NT 6.1; rv:11.0a1) Gecko/20111208 Firefox/11.0a1

Mozilla/5.0 (X11; Linux i686; rv:9.0) Gecko/20100101 Firefox/9.0 (20111206234556)
Mozilla/5.0 (X11; Linux i686; rv:10.0a2) Gecko/20111208 Firefox/10.0a2
Mozilla/5.0 (X11; Linux i686; rv:11.0a1) Gecko/20111208 Firefox/11.0a1

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0) Gecko/20100101 Firefox/9.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0a2) Gecko/20111208 Firefox/10.0a2
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0a1) Gecko/20111207 Firefox/11.0a1

I loaded http://jsfiddle.net/xSMmW/ in all the builds several times and everything worked fine. There was no crash.

I also verified the crash stats and I didn't find any crashes with both signatures    from the Crash Signature section.