582111 - Crash [@ nsCSSValue::GetStringValue], [@ nsStyleTransformMatrix::TransformFunctionOf] with -moz-transition and -moz-transform

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect, P1)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, which crashes current trunk build, after you've pressed ctrl-+ to change the zoom level.

http://crash-stats.mozilla.com/report/index/9c93c815-1608-45a3-8d29-bddf22100726
0  	xul.dll  	nsCSSValue::GetStringValue  	 layout/style/nsCSSValue.h:278
1 	xul.dll 	nsStyleTransformMatrix::TransformFunctionOf 	layout/style/nsStyleTransformMatrix.cpp:550
2 	xul.dll 	nsStyleAnimation::AddWeighted 	
3 	xul.dll 	nsTransitionManager::ConsiderStartingTransition 	layout/style/nsTransitionManager.cpp:624
4 	xul.dll 	nsTransitionManager::StyleContextChanged 	
5 	xul.dll 	nsTArray_base::EnsureCapacity 	obj-firefox/xpcom/build/nsTArray.cpp:76
6 	xul.dll 	nsTArray<nsIFrame*>::AppendElements<nsIFrame*> 	obj-firefox/dist/include/nsTArray.h:657
7 	xul.dll 	nsFrameManager::ReResolveStyleContext 	layout/base/nsFrameManager.cpp:1229

Comment 1

[Daniel Holbert [:dholbert]]

Confirmed on Linux, though my crash is one stack-level up from Martijn's (looks like that's just because nsCSSValue::GetStringValue gets inlined in my build).

bp-b5017a84-a9be-4f38-a899-933182100726
bp-501d0ce8-9fcd-4f37-ad15-673122100726
Mozilla/5.0 (X11; Linux x86_64; rv:2.0b3pre) Gecko/20100726 Minefield/4.0b3pre

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: backtrace

Here's the backtrace of the crash, in my x86_64 debug build.

Note: the "aStyleContext" in stacklevels 1-3 is "aOldStyleContext" from stacklevels 4-6.

Comment 3

[Daniel Holbert [:dholbert]]

(note that the debug build crashes in a different spot -- probably earlier -- than the opt build)

Comment 4

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

Attachment: valgrind output

Comment 5

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

I think this is related to the rule immutability violation by ElementTransitionsStyleRule.  I'm not sure if it's better to fully fix following rule immutability, do some ownership mangling, or stop trying to make style contexts avoid owning a clone of the transform list.

Comment 7

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

It turns out that preserving immutability is actually a code *simplification*, since I can share code between the transitions style rule and the covering style rule.  Patch coming soon, I hope.

Comment 8

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

Attachment: patch

I believe this should fix it, although I couldn't reproduce the crash reliably on any of the testcases, so I'm not 100% sure.  I'll try writing a more reliable testcase...

Comment 9

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

Attachment: and while I'm here

Comment 10

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

The basic idea of the main patch is that we want to store property-value pairs in the ElementTransitionsStyleRule.  But it turns out we already have a class that does exactly that - the CoverTransitionStartStyleRule.  So this patch renames CoverTransitionStartStyleRule to AnimValuesStyleRule (since it's a style rule representing property-nsStyleAnimation::Value pairs), removes ElementTransitionsStyleRule, and moves part of the code from ElementTransitionsStyleRule::MapRuleInfoInto into ElementTransitions::EnsureStyleRuleFor.

diff didn't do a particularly good job on it, though it's not horrible.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 461012 [details] [diff] [review]
and while I'm here

r=bzbarsky

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 461011 [details] [diff] [review]
patch

r=bzbarsky.  I wonder whether this helps with bug 582379.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Oh, and fix this, please?

>   // nsISupportsImplementation

Comment 14

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

(In reply to comment #12)
> r=bzbarsky.  I wonder whether this helps with bug 582379.

In fact, I had noticed that I couldn't reproduce that bug anymore.

Comment 15

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

http://hg.mozilla.org/mozilla-central/rev/7e1bcdcb84a8
http://hg.mozilla.org/mozilla-central/rev/bb91308af252

Comment 17

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Windows; Windows NT 6.1; rv:2.0b3pre) Gecko/20100729 Minefield/4.0b3pre

Comment 21

[Sebastian Zartner [:sebo]]

Just saw that crash when wanting to inspect a CSS containing an @counter-style rule within Firebug:

https://crash-stats.mozilla.com/report/index/9e27ce8f-5be3-4764-b6ed-a3b822140724

Sebastian

Comment 22

[Sebastian Zartner [:sebo]]

Sorry, missed that this bug is already closed. I created bug 1043181.

Sebastian