Changeset 255810 in webkit

Timestamp:

Feb 5, 2020 3:51:40 AM (4 years ago)

Author:

graouts@webkit.org

Message:

[Web Animations] Canceling an accelerated animation before it was committed does not prevent it from playing
​https://bugs.webkit.org/show_bug.cgi?id=207253
<rdar://problem/59143624>

Reviewed by Antti Koivisto.

Source/WebCore:

Test: webanimations/accelerated-animation-canceled-before-commit.html

Merely checking whether an accelerated animation is running prior to enqueuing an action to cancel it is not sufficient
since there could be an uncommitted change to start it upon the next animation frame. The same logic would need to apply
in other situations where the playback state changes for a potentially in-flight animation.

• animation/KeyframeEffect.cpp:

(WebCore::KeyframeEffect::animationDidSeek):
(WebCore::KeyframeEffect::animationWasCanceled):
(WebCore::KeyframeEffect::willChangeRenderer):
(WebCore::KeyframeEffect::animationSuspensionStateDidChange):

LayoutTests:

Add a new test that checks that an accelerated animation that has been enqueued to start but has
not yet been committed is correctly canceled when the cancel() method is called. This test fails
prior to this source change.

• webanimations/accelerated-animation-canceled-before-commit-expected.html: Added.
• webanimations/accelerated-animation-canceled-before-commit.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/webanimations/accelerated-animation-canceled-before-commit-expected.html (added)
• LayoutTests/webanimations/accelerated-animation-canceled-before-commit.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/animation/KeyframeEffect.cpp (modified) (4 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2020-02-05  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Canceling an accelerated animation before it was committed does not prevent it from playing
+        https://bugs.webkit.org/show_bug.cgi?id=207253
+        <rdar://problem/59143624>
+
+        Reviewed by Antti Koivisto.
+
+        Add a new test that checks that an accelerated animation that has been enqueued to start but has
+        not yet been committed is correctly canceled when the cancel() method is called. This test fails
+        prior to this source change.
+
+        * webanimations/accelerated-animation-canceled-before-commit-expected.html: Added.
+        * webanimations/accelerated-animation-canceled-before-commit.html: Added.
+
 2020-02-04  Lauro Moura  <lmoura@igalia.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-02-05  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Canceling an accelerated animation before it was committed does not prevent it from playing
+        https://bugs.webkit.org/show_bug.cgi?id=207253
+        <rdar://problem/59143624>
+
+        Reviewed by Antti Koivisto.
+
+        Test: webanimations/accelerated-animation-canceled-before-commit.html
+
+        Merely checking whether an accelerated animation is running prior to enqueuing an action to cancel it is not sufficient
+        since there could be an uncommitted change to start it upon the next animation frame. The same logic would need to apply
+        in other situations where the playback state changes for a potentially in-flight animation. 
+
+        * animation/KeyframeEffect.cpp:
+        (WebCore::KeyframeEffect::animationDidSeek):
+        (WebCore::KeyframeEffect::animationWasCanceled):
+        (WebCore::KeyframeEffect::willChangeRenderer):
+        (WebCore::KeyframeEffect::animationSuspensionStateDidChange):
+
 2020-02-05  Philippe Normand  <philn@igalia.com>
 

trunk/Source/WebCore/animation/KeyframeEffect.cpp

@@ -1386 +1386 @@
     // There is no need to seek if we're not playing an animation already. If seeking
     // means we're moving into an active lexicalGlobalObject, we'll pick this up in apply().
-    if (m_isRunningAccelerated)
+    if (m_isRunningAccelerated || isAboutToRunAccelerated())
         addPendingAcceleratedAction(AcceleratedAction::Seek);
 }
@@ -1392 +1392 @@
 void KeyframeEffect::animationWasCanceled()
 {
-    if (m_isRunningAccelerated)
+    if (m_isRunningAccelerated || isAboutToRunAccelerated())
         addPendingAcceleratedAction(AcceleratedAction::Stop);
 }
@@ -1398 +1398 @@
 void KeyframeEffect::willChangeRenderer()
 {
-    if (m_isRunningAccelerated)
+    if (m_isRunningAccelerated || isAboutToRunAccelerated())
         addPendingAcceleratedAction(AcceleratedAction::Stop);
 }
@@ -1404 +1404 @@
 void KeyframeEffect::animationSuspensionStateDidChange(bool animationIsSuspended)
 {
-    if (m_isRunningAccelerated)
+    if (m_isRunningAccelerated || isAboutToRunAccelerated())
         addPendingAcceleratedAction(animationIsSuspended ? AcceleratedAction::Pause : AcceleratedAction::Play);
 }