REGRESSION (r217997): mint.com header renders incorrectly when initially loaded
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Jun 2017 15:40:51 +0000 (15:40 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Jun 2017 15:40:51 +0000 (15:40 +0000)
https://bugs.webkit.org/show_bug.cgi?id=173302
<rdar://problem/32731747>

Reviewed by Darin Adler.

Source/WebCore:

r217997 updated ImplicitAnimation::reset() to not call updateStateMachine(AnimationStateInput::RestartAnimation)
if the compositeAnimation is suspended. If the compositeAnimation is suspended, we would call
updateStateMachine(AnimationStateInput::AnimationStateInput::PlayStatePaused), which was expected to be a no-op.
This was needed because otherwise, changing the style of the animated element would restart the animation
even though it was supposed to be suspended. One thing I did not realize is that calling
updateStateMachine(AnimationStateInput::AnimationStateInput::PlayStatePaused) on an animation that is already
in PausedNew state, will cause it to move to PausedWaitResponse state. This is an issue because upon resuming
we would call AnimationBase::updatePlayState(AnimPlayStatePlaying) which would return early because
AnimationBase::paused() would return false. To address the issue, we no longer call updateStateMachine(PlayStatePaused)
in ImplicitAnimation::reset() when the compositeAnimation is suspended, so that the animation stays in
PausedNew state until we resume. When we resume, AnimationBase::paused() returns false and we actually resume
the animation.

Tests:
fast/animation/css-animation-resuming-when-visible-with-style-change.html
fast/animation/css-animation-resuming-when-visible-with-style-change2.html

* page/animation/ImplicitAnimation.cpp:
(WebCore::ImplicitAnimation::reset):

LayoutTests:

Add layout test coverage. We change the style of the animated element while the animation is paused,
and then we resume the animations.

* fast/animation/css-animation-resuming-when-visible-with-style-change-expected.txt: Added.
* fast/animation/css-animation-resuming-when-visible-with-style-change.html: Added.
* fast/animation/css-animation-resuming-when-visible-with-style-change2-expected.txt: Added.
* fast/animation/css-animation-resuming-when-visible-with-style-change2.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/animation/css-animation-resuming-when-visible-with-style-change-expected.txt [new file with mode: 0644]
LayoutTests/fast/animation/css-animation-resuming-when-visible-with-style-change.html [new file with mode: 0644]
LayoutTests/fast/animation/css-animation-resuming-when-visible-with-style-change2-expected.txt [new file with mode: 0644]
LayoutTests/fast/animation/css-animation-resuming-when-visible-with-style-change2.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/animation/ImplicitAnimation.cpp

index c273538..3d49658 100644 (file)
@@ -1,3 +1,19 @@
+2017-06-14  Chris Dumez  <cdumez@apple.com>
+
+        REGRESSION (r217997): mint.com header renders incorrectly when initially loaded
+        https://bugs.webkit.org/show_bug.cgi?id=173302
+        <rdar://problem/32731747>
+
+        Reviewed by Darin Adler.
+
+        Add layout test coverage. We change the style of the animated element while the animation is paused,
+        and then we resume the animations.
+
+        * fast/animation/css-animation-resuming-when-visible-with-style-change-expected.txt: Added.
+        * fast/animation/css-animation-resuming-when-visible-with-style-change.html: Added.
+        * fast/animation/css-animation-resuming-when-visible-with-style-change2-expected.txt: Added.
+        * fast/animation/css-animation-resuming-when-visible-with-style-change2.html: Added.
+
 2017-06-14  Per Arne Vollan  <pvollan@apple.com>
 
         [Win] Update expectations for layout tests.
diff --git a/LayoutTests/fast/animation/css-animation-resuming-when-visible-with-style-change-expected.txt b/LayoutTests/fast/animation/css-animation-resuming-when-visible-with-style-change-expected.txt
new file mode 100644 (file)
index 0000000..bec7c30
--- /dev/null
@@ -0,0 +1,18 @@
+Tests that CSS animations that are created while the page is hidden are properly resumed when the page becomes visible.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Suspend animations
+PASS internals.animationsAreSuspended() is true
+PASS internals.numberOfActiveAnimations() is 0
+PASS internals.animationsAreSuspended() is true
+PASS internals.numberOfActiveAnimations() is 0
+PASS window.getComputedStyle(testDiv).transform is pausedTransform
+Resume animations
+PASS internals.numberOfActiveAnimations() became 1
+PASS window.getComputedStyle(testDiv).transform != pausedTransform became true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+TEST
diff --git a/LayoutTests/fast/animation/css-animation-resuming-when-visible-with-style-change.html b/LayoutTests/fast/animation/css-animation-resuming-when-visible-with-style-change.html
new file mode 100644 (file)
index 0000000..c682622
--- /dev/null
@@ -0,0 +1,59 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+#testDiv {
+    transition: transform 30s linear, color 2s, left 4s linear, top 4s linear;
+    position: absolute;
+}
+</style>
+</head>
+<body>
+<script src="../../resources/js-test.js"></script>
+<div id="testDiv">TEST</div>
+<script>
+description("Tests that CSS animations that are created while the page is hidden are properly resumed when the page becomes visible.");
+jsTestIsAsync = true;
+
+function registerAnimation()
+{
+    testDiv.style.transform = "rotate(170deg) scale(0.2781941414347284)";
+}
+
+function checkTransformAndFinishTest()
+{
+    shouldBecomeEqual("window.getComputedStyle(testDiv).transform != pausedTransform", "true", finishJSTest);
+}
+
+onload = function() {
+    debug("Suspend animations");
+    internals.suspendAnimations();
+
+    setTimeout(function() {
+        registerAnimation("170");
+        setTimeout(function() {
+            shouldBeTrue("internals.animationsAreSuspended()");
+            shouldBe("internals.numberOfActiveAnimations()", "0");
+
+            pausedTransform = window.getComputedStyle(testDiv).transform;
+
+            // Change style to make sure it does not make the animation active.
+            testDiv.style.backgroundColor = "green";
+            testDiv.offsetLeft;
+
+            shouldBeTrue("internals.animationsAreSuspended()");
+            shouldBe("internals.numberOfActiveAnimations()", "0");
+
+            setTimeout(function() {
+                shouldBe("window.getComputedStyle(testDiv).transform", "pausedTransform");
+
+                debug("Resume animations");
+                internals.resumeAnimations();
+                shouldBecomeEqual("internals.numberOfActiveAnimations()", "1", checkTransformAndFinishTest);
+            }, 50);
+        }, 500);
+    }, 0);
+}
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/animation/css-animation-resuming-when-visible-with-style-change2-expected.txt b/LayoutTests/fast/animation/css-animation-resuming-when-visible-with-style-change2-expected.txt
new file mode 100644 (file)
index 0000000..5c42594
--- /dev/null
@@ -0,0 +1,18 @@
+Tests that CSS animations that are created while the page is hidden are properly resumed when the page becomes visible.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS internals.animationsAreSuspended() is false
+PASS internals.numberOfActiveAnimations() is 1
+Suspend animations
+PASS internals.animationsAreSuspended() is true
+PASS internals.animationsAreSuspended() is true
+PASS internals.animationsAreSuspended() is true
+PASS window.getComputedStyle(testDiv).transform is pausedTransform
+Resume animations...
+PASS window.getComputedStyle(testDiv).transform != pausedTransform became true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+TEST
diff --git a/LayoutTests/fast/animation/css-animation-resuming-when-visible-with-style-change2.html b/LayoutTests/fast/animation/css-animation-resuming-when-visible-with-style-change2.html
new file mode 100644 (file)
index 0000000..fdf3889
--- /dev/null
@@ -0,0 +1,58 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+#testDiv {
+    transition: transform 30s linear, color 2s, left 4s linear, top 4s linear;
+    position: absolute;
+}
+</style>
+</head>
+<body>
+<script src="../../resources/js-test.js"></script>
+<div id="testDiv">TEST</div>
+<script>
+description("Tests that CSS animations that are created while the page is hidden are properly resumed when the page becomes visible.");
+jsTestIsAsync = true;
+
+function registerAnimation()
+{
+    testDiv.style.transform = "rotate(170deg) scale(0.2781941414347284)";
+}
+
+onload = function() {
+    setTimeout(function() {
+        registerAnimation("170");
+
+        setTimeout(function() {
+            shouldBeFalse("internals.animationsAreSuspended()");
+            shouldBe("internals.numberOfActiveAnimations()", "1");
+
+            debug("Suspend animations");
+            internals.suspendAnimations();
+            setTimeout(function() {
+                shouldBeTrue("internals.animationsAreSuspended()");
+
+                pausedTransform = window.getComputedStyle(testDiv).transform;
+
+                // Change style to make sure it does not make the animation active.
+                testDiv.style.backgroundColor = "green";
+                testDiv.offsetLeft;
+
+                shouldBeTrue("internals.animationsAreSuspended()");
+
+                setTimeout(function() {
+                    shouldBeTrue("internals.animationsAreSuspended()");
+                    shouldBe("window.getComputedStyle(testDiv).transform", "pausedTransform");
+
+                    debug("Resume animations...");
+                    internals.resumeAnimations();
+                    shouldBecomeEqual("window.getComputedStyle(testDiv).transform != pausedTransform", "true", finishJSTest);
+                }, 100);
+            }, 0);
+        }, 500);
+    }, 0);
+}
+</script>
+</body>
+</html>
index 0d56222..eef11f0 100644 (file)
@@ -1,3 +1,31 @@
+2017-06-14  Chris Dumez  <cdumez@apple.com>
+
+        REGRESSION (r217997): mint.com header renders incorrectly when initially loaded
+        https://bugs.webkit.org/show_bug.cgi?id=173302
+        <rdar://problem/32731747>
+
+        Reviewed by Darin Adler.
+
+        r217997 updated ImplicitAnimation::reset() to not call updateStateMachine(AnimationStateInput::RestartAnimation)
+        if the compositeAnimation is suspended. If the compositeAnimation is suspended, we would call
+        updateStateMachine(AnimationStateInput::AnimationStateInput::PlayStatePaused), which was expected to be a no-op.
+        This was needed because otherwise, changing the style of the animated element would restart the animation
+        even though it was supposed to be suspended. One thing I did not realize is that calling
+        updateStateMachine(AnimationStateInput::AnimationStateInput::PlayStatePaused) on an animation that is already
+        in PausedNew state, will cause it to move to PausedWaitResponse state. This is an issue because upon resuming
+        we would call AnimationBase::updatePlayState(AnimPlayStatePlaying) which would return early because
+        AnimationBase::paused() would return false. To address the issue, we no longer call updateStateMachine(PlayStatePaused)
+        in ImplicitAnimation::reset() when the compositeAnimation is suspended, so that the animation stays in
+        PausedNew state until we resume. When we resume, AnimationBase::paused() returns false and we actually resume
+        the animation.
+
+        Tests:
+        fast/animation/css-animation-resuming-when-visible-with-style-change.html
+        fast/animation/css-animation-resuming-when-visible-with-style-change2.html
+
+        * page/animation/ImplicitAnimation.cpp:
+        (WebCore::ImplicitAnimation::reset):
+
 2017-06-14  Miguel Gomez  <magomez@igalia.com>
 
         REGRESSION(r216901): ImageDecoders: rendering of large images is broken since r216901
index f7f401a..df6ce29 100644 (file)
@@ -208,11 +208,11 @@ void ImplicitAnimation::reset(const RenderStyle& to, CompositeAnimation& composi
     if (m_object && m_object->element())
         Style::loadPendingResources(*m_toStyle, m_object->element()->document(), m_object->element());
 
-    // Restart the transition
-    if (m_fromStyle && m_toStyle)
-        updateStateMachine(compositeAnimation.isSuspended() ? AnimationStateInput::PlayStatePaused : AnimationStateInput::RestartAnimation, -1);
-        
-    // set the transform animation list
+    // Restart the transition.
+    if (m_fromStyle && m_toStyle && !compositeAnimation.isSuspended())
+        updateStateMachine(AnimationStateInput::RestartAnimation, -1);
+
+    // Set the transform animation list.
     validateTransformFunctionList();
     checkForMatchingFilterFunctionLists();
 #if ENABLE(FILTERS_LEVEL_2)