Title: [268809] trunk
- Revision
- 268809
- Author
- grao...@webkit.org
- Date
- 2020-10-21 11:10:52 -0700 (Wed, 21 Oct 2020)
Log 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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (268808 => 268809)
--- trunk/LayoutTests/ChangeLog 2020-10-21 18:09:02 UTC (rev 268808)
+++ trunk/LayoutTests/ChangeLog 2020-10-21 18:10:52 UTC (rev 268809)
@@ -1,5 +1,19 @@
2020-10-21 Antoine Quint <grao...@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 <grao...@webkit.org>
+
[WK1] webanimations/accelerated-overlapping-transform-animations.html is a failure
https://bugs.webkit.org/show_bug.cgi?id=217997
<rdar://problem/70505533>
Added: trunk/LayoutTests/webanimations/transition-restart-after-style-recalc-during-transitionend-expected.txt (0 => 268809)
--- trunk/LayoutTests/webanimations/transition-restart-after-style-recalc-during-transitionend-expected.txt (rev 0)
+++ trunk/LayoutTests/webanimations/transition-restart-after-style-recalc-during-transitionend-expected.txt 2020-10-21 18:10:52 UTC (rev 268809)
@@ -0,0 +1,3 @@
+
+PASS Forcing a style update in a transitionend event for an accelerated animation should not prevent another transition for the same property from running later.
+
Added: trunk/LayoutTests/webanimations/transition-restart-after-style-recalc-during-transitionend.html (0 => 268809)
--- trunk/LayoutTests/webanimations/transition-restart-after-style-recalc-during-transitionend.html (rev 0)
+++ trunk/LayoutTests/webanimations/transition-restart-after-style-recalc-during-transitionend.html 2020-10-21 18:10:52 UTC (rev 268809)
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>Forcing a style update in a transitionend event for an accelerated animation should not prevent another transition for the same property from running later</title>
+<body>
+<script src=""
+<script src=""
+<script src=""
+<script>
+
+'use strict';
+
+promise_test(async t => {
+ const target = document.body.appendChild(document.createElement("div"));
+ const animatedStyle = "transition: transform 1ms linear; transform: translateX(100px)";
+
+ // Start a transform transition.
+ getComputedStyle(target).transform;
+ target.setAttribute("style", animatedStyle);
+
+ // Wait until it completes, and in the "transitionend" event handler, force a style recalc
+ // before removing the transition style.
+ await new Promise(resolve => {
+ target.addEventListener("transitionend", event => {
+ target.getAnimations();
+ target.removeAttribute("style");
+ resolve();
+ }, { once: true });
+ });
+
+ // Wait until the accelerated animation has completed.
+ await UIHelper.ensurePresentationUpdate();
+
+ // Start another transform transition.
+ getComputedStyle(target).transform;
+ target.setAttribute("style", animatedStyle);
+
+ // Check that it starts and ends.
+ await new Promise(resolve => target.addEventListener("transitionend", resolve, { once: true }));
+}, "Forcing a style update in a transitionend event for an accelerated animation should not prevent another transition for the same property from running later.");
+
+</script>
+</body>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (268808 => 268809)
--- trunk/Source/WebCore/ChangeLog 2020-10-21 18:09:02 UTC (rev 268808)
+++ trunk/Source/WebCore/ChangeLog 2020-10-21 18:10:52 UTC (rev 268809)
@@ -1,5 +1,39 @@
2020-10-21 Antoine Quint <grao...@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 <grao...@webkit.org>
+
Rename hasRunningTransitionsForProperty() and hasCompletedTransitionsForProperty() to be singular
https://bugs.webkit.org/show_bug.cgi?id=218014
Modified: trunk/Source/WebCore/animation/DocumentTimeline.cpp (268808 => 268809)
--- trunk/Source/WebCore/animation/DocumentTimeline.cpp 2020-10-21 18:09:02 UTC (rev 268808)
+++ trunk/Source/WebCore/animation/DocumentTimeline.cpp 2020-10-21 18:10:52 UTC (rev 268809)
@@ -297,8 +297,11 @@
ASSERT(transition);
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);
+ }
}
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
