animation-play-state: paused causes very high cpu load because of style invalidation...
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 May 2018 08:59:53 +0000 (08:59 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 May 2018 08:59:53 +0000 (08:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=182436
<rdar://problem/37182562>

Reviewed by Dean Jackson.

Source/WebCore:

Test: animations/animation-playstate-paused-style-resolution.html

If the style of an element with 'animation-play-state: paused' is recomputed so it stays
paused we would enter zero-duration animation timer loop.

* page/animation/AnimationBase.cpp:
(WebCore::AnimationBase::updateStateMachine):

Don't move to AnimationState::PausedWaitResponse unless we get AnimationStateInput::StyleAvailable
(matching the comments). Otherwise just stay in the existing paused state.

Remove AnimationStateInput::StartAnimation from assertion as the case can't happen.

LayoutTests:

* animations/animation-playstate-paused-style-resolution-expected.txt: Added.
* animations/animation-playstate-paused-style-resolution.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@231794 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/animations/animation-playstate-paused-style-resolution-expected.txt [new file with mode: 0644]
LayoutTests/animations/animation-playstate-paused-style-resolution.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/animation/AnimationBase.cpp

index adc2df5..de034cd 100644 (file)
@@ -1,3 +1,14 @@
+2018-05-15  Antti Koivisto  <antti@apple.com>
+
+        animation-play-state: paused causes very high cpu load because of style invalidation loop
+        https://bugs.webkit.org/show_bug.cgi?id=182436
+        <rdar://problem/37182562>
+
+        Reviewed by Dean Jackson.
+
+        * animations/animation-playstate-paused-style-resolution-expected.txt: Added.
+        * animations/animation-playstate-paused-style-resolution.html: Added.
+
 2018-05-14  Youenn Fablet  <youenn@apple.com>
 
         readableStreamDefaultControllerError should return early if stream is not readable
diff --git a/LayoutTests/animations/animation-playstate-paused-style-resolution-expected.txt b/LayoutTests/animations/animation-playstate-paused-style-resolution-expected.txt
new file mode 100644 (file)
index 0000000..e1016e7
--- /dev/null
@@ -0,0 +1,2 @@
+Paused animation
+style recalc count: 1
diff --git a/LayoutTests/animations/animation-playstate-paused-style-resolution.html b/LayoutTests/animations/animation-playstate-paused-style-resolution.html
new file mode 100644 (file)
index 0000000..cfa6d8d
--- /dev/null
@@ -0,0 +1,37 @@
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+</script>
+<style>
+.anim {
+    animation-duration: 20s;
+    animation-name: slidein;
+    animation-play-state: paused;
+}
+@keyframes slidein {
+    from { margin-left: 50%; width: 300%; }
+    to { margin-left: 0%; width: 100%; }
+}
+</style>
+<div class=anim>
+Paused animation
+</div>
+<div id=log></div>
+<script>
+document.body.offsetLeft;
+if (window.testRunner) {
+    internals.startTrackingStyleRecalcs();
+    setTimeout(() => {
+        document.body.offsetLeft;
+        log.innerHTML = "style recalc count: " + internals.styleRecalcCount();
+        testRunner.notifyDone();
+    }, 50);
+}
+</script>
+<style>
+.anim {
+    color: green;
+}
+</style>
index 0ce1f28..7b17536 100644 (file)
@@ -1,3 +1,24 @@
+2018-05-15  Antti Koivisto  <antti@apple.com>
+
+        animation-play-state: paused causes very high cpu load because of style invalidation loop
+        https://bugs.webkit.org/show_bug.cgi?id=182436
+        <rdar://problem/37182562>
+
+        Reviewed by Dean Jackson.
+
+        Test: animations/animation-playstate-paused-style-resolution.html
+
+        If the style of an element with 'animation-play-state: paused' is recomputed so it stays
+        paused we would enter zero-duration animation timer loop.
+
+        * page/animation/AnimationBase.cpp:
+        (WebCore::AnimationBase::updateStateMachine):
+
+        Don't move to AnimationState::PausedWaitResponse unless we get AnimationStateInput::StyleAvailable
+        (matching the comments). Otherwise just stay in the existing paused state.
+
+        Remove AnimationStateInput::StartAnimation from assertion as the case can't happen.
+
 2018-05-14  Youenn Fablet  <youenn@apple.com>
 
         readableStreamDefaultControllerError should return early if stream is not readable
index 73800a9..94a61b3 100644 (file)
@@ -395,7 +395,7 @@ void AnimationBase::updateStateMachine(AnimationStateInput input, double param)
             // AnimationState::PausedWaitResponse, we don't yet have a valid startTime, so we send 0 to startAnimation.
             // When the AnimationStateInput::StartTimeSet comes in and we were in AnimationState::PausedRun, we will notice
             // that we have already set the startTime and will ignore it.
-            ASSERT(input == AnimationStateInput::PlayStatePaused || input == AnimationStateInput::PlayStateRunning || input == AnimationStateInput::StartTimeSet || input == AnimationStateInput::StyleAvailable || input == AnimationStateInput::StartAnimation);
+            ASSERT(input == AnimationStateInput::PlayStatePaused || input == AnimationStateInput::PlayStateRunning || input == AnimationStateInput::StartTimeSet || input == AnimationStateInput::StyleAvailable);
             ASSERT(paused());
 
             if (input == AnimationStateInput::PlayStateRunning) {
@@ -456,6 +456,12 @@ void AnimationBase::updateStateMachine(AnimationStateInput input, double param)
             }
 
             ASSERT(m_animationState == AnimationState::PausedNew || m_animationState == AnimationState::PausedWaitStyleAvailable);
+
+            if (input == AnimationStateInput::PlayStatePaused)
+                break;
+
+            ASSERT(input == AnimationStateInput::StyleAvailable);
+
             // We are paused but we got the callback that notifies us that style has been updated.
             // We move to the AnimationState::PausedWaitResponse state
             LOG(Animations, "%p AnimationState %s -> PausedWaitResponse", this, nameForState(m_animationState));