CSS transitions added while page is not visible do not start when the page becomes...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 9 Jun 2017 17:52:42 +0000 (17:52 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 9 Jun 2017 17:52:42 +0000 (17:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=173166
<rdar://problem/32250351>

Reviewed by Darin Adler.

Source/WebCore:

CSS transitions added while page is not visible would not start when the page becomes
visible. The issue was that when CompositeAnimation::updateTransitions() was called
while the page is hidden (and animations are therefore suspended), it would not
populate m_transations with ImplicitAnimation objects. We would therefore have later
no transitions to resume when the page becomes visible later on. This patch updates
CompositeAnimation::updateTransitions() to properly populate m_transitions and instead
pause the ImplicitAnimation it creates if animations are currently suspended. This
behavior is more consistent with the one of CompositeAnimation::updateKeyframeAnimations().

I also needed to update ImplicitAnimation::animate() to not restart a paused animation
if the animation is currently paused. This is similar to what is done in
KeyframeAnimation::animate(). Without this, the paused ImplicitAnimation we add to
m_transition would incorrectly get unpaused while the page is still hidden and the
animations are still supposed to be suspended. This issue was showing when running the
test I am adding in this patch.

Test: fast/animation/css-animation-resuming-when-visible.html

* page/animation/CompositeAnimation.cpp:
(WebCore::CompositeAnimation::updateTransitions):
* page/animation/ImplicitAnimation.cpp:
(WebCore::ImplicitAnimation::animate):
(WebCore::ImplicitAnimation::reset):
* page/animation/ImplicitAnimation.h:

LayoutTests:

Add layout test coverage.

* fast/animation/css-animation-resuming-when-visible-expected.txt: Added.
* fast/animation/css-animation-resuming-when-visible.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/animation/css-animation-resuming-when-visible-expected.txt [new file with mode: 0644]
LayoutTests/fast/animation/css-animation-resuming-when-visible.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/animation/AnimationBase.h
Source/WebCore/page/animation/CompositeAnimation.cpp
Source/WebCore/page/animation/ImplicitAnimation.cpp
Source/WebCore/page/animation/ImplicitAnimation.h
Source/WebCore/page/animation/KeyframeAnimation.cpp
Source/WebCore/page/animation/KeyframeAnimation.h

index 9c2bfdb..258eff0 100644 (file)
@@ -1,3 +1,16 @@
+2017-06-09  Chris Dumez  <cdumez@apple.com>
+
+        CSS transitions added while page is not visible do not start when the page becomes visible
+        https://bugs.webkit.org/show_bug.cgi?id=173166
+        <rdar://problem/32250351>
+
+        Reviewed by Darin Adler.
+
+        Add layout test coverage.
+
+        * fast/animation/css-animation-resuming-when-visible-expected.txt: Added.
+        * fast/animation/css-animation-resuming-when-visible.html: Added.
+
 2017-06-09  Eric Carlson  <eric.carlson@apple.com>
 
         fast/mediastream/MediaStream-page-muted.html times out and asserts
diff --git a/LayoutTests/fast/animation/css-animation-resuming-when-visible-expected.txt b/LayoutTests/fast/animation/css-animation-resuming-when-visible-expected.txt
new file mode 100644 (file)
index 0000000..a524bd0
--- /dev/null
@@ -0,0 +1,12 @@
+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 true
+PASS internals.numberOfActiveAnimations() is 0
+PASS internals.numberOfActiveAnimations() became 1
+PASS successfullyParsed is true
+
+TEST COMPLETE
+TEST
diff --git a/LayoutTests/fast/animation/css-animation-resuming-when-visible.html b/LayoutTests/fast/animation/css-animation-resuming-when-visible.html
new file mode 100644 (file)
index 0000000..330bf84
--- /dev/null
@@ -0,0 +1,40 @@
+<!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() {
+    internals.suspendAnimations();
+
+    setTimeout(function() {
+        registerAnimation();
+        setTimeout(function() {
+            shouldBeTrue("internals.animationsAreSuspended()");
+            shouldBe("internals.numberOfActiveAnimations()", "0");
+
+            internals.resumeAnimations();
+            shouldBecomeEqual("internals.numberOfActiveAnimations()", "1", finishJSTest);
+        }, 500);
+    }, 0);
+}
+
+</script>
+</body>
+</html>
index 2f2fddd..706e1eb 100644 (file)
@@ -1,3 +1,36 @@
+2017-06-09  Chris Dumez  <cdumez@apple.com>
+
+        CSS transitions added while page is not visible do not start when the page becomes visible
+        https://bugs.webkit.org/show_bug.cgi?id=173166
+        <rdar://problem/32250351>
+
+        Reviewed by Darin Adler.
+
+        CSS transitions added while page is not visible would not start when the page becomes
+        visible. The issue was that when CompositeAnimation::updateTransitions() was called
+        while the page is hidden (and animations are therefore suspended), it would not
+        populate m_transations with ImplicitAnimation objects. We would therefore have later
+        no transitions to resume when the page becomes visible later on. This patch updates
+        CompositeAnimation::updateTransitions() to properly populate m_transitions and instead
+        pause the ImplicitAnimation it creates if animations are currently suspended. This
+        behavior is more consistent with the one of CompositeAnimation::updateKeyframeAnimations().
+
+        I also needed to update ImplicitAnimation::animate() to not restart a paused animation
+        if the animation is currently paused. This is similar to what is done in
+        KeyframeAnimation::animate(). Without this, the paused ImplicitAnimation we add to
+        m_transition would incorrectly get unpaused while the page is still hidden and the
+        animations are still supposed to be suspended. This issue was showing when running the
+        test I am adding in this patch.
+
+        Test: fast/animation/css-animation-resuming-when-visible.html
+
+        * page/animation/CompositeAnimation.cpp:
+        (WebCore::CompositeAnimation::updateTransitions):
+        * page/animation/ImplicitAnimation.cpp:
+        (WebCore::ImplicitAnimation::animate):
+        (WebCore::ImplicitAnimation::reset):
+        * page/animation/ImplicitAnimation.h:
+
 2017-06-09  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         Unreviewed, Use FALLTHROUGH
index cbeb748..3ab5cbf 100644 (file)
@@ -134,7 +134,7 @@ public:
     double progress(double scale = 1, double offset = 0, const TimingFunction* = nullptr) const;
 
     // Returns true if the animation state changed.
-    virtual bool animate(CompositeAnimation*, RenderElement*, const RenderStyle* /*currentStyle*/, const RenderStyle* /*targetStyle*/, std::unique_ptr<RenderStyle>& /*animatedStyle*/, bool& didBlendStyle) = 0;
+    virtual bool animate(CompositeAnimation&, RenderElement*, const RenderStyle* /*currentStyle*/, const RenderStyle& /*targetStyle*/, std::unique_ptr<RenderStyle>& /*animatedStyle*/, bool& didBlendStyle) = 0;
     virtual void getAnimatedStyle(std::unique_ptr<RenderStyle>& /*animatedStyle*/) = 0;
 
     virtual bool computeExtentOfTransformAnimation(LayoutRect&) const = 0;
index 30b7b3b..d8e1b2d 100644 (file)
@@ -94,7 +94,7 @@ void CompositeAnimation::updateTransitions(RenderElement* renderer, const Render
     if (targetStyle->transitions()) {
         for (size_t i = 0; i < targetStyle->transitions()->size(); ++i) {
             auto& animation = targetStyle->transitions()->animation(i);
-            bool isActiveTransition = !m_suspended && (animation.duration() || animation.delay() > 0);
+            bool isActiveTransition = animation.duration() || animation.delay() > 0;
 
             Animation::AnimationMode mode = animation.animationMode();
             if (mode == Animation::AnimateNone || mode == Animation::AnimateUnknownProperty)
@@ -169,6 +169,9 @@ void CompositeAnimation::updateTransitions(RenderElement* renderer, const Render
                 if (!equal && isActiveTransition) {
                     // Add the new transition
                     auto implicitAnimation = ImplicitAnimation::create(animation, prop, renderer, this, modifiedCurrentStyle ? modifiedCurrentStyle.get() : fromStyle);
+                    if (m_suspended && implicitAnimation->hasStyle())
+                        implicitAnimation->updatePlayState(AnimPlayStatePaused);
+
                     LOG(Animations, "Created ImplicitAnimation %p on renderer %p for property %s duration %.2f delay %.2f", implicitAnimation.ptr(), renderer, getPropertyName(prop), animation.duration(), animation.delay());
                     m_transitions.set(prop, WTFMove(implicitAnimation));
                 }
@@ -300,7 +303,7 @@ bool CompositeAnimation::animate(RenderElement& renderer, const RenderStyle* cur
         bool checkForStackingContext = false;
         for (auto& transition : m_transitions.values()) {
             bool didBlendStyle = false;
-            if (transition->animate(this, &renderer, currentStyle, &targetStyle, blendedStyle, didBlendStyle))
+            if (transition->animate(*this, &renderer, currentStyle, targetStyle, blendedStyle, didBlendStyle))
                 animationStateChanged = true;
 
             if (didBlendStyle)
@@ -330,7 +333,7 @@ bool CompositeAnimation::animate(RenderElement& renderer, const RenderStyle* cur
         RefPtr<KeyframeAnimation> keyframeAnim = m_keyframeAnimations.get(name);
         if (keyframeAnim) {
             bool didBlendStyle = false;
-            if (keyframeAnim->animate(this, &renderer, currentStyle, &targetStyle, blendedStyle, didBlendStyle))
+            if (keyframeAnim->animate(*this, &renderer, currentStyle, targetStyle, blendedStyle, didBlendStyle))
                 animationStateChanged = true;
 
             forceStackingContext |= didBlendStyle && keyframeAnim->triggersStackingContext();
index 13976af..f7f401a 100644 (file)
@@ -61,7 +61,7 @@ bool ImplicitAnimation::shouldSendEventForListener(Document::ListenerType inList
     return m_object->document().hasListenerType(inListenerType);
 }
 
-bool ImplicitAnimation::animate(CompositeAnimation*, RenderElement*, const RenderStyle*, const RenderStyle* targetStyle, std::unique_ptr<RenderStyle>& animatedStyle, bool& didBlendStyle)
+bool ImplicitAnimation::animate(CompositeAnimation& compositeAnimation, RenderElement*, const RenderStyle*, const RenderStyle& targetStyle, std::unique_ptr<RenderStyle>& animatedStyle, bool& didBlendStyle)
 {
     // If we get this far and the animation is done, it means we are cleaning up a just finished animation.
     // So just return. Everything is already all cleaned up.
@@ -72,12 +72,12 @@ bool ImplicitAnimation::animate(CompositeAnimation*, RenderElement*, const Rende
 
     // Reset to start the transition if we are new
     if (isNew())
-        reset(targetStyle);
+        reset(targetStyle, compositeAnimation);
 
     // Run a cycle of animation.
     // We know we will need a new render style, so make one if needed
     if (!animatedStyle)
-        animatedStyle = RenderStyle::clonePtr(*targetStyle);
+        animatedStyle = RenderStyle::clonePtr(targetStyle);
 
     CSSPropertyAnimation::blendProperties(this, m_animatingProperty, animatedStyle.get(), m_fromStyle.get(), m_toStyle.get(), progress());
     // FIXME: we also need to detect cases where we have to software animate for other reasons,
@@ -199,19 +199,18 @@ bool ImplicitAnimation::sendTransitionEvent(const AtomicString& eventType, doubl
     return false; // Didn't dispatch an event
 }
 
-void ImplicitAnimation::reset(const RenderStyle* to)
+void ImplicitAnimation::reset(const RenderStyle& to, CompositeAnimation& compositeAnimation)
 {
-    ASSERT(to);
     ASSERT(m_fromStyle);
 
-    m_toStyle = RenderStyle::clonePtr(*to);
+    m_toStyle = RenderStyle::clonePtr(to);
 
     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(AnimationStateInput::RestartAnimation, -1);
+        updateStateMachine(compositeAnimation.isSuspended() ? AnimationStateInput::PlayStatePaused : AnimationStateInput::RestartAnimation, -1);
         
     // set the transform animation list
     validateTransformFunctionList();
index 8e17195..0f061c5 100644 (file)
@@ -53,9 +53,9 @@ public:
     void pauseAnimation(double timeOffset) override;
     void endAnimation() override;
 
-    bool animate(CompositeAnimation*, RenderElement*, const RenderStyle* currentStyle, const RenderStyle* targetStyle, std::unique_ptr<RenderStyle>& animatedStyle, bool& didBlendStyle) override;
+    bool animate(CompositeAnimation&, RenderElement*, const RenderStyle* currentStyle, const RenderStyle& targetStyle, std::unique_ptr<RenderStyle>& animatedStyle, bool& didBlendStyle) override;
     void getAnimatedStyle(std::unique_ptr<RenderStyle>& animatedStyle) override;
-    void reset(const RenderStyle* to);
+    void reset(const RenderStyle& to, CompositeAnimation&);
 
     bool computeExtentOfTransformAnimation(LayoutRect&) const override;
 
index 00f72a9..d42dcbb 100644 (file)
@@ -154,14 +154,14 @@ void KeyframeAnimation::fetchIntervalEndpointsForProperty(CSSPropertyID property
     prog = progress(scale, offset, prevKeyframe.timingFunction(name()));
 }
 
-bool KeyframeAnimation::animate(CompositeAnimation* compositeAnimation, RenderElement*, const RenderStyle*, const RenderStyle* targetStyle, std::unique_ptr<RenderStyle>& animatedStyle, bool& didBlendStyle)
+bool KeyframeAnimation::animate(CompositeAnimation& compositeAnimation, RenderElement*, const RenderStyle*, const RenderStyle& targetStyle, std::unique_ptr<RenderStyle>& animatedStyle, bool& didBlendStyle)
 {
     // Fire the start timeout if needed
     fireAnimationEventsIfNeeded();
     
     // If we have not yet started, we will not have a valid start time, so just start the animation if needed.
     if (isNew()) {
-        if (m_animation->playState() == AnimPlayStatePlaying && !compositeAnimation->isSuspended())
+        if (m_animation->playState() == AnimPlayStatePlaying && !compositeAnimation.isSuspended())
             updateStateMachine(AnimationStateInput::StartAnimation, -1);
         else if (m_animation->playState() == AnimPlayStatePaused)
             updateStateMachine(AnimationStateInput::PlayStatePaused, -1);
@@ -171,7 +171,7 @@ bool KeyframeAnimation::animate(CompositeAnimation* compositeAnimation, RenderEl
     // If so, we need to send back the targetStyle.
     if (postActive()) {
         if (!animatedStyle)
-            animatedStyle = RenderStyle::clonePtr(*targetStyle);
+            animatedStyle = RenderStyle::clonePtr(targetStyle);
         return false;
     }
 
@@ -195,7 +195,7 @@ bool KeyframeAnimation::animate(CompositeAnimation* compositeAnimation, RenderEl
     // Run a cycle of animation.
     // We know we will need a new render style, so make one if needed.
     if (!animatedStyle)
-        animatedStyle = RenderStyle::clonePtr(*targetStyle);
+        animatedStyle = RenderStyle::clonePtr(targetStyle);
 
     // FIXME: we need to be more efficient about determining which keyframes we are animating between.
     // We should cache the last pair or something.
index d220edf..06db899 100644 (file)
@@ -44,7 +44,7 @@ public:
         return adoptRef(*new KeyframeAnimation(animation, renderer, compositeAnimation, unanimatedStyle));
     }
 
-    bool animate(CompositeAnimation*, RenderElement*, const RenderStyle* currentStyle, const RenderStyle* targetStyle, std::unique_ptr<RenderStyle>& animatedStyle, bool& didBlendStyle) override;
+    bool animate(CompositeAnimation&, RenderElement*, const RenderStyle* currentStyle, const RenderStyle& targetStyle, std::unique_ptr<RenderStyle>& animatedStyle, bool& didBlendStyle) override;
     void getAnimatedStyle(std::unique_ptr<RenderStyle>&) override;
 
     bool computeExtentOfTransformAnimation(LayoutRect&) const override;