Changeset 268809 in webkit

Timestamp:

Oct 21, 2020, 11:10:52 AM (6 years ago)

Author:

graouts@webkit.org

Message:

REGRESSION (r263729): transform transition doesn't restart
​https://bugs.webkit.org/show_bug.cgi?id=218011
Source/WebCore:

Reviewed by Dean Jackson.
<rdar://problem/70035130>

In the case of accelerated animations, style for the targeted element is not updated while the animation is
in flight since the animation is performed by Core Animation. This means that by the time the "transitionend"
event is fired, a transition has not yet performed style updates for the element that would have already been
performed for a software animations and thus the updates to the completed and running transition maps performed
under AnimationTimeline::updateCSSTransitionsForStyleableAndProperty() may be out of sync with the assumptions
made in DocumentTimelinesController::updateAnimationsAndSendEvents() that a transition newly marked as finished
should be added to the list of completed transitions.

Indeed, in the newly-added test, if a style update is forced during the "transitionend" event listener before also
clearing the transition styles, two style updates, and thus two calls to updateCSSTransitionsForStyleableAndProperty(),
will have been performed by the time updateAnimationsAndSendEvents() runs again and collects completed transitions,
during which time the transition in question will have been removed from the list of running transitions and added
to the list of completed transitions (first update) and subsequently removed from the list of completed transitions
(second update). At this point, setting styles that would start a new transition for this property will not yield a
transition since we won't be able to satisfy the requirement that the new target value does not match that of the
completed transition.

In this change we stop assuming that just because updateAnimationsAndSendEvents() sees a transition as finished for
the first time it implies that the transition is still considered a running transition. As such we only add a transition
to the list of completed transitions should it be in the list of running transitions.

Test: webanimations/transition-restart-after-style-recalc-during-transitionend.html

• animation/DocumentTimeline.cpp:

(WebCore::DocumentTimeline::transitionDidComplete):

LayoutTests:

<rdar://problem/70035130>

Reviewed by Dean Jackson.

Add a new test that checks that forcing a style update during dispatch of the "transitionend" event
does not prevent the completed transition from starting again if reset.

• webanimations/transition-restart-after-style-recalc-during-transitionend-expected.txt: Added.
• webanimations/transition-restart-after-style-recalc-during-transitionend.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/webanimations/transition-restart-after-style-recalc-during-transitionend-expected.txt (added)
• LayoutTests/webanimations/transition-restart-after-style-recalc-during-transitionend.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/animation/DocumentTimeline.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2020-10-21  Antoine Quint  <graouts@webkit.org>
+
+        REGRESSION (r263729): transform transition doesn't restart
+        https://bugs.webkit.org/show_bug.cgi?id=218011
+        <rdar://problem/70035130>
+
+        Reviewed by Dean Jackson.
+
+        Add a new test that checks that forcing a style update during dispatch of the "transitionend" event
+        does not prevent the completed transition from starting again if reset.
+
+        * webanimations/transition-restart-after-style-recalc-during-transitionend-expected.txt: Added.
+        * webanimations/transition-restart-after-style-recalc-during-transitionend.html: Added.
+
 2020-10-21  Antoine Quint  <graouts@webkit.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-10-21  Antoine Quint  <graouts@webkit.org>
+
+        REGRESSION (r263729): transform transition doesn't restart
+        https://bugs.webkit.org/show_bug.cgi?id=218011
+
+        Reviewed by Dean Jackson.
+        <rdar://problem/70035130>
+
+        In the case of accelerated animations, style for the targeted element is not updated while the animation is
+        in flight since the animation is performed by Core Animation. This means that by the time the "transitionend"
+        event is fired, a transition has not yet performed style updates for the element that would have already been
+        performed for a software animations and thus the updates to the completed and running transition maps performed
+        under AnimationTimeline::updateCSSTransitionsForStyleableAndProperty() may be out of sync with the assumptions
+        made in DocumentTimelinesController::updateAnimationsAndSendEvents() that a transition newly marked as finished
+        should be added to the list of completed transitions.
+        
+        Indeed, in the newly-added test, if a style update is forced during the "transitionend" event listener before also
+        clearing the transition styles, two style updates, and thus two calls to updateCSSTransitionsForStyleableAndProperty(),
+        will have been performed by the time updateAnimationsAndSendEvents() runs again and collects completed transitions,
+        during which time the transition in question will have been removed from the list of running transitions and added
+        to the list of completed transitions (first update) and subsequently removed from the list of completed transitions
+        (second update). At this point, setting styles that would start a new transition for this property will not yield a
+        transition since we won't be able to satisfy the requirement that the new target value does not match that of the
+        completed transition.
+
+        In this change we stop assuming that just because updateAnimationsAndSendEvents() sees a transition as finished for
+        the first time it implies that the transition is still considered a running transition. As such we only add a transition
+        to the list of completed transitions should it be in the list of running transitions.
+
+        Test: webanimations/transition-restart-after-style-recalc-during-transitionend.html
+
+        * animation/DocumentTimeline.cpp:
+        (WebCore::DocumentTimeline::transitionDidComplete):
+
 2020-10-21  Antoine Quint  <graouts@webkit.org>
 

trunk/Source/WebCore/animation/DocumentTimeline.cpp

@@ -298 +298 @@
     removeAnimation(*transition);
     if (is<KeyframeEffect>(transition->effect())) {
-        if (auto styleable = downcast<KeyframeEffect>(transition->effect())->targetStyleable())
-            styleable->ensureCompletedTransitionsByProperty().set(transition->property(), transition);
+        if (auto styleable = downcast<KeyframeEffect>(transition->effect())->targetStyleable()) {
+            auto property = transition->property();
+            if (styleable->hasRunningTransitionForProperty(property))
+                styleable->ensureCompletedTransitionsByProperty().set(property, transition);
+        }
     }
 }