[Web Animations] REGRESSION (r236809): crash under AnimationTimeline::updateCSSAnimat...
authorgraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Oct 2018 10:57:43 +0000 (10:57 +0000)
committergraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Oct 2018 10:57:43 +0000 (10:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=190307
<rdar://problem/45009901>

Reviewed by Dean Jackson.

Source/WebCore:

We could crash with an invalid access to cssAnimationsByName since cancelOrRemoveDeclarativeAnimation() already
does the job of clearing the m_elementToCSSAnimationByName entry for this particular element if there are no
animations targeting it anymore. This started happening in r236809 when we switched from a simple call to to cancel()
to a call to cancelOrRemoveDeclarativeAnimation(). We can safely remove the removal here since cancelOrRemoveDeclarativeAnimation()
will already have performed this task safely if needed.

* animation/AnimationTimeline.cpp:
(WebCore::AnimationTimeline::updateCSSAnimationsForElement):

LayoutTests:

This test was also crashing even though it should not have been using the new animation engine. Adding the
flag to opt into the legacy animation engine.

* legacy-animation-engine/animations/animation-shorthand-removed.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@236871 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/legacy-animation-engine/animations/animation-shorthand-removed.html
Source/WebCore/ChangeLog
Source/WebCore/animation/AnimationTimeline.cpp

index 59de6c5..b8e7c8b 100644 (file)
@@ -1,3 +1,16 @@
+2018-10-05  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] REGRESSION (r236809): crash under AnimationTimeline::updateCSSAnimationsForElement()
+        https://bugs.webkit.org/show_bug.cgi?id=190307
+        <rdar://problem/45009901>
+
+        Reviewed by Dean Jackson.
+
+        This test was also crashing even though it should not have been using the new animation engine. Adding the
+        flag to opt into the legacy animation engine.
+
+        * legacy-animation-engine/animations/animation-shorthand-removed.html:
+
 2018-10-04  Chris Dumez  <cdumez@apple.com>
 
         A Document / Window should lose its browsing context as soon as its iframe is removed from the document
index c097856..0d6a023 100644 (file)
@@ -1,4 +1,4 @@
-<html>
+<html><!-- webkit-test-runner [ experimental:WebAnimationsCSSIntegrationEnabled=false ] -->
 <head>
 <title>Test removal of animation shorthand property</title>
 <style type="text/css" media="screen">
index 36eab33..a54dece 100644 (file)
@@ -1,3 +1,20 @@
+2018-10-05  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] REGRESSION (r236809): crash under AnimationTimeline::updateCSSAnimationsForElement()
+        https://bugs.webkit.org/show_bug.cgi?id=190307
+        <rdar://problem/45009901>
+
+        Reviewed by Dean Jackson.
+
+        We could crash with an invalid access to cssAnimationsByName since cancelOrRemoveDeclarativeAnimation() already
+        does the job of clearing the m_elementToCSSAnimationByName entry for this particular element if there are no
+        animations targeting it anymore. This started happening in r236809 when we switched from a simple call to to cancel()
+        to a call to cancelOrRemoveDeclarativeAnimation(). We can safely remove the removal here since cancelOrRemoveDeclarativeAnimation()
+        will already have performed this task safely if needed.
+
+        * animation/AnimationTimeline.cpp:
+        (WebCore::AnimationTimeline::updateCSSAnimationsForElement):
+
 2018-10-04  Jer Noble  <jer.noble@apple.com>
 
         Add support for reporting "display composited video frames" through the VideoPlaybackQuality object.
index a651a0b..936ea2c 100644 (file)
@@ -276,10 +276,6 @@ void AnimationTimeline::updateCSSAnimationsForElement(Element& element, const Re
         if (auto animation = cssAnimationsByName.take(nameOfAnimationToRemove))
             cancelOrRemoveDeclarativeAnimation(animation);
     }
-
-    // Remove the map of CSSAnimations by animation name for this element if it's now empty.
-    if (cssAnimationsByName.isEmpty())
-        m_elementToCSSAnimationByName.remove(&element);
 }
 
 RefPtr<WebAnimation> AnimationTimeline::cssAnimationForElementAndProperty(Element& element, CSSPropertyID property)