diff options
| author | Douglas Stockwell <dstockwell@chromium.org> | 2016-01-04 10:28:25 +1100 |
|---|---|---|
| committer | Douglas Stockwell <dstockwell@chromium.org> | 2016-01-03 23:30:13 +0000 |
| commit | 19e6a44082f36d5ac3a76bc61734b859abee4dee (patch) | |
| tree | b01ca8b3002525e4fe235c0a4e8cc943502519e5 | |
| parent | 76792445d764ed2d34fd0a6af99e489628ff4885 (diff) | |
| download | chromium_src-19e6a44082f36d5ac3a76bc61734b859abee4dee.zip chromium_src-19e6a44082f36d5ac3a76bc61734b859abee4dee.tar.gz chromium_src-19e6a44082f36d5ac3a76bc61734b859abee4dee.tar.bz2 | |
Web Animations: Fix scheduling loop in race between visibility and start time notification
After r202085[1] the change in an underlying value could delay the restart of a
compositor animation until the previous start time was known. If this raced
with the page visibility state becoming hidden we could get into a tight
scheduling loop.
This case handles this case explicitly and adds additional guarding asserts.
BUG=560459
Review URL: https://codereview.chromium.org/1500383002
Cr-Commit-Position: refs/heads/master@{#363420}
(cherry picked from commit 047977b0a464789957a6680972647c8e07cb2065)
Review URL: https://codereview.chromium.org/1554023002 .
Cr-Commit-Position: refs/branch-heads/2526@{#540}
Cr-Branched-From: cb947c0153db0ec02a8abbcb3ca086d88bf6006f-refs/heads/master@{#352221}
3 files changed, 36 insertions, 5 deletions
diff --git a/third_party/WebKit/LayoutTests/animations/restart-not-visible.html b/third_party/WebKit/LayoutTests/animations/restart-not-visible.html new file mode 100644 index 0000000..f8c094b7 --- /dev/null +++ b/third_party/WebKit/LayoutTests/animations/restart-not-visible.html @@ -0,0 +1,31 @@ +<!DOCTYPE html> +<style> +.run { + animation: foo 1s infinite alternate; +} + +@keyframes foo { + 100% { + transform: translateX(100px) + } +} +</style> +<div id="target"></div> +<script src="../resources/testharness.js"></script> +<script src="../resources/testharnessreport.js"></script> +<script> +var test = async_test('Race between visibility and set compositor pending should not crash'); +requestAnimationFrame(t => { + requestAnimationFrame(t => { + target.classList.add('run'); + setTimeout(() => { + testRunner.setPageVisibility("hidden"); + target.style.transform = 'translateX(50px)'; + target.offsetTop; + setTimeout(() => { + test.done(); + }, 0); + }, 0); + }); +}); +</script> diff --git a/third_party/WebKit/Source/core/animation/Animation.cpp b/third_party/WebKit/Source/core/animation/Animation.cpp index dc4aeca..60008e2 100644 --- a/third_party/WebKit/Source/core/animation/Animation.cpp +++ b/third_party/WebKit/Source/core/animation/Animation.cpp @@ -251,7 +251,7 @@ bool Animation::preCommit(int compositorGroup, bool startOnCompositor) bool shouldCancel = (!playing() && m_compositorState) || changed; bool shouldStart = playing() && (!m_compositorState || changed); - if (shouldCancel && shouldStart && m_compositorState && m_compositorState->pendingAction == Start) { + if (startOnCompositor && shouldCancel && shouldStart && m_compositorState && m_compositorState->pendingAction == Start) { // Restarting but still waiting for a start time. return false; } diff --git a/third_party/WebKit/Source/core/animation/CompositorPendingAnimations.cpp b/third_party/WebKit/Source/core/animation/CompositorPendingAnimations.cpp index 28ee306..2ad466c8 100644 --- a/third_party/WebKit/Source/core/animation/CompositorPendingAnimations.cpp +++ b/third_party/WebKit/Source/core/animation/CompositorPendingAnimations.cpp @@ -77,6 +77,7 @@ bool CompositorPendingAnimations::update(bool startOnCompositor) // Animations with a start time do not participate in compositor start-time grouping. if (animation->preCommit(animation->hasStartTime() ? 1 : compositorGroup, startOnCompositor)) { if (animation->hasActiveAnimationsOnCompositor() && !hadCompositorAnimation) { + startedSynchronizedOnCompositor = true; } @@ -110,11 +111,10 @@ bool CompositorPendingAnimations::update(bool startOnCompositor) animation->postCommit(animation->timeline()->currentTimeInternal()); ASSERT(m_pending.isEmpty()); + ASSERT(startOnCompositor || deferred.isEmpty()); for (auto& animation : deferred) animation->setCompositorPending(); -#if ENABLE(ASSERT) - size_t pendingSize = m_pending.size(); -#endif + ASSERT(m_pending.size() == deferred.size()); if (startedSynchronizedOnCompositor) return true; @@ -131,7 +131,7 @@ bool CompositorPendingAnimations::update(bool startOnCompositor) // If not, go ahead and start any animations that were waiting. notifyCompositorAnimationStarted(monotonicallyIncreasingTime()); - ASSERT(pendingSize == m_pending.size()); + ASSERT(m_pending.size() == deferred.size()); return false; } |