Title: [227493] releases/WebKitGTK/webkit-2.18/Source/WebCore
Revision
227493
Author
carlo...@webkit.org
Date
2018-01-24 01:37:27 -0800 (Wed, 24 Jan 2018)

Log Message

Merge r226457 - 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 <sabouhall...@apple.com> 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: releases/WebKitGTK/webkit-2.18/Source/WebCore/ChangeLog (227492 => 227493)


--- releases/WebKitGTK/webkit-2.18/Source/WebCore/ChangeLog	2018-01-24 09:37:21 UTC (rev 227492)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/ChangeLog	2018-01-24 09:37:27 UTC (rev 227493)
@@ -1,3 +1,24 @@
+2018-01-05  Said Abou-Hallawa  <sabouhall...@apple.com>
+
+        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):
+
 2017-11-03  Daniel Bates  <daba...@apple.com>
 
         Invalidate node list when associated form control element is removed

Modified: releases/WebKitGTK/webkit-2.18/Source/WebCore/svg/SVGAnimatedTypeAnimator.h (227492 => 227493)


--- releases/WebKitGTK/webkit-2.18/Source/WebCore/svg/SVGAnimatedTypeAnimator.h	2018-01-24 09:37:21 UTC (rev 227492)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/svg/SVGAnimatedTypeAnimator.h	2018-01-24 09:37:27 UTC (rev 227493)
@@ -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: releases/WebKitGTK/webkit-2.18/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h (227492 => 227493)


--- releases/WebKitGTK/webkit-2.18/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h	2018-01-24 09:37:21 UTC (rev 227492)
+++ releases/WebKitGTK/webkit-2.18/Source/WebCore/svg/properties/SVGAnimatedListPropertyTearOff.h	2018-01-24 09:37:27 UTC (rev 227493)
@@ -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
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to