Changeset 209675 in webkit

Timestamp:

Dec 10, 2016 2:29:24 PM (7 years ago)

Author:

Simon Fraser

Message:

Animation followed by transition doesn't always fire transitionend event
​https://bugs.webkit.org/show_bug.cgi?id=165731
rdar://problem/28471240

Reviewed by Zalan Bujtas.
Source/WebCore:

After r200047, a keyframe animation of an accelerated property followed by a
transition didn't always fire a transitionend event.

This happened if CompositeAnimation::timeToNextService() happend to be called
when the transitions's timeToNextService() returned a positive value, but the
keyframe animation still existed, but its timeToNextService() returned -1. In
this case that -1 would clobber the positing minT.

Fix by just continuing in each loop when the timeToNextService() returns -1.

This code should probably be rewritten to use std::optional<double> rather than
magic values.

Test: animations/animation-followed-by-transition.html

• page/animation/CompositeAnimation.cpp:

(WebCore::CompositeAnimation::timeToNextService):

• platform/graphics/ca/GraphicsLayerCA.cpp:

(WebCore::GraphicsLayerCA::addAnimation):
(WebCore::GraphicsLayerCA::pauseAnimation):
(WebCore::GraphicsLayerCA::removeAnimation):
(WebCore::GraphicsLayerCA::platformCALayerAnimationStarted):
(WebCore::GraphicsLayerCA::platformCALayerAnimationEnded):

LayoutTests:

• animations/animation-followed-by-transition-expected.txt: Added.
• animations/animation-followed-by-transition.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/animations/animation-followed-by-transition-expected.txt (added)
• LayoutTests/animations/animation-followed-by-transition.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/animation/CompositeAnimation.cpp (modified) (2 diffs)
• Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp (modified) (5 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-12-10  Simon Fraser  <simon.fraser@apple.com>
+
+        Animation followed by transition doesn't always fire transitionend event
+        https://bugs.webkit.org/show_bug.cgi?id=165731
+        rdar://problem/28471240
+
+        Reviewed by Zalan Bujtas.
+
+        * animations/animation-followed-by-transition-expected.txt: Added.
+        * animations/animation-followed-by-transition.html: Added.
+
 2016-12-09  Sam Weinig  <sam@webkit.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-12-10  Simon Fraser  <simon.fraser@apple.com>
+
+        Animation followed by transition doesn't always fire transitionend event
+        https://bugs.webkit.org/show_bug.cgi?id=165731
+        rdar://problem/28471240
+
+        Reviewed by Zalan Bujtas.
+        
+        After r200047, a keyframe animation of an accelerated property followed by a
+        transition didn't always fire a transitionend event.
+        
+        This happened if CompositeAnimation::timeToNextService() happend to be called
+        when the transitions's timeToNextService() returned a positive value, but the
+        keyframe animation still existed, but its timeToNextService() returned -1. In
+        this case that -1 would clobber the positing minT.
+
+        Fix by just continuing in each loop when the timeToNextService() returns -1.
+
+        This code should probably be rewritten to use std::optional<double> rather than
+        magic values.
+
+        Test: animations/animation-followed-by-transition.html
+
+        * page/animation/CompositeAnimation.cpp:
+        (WebCore::CompositeAnimation::timeToNextService):
+        * platform/graphics/ca/GraphicsLayerCA.cpp:
+        (WebCore::GraphicsLayerCA::addAnimation):
+        (WebCore::GraphicsLayerCA::pauseAnimation):
+        (WebCore::GraphicsLayerCA::removeAnimation):
+        (WebCore::GraphicsLayerCA::platformCALayerAnimationStarted):
+        (WebCore::GraphicsLayerCA::platformCALayerAnimationEnded):
+
 2016-12-10  Sam Weinig  <sam@webkit.org>
 

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

@@ -376 +376 @@
         for (auto& transition : m_transitions.values()) {
             double t = transition->timeToNextService();
+            if (t == -1)
+                continue;
             if (t < minT || minT == -1)
                 minT = t;
@@ -386 +388 @@
         for (auto& animation : m_keyframeAnimations.values()) {
             double t = animation->timeToNextService();
+            if (t == -1)
+                continue;
             if (t < minT || minT == -1)
                 minT = t;

trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp

@@ -937 +937 @@
 bool GraphicsLayerCA::addAnimation(const KeyframeValueList& valueList, const FloatSize& boxSize, const Animation* anim, const String& animationName, double timeOffset)
 {
+    LOG(Animations, "GraphicsLayerCA %p addAnimation %s (can be accelerated %d)", this, animationName.utf8().data(), animationCanBeAccelerated(valueList, anim));
+
     ASSERT(!animationName.isEmpty());
 
@@ -966 +968 @@
 void GraphicsLayerCA::pauseAnimation(const String& animationName, double timeOffset)
 {
+    LOG(Animations, "GraphicsLayerCA %p pauseAnimation %s (running %d)", this, animationName.utf8().data(), animationIsRunning(animationName));
+
     if (!animationIsRunning(animationName))
         return;
@@ -983 +987 @@
 void GraphicsLayerCA::removeAnimation(const String& animationName)
 {
+    LOG(Animations, "GraphicsLayerCA %p removeAnimation %s (running %d)", this, animationName.utf8().data(), animationIsRunning(animationName));
+
     if (!animationIsRunning(animationName))
         return;
@@ -992 +998 @@
 void GraphicsLayerCA::platformCALayerAnimationStarted(const String& animationKey, CFTimeInterval startTime)
 {
+    LOG(Animations, "GraphicsLayerCA %p platformCALayerAnimationStarted %s at %f", this, animationKey.utf8().data(), startTime);
     client().notifyAnimationStarted(this, animationKey, startTime);
 }
@@ -997 +1004 @@
 void GraphicsLayerCA::platformCALayerAnimationEnded(const String& animationKey)
 {
+    LOG(Animations, "GraphicsLayerCA %p platformCALayerAnimationEnded %s", this, animationKey.utf8().data());
     client().notifyAnimationEnded(this, animationKey);
 }