SVG Animations update baseVal instead of animVal

https://bugs.webkit.org/show_bug.cgi?id=12437

Reviewed by Dirk Schulze.

Blind fix for some GC related assertions firing on v8.

Assure that animationStarted/animationEnded calls are happening
on the same SVGAnimatedProperty. Always call animationEnded(),
even if we shouldn't do anything for the target element, as its
destructed, as we still have to reset m_isAnimating.

* svg/SVGAnimateElement.cpp:
(WebCore::SVGAnimateElement::SVGAnimateElement):
(WebCore::SVGAnimateElement::resetToBaseValue):
(WebCore::SVGAnimateElement::targetElementWillChange):
* svg/SVGAnimateElement.h:
(SVGAnimateElement):
* svg/properties/SVGAnimatedPropertyTearOff.h:
(WebCore::SVGAnimatedPropertyTearOff::animationEnded):

Canonical link: https://commits.webkit.org/98168@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@110591 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog

@@ -1,3 +1,26 @@
+2012-03-13  Nikolas Zimmermann  <nzimmermann@rim.com>
+
+        SVG Animations update baseVal instead of animVal
+        https://bugs.webkit.org/show_bug.cgi?id=12437
+
+        Reviewed by Dirk Schulze.
+
+        Blind fix for some GC related assertions firing on v8.
+
+        Assure that animationStarted/animationEnded calls are happening
+        on the same SVGAnimatedProperty. Always call animationEnded(),
+        even if we shouldn't do anything for the target element, as its
+        destructed, as we still have to reset m_isAnimating.
+
+        * svg/SVGAnimateElement.cpp:
+        (WebCore::SVGAnimateElement::SVGAnimateElement):
+        (WebCore::SVGAnimateElement::resetToBaseValue):
+        (WebCore::SVGAnimateElement::targetElementWillChange):
+        * svg/SVGAnimateElement.h:
+        (SVGAnimateElement):
+        * svg/properties/SVGAnimatedPropertyTearOff.h:
+        (WebCore::SVGAnimatedPropertyTearOff::animationEnded):
+
 2012-03-13  Mark Pilgrim  <pilgrim@chromium.org>
 
         Move DirectoryEntry and DirectoryReader to new Modules/filesystem/ directory

Source/WebCore/svg/SVGAnimateElement.cpp

@@ -41,6 +41,7 @@ SVGAnimateElement::SVGAnimateElement(const QualifiedName& tagName, Document* doc
     , m_animatedPropertyType(AnimatedString)
     , m_fromPropertyValueType(RegularPropertyValue)
     , m_toPropertyValueType(RegularPropertyValue)
+    , m_animatedProperty(0)
 {
     ASSERT(hasTagName(SVGNames::animateTag) || hasTagName(SVGNames::setTag) || hasTagName(SVGNames::animateColorTag));
 }
@@ -219,13 +220,15 @@ void SVGAnimateElement::resetToBaseValue(const String& baseString)
     // In that case we don't need any base value strings, but can directly operate on these
     // SVG DOM primitives, like SVGLength.
     SVGAnimatedTypeAnimator* animator = ensureAnimator();
-    if (SVGAnimatedProperty* animatedProperty = animatedPropertyForType(animator->type())) {
+    m_animatedProperty = animatedPropertyForType(animator->type());
+    if (m_animatedProperty) {
         if (!m_animatedType) {
-            m_animatedType = animator->constructFromCopy(animatedProperty->currentBaseValue(animator->type()));
-            animationStarted(animatedProperty, m_animatedType.get());
+            m_animatedType = animator->constructFromCopy(m_animatedProperty->currentBaseValue(animator->type()));
+            animationStarted(m_animatedProperty, m_animatedType.get());
         } else
-            m_animatedType->setVariantValue(animatedProperty->currentBaseValue(m_animator->type()));
+            m_animatedType->setVariantValue(m_animatedProperty->currentBaseValue(m_animator->type()));
         ASSERT(m_animatedPropertyType == animator->type());
+        ASSERT(m_animatedPropertyType == m_animatedProperty->animatedPropertyType());
         return;
     }
 
@@ -259,11 +262,9 @@ void SVGAnimateElement::targetElementWillChange(SVGElement* currentTarget, SVGEl
 {
     SVGSMILElement::targetElementWillChange(currentTarget, newTarget);
 
-    // Be sure to never execute any of this, while the target element is being removed from the document or destructed.
-    if (currentTarget && currentTarget->inDocument() && currentTarget->parentNode()) {
-        ASSERT(!currentTarget->m_deletionHasBegun);
-        if (SVGAnimatedProperty* animatedProperty = animatedPropertyForType(m_animatedPropertyType))
-            animationEnded(animatedProperty);
+    if (m_animatedProperty) {
+        animationEnded(m_animatedProperty);
+        m_animatedProperty = 0;
     }
 
     m_animatedType.clear();

Source/WebCore/svg/SVGAnimateElement.h

@@ -40,6 +40,8 @@ enum AnimatedPropertyValueType {
     InheritValue
 };
 
+class SVGAnimatedProperty;
+
 class SVGAnimateElement : public SVGAnimationElement {
 public:
     static PassRefPtr<SVGAnimateElement> create(const QualifiedName&, Document*);
@@ -79,7 +81,8 @@ class SVGAnimateElement : public SVGAnimationElement {
     OwnPtr<SVGAnimatedType> m_fromType;
     OwnPtr<SVGAnimatedType> m_toType;
     OwnPtr<SVGAnimatedType> m_animatedType;
-
+
+    SVGAnimatedProperty* m_animatedProperty;
     OwnPtr<SVGAnimatedTypeAnimator> m_animator;
 };
 

Source/WebCore/svg/properties/SVGAnimatedPropertyTearOff.h

@@ -79,8 +79,11 @@ class SVGAnimatedPropertyTearOff : public SVGAnimatedProperty {
         static_cast<SVGPropertyTearOff<PropertyType>*>(animVal())->setValue(m_property);
         m_isAnimating = false;
 
-        ASSERT(contextElement());
-        contextElement()->svgAttributeChanged(attributeName());
+        SVGElement* element = contextElement();
+        if (!element || !element->inDocument() || !element->parentNode())
+            return;
+        ASSERT(!element->m_deletionHasBegun);
+        element->svgAttributeChanged(attributeName());
     }
 
     virtual void animationValueChanged()