Title: [226457] trunk/Source/WebCore
- Revision
- 226457
- Author
- [email protected]
- Date
- 2018-01-05 12:10:44 -0800 (Fri, 05 Jan 2018)
Log Message
SVGAnimatedListPropertyTearOff::synchronizeWrappersIfNeeded() should do nothing if the property is not animating
https://bugs.webkit.org/show_bug.cgi?id=181316
<rdar://problem/36147545>
Patch by Said Abou-Hallawa <[email protected]> on 2018-01-05
Reviewed by Simon Fraser.
This is a speculative change to fix a crash which appeared after r226065.
The crash is very intermittent and sometimes very hard to reproduce. The
basic code analysis did not show how this crash can even happen.
* svg/SVGAnimatedTypeAnimator.h:
(WebCore::SVGAnimatedTypeAnimator::resetFromBaseValues): For SVG property
with two values, e.g. <SVGAngleValue, SVGMarkerOrientType>, we need to
detach the wrappers of the animated property if the animated values are
going to change. This is similar to what we did in resetFromBaseValue().
* svg/properties/SVGAnimatedListPropertyTearOff.h:
(WebCore::SVGAnimatedListPropertyTearOff::synchronizeWrappersIfNeeded):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (226456 => 226457)
--- trunk/Source/WebCore/ChangeLog 2018-01-05 19:51:49 UTC (rev 226456)
+++ trunk/Source/WebCore/ChangeLog 2018-01-05 20:10:44 UTC (rev 226457)
@@ -1,3 +1,24 @@
+2018-01-05 Said Abou-Hallawa <[email protected]>
+
+ SVGAnimatedListPropertyTearOff::synchronizeWrappersIfNeeded() should do nothing if the property is not animating
+ https://bugs.webkit.org/show_bug.cgi?id=181316
+ <rdar://problem/36147545>
+
+ Reviewed by Simon Fraser.
+
+ This is a speculative change to fix a crash which appeared after r226065.
+ The crash is very intermittent and sometimes very hard to reproduce. The
+ basic code analysis did not show how this crash can even happen.
+
+ * svg/SVGAnimatedTypeAnimator.h:
+ (WebCore::SVGAnimatedTypeAnimator::resetFromBaseValues): For SVG property
+ with two values, e.g. <SVGAngleValue, SVGMarkerOrientType>, we need to
+ detach the wrappers of the animated property if the animated values are
+ going to change. This is similar to what we did in resetFromBaseValue().
+
+ * svg/properties/SVGAnimatedListPropertyTearOff.h:
+ (WebCore::SVGAnimatedListPropertyTearOff::synchronizeWrappersIfNeeded):
+
2018-01-05 Matt Lewis <[email protected]>
Unreviewed, rolling out r226401.
Modified: trunk/Source/WebCore/svg/SVGAnimatedTypeAnimator.h (226456 => 226457)
--- trunk/Source/WebCore/svg/SVGAnimatedTypeAnimator.h 2018-01-05 19:51:49 UTC (rev 226456)
+++ trunk/Source/WebCore/svg/SVGAnimatedTypeAnimator.h 2018-01-05 20:10:44 UTC (rev 226457)
@@ -127,10 +127,14 @@
{
ASSERT(animatedTypes[0].properties.size() == 2);
ASSERT(type.type() == m_type);
+ auto* firstProperty = castAnimatedPropertyToActualType<AnimValType1>(animatedTypes[0].properties[0].get());
+ auto* secondProperty = castAnimatedPropertyToActualType<AnimValType2>(animatedTypes[0].properties[1].get());
+ firstProperty->synchronizeWrappersIfNeeded();
+ secondProperty->synchronizeWrappersIfNeeded();
std::pair<typename AnimValType1::ContentType, typename AnimValType2::ContentType>& animatedTypeValue = (type.*getter)();
- animatedTypeValue.first = castAnimatedPropertyToActualType<AnimValType1>(animatedTypes[0].properties[0].get())->currentBaseValue();
- animatedTypeValue.second = castAnimatedPropertyToActualType<AnimValType2>(animatedTypes[0].properties[1].get())->currentBaseValue();
+ animatedTypeValue.first = firstProperty->currentBaseValue();
+ animatedTypeValue.second = secondProperty->currentBaseValue();
executeAction<AnimValType1>(StartAnimationAction, animatedTypes, 0, &animatedTypeValue.first);
executeAction<AnimValType2>(StartAnimationAction, animatedTypes, 1, &animatedTypeValue.second);
Modified: trunk/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h (226456 => 226457)
--- trunk/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h 2018-01-05 19:51:49 UTC (rev 226456)
+++ trunk/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h 2018-01-05 20:10:44 UTC (rev 226457)
@@ -143,7 +143,11 @@
void synchronizeWrappersIfNeeded()
{
- ASSERT(isAnimating());
+ if (!isAnimating()) {
+ // This should never happen, but we've seen it in the field. Please comment in bug #181316 if you hit this.
+ ASSERT_NOT_REACHED();
+ return;
+ }
// Eventually the wrapper list needs synchronization because any SVGAnimateLengthList::calculateAnimatedValue() call may
// mutate the length of our values() list, and thus the wrapper() cache needs synchronization, to have the same size.
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
