git-svn-id: https://svn.webkit.org/repository/webkit/trunk@179772 268f45cc-cd09-0410...
authorsaid@apple.com <said@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 7 Feb 2015 01:24:48 +0000 (01:24 +0000)
committersaid@apple.com <said@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 7 Feb 2015 01:24:48 +0000 (01:24 +0000)
LayoutTests/ChangeLog
LayoutTests/svg/animations/animate-montion-invalid-attribute-expected.svg [new file with mode: 0644]
LayoutTests/svg/animations/animate-montion-invalid-attribute.svg [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/svg/SVGAnimateElementBase.cpp
Source/WebCore/svg/SVGAnimationElement.cpp
Source/WebCore/svg/SVGAnimationElement.h

index 7b03ca8..69f5943 100644 (file)
@@ -1,3 +1,15 @@
+2015-02-06  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        Invalid cast in WebCore::SVGAnimateElement::calculateAnimatedValue.
+        https://bugs.webkit.org/show_bug.cgi?id=135171.
+
+        Reviewed by Dean Jackson.
+
+        * svg/animations/animate-montion-invalid-attribute-expected.svg: Added.
+        * svg/animations/animate-montion-invalid-attribute.svg: Added.
+        Make sure that adding the same attribute to <animateMotion> and <animate>, which both
+        animate the same target element, will be ignored and we won't crash.
+
 2015-02-06  Simon Fraser  <simon.fraser@apple.com>
 
         Convert the compositing overlap map to use LayoutRects
diff --git a/LayoutTests/svg/animations/animate-montion-invalid-attribute-expected.svg b/LayoutTests/svg/animations/animate-montion-invalid-attribute-expected.svg
new file mode 100644 (file)
index 0000000..ea77c80
--- /dev/null
@@ -0,0 +1,4 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+  <circle cx="60" cy="60" r="50" fill="Lime"/>
+  <circle cx="160" cy="160" r="50" fill="Green"/>
+</svg>
diff --git a/LayoutTests/svg/animations/animate-montion-invalid-attribute.svg b/LayoutTests/svg/animations/animate-montion-invalid-attribute.svg
new file mode 100644 (file)
index 0000000..ddf3f4e
--- /dev/null
@@ -0,0 +1,38 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+  <circle cx="60" cy="60" r="50" fill="DarkRed">
+    <!-- animateMotion should not have animation attribute target attributes -->
+    <animateMotion attributeName="fill"
+                    path="M 0 0 L 100 100"
+                    begin="0ms"
+                    dur="100ms"
+                    fill="freeze"/>
+    <animateColor attributeName="fill" 
+                    attributeType="XML"
+                    from="DarkRed"
+                    to="Green"
+                    begin="0ms"
+                    dur="100ms"
+                    fill="freeze"/>
+  </circle>
+  <circle cx="160" cy="160" r="50" fill="Red">
+    <!-- animateMotion should not have animation attribute target attributes -->
+    <animateMotion attributeName="fill"
+                    path="M 0 0 L -100 -100"
+                    begin="0ms"
+                    dur="100ms"
+                    fill="freeze"/>
+    <animate attributeName="fill" 
+                    attributeType="XML"
+                    from="Red"
+                    to="Lime"
+                    begin="0ms"
+                    dur="100ms"
+                    fill="freeze"/>
+  </circle>
+  <script>
+    if (window.testRunner) {
+      testRunner.waitUntilDone();
+      window.setTimeout(function() { testRunner.notifyDone(); }, 500);
+    }
+  </script>
+</svg>
index 2c21297..9685331 100644 (file)
@@ -1,3 +1,38 @@
+2015-02-06  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        Invalid cast in WebCore::SVGAnimateElement::calculateAnimatedValue.
+        https://bugs.webkit.org/show_bug.cgi?id=135171.
+
+        Reviewed by Dean Jackson.
+
+        The bug happens when an SVG element is animated by <animateMotion> followed by an
+        <animateColor> or an <animate> and the values of the "attributeName" in both elements
+        are the same. The problem is <animateMotion> should not have an attribute to animate.
+        If it does by fuzz or by mistake, then we assume the <animateMotion> and the <animate>
+        animate the same attribute for the same element target. Therefore we schedule them in
+        the same AnimationVector in SMILTimeContainer::schedule(). When we call
+        SVGAnimateElementBase::calculateAnimatedValue() for an SVGAnimateColorElement and the
+        resultElement is SVGAnimateMotionElement, we fail to cast it to SVGAnimateElementBase
+        because SVGAnimateMotionElement is derived from SVGAnimationElement which is the base
+        class of all animate elements including SVGAnimateElementBase.
+
+        The fix is to nullify setting "attributeName" of an SVGAnimationElement. By doing so,
+        "attributeName" and its value will be ignored from the <animateMotion> which is correct.
+        
+        Tests: svg/animations/animate-montion-invalid-attribute.svg.
+
+        * svg/SVGAnimateElementBase.cpp:
+        (WebCore::SVGAnimateElementBase::setAttributeName):
+        Do not call SVGAnimationElement::setAttributeName() since SVGAnimationElement should
+        not have an attribute to animate. We prevent this by bypassing the parent in the class 
+        hierarchy: SVGAnimationElement and calling SVGSMILElement::setAttributeName() directly.
+        
+        * svg/SVGAnimationElement.cpp:
+        (WebCore::SVGAnimationElement::setAttributeName): Deleted.
+        * svg/SVGAnimationElement.h:
+        SVGAnimationElement should not have an attribute to animate. So implement its
+        setAttributeName() as a null function.
+
 2015-02-06  Simon Fraser  <simon.fraser@apple.com>
 
         Convert the compositing overlap map to use LayoutRects
index c1ccdc2..82a65ce 100644 (file)
@@ -424,7 +424,8 @@ void SVGAnimateElementBase::setTargetElement(SVGElement* target)
 
 void SVGAnimateElementBase::setAttributeName(const QualifiedName& attributeName)
 {
-    SVGAnimationElement::setAttributeName(attributeName);
+    SVGSMILElement::setAttributeName(attributeName);
+    checkInvalidCSSAttributeType(targetElement());
     resetAnimatedPropertyType();
 }
 
index 2c7f51f..737a84e 100644 (file)
@@ -691,12 +691,6 @@ void SVGAnimationElement::setTargetElement(SVGElement* target)
     checkInvalidCSSAttributeType(target);
 }
 
-void SVGAnimationElement::setAttributeName(const QualifiedName& attributeName)
-{
-    SVGSMILElement::setAttributeName(attributeName);
-    checkInvalidCSSAttributeType(targetElement());
-}
-
 void SVGAnimationElement::checkInvalidCSSAttributeType(SVGElement* target)
 {
     m_hasInvalidCSSAttributeType = target && hasValidAttributeName() && attributeType() == AttributeTypeCSS && !isTargetAttributeCSSProperty(target, attributeName());
index e1712c9..b3cb174 100644 (file)
@@ -194,8 +194,9 @@ protected:
     AnimatedPropertyValueType m_toPropertyValueType;
 
     virtual void setTargetElement(SVGElement*) override;
-    virtual void setAttributeName(const QualifiedName&) override;
+    virtual void setAttributeName(const QualifiedName&) override { }
     bool hasInvalidCSSAttributeType() const { return m_hasInvalidCSSAttributeType; }
+    void checkInvalidCSSAttributeType(SVGElement*);
 
     virtual void updateAnimationMode();
     void setAnimationMode(AnimationMode animationMode) { m_animationMode = animationMode; }
@@ -205,8 +206,6 @@ private:
     virtual void animationAttributeChanged() override;
     void setAttributeType(const AtomicString&);
 
-    void checkInvalidCSSAttributeType(SVGElement*);
-
     virtual bool calculateToAtEndOfDurationValue(const String& toAtEndOfDurationString) = 0;
     virtual bool calculateFromAndToValues(const String& fromString, const String& toString) = 0;
     virtual bool calculateFromAndByValues(const String& fromString, const String& byString) = 0;