[Web Animations] Accelerated animations don't get suspended
authorgraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Sep 2018 13:09:45 +0000 (13:09 +0000)
committergraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Sep 2018 13:09:45 +0000 (13:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=189783
<rdar://problem/43033568>

Reviewed by Dean Jackson.

Source/WebCore:

Test: webanimations/accelerated-animation-suspension.html

We used to set the flag that marked the timeline as suspended prior to notifying animations that they need to be suspended.
However, since the timeline was marked as suspended, querying the running state of the animations would indicate that the
animations weren't running since a suspended timeline would identify its animations as not running. As such we would fail
to pause the accelerated animations because they were already not marked as running. We now set the suspended flag on the
timeline _after_ suspending its animations.

We also fix a bug in the new internals.acceleratedAnimationsForElement() test function so that we read from the actual
CA animations and not from a stale list of animations which would not indicate the correct animation speeds.

* animation/DocumentTimeline.cpp:
(WebCore::DocumentTimeline::suspendAnimations):
* platform/graphics/ca/GraphicsLayerCA.cpp:
(WebCore::GraphicsLayerCA::acceleratedAnimationsForTesting):

LayoutTests:

Add a new test that checks whether an accelerated animation is correctly paused after suspending animations.

* webanimations/accelerated-animation-suspension-expected.txt: Added.
* webanimations/accelerated-animation-suspension.html: Added.
* platform/win/TestExpectations:

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

LayoutTests/ChangeLog
LayoutTests/animations/suspend-resume-animation-events.html
LayoutTests/webanimations/accelerated-animation-suspension-expected.txt [new file with mode: 0644]
LayoutTests/webanimations/accelerated-animation-suspension.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/animation/DocumentTimeline.cpp
Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp

index 6b6c9c6..57943d2 100644 (file)
@@ -1,5 +1,19 @@
 2018-09-21  Antoine Quint  <graouts@apple.com>
 
+        [Web Animations] Accelerated animations don't get suspended
+        https://bugs.webkit.org/show_bug.cgi?id=189783
+        <rdar://problem/43033568>
+
+        Reviewed by Dean Jackson.
+
+        Add a new test that checks whether an accelerated animation is correctly paused after suspending animations.
+
+        * webanimations/accelerated-animation-suspension-expected.txt: Added.
+        * webanimations/accelerated-animation-suspension.html: Added.
+        * platform/win/TestExpectations:
+
+2018-09-21  Antoine Quint  <graouts@apple.com>
+
         [Web Animations] DocumentTimeline::updateAnimations() is called endlessly
         https://bugs.webkit.org/show_bug.cgi?id=189784
         <rdar://problem/41705679>
index 6ce62e9..1bf7e9a 100644 (file)
@@ -1,4 +1,4 @@
-<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd"><!-- webkit-test-runner [ experimental:WebAnimationsCSSIntegrationEnabled=true ] -->
 <html lang="en">
 <head>
   <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
diff --git a/LayoutTests/webanimations/accelerated-animation-suspension-expected.txt b/LayoutTests/webanimations/accelerated-animation-suspension-expected.txt
new file mode 100644 (file)
index 0000000..bc04003
--- /dev/null
@@ -0,0 +1,3 @@
+
+PASS Suspending animations should pause running accelerated animations. 
+
diff --git a/LayoutTests/webanimations/accelerated-animation-suspension.html b/LayoutTests/webanimations/accelerated-animation-suspension.html
new file mode 100644 (file)
index 0000000..0654a69
--- /dev/null
@@ -0,0 +1,57 @@
+<!DOCTYPE html><!-- webkit-test-runner [ experimental:WebAnimationsCSSIntegrationEnabled=true ] -->
+<html>
+<head>
+<style>
+
+#target {
+    width: 100px;
+    height: 100px;
+    background-color: black;
+}
+
+</style>
+</head>
+<body>
+<script src="../resources/testharness.js"></script>
+<script src="../resources/testharnessreport.js"></script>
+<div id="target"></div>
+
+<script>
+
+function waitNFrames(numberOfFrames, continuation)
+{
+    let elapsedFrames = 0;
+    (function rAFCallback() {
+        if (elapsedFrames++ >= numberOfFrames)
+            continuation();
+        else
+            requestAnimationFrame(rAFCallback);
+    })();
+}
+
+async_test(t => {
+    const target = document.getElementById("target");
+    target.animate({ transform: ["translateX(100px)", "none"] }, 1000 * 1000);
+
+    waitNFrames(3, () => {
+        const initialAnimations = internals.acceleratedAnimationsForElement(target);
+        assert_equals(initialAnimations.length, 1, "There should be a single accelerated animation before suspension.");
+        assert_object_equals(initialAnimations[0], { property: "transform", speed: 1 },
+                             'The single accelerated animation before suspension should be running and targeting the "transform" property.');
+
+        internals.suspendAnimations();
+
+        waitNFrames(2, () => {
+            const suspendedAnimations = internals.acceleratedAnimationsForElement(target);
+            assert_equals(suspendedAnimations.length, 1, "There should be a single accelerated animation after suspension.");
+            assert_object_equals(suspendedAnimations[0], { property: "transform", speed: 0 },
+                                 'The single accelerated animation after suspension should be paused and targeting the "transform" property.');
+            internals.resumeAnimations();
+            setTimeout(() => t.done());
+        });
+    });
+}, "Suspending animations should pause running accelerated animations.");
+
+</script>
+</body>
+</html>
index 66801c5..18be49b 100644 (file)
@@ -1,3 +1,27 @@
+2018-09-21  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Accelerated animations don't get suspended
+        https://bugs.webkit.org/show_bug.cgi?id=189783
+        <rdar://problem/43033568>
+
+        Reviewed by Dean Jackson.
+
+        Test: webanimations/accelerated-animation-suspension.html
+
+        We used to set the flag that marked the timeline as suspended prior to notifying animations that they need to be suspended.
+        However, since the timeline was marked as suspended, querying the running state of the animations would indicate that the
+        animations weren't running since a suspended timeline would identify its animations as not running. As such we would fail
+        to pause the accelerated animations because they were already not marked as running. We now set the suspended flag on the
+        timeline _after_ suspending its animations.
+
+        We also fix a bug in the new internals.acceleratedAnimationsForElement() test function so that we read from the actual
+        CA animations and not from a stale list of animations which would not indicate the correct animation speeds.
+
+        * animation/DocumentTimeline.cpp:
+        (WebCore::DocumentTimeline::suspendAnimations):
+        * platform/graphics/ca/GraphicsLayerCA.cpp:
+        (WebCore::GraphicsLayerCA::acceleratedAnimationsForTesting):
+
 2018-09-21  Zan Dobersek  <zdobersek@igalia.com>
 
         TransformationMatrix::toColumnMajorFloatArray() should return a std::array<> object
index 3c13948..aed1ba7 100644 (file)
@@ -98,8 +98,6 @@ void DocumentTimeline::suspendAnimations()
     if (animationsAreSuspended())
         return;
 
-    m_isSuspended = true;
-
     m_invalidationTaskQueue.cancelAllTasks();
     if (m_animationScheduleTimer.isActive())
         m_animationScheduleTimer.stop();
@@ -107,6 +105,8 @@ void DocumentTimeline::suspendAnimations()
     for (const auto& animation : animations())
         animation->setSuspended(true);
 
+    m_isSuspended = true;
+
     applyPendingAcceleratedAnimations();
 }
 
@@ -216,9 +216,11 @@ void DocumentTimeline::scheduleInvalidationTaskIfNeeded()
 void DocumentTimeline::performInvalidationTask()
 {
     // Now that the timing model has changed we can see if there are DOM events to dispatch for declarative animations.
-    for (auto& animation : animations()) {
-        if (is<DeclarativeAnimation>(animation))
-            downcast<DeclarativeAnimation>(*animation).invalidateDOMEvents();
+    if (!m_isSuspended) {
+        for (auto& animation : animations()) {
+            if (is<DeclarativeAnimation>(animation))
+                downcast<DeclarativeAnimation>(*animation).invalidateDOMEvents();
+        }
     }
 
     applyPendingAcceleratedAnimations();
index 76484cf..81e5c03 100644 (file)
@@ -4272,7 +4272,8 @@ Vector<std::pair<String, double>> GraphicsLayerCA::acceleratedAnimationsForTesti
             size_t numAnimations = propertyAnimations.size();
             for (size_t i = 0; i < numAnimations; ++i) {
                 const LayerPropertyAnimation& currAnimation = propertyAnimations[i];
-                animations.append({ animatedPropertyIDAsString(currAnimation.m_property), currAnimation.m_animation->speed() });
+                auto caAnimation = animatedLayer(currAnimation.m_property)->animationForKey(animationIdentifier(currAnimation.m_name, currAnimation.m_property, currAnimation.m_index, currAnimation.m_subIndex));
+                animations.append({ animatedPropertyIDAsString(currAnimation.m_property), caAnimation->speed() });
             }
         }
     }