Attachment 349144 Details for Bug 189405 – Patch

[patch] Patch

bug-189405-20180907155146.patch (text/plain), 5.23 KB, created by Antoine Quint on 2018-09-07 06:51:47 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antoine Quint

Created: 2018-09-07 06:51:47 PDT

Size: 5.23 KB

patch

obsolete

>Subversion Revision: 235781
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 68c6ab7b8e58805e4223fe6c4e009ddb9532807e..5a7ef30168c49a713cf10904aadba8f8a3b5cefe 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,24 @@
>+2018-09-07  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 NOBODY (OOPS!).
>+
>+        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-06  Ryosuke Niwa  <rniwa@webkit.org>
> 
>         ShadowRoot should have its own node flag
>diff --git a/Source/WebCore/animation/KeyframeEffectReadOnly.cpp b/Source/WebCore/animation/KeyframeEffectReadOnly.cpp
>index 362dab0602b460be3f1ad2213bf89c17e1258a6e..135adddaf53c8e7a47f0ec11767b655faaff8200 100644
>--- a/Source/WebCore/animation/KeyframeEffectReadOnly.cpp
>+++ b/Source/WebCore/animation/KeyframeEffectReadOnly.cpp
>@@ -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();
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index f5a27fd3cfbd5fc5573de8920579ce7e7066617d..6e47dfb8d1259cb49c13d2e01c675cb78d651c3f 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,18 @@
>+2018-09-07  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 NOBODY (OOPS!).
>+
>+        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.
>+
>+        * webanimations/accelerated-transition-interrupted-on-composited-element-expected.html: Added.
>+        * webanimations/accelerated-transition-interrupted-on-composited-element.html: Added.
>+
> 2018-09-06  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC] Add support for min/max-height percentage values.
>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
>index 0000000000000000000000000000000000000000..4a2d2dfe7f30d60a8fcd97c656997f5301084960
>--- /dev/null
>+++ b/LayoutTests/webanimations/accelerated-transition-interrupted-on-composited-element-expected.html
>@@ -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
>index 0000000000000000000000000000000000000000..b63caeaff5875b9d69538bd8ead2f22532f65a78
>--- /dev/null
>+++ b/LayoutTests/webanimations/accelerated-transition-interrupted-on-composited-element.html
>@@ -0,0 +1,32 @@
>+<!DOCTYPE html>
>+<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>

Flags:

simon.fraser: review+
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 189405: 349144 | 349146