Ensure animations that lose their effect don't schedule an animation update
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Feb 2020 18:29:47 +0000 (18:29 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Feb 2020 18:29:47 +0000 (18:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=207713
rdar://59174840

Patch by Sunny He <sunny_he@apple.com> on 2020-02-14
Reviewed by Antoine Quint.

Source/WebCore:
An active animation for which the effect is removed may be considered for
an upcoming animation resolution. However, WebAnimation::timeToNextTick()
expects a valid effect to be available to be able to determine timing.
We now check an animation is relevant before calling timeToNextTick() and
add an ASSERT() in that function to catch cases where an animation effect
might not be available.

Source/WebCore:

Test: webanimations/animation-null-effect.html

* animation/DocumentTimeline.cpp:
(WebCore::DocumentTimeline::scheduleNextTick):
* animation/WebAnimation.cpp:
(WebCore::WebAnimation::timeToNextTick const):

LayoutTests:

* webanimations/animation-null-effect-expected.txt: Added.
* webanimations/animation-null-effect.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/webanimations/animation-null-effect-expected.txt [new file with mode: 0644]
LayoutTests/webanimations/animation-null-effect.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/animation/DocumentTimeline.cpp
Source/WebCore/animation/WebAnimation.cpp

index 6fe60d8..70cb8af 100644 (file)
@@ -1,3 +1,22 @@
+2020-02-14  Sunny He  <sunny_he@apple.com>
+
+        Ensure animations that lose their effect don't schedule an animation update
+        https://bugs.webkit.org/show_bug.cgi?id=207713
+        rdar://59174840
+
+        Reviewed by Antoine Quint.
+
+        Source/WebCore:
+        An active animation for which the effect is removed may be considered for
+        an upcoming animation resolution. However, WebAnimation::timeToNextTick()
+        expects a valid effect to be available to be able to determine timing.
+        We now check an animation is relevant before calling timeToNextTick() and
+        add an ASSERT() in that function to catch cases where an animation effect
+        might not be available.
+
+        * webanimations/animation-null-effect-expected.txt: Added.
+        * webanimations/animation-null-effect.html: Added.
+
 2020-02-14  Charles Turner  <cturner@igalia.com>
 
         [EME][GStreamer] REGRESSION(r256429): Several encrypted-media tests are crashing or failing
diff --git a/LayoutTests/webanimations/animation-null-effect-expected.txt b/LayoutTests/webanimations/animation-null-effect-expected.txt
new file mode 100644 (file)
index 0000000..3ac7fb6
--- /dev/null
@@ -0,0 +1,3 @@
+Ensure removing the effect from an active animation is handled correctly.
+
+PASS if test does not crash.
diff --git a/LayoutTests/webanimations/animation-null-effect.html b/LayoutTests/webanimations/animation-null-effect.html
new file mode 100644 (file)
index 0000000..d87037d
--- /dev/null
@@ -0,0 +1,16 @@
+<script>
+    if (window.testRunner) {
+        window.testRunner.dumpAsText();
+    }
+    function eventhandler() {
+        var animation = element.animate({ "padding-left": [0, 1] }, 0.5);
+        animation.reverse();
+        animation.effect = null;
+    }
+    window.requestAnimationFrame(eventhandler);
+</script>
+<body>
+    <div id="element"></div>
+    <p>Ensure removing the effect from an active animation is handled correctly.</p>
+    <p>PASS if test does not crash.</p>
+</body>
index fb02379..8935888 100644 (file)
@@ -1,3 +1,26 @@
+2020-02-14  Sunny He  <sunny_he@apple.com>
+
+        Ensure animations that lose their effect don't schedule an animation update
+        https://bugs.webkit.org/show_bug.cgi?id=207713
+        rdar://59174840
+
+        Reviewed by Antoine Quint.
+
+        Source/WebCore:
+        An active animation for which the effect is removed may be considered for
+        an upcoming animation resolution. However, WebAnimation::timeToNextTick()
+        expects a valid effect to be available to be able to determine timing.
+        We now check an animation is relevant before calling timeToNextTick() and
+        add an ASSERT() in that function to catch cases where an animation effect
+        might not be available.
+
+        Test: webanimations/animation-null-effect.html
+
+        * animation/DocumentTimeline.cpp:
+        (WebCore::DocumentTimeline::scheduleNextTick):
+        * animation/WebAnimation.cpp:
+        (WebCore::WebAnimation::timeToNextTick const):
+
 2020-02-14  Sihui Liu  <sihui_liu@apple.com>
 
         IndexedDB: prefetch cursor records on client side
index 8f36a4e..3f06d0a 100644 (file)
@@ -569,6 +569,8 @@ void DocumentTimeline::scheduleNextTick()
     Seconds scheduleDelay = Seconds::infinity();
 
     for (const auto& animation : m_animations) {
+        if (!animation->isRelevant())
+            continue;
         auto animationTimeToNextRequiredTick = animation->timeToNextTick();
         if (animationTimeToNextRequiredTick < animationInterval()) {
             scheduleAnimationResolution();
index 45e247f..c19e4fd 100644 (file)
@@ -1445,6 +1445,8 @@ ExceptionOr<void> WebAnimation::commitStyles()
 
 Seconds WebAnimation::timeToNextTick() const
 {
+    ASSERT(effect());
+
     if (pending())
         return 0_s;