Changeset 217997 in webkit

Timestamp:

Jun 9, 2017 10:52:42 AM (7 years ago)

Author:

Chris Dumez

Message:

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.

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/animation/css-animation-resuming-when-visible-expected.txt (added)
• LayoutTests/fast/animation/css-animation-resuming-when-visible.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/animation/AnimationBase.h (modified) (1 diff)
• Source/WebCore/page/animation/CompositeAnimation.cpp (modified) (4 diffs)
• Source/WebCore/page/animation/ImplicitAnimation.cpp (modified) (4 diffs)
• Source/WebCore/page/animation/ImplicitAnimation.h (modified) (1 diff)
• Source/WebCore/page/animation/KeyframeAnimation.cpp (modified) (4 diffs)
• Source/WebCore/page/animation/KeyframeAnimation.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/page/animation/AnimationBase.h

@@ -135 +135 @@
 
     // 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;
 

trunk/Source/WebCore/page/animation/CompositeAnimation.cpp

@@ -95 +95 @@
         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();
@@ -170 +170 @@
                     // 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));
@@ -301 +304 @@
         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;
 
@@ -331 +334 @@
         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;
 

trunk/Source/WebCore/page/animation/ImplicitAnimation.cpp

@@ -62 +62 @@
 }
 
-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.
@@ -73 +73 @@
     // 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());
@@ -200 +200 @@
 }
 
-void ImplicitAnimation::reset(const RenderStyle* to)
-{
-    ASSERT(to);
+void ImplicitAnimation::reset(const RenderStyle& to, CompositeAnimation& compositeAnimation)
+{
     ASSERT(m_fromStyle);
 
-    m_toStyle = RenderStyle::clonePtr(*to);
+    m_toStyle = RenderStyle::clonePtr(to);
 
     if (m_object && m_object->element())
@@ -212 +211 @@
     // 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

trunk/Source/WebCore/page/animation/ImplicitAnimation.h

@@ -54 +54 @@
     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;

trunk/Source/WebCore/page/animation/KeyframeAnimation.cpp

@@ -155 +155 @@
 }
 
-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
@@ -162 +162 @@
     // 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)
@@ -172 +172 @@
     if (postActive()) {
         if (!animatedStyle)
-            animatedStyle = RenderStyle::clonePtr(*targetStyle);
+            animatedStyle = RenderStyle::clonePtr(targetStyle);
         return false;
     }
@@ -196 +196 @@
     // 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.

trunk/Source/WebCore/page/animation/KeyframeAnimation.h

@@ -45 +45 @@
     }
 
-    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;