Attachment 350187 Details for Bug 189783 – Patch

[patch] Patch

bug-189783-20180920140807.patch (text/plain), 7.34 KB, created by Antoine Quint on 2018-09-20 05:08:09 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antoine Quint

Created: 2018-09-20 05:08:09 PDT

Size: 7.34 KB

patch

obsolete

>Subversion Revision: 236247
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c8c3b8bab552e8c3c62742b3e3bdc58472b823ab..50513281cdd3ca91bae574543634b9f2c72566d1 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,27 @@
>+2018-09-20  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 NOBODY (OOPS!).
>+
>+        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-20  Antoine Quint  <graouts@apple.com>
> 
>         [Web Animations] Provide a way to query accelerated animations for internal testing
>diff --git a/Source/WebCore/animation/DocumentTimeline.cpp b/Source/WebCore/animation/DocumentTimeline.cpp
>index 772b491038916f3ab44f1cde2449c46d532b9aaa..b29197b3ca54e149910284c302ecdfca44a811f3 100644
>--- a/Source/WebCore/animation/DocumentTimeline.cpp
>+++ b/Source/WebCore/animation/DocumentTimeline.cpp
>@@ -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();
> }
> 
>diff --git a/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp b/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
>index d8a4372a784af7bb5b05637a5ec311c50e08fae8..51bb83fe9d81e269db5e0e0ca55869a6ae4492f8 100644
>--- a/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
>+++ b/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
>@@ -4245,7 +4245,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() });
>             }
>         }
>     }
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 460d3adf96b9faa4ce0d9a56d489a4b32d3d5245..a4825623d8d9aad55f39bd0a8361b6d2989feab1 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,16 @@
>+2018-09-20  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 NOBODY (OOPS!).
>+
>+        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.
>+
> 2018-09-19  Yacine Bandou  <yacine.bandou@softathome.com>
> 
>         [EME][WPE] Add WebM initData in the expected result of clearkey-generate-request-disallowed-input test
>diff --git a/LayoutTests/webanimations/accelerated-animation-suspension-expected.txt b/LayoutTests/webanimations/accelerated-animation-suspension-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..bc04003d43036c6d39e8c2a8e564e9e1bc3d37d3
>--- /dev/null
>+++ b/LayoutTests/webanimations/accelerated-animation-suspension-expected.txt
>@@ -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
>index 0000000000000000000000000000000000000000..0654a695f5b24bc42fca7f1d2381da0f72ab688c
>--- /dev/null
>+++ b/LayoutTests/webanimations/accelerated-animation-suspension.html
>@@ -0,0 +1,57 @@
>+<!DOCTYPE html>
>+<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>

Flags:

dino: review+
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 189783: 350187 | 350191 | 350194 | 350196