- Revision
- 236308
- Author
- [email protected]
- Date
- 2018-09-21 04:28:40 -0700 (Fri, 21 Sep 2018)
Log Message
[Web Animations] DocumentTimeline::updateAnimations() is called endlessly
https://bugs.webkit.org/show_bug.cgi?id=189784
<rdar://problem/41705679>
Reviewed by Dean Jackson.
Source/WebCore:
Test: webanimations/accelerated-animation-interruption-display-none.html
We have code that keeps queueing pending accelerated actions for an animation that does not have a renderer until it has one
so that we can deal with situations where animations are ready to commited before its composited renderer is available. This
code ended up running continuously when an element with an accelerated animation had its renderer removed without the animation
being removed itself, such as setting "display: none" on an element with an acceelerated CSS Animation targeting it.
We fix this by queueing up a "Stop" accelerated action when updating the accelerated state if there is no renderer for the current
animation target. Then, we no longer re-queue pending accelerated actions if the last queued operation is "Stop". This ensures that
we no longer queue actions endlessly when there is no longer a visible animation.
To test this, we add a new internals.numberOfAnimationTimelineInvalidations() method that indicates the number of times the current
document's animation timeline was invalidated.
* animation/DocumentTimeline.cpp:
(WebCore::DocumentTimeline::updateAnimations):
(WebCore::DocumentTimeline::numberOfAnimationTimelineInvalidationsForTesting const):
* animation/DocumentTimeline.h:
* animation/KeyframeEffectReadOnly.cpp:
(WebCore::KeyframeEffectReadOnly::updateAcceleratedAnimationState): If the animation target does not have a renderer and it's still
marked as running, enqueue a "Stop" accelerated action.
(WebCore::KeyframeEffectReadOnly::addPendingAcceleratedAction): If we enqueue a "Stop" accelerated action, remove any other queued
action so that we only process the "Stop" action, which would have superseded all previously queued actions anyway.
(WebCore::KeyframeEffectReadOnly::applyPendingAcceleratedActions): Only re-queue pending accelerated actions when a composited renderer
is not yet available if we don't have a "Stop" action queued.
* testing/Internals.cpp:
(WebCore::Internals::numberOfAnimationTimelineInvalidations const):
* testing/Internals.h:
* testing/Internals.idl:
LayoutTests:
Add a new test that checks that setting "display: none" on an element with an accelerated CSS animation on it
will no longer update the animation timeline.
* webanimations/accelerated-animation-interruption-display-none-expected.txt: Added.
* webanimations/accelerated-animation-interruption-display-none.html: Added.
* platform/win/TestExpectations:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (236307 => 236308)
--- trunk/LayoutTests/ChangeLog 2018-09-21 07:37:14 UTC (rev 236307)
+++ trunk/LayoutTests/ChangeLog 2018-09-21 11:28:40 UTC (rev 236308)
@@ -1,3 +1,18 @@
+2018-09-21 Antoine Quint <[email protected]>
+
+ [Web Animations] DocumentTimeline::updateAnimations() is called endlessly
+ https://bugs.webkit.org/show_bug.cgi?id=189784
+ <rdar://problem/41705679>
+
+ Reviewed by Dean Jackson.
+
+ Add a new test that checks that setting "display: none" on an element with an accelerated CSS animation on it
+ will no longer update the animation timeline.
+
+ * webanimations/accelerated-animation-interruption-display-none-expected.txt: Added.
+ * webanimations/accelerated-animation-interruption-display-none.html: Added.
+ * platform/win/TestExpectations:
+
2018-09-20 Dean Jackson <[email protected]>
Restrict the total combined size of backdrop filters
Modified: trunk/LayoutTests/platform/win/TestExpectations (236307 => 236308)
--- trunk/LayoutTests/platform/win/TestExpectations 2018-09-21 07:37:14 UTC (rev 236307)
+++ trunk/LayoutTests/platform/win/TestExpectations 2018-09-21 11:28:40 UTC (rev 236308)
@@ -4144,6 +4144,7 @@
webkit.org/b/188166 webanimations/setting-css-animation-timing-property-via-style-after-clearing-effect.html [ Failure ]
webkit.org/b/189468 webanimations/accelerated-transition-interrupted-on-composited-element.html [ Skip ]
+webkit.org/b/189825 webanimations/accelerated-animation-interruption-display-none.html [ Skip ]
webkit.org/b/188167 fast/repaint/canvas-object-fit.html [ Failure ]
Added: trunk/LayoutTests/webanimations/accelerated-animation-interruption-display-none-expected.txt (0 => 236308)
--- trunk/LayoutTests/webanimations/accelerated-animation-interruption-display-none-expected.txt (rev 0)
+++ trunk/LayoutTests/webanimations/accelerated-animation-interruption-display-none-expected.txt 2018-09-21 11:28:40 UTC (rev 236308)
@@ -0,0 +1,3 @@
+
+PASS Interrupting an animation by setting 'display: none' should stop invalidating the animation timeline.
+
Added: trunk/LayoutTests/webanimations/accelerated-animation-interruption-display-none.html (0 => 236308)
--- trunk/LayoutTests/webanimations/accelerated-animation-interruption-display-none.html (rev 0)
+++ trunk/LayoutTests/webanimations/accelerated-animation-interruption-display-none.html 2018-09-21 11:28:40 UTC (rev 236308)
@@ -0,0 +1,57 @@
+<!DOCTYPE html><!-- webkit-test-runner [ experimental:WebAnimationsCSSIntegrationEnabled=true ] -->
+<html>
+<head>
+<style>
+
+#target {
+ width: 100px;
+ height: 100px;
+ background-color: black;
+ animation: foo linear 10s;
+}
+
+@keyframes foo {
+ from { transform: "translateX(100px)" };
+ to { transform: "none" };
+}
+
+</style>
+</head>
+<body>
+<script src=""
+<script src=""
+<div id="target"></div>
+
+<script>
+
+function waitNFrames(numberOfFrames, continuation)
+{
+ let elapsedFrames = 0;
+ (function rAFCallback() {
+ if (elapsedFrames++ >= numberOfFrames)
+ continuation();
+ else
+ requestAnimationFrame(rAFCallback);
+ })();
+}
+
+async_test(t => {
+ const initialNumberOfTimelineInvalidations = internals.numberOfAnimationTimelineInvalidations();
+ waitNFrames(3, () => {
+ assert_greater_than(internals.numberOfAnimationTimelineInvalidations(), initialNumberOfTimelineInvalidations, "There should be updates made to the timeline as an animation is set up.");
+
+ document.getElementById("target").style.display = "none";
+
+ waitNFrames(2, () => {
+ const numberOfTimelineInvalidationsAfterInterruption = internals.numberOfAnimationTimelineInvalidations();
+ requestAnimationFrame(() => {
+ assert_equals(internals.numberOfAnimationTimelineInvalidations(), numberOfTimelineInvalidationsAfterInterruption, "There should not be any updates made to the timeline after interrupting the single running animation.");
+ t.done();
+ });
+ });
+ });
+}, "Interrupting an animation by setting 'display: none' should stop invalidating the animation timeline.");
+
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (236307 => 236308)
--- trunk/Source/WebCore/ChangeLog 2018-09-21 07:37:14 UTC (rev 236307)
+++ trunk/Source/WebCore/ChangeLog 2018-09-21 11:28:40 UTC (rev 236308)
@@ -1,3 +1,41 @@
+2018-09-20 Antoine Quint <[email protected]>
+
+ [Web Animations] DocumentTimeline::updateAnimations() is called endlessly
+ https://bugs.webkit.org/show_bug.cgi?id=189784
+ <rdar://problem/41705679>
+
+ Reviewed by Dean Jackson.
+
+ Test: webanimations/accelerated-animation-interruption-display-none.html
+
+ We have code that keeps queueing pending accelerated actions for an animation that does not have a renderer until it has one
+ so that we can deal with situations where animations are ready to commited before its composited renderer is available. This
+ code ended up running continuously when an element with an accelerated animation had its renderer removed without the animation
+ being removed itself, such as setting "display: none" on an element with an acceelerated CSS Animation targeting it.
+
+ We fix this by queueing up a "Stop" accelerated action when updating the accelerated state if there is no renderer for the current
+ animation target. Then, we no longer re-queue pending accelerated actions if the last queued operation is "Stop". This ensures that
+ we no longer queue actions endlessly when there is no longer a visible animation.
+
+ To test this, we add a new internals.numberOfAnimationTimelineInvalidations() method that indicates the number of times the current
+ document's animation timeline was invalidated.
+
+ * animation/DocumentTimeline.cpp:
+ (WebCore::DocumentTimeline::updateAnimations):
+ (WebCore::DocumentTimeline::numberOfAnimationTimelineInvalidationsForTesting const):
+ * animation/DocumentTimeline.h:
+ * animation/KeyframeEffectReadOnly.cpp:
+ (WebCore::KeyframeEffectReadOnly::updateAcceleratedAnimationState): If the animation target does not have a renderer and it's still
+ marked as running, enqueue a "Stop" accelerated action.
+ (WebCore::KeyframeEffectReadOnly::addPendingAcceleratedAction): If we enqueue a "Stop" accelerated action, remove any other queued
+ action so that we only process the "Stop" action, which would have superseded all previously queued actions anyway.
+ (WebCore::KeyframeEffectReadOnly::applyPendingAcceleratedActions): Only re-queue pending accelerated actions when a composited renderer
+ is not yet available if we don't have a "Stop" action queued.
+ * testing/Internals.cpp:
+ (WebCore::Internals::numberOfAnimationTimelineInvalidations const):
+ * testing/Internals.h:
+ * testing/Internals.idl:
+
2018-09-21 Yacine Bandou <[email protected]>
[EME] Fix typo in WebM sanitization variable
Modified: trunk/Source/WebCore/animation/DocumentTimeline.cpp (236307 => 236308)
--- trunk/Source/WebCore/animation/DocumentTimeline.cpp 2018-09-21 07:37:14 UTC (rev 236307)
+++ trunk/Source/WebCore/animation/DocumentTimeline.cpp 2018-09-21 11:28:40 UTC (rev 236308)
@@ -291,6 +291,8 @@
void DocumentTimeline::updateAnimations()
{
+ m_numberOfAnimationTimelineInvalidationsForTesting++;
+
for (const auto& animation : animations())
animation->runPendingTasks();
@@ -499,4 +501,9 @@
return { };
}
+unsigned DocumentTimeline::numberOfAnimationTimelineInvalidationsForTesting() const
+{
+ return m_numberOfAnimationTimelineInvalidationsForTesting;
+}
+
} // namespace WebCore
Modified: trunk/Source/WebCore/animation/DocumentTimeline.h (236307 => 236308)
--- trunk/Source/WebCore/animation/DocumentTimeline.h 2018-09-21 07:37:14 UTC (rev 236307)
+++ trunk/Source/WebCore/animation/DocumentTimeline.h 2018-09-21 11:28:40 UTC (rev 236308)
@@ -76,6 +76,7 @@
WEBCORE_EXPORT bool animationsAreSuspended();
WEBCORE_EXPORT unsigned numberOfActiveAnimationsForTesting() const;
WEBCORE_EXPORT Vector<std::pair<String, double>> acceleratedAnimationsForElement(Element&) const;
+ WEBCORE_EXPORT unsigned numberOfAnimationTimelineInvalidationsForTesting() const;
private:
DocumentTimeline(Document&, Seconds);
@@ -101,6 +102,7 @@
Timer m_animationScheduleTimer;
HashSet<RefPtr<WebAnimation>> m_acceleratedAnimationsPendingRunningStateChange;
Vector<Ref<AnimationPlaybackEvent>> m_pendingAnimationEvents;
+ unsigned m_numberOfAnimationTimelineInvalidationsForTesting { 0 };
#if !USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
void animationResolutionTimerFired();
Modified: trunk/Source/WebCore/animation/KeyframeEffectReadOnly.cpp (236307 => 236308)
--- trunk/Source/WebCore/animation/KeyframeEffectReadOnly.cpp 2018-09-21 07:37:14 UTC (rev 236307)
+++ trunk/Source/WebCore/animation/KeyframeEffectReadOnly.cpp 2018-09-21 11:28:40 UTC (rev 236308)
@@ -1199,8 +1199,11 @@
if (!m_shouldRunAccelerated)
return;
- if (!renderer())
+ if (!renderer()) {
+ if (isRunningAccelerated())
+ addPendingAcceleratedAction(AcceleratedAction::Stop);
return;
+ }
auto localTime = animation()->currentTime();
@@ -1225,6 +1228,10 @@
if (playState == WebAnimation::PlayState::Finished) {
if (isRunningAccelerated())
addPendingAcceleratedAction(AcceleratedAction::Stop);
+ else {
+ m_lastRecordedAcceleratedAction = AcceleratedAction::Stop;
+ m_pendingAcceleratedActions.clear();
+ }
return;
}
@@ -1237,6 +1244,8 @@
void KeyframeEffectReadOnly::addPendingAcceleratedAction(AcceleratedAction action)
{
+ if (action == AcceleratedAction::Stop)
+ m_pendingAcceleratedActions.clear();
m_pendingAcceleratedActions.append(action);
if (action != AcceleratedAction::Seek)
m_lastRecordedAcceleratedAction = action;
@@ -1269,7 +1278,8 @@
auto* renderer = this->renderer();
if (!renderer || !renderer->isComposited()) {
- animation()->acceleratedStateDidChange();
+ if (m_lastRecordedAcceleratedAction != AcceleratedAction::Stop || m_pendingAcceleratedActions.last() != AcceleratedAction::Stop)
+ animation()->acceleratedStateDidChange();
return;
}
Modified: trunk/Source/WebCore/testing/Internals.cpp (236307 => 236308)
--- trunk/Source/WebCore/testing/Internals.cpp 2018-09-21 07:37:14 UTC (rev 236307)
+++ trunk/Source/WebCore/testing/Internals.cpp 2018-09-21 11:28:40 UTC (rev 236308)
@@ -1076,6 +1076,13 @@
return animations;
}
+unsigned Internals::numberOfAnimationTimelineInvalidations() const
+{
+ if (RuntimeEnabledFeatures::sharedFeatures().webAnimationsCSSIntegrationEnabled())
+ return frame()->document()->timeline().numberOfAnimationTimelineInvalidationsForTesting();
+ return 0;
+}
+
ExceptionOr<RefPtr<Element>> Internals::pseudoElement(Element& element, const String& pseudoId)
{
if (pseudoId != "before" && pseudoId != "after")
Modified: trunk/Source/WebCore/testing/Internals.h (236307 => 236308)
--- trunk/Source/WebCore/testing/Internals.h 2018-09-21 07:37:14 UTC (rev 236307)
+++ trunk/Source/WebCore/testing/Internals.h 2018-09-21 11:28:40 UTC (rev 236308)
@@ -206,6 +206,7 @@
double speed;
};
Vector<AcceleratedAnimation> acceleratedAnimationsForElement(Element&);
+ unsigned numberOfAnimationTimelineInvalidations() const;
// For animations testing, we need a way to get at pseudo elements.
ExceptionOr<RefPtr<Element>> pseudoElement(Element&, const String&);
Modified: trunk/Source/WebCore/testing/Internals.idl (236307 => 236308)
--- trunk/Source/WebCore/testing/Internals.idl 2018-09-21 07:37:14 UTC (rev 236307)
+++ trunk/Source/WebCore/testing/Internals.idl 2018-09-21 11:28:40 UTC (rev 236308)
@@ -211,6 +211,7 @@
// Web Animations testing.
sequence<AcceleratedAnimation> acceleratedAnimationsForElement(Element element);
+ unsigned long numberOfAnimationTimelineInvalidations();
// For animations testing, we need a way to get at pseudo elements.
[MayThrowException] Element? pseudoElement(Element element, DOMString pseudoId);
