Created attachment 251234 [details] Test case (will crash) Open the attached file. Result: WebKit crashes with the following call stack: WebCore::SVGAnimateElementBase::calculateAnimatedValue WebCore::SVGAnimationElement::updateAnimation WebCore::SVGSMILElement::progress WebCore::SMILTimeContainer::updateAnimations WebCore::SMILTimeContainer::timerFired Notes: The crash happens when removing an animating SVG element and inserting it back while animating within the same animation limits. The reason for the crash is when removing an animating element from the SVG document, we call SVGAnimateElementBase::resetAnimatedPropertyType() which sets SVGAnimateElementBase::m_fromType and SVGAnimateElementBase::m_toType to nullptr. When the element is inserted back to the SVG document, SVGAnimationElement::updateAnimation() is called to get the new animated value. Before doing that we check if the animation limits have changed or not. But since in this case, the limits are not changed, we do not call SVGAnimateElementBase::calculateFromAndToValues() which is supposed to call SVGAnimatedTypeAnimator::calculateFromAndToValues(). And this later function is supposed to set valid values to SVGAnimateElementBase::m_fromType and SVGAnimateElementBase::m_toType. But since this does not happen, we end up calling SSVGAnimateElementBase::calculateAnimatedValue() which asserts ASSERT(m_fromType); ASSERT(m_fromType->type() == m_animatedPropertyType); ASSERT(m_toType);
Created attachment 251236 [details] Patch
Created attachment 251238 [details] Patch
This is the crash trace we get when opening the attached test case. The assert in SVGAnimateElementBase::calculateAnimatedValue() should happen before this crash in debug build. SVGAnimatedLengthAnimator::calculateAnimatedValue(float, unsigned int, SVGAnimatedType*, WebCore::SVGAnimatedType*, WebCore::SVGAnimatedType*, WebCore::SVGAnimatedType*) + 60 WebCore::SVGAnimationElement::updateAnimation(float, unsigned int, WebCore::SVGSMILElement*) + 747 WebCore::SVGSMILElement::progress(WebCore::SMILTime, WebCore::SVGSMILElement*, bool) + 607 WebCore::SMILTimeContainer::updateAnimations(WebCore::SMILTime, bool) + 426
Created attachment 251250 [details] transform test case (will crash) This is another test case with a different crash trace. However it happens because of the same reason mentioned above. In the debug build SVGAnimateElementBase::calculateAnimatedValue() asserts before this crash: WebCore::SVGAnimatedTransformListAnimator::calculateAnimatedValue(float, unsigned int, WebCore::SVGAnimatedType*, WebCore::SVGAnimatedType*, WebCore::SVGAnimatedType*, WebCore::SVGAnimatedType*) + 34 WebCore::SVGAnimationElement::updateAnimation(float, unsigned int, WebCore::SVGSMILElement*) + 747 WebCore::SVGSMILElement::progress(WebCore::SMILTime, WebCore::SVGSMILElement*, bool) + 607 WebCore::SMILTimeContainer::updateAnimations(WebCore::SMILTime, bool) + 509
Created attachment 251255 [details] Patch
<rdar://problem/18921450>
Comment on attachment 251255 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=251255&action=review > LayoutTests/svg/animations/crash-reinsert-animate-length-same-limits.svg:19 > + }, 200); Try to find a way to reduce or eliminate this timeout. > LayoutTests/svg/animations/crash-reinsert-animate-transform-same-limits.svg:19 > + }, 200); Try to find a way to reduce or eliminate this timeout.
Created attachment 251271 [details] Patch
(In reply to comment #7) > Comment on attachment 251255 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=251255&action=review > > > LayoutTests/svg/animations/crash-reinsert-animate-length-same-limits.svg:19 > > + }, 200); > > Try to find a way to reduce or eliminate this timeout. > > > LayoutTests/svg/animations/crash-reinsert-animate-transform-same-limits.svg:19 > > + }, 200); > > Try to find a way to reduce or eliminate this timeout. The 200ms is now replaced by 0. The setTimeout(...,0); has to stay to ensure that the animation timer will fire at least once after reinserting the animating element and before the test ends. I also confirmed that WebKit crashes with the modified tests.
Comment on attachment 251271 [details] Patch Clearing flags on attachment: 251271 Committed r183085: <http://trac.webkit.org/changeset/183085>
All reviewed patches have been landed. Closing bug.