Title: [250175] trunk
- Revision
- 250175
- Author
- commit-qu...@webkit.org
- Date
- 2019-09-20 18:24:45 -0700 (Fri, 20 Sep 2019)
Log Message
Assertion fires when animating a discrete property with values range and multiple animators
https://bugs.webkit.org/show_bug.cgi?id=201926
Patch by Said Abou-Hallawa <sabouhall...@apple.com> on 2019-09-20
Reviewed by Darin Adler.
Source/WebCore:
The first animator of a property is considered the result element. The
other animators will be just contributers to the first animator. For the
first animator and in SVGSMILElement::progress(), we call resetAnimatedType()
which creates m_animator in SVGAnimateElementBase::animator(). But for
the other animators we do not call resetAnimatedType(). So their m_animator
will stay null until they are used for the first time.
If SVGAnimationElement::startedActiveInterval() calls calculateToAtEndOfDurationValue()
for a discrete property this will have no effect and the call should be
ignored. So SVGAnimateElementBase::calculateToAtEndOfDurationValue()
should bail out early if isDiscreteAnimator() is true.
The bug is isDiscreteAnimator() will return false if the m_animator is
null even if the animated property is discrete, e.g. SVGAnimatedString.
The fix is to make isDiscreteAnimator() ensure m_animator is created.
Unrelated change:
Make most of the protected methods of SVGAnimateElementBase be private.
Test: svg/animations/multiple-discrete-values-animate.svg
* svg/SVGAnimateElementBase.cpp:
(WebCore::SVGAnimateElementBase::calculateFromAndByValues):
(WebCore::SVGAnimateElementBase::calculateToAtEndOfDurationValue):
LayoutTests:
Animate a discrete property, such as SVGAnimatedString. There should be
multiple animators and the range of animation has to be set by the 'values'
attribute.
* svg/animations/multiple-discrete-values-animate-expected.txt: Added.
* svg/animations/multiple-discrete-values-animate.svg: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (250174 => 250175)
--- trunk/LayoutTests/ChangeLog 2019-09-21 01:07:49 UTC (rev 250174)
+++ trunk/LayoutTests/ChangeLog 2019-09-21 01:24:45 UTC (rev 250175)
@@ -1,3 +1,17 @@
+2019-09-20 Said Abou-Hallawa <sabouhall...@apple.com>
+
+ Assertion fires when animating a discrete property with values range and multiple animators
+ https://bugs.webkit.org/show_bug.cgi?id=201926
+
+ Reviewed by Darin Adler.
+
+ Animate a discrete property, such as SVGAnimatedString. There should be
+ multiple animators and the range of animation has to be set by the 'values'
+ attribute.
+
+ * svg/animations/multiple-discrete-values-animate-expected.txt: Added.
+ * svg/animations/multiple-discrete-values-animate.svg: Added.
+
2019-09-20 Chris Dumez <cdu...@apple.com>
REGRESSION (iOS 13): rAF stops firing when navigating away cross-origin and then back
Added: trunk/LayoutTests/svg/animations/multiple-discrete-values-animate-expected.txt (0 => 250175)
--- trunk/LayoutTests/svg/animations/multiple-discrete-values-animate-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/animations/multiple-discrete-values-animate-expected.txt 2019-09-21 01:24:45 UTC (rev 250175)
@@ -0,0 +1 @@
+Passes if we did not debug assert.
Added: trunk/LayoutTests/svg/animations/multiple-discrete-values-animate.svg (0 => 250175)
--- trunk/LayoutTests/svg/animations/multiple-discrete-values-animate.svg (rev 0)
+++ trunk/LayoutTests/svg/animations/multiple-discrete-values-animate.svg 2019-09-21 01:24:45 UTC (rev 250175)
@@ -0,0 +1,18 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+ <g>
+ <text>Passes if we did not debug assert.</text>
+ <animate attributeName="class" dur="20ms" values="box; circle" _onbegin_="animationStarted()"/>
+ <animate attributeName="class" dur="20ms" values="box; circle"/>
+ </g>
+ <script>
+ if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+ }
+
+ function animationStarted() {
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }
+ </script>
+</svg>
Modified: trunk/Source/WebCore/ChangeLog (250174 => 250175)
--- trunk/Source/WebCore/ChangeLog 2019-09-21 01:07:49 UTC (rev 250174)
+++ trunk/Source/WebCore/ChangeLog 2019-09-21 01:24:45 UTC (rev 250175)
@@ -1,3 +1,35 @@
+2019-09-20 Said Abou-Hallawa <sabouhall...@apple.com>
+
+ Assertion fires when animating a discrete property with values range and multiple animators
+ https://bugs.webkit.org/show_bug.cgi?id=201926
+
+ Reviewed by Darin Adler.
+
+ The first animator of a property is considered the result element. The
+ other animators will be just contributers to the first animator. For the
+ first animator and in SVGSMILElement::progress(), we call resetAnimatedType()
+ which creates m_animator in SVGAnimateElementBase::animator(). But for
+ the other animators we do not call resetAnimatedType(). So their m_animator
+ will stay null until they are used for the first time.
+
+ If SVGAnimationElement::startedActiveInterval() calls calculateToAtEndOfDurationValue()
+ for a discrete property this will have no effect and the call should be
+ ignored. So SVGAnimateElementBase::calculateToAtEndOfDurationValue()
+ should bail out early if isDiscreteAnimator() is true.
+
+ The bug is isDiscreteAnimator() will return false if the m_animator is
+ null even if the animated property is discrete, e.g. SVGAnimatedString.
+ The fix is to make isDiscreteAnimator() ensure m_animator is created.
+
+ Unrelated change:
+ Make most of the protected methods of SVGAnimateElementBase be private.
+
+ Test: svg/animations/multiple-discrete-values-animate.svg
+
+ * svg/SVGAnimateElementBase.cpp:
+ (WebCore::SVGAnimateElementBase::calculateFromAndByValues):
+ (WebCore::SVGAnimateElementBase::calculateToAtEndOfDurationValue):
+
2019-09-20 Keith Rollin <krol...@apple.com>
Remove dead code for a specific macOS and iOS SDK
Modified: trunk/Source/WebCore/svg/SVGAnimateElementBase.cpp (250174 => 250175)
--- trunk/Source/WebCore/svg/SVGAnimateElementBase.cpp 2019-09-21 01:07:49 UTC (rev 250174)
+++ trunk/Source/WebCore/svg/SVGAnimateElementBase.cpp 2019-09-21 01:24:45 UTC (rev 250175)
@@ -75,7 +75,11 @@
bool SVGAnimateElementBase::isDiscreteAnimator() const
{
- return hasValidAttributeType() && animatorIfExists() && animatorIfExists()->isDiscrete();
+ if (!hasValidAttributeType())
+ return false;
+
+ auto* animator = this->animator();
+ return animator && animator->isDiscrete();
}
void SVGAnimateElementBase::setTargetElement(SVGElement* target)
Modified: trunk/Source/WebCore/svg/SVGAnimateElementBase.h (250174 => 250175)
--- trunk/Source/WebCore/svg/SVGAnimateElementBase.h 2019-09-21 01:07:49 UTC (rev 250174)
+++ trunk/Source/WebCore/svg/SVGAnimateElementBase.h 2019-09-21 01:24:45 UTC (rev 250175)
@@ -38,11 +38,13 @@
protected:
SVGAnimateElementBase(const QualifiedName&, Document&);
+ bool hasValidAttributeType() const override;
+ virtual String animateRangeString(const String& string) const { return string; }
+
+private:
SVGAttributeAnimator* animator() const;
SVGAttributeAnimator* animatorIfExists() const { return m_animator.get(); }
- bool hasValidAttributeType() const override;
-
void setTargetElement(SVGElement*) override;
void setAttributeName(const QualifiedName&) override;
void resetAnimation() override;
@@ -57,9 +59,6 @@
void clearAnimatedType(SVGElement* targetElement) override;
Optional<float> calculateDistance(const String& fromString, const String& toString) override;
- virtual String animateRangeString(const String& string) const { return string; }
-
-private:
bool hasInvalidCSSAttributeType() const;
mutable std::unique_ptr<SVGAttributeAnimator> m_animator;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
