Closed Bug 582111 Opened 12 years ago Closed 12 years ago

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

Categories

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

defect

Tracking

()

VERIFIED FIXED
mozilla2.0b3
Tracking Status
blocking2.0 --- final+

People

(Reporter: martijn.martijn, Assigned: dbaron)

References

Details

(Keywords: crash, testcase)

Crash Data

Attachments

(5 files)

Attached file 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
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
Summary: Crash [@ nsCSSValue::GetStringValue] with -moz-transition and -moz-transform → Crash [@ nsCSSValue::GetStringValue], [@ nsStyleTransformMatrix::TransformFunctionOf] with -moz-transition and -moz-transform
OS: Windows 7 → All
Hardware: x86 → All
Blocks: 531344
Attached file 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.
(note that the debug build crashes in a different spot -- probably earlier -- than the opt build)
blocking2.0: --- → final+
Attached file valgrind output
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.
Blocks: 578530
Assignee: nobody → dbaron
Duplicate of this bug: 582564
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.
Attached patch patchSplinter Review
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...
Attachment #461011 - Flags: review?(bzbarsky)
Attachment #461012 - Flags: review?(bzbarsky)
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 on attachment 461012 [details] [diff] [review]
and while I'm here

r=bzbarsky
Attachment #461012 - Flags: review?(bzbarsky) → review+
Comment on attachment 461011 [details] [diff] [review]
patch

r=bzbarsky.  I wonder whether this helps with bug 582379.
Attachment #461011 - Flags: review?(bzbarsky) → review+
Oh, and fix this, please?

>   // nsISupportsImplementation
(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.
Blocks: 582379
http://hg.mozilla.org/mozilla-central/rev/7e1bcdcb84a8
http://hg.mozilla.org/mozilla-central/rev/bb91308af252
Status: NEW → RESOLVED
Closed: 12 years ago
Priority: -- → P1
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b3
Duplicate of this bug: 576980
Flags: in-testsuite?
Verified fixed, using:
Mozilla/5.0 (Windows; Windows NT 6.1; rv:2.0b3pre) Gecko/20100729 Minefield/4.0b3pre
Status: RESOLVED → VERIFIED
Duplicate of this bug: 582958
Duplicate of this bug: 578530
Duplicate of this bug: 585705
Crash Signature: [@ nsCSSValue::GetStringValue] [@ nsStyleTransformMatrix::TransformFunctionOf]
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
Sorry, missed that this bug is already closed. I created bug 1043181.

Sebastian
You need to log in before you can comment on or make changes to this bug.