[Web Animations] Interrupting an accelerated CSS transition on a composited element...
authorgraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Sep 2018 07:13:19 +0000 (07:13 +0000)
committergraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Sep 2018 07:13:19 +0000 (07:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=189405
<rdar://problem/43342639>

Reviewed by Simon Fraser.

Source/WebCore:

Test: webanimations/accelerated-transition-interrupted-on-composited-element.html

If we interrupt an animation on an element that is composited also outside of the duration of the animation,
the "stop" accelerated action would fail to be performed because we no longer had a resolved current time and
the accelerated animation applied to the layer would never be removed.

However, having a resolved current time is not necessary to stop an animation, only for the other types of
actions (play, pause and seek). So we now simply default to a 0s time for an unresolved current time for a
simple fix to this issue.

* animation/KeyframeEffectReadOnly.cpp:
(WebCore::KeyframeEffectReadOnly::applyPendingAcceleratedActions):

LayoutTests:

Add a new test that checks that interrupting a CSS transition targeting an accelerated property for an element
that is composited outside the duration of the transition correctly interrupts the animation and jumps straight
to the target value.

* platform/win/TestExpectations:
* webanimations/accelerated-transition-interrupted-on-composited-element-expected.html: Added.
* webanimations/accelerated-transition-interrupted-on-composited-element.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/platform/win/TestExpectations
LayoutTests/webanimations/accelerated-transition-interrupted-on-composited-element-expected.html [new file with mode: 0644]
LayoutTests/webanimations/accelerated-transition-interrupted-on-composited-element.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/animation/KeyframeEffectReadOnly.cpp

index 1247196..e03cb26 100644 (file)
@@ -1,3 +1,19 @@
+2018-09-10  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Interrupting an accelerated CSS transition on a composited element in flight fails
+        https://bugs.webkit.org/show_bug.cgi?id=189405
+        <rdar://problem/43342639>
+
+        Reviewed by Simon Fraser.
+
+        Add a new test that checks that interrupting a CSS transition targeting an accelerated property for an element
+        that is composited outside the duration of the transition correctly interrupts the animation and jumps straight
+        to the target value.
+
+        * platform/win/TestExpectations:
+        * webanimations/accelerated-transition-interrupted-on-composited-element-expected.html: Added.
+        * webanimations/accelerated-transition-interrupted-on-composited-element.html: Added.
+
 2018-09-08  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         REGRESSION (r235153): [iOS] Can't move selection start grabber when selecting text in a subframe
index e8d4de9..2846a93 100644 (file)
@@ -4143,6 +4143,8 @@ webkit.org/b/188166 webanimations/accessing-current-time-after-clearing-css-anim
 webkit.org/b/188166 webanimations/setting-css-animation-none-after-clearing-effect.html [ Failure ]
 webkit.org/b/188166 webanimations/setting-css-animation-timing-property-via-style-after-clearing-effect.html [ Failure ]
 
+webkit.org/b/189468 webanimations/accelerated-transition-interrupted-on-composited-element.html [ Failure ]
+
 webkit.org/b/188167 fast/repaint/canvas-object-fit.html [ Failure ]
 
 webkit.org/b/188168 fast/text/complex-first-glyph-with-initial-advance.html [ ImageOnlyFailure ]
diff --git a/LayoutTests/webanimations/accelerated-transition-interrupted-on-composited-element-expected.html b/LayoutTests/webanimations/accelerated-transition-interrupted-on-composited-element-expected.html
new file mode 100644 (file)
index 0000000..4a2d2df
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<body>
+<style>
+div {
+    width: 100px;
+    height: 100px;
+    background-color: black;
+    will-change: transform;
+    transform: translateX(100px);
+}
+</style>
+<div></div>
+</body>
diff --git a/LayoutTests/webanimations/accelerated-transition-interrupted-on-composited-element.html b/LayoutTests/webanimations/accelerated-transition-interrupted-on-composited-element.html
new file mode 100644 (file)
index 0000000..b63caea
--- /dev/null
@@ -0,0 +1,32 @@
+<!DOCTYPE html><!-- webkit-test-runner [ enableWebAnimationsCSSIntegration=true ] -->
+<body>
+<style>
+div {
+    width: 100px;
+    height: 100px;
+    background-color: black;
+    will-change: transform;
+}
+</style>
+<script>
+
+if (window.testRunner)
+    testRunner.waitUntilDone();
+
+const div = document.body.appendChild(document.createElement("div"));
+
+// Wait for a transition to start and abort it.
+div.addEventListener("transitionstart", event => {
+    div.style.transition = "none";
+    if (window.testRunner)
+        requestAnimationFrame(() => testRunner.notifyDone());
+});
+
+// Initiate a transform transition.
+setTimeout(() => {
+    div.style.transition = "transform 2s";
+    div.style.transform = "translateX(100px)";
+});
+
+</script>
+</body>
index e46dbe3..7ce60b1 100644 (file)
@@ -1,3 +1,24 @@
+2018-09-10  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Interrupting an accelerated CSS transition on a composited element in flight fails
+        https://bugs.webkit.org/show_bug.cgi?id=189405
+        <rdar://problem/43342639>
+
+        Reviewed by Simon Fraser.
+
+        Test: webanimations/accelerated-transition-interrupted-on-composited-element.html
+
+        If we interrupt an animation on an element that is composited also outside of the duration of the animation,
+        the "stop" accelerated action would fail to be performed because we no longer had a resolved current time and
+        the accelerated animation applied to the layer would never be removed.
+
+        However, having a resolved current time is not necessary to stop an animation, only for the other types of
+        actions (play, pause and seek). So we now simply default to a 0s time for an unresolved current time for a
+        simple fix to this issue.
+
+        * animation/KeyframeEffectReadOnly.cpp:
+        (WebCore::KeyframeEffectReadOnly::applyPendingAcceleratedActions):
+
 2018-09-09  Fujii Hironori  <Hironori.Fujii@sony.com>
 
         Add specialized template declarations of HashTraits and DefaultHash to detect misuse
index 362dab0..135addd 100644 (file)
@@ -1278,11 +1278,8 @@ void KeyframeEffectReadOnly::applyPendingAcceleratedActions()
 
     auto* compositedRenderer = downcast<RenderBoxModelObject>(renderer);
 
-    auto currentTime = animation()->currentTime();
-    if (!currentTime)
-        return;
-
-    auto timeOffset = currentTime->seconds();
+    // To simplify the code we use a default of 0s for an unresolved current time since for a Stop action that is acceptable.
+    auto timeOffset = animation()->currentTime().value_or(0_s).seconds();
     if (timing()->delay() < 0_s)
         timeOffset = -timing()->delay().seconds();