Attachment 342245 Details for Bug 170784 – Patch

[patch] Patch

bug-170784-20180608103302.patch (text/plain), 8.57 KB, created by Frédéric Wang (:fredw) on 2018-06-08 01:33:04 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Frédéric Wang (:fredw)

Created: 2018-06-08 01:33:04 PDT

Size: 8.57 KB

patch

obsolete

>Subversion Revision: 232617
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index d2fbb2d902a120d0eba42c990ee5daba62c76809..c0ee0c09a8be1329b1498a23a4995213e5060c39 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,18 @@
>+2018-06-08  Frederic Wang  <fred.wang@free.fr>
>+
>+        [iOS] Animations with BÃ©zier timing function not suspended on UI process when animation-play-state is set to "paused"
>+        https://bugs.webkit.org/show_bug.cgi?id=170784
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        In order to pause a running animation, GraphicsLayerCA::pauseCAAnimationOnLayer calls
>+        PlatformCALayer::addAnimationForKey, assuming it will replace the current animation.
>+        This patch fixes PlatformCALayerRemote::addAnimationForKey to ensure this assumption holds.
>+
>+        * WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.cpp:
>+        (WebKit::PlatformCALayerRemote::addAnimationForKey): If the animation was already sent to
>+        the UI process, make sure it is properly updated.
>+
> 2018-06-07  Chris Dumez  <cdumez@apple.com>
> 
>         Add base class to get WeakPtrFactory member and avoid some boilerplate code
>diff --git a/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.cpp b/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.cpp
>index 53968fcdb38ccdf4266746f3f1712c09eaadeee8..312399cd98667fe4a2e4b84f1c948866486070e4 100644
>--- a/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.cpp
>@@ -328,16 +328,23 @@ void PlatformCALayerRemote::adoptSublayers(PlatformCALayer& source)
> void PlatformCALayerRemote::addAnimationForKey(const String& key, PlatformCAAnimation& animation)
> {
>     auto addResult = m_animations.set(key, &animation);
>-    if (addResult.isNewEntry)
>-        m_properties.addedAnimations.append(std::pair<String, PlatformCAAnimationRemote::Properties>(key, downcast<PlatformCAAnimationRemote>(animation).properties()));
>-    else {
>+    bool appendToAddedAnimations = true;
>+    if (!addResult.isNewEntry) {
>+        // There is already an animation for this key. Verify whether it is pending to be sent to
>+        // the UI process and update the key accordingly.
>         for (auto& keyAnimationPair : m_properties.addedAnimations) {
>             if (keyAnimationPair.first == key) {
>                 keyAnimationPair.second = downcast<PlatformCAAnimationRemote>(animation).properties();
>+                appendToAddedAnimations = false;
>                 break;
>             }
>         }
>+        // The animation has already been received by the UI Process so it must be replaced.
>+        if (appendToAddedAnimations)
>+            m_properties.keyPathsOfAnimationsToRemove.add(key);
>     }
>+    if (appendToAddedAnimations)
>+        m_properties.addedAnimations.append(std::pair<String, PlatformCAAnimationRemote::Properties>(key, downcast<PlatformCAAnimationRemote>(animation).properties()));
>     
>     m_properties.notePropertiesChanged(RemoteLayerTreeTransaction::AnimationsChanged);
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index b4d409afd9ba15e441b629ca07423cd74245262f..3af8b5cbc938d3381d449fd2fab01269edf0ce20 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-06-08  Frederic Wang  <fred.wang@free.fr>
>+
>+        [iOS] Animations with BÃ©zier timing function not suspended on UI process when animation-play-state is set to "paused"
>+        https://bugs.webkit.org/show_bug.cgi?id=170784
>+
>+        Add a reftest to visually check pausing of transform animations.
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * http/wpt/css/css-animations/set-animation-play-state-to-paused-001-expected.html: Added.
>+        * http/wpt/css/css-animations/set-animation-play-state-to-paused-001.html: Added.
>+
> 2018-06-07  Mark Lam  <mark.lam@apple.com>
> 
>         Enhance run-jsc-stress-tests to allow a test to specify test specific options required for it to run.
>diff --git a/LayoutTests/http/wpt/css/css-animations/set-animation-play-state-to-paused-001-expected.html b/LayoutTests/http/wpt/css/css-animations/set-animation-play-state-to-paused-001-expected.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..26d8c37c81ef3d5a40e82f34d607161fdc326ebe
>--- /dev/null
>+++ b/LayoutTests/http/wpt/css/css-animations/set-animation-play-state-to-paused-001-expected.html
>@@ -0,0 +1,29 @@
>+<!DOCTYPE html>
>+<html>
>+  <head>
>+    <meta charset="utf-8">
>+    <title>Dynamically set animation-play-state to paused (reference)</title>
>+    <style>
>+      #container {
>+        position: absolute;
>+        left: 0;
>+        top: 3em;
>+      }
>+      #coveringRect {
>+        position: absolute;
>+        background: lightgreen;
>+        width: 150px;
>+        height: 300px;
>+        left: -75px;
>+        top: 0px;
>+        transform: translate(150px);
>+      }
>+    </style>
>+  </head>
>+  <body>
>+    <p>This test passes if you see a green rectangle and no red.</p>
>+    <div id="container">
>+      <div id="coveringRect"></div>
>+    </div>
>+  </body>
>+</html>
>diff --git a/LayoutTests/http/wpt/css/css-animations/set-animation-play-state-to-paused-001.html b/LayoutTests/http/wpt/css/css-animations/set-animation-play-state-to-paused-001.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..2726b903fb81a1a516cf2152c868a0a7c79c36f3
>--- /dev/null
>+++ b/LayoutTests/http/wpt/css/css-animations/set-animation-play-state-to-paused-001.html
>@@ -0,0 +1,97 @@
>+<!DOCTYPE html>
>+<html>
>+  <head>
>+    <meta charset="utf-8">
>+    <title>Dynamically set animation-play-state to paused</title>
>+    <link rel="author" title="Igalia S.L." href="https://www.igalia.com/">
>+    <link rel="help" href="https://drafts.csswg.org/css-animations-1/#animation-play-state">
>+    <meta name="assert" content="Visually check that transform animations stop
>+                                 when animation-play-state is set to paused">
>+    <link rel="match" href="set-animation-play-state-to-paused-001-ref.html">
>+    <style>
>+      #container {
>+        position: absolute;
>+        left: 0;
>+        top: 3em;
>+      }
>+      #squareLinear, #squareSteps  {
>+        width: 100px;
>+        height: 100px;
>+        background: red;
>+        position: absolute;
>+        left: -50px;
>+      }
>+      #squareLinear {
>+        top: 25px;
>+        animation: move 2s linear;
>+      }
>+      #squareSteps {
>+        top: 175px;
>+        animation: move 2s steps(1000, end);
>+      }
>+      #coveringRect {
>+        position: absolute;
>+        background: lightgreen;
>+        width: 150px;
>+        height: 300px;
>+        left: -75px;
>+        top: 0px;
>+        transform: translate(150px);
>+      }
>+      .pause {
>+        opacity: 0.6;
>+        animation-play-state: paused !important;
>+      }
>+      @keyframes move
>+      {
>+        100% {
>+          transform: translate(500px);
>+        }
>+      }
>+    </style>
>+    <script>
>+      var start = null;
>+      var animationDuration = 2000;
>+      function halfWidth(id)
>+      {
>+        return document.getElementById(id).scrollWidth / 2;
>+      }
>+      function shift(id)
>+      {
>+        var transform = window.getComputedStyle(document.getElementById(id)).transform;
>+        if (transform === "none")
>+          return 0;
>+        // See https://drafts.csswg.org/css-transforms-2/#serialization-of-the-computed-value
>+        return parseFloat(transform.split(/,\s*/)[4]);
>+      }
>+      function step(timestamp) {
>+        // For each square, pause the animation as soon as it passes under the covering rect.
>+        ["squareLinear", "squareSteps"].forEach((id) => {
>+          if (Math.abs(shift("coveringRect") - shift(id)) < halfWidth("coveringRect") - halfWidth(id))
>+            document.getElementById(id).classList.add("pause");
>+        });
>+
>+        // Stop the reftest after the time when the animations would have stop.
>+        // If pausing failed for some reason, the squares will be visible.
>+        if (!start) start = timestamp;
>+        if (timestamp - start < animationDuration)
>+          window.requestAnimationFrame(step);
>+        else if (window.testRunner)
>+            testRunner.notifyDone();
>+      }
>+      function runTest() {
>+          if (window.testRunner)
>+              testRunner.waitUntilDone();
>+          window.requestAnimationFrame(step);
>+      }
>+    </script>
>+  </head>
>+  <body onload="runTest()">
>+    <p>This test passes if you see a green rectangle and no red.</p>
>+    <div id="container">
>+      <div id="squareLinear"></div>
>+      <div id="squareSteps"></div>
>+      <div id="coveringRect"></div>
>+    </div>
>+  </body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 170784: 340559 | 340560 | 341453 | 341454 | 341646 | 341650 | 342245 | 342414 | 342422 | 344170