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

VERIFIED FIXED in mozilla2.0b3

Status

()

P1
critical
VERIFIED FIXED
8 years ago
4 years ago

People

(Reporter: martijn.martijn, Assigned: dbaron)

Tracking

({crash, testcase})

Trunk
mozilla2.0b3
crash, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(blocking2.0 final+)

Details

(crash signature)

Attachments

(5 attachments)

(Reporter)

Description

8 years ago
Created attachment 460363 [details]
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
Created attachment 460373 [details]
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)
(Assignee)

Updated

8 years ago
blocking2.0: --- → final+
Created attachment 460403 [details]
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.
(Assignee)

Updated

8 years ago
Blocks: 578530
(Assignee)

Updated

8 years ago
Assignee: nobody → dbaron
(Assignee)

Updated

8 years ago
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.
Created attachment 461011 [details] [diff] [review]
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...
Attachment #461011 - Flags: review?(bzbarsky)
Created attachment 461012 [details] [diff] [review]
and while I'm here
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.
http://hg.mozilla.org/mozilla-central/rev/7e1bcdcb84a8
http://hg.mozilla.org/mozilla-central/rev/bb91308af252
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Priority: -- → P1
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b3
(Assignee)

Updated

8 years ago
Duplicate of this bug: 576980
(Assignee)

Updated

8 years ago
Flags: in-testsuite?
(Reporter)

Comment 17

8 years ago
Verified fixed, using:
Mozilla/5.0 (Windows; Windows NT 6.1; rv:2.0b3pre) Gecko/20100729 Minefield/4.0b3pre
Status: RESOLVED → VERIFIED

Updated

8 years ago
Duplicate of this bug: 582958
(Assignee)

Updated

8 years ago
Duplicate of this bug: 578530
(Assignee)

Updated

8 years ago
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.