From 49f31bb4143ad0629c5c4cc4a61a460cd2ff1236 Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Mon, 20 Nov 2017 13:56:26 +0900 Subject: [PATCH] Bug 1418059 - Stop eagerly CSS animations on the root of display:none subtree. r=birtles Otherwise we do update keyframes data unnecessarily. MozReview-Commit-ID: ys4BEF1kxX --HG-- extra : rebase_source : 725aef9a4be9296bc992f6128be7c62b4c2b01e1 --- layout/style/crashtests/1418059.html | 24 ++++++++++++++++++++++++ layout/style/crashtests/crashtests.list | 1 + layout/style/nsAnimationManager.cpp | 15 ++++++++++----- 3 files changed, 35 insertions(+), 5 deletions(-) create mode 100644 layout/style/crashtests/1418059.html diff --git a/layout/style/crashtests/1418059.html b/layout/style/crashtests/1418059.html new file mode 100644 index 000000000000..a799e162a9b9 --- /dev/null +++ b/layout/style/crashtests/1418059.html @@ -0,0 +1,24 @@ + + + +
+ + diff --git a/layout/style/crashtests/crashtests.list b/layout/style/crashtests/crashtests.list index ae40b1822e34..f9877c0eed1d 100644 --- a/layout/style/crashtests/crashtests.list +++ b/layout/style/crashtests/crashtests.list @@ -263,3 +263,4 @@ load 1413361.html load 1415663.html pref(dom.webcomponents.enabled,true) load 1415353.html load 1415021.html # This should have dom.webcomponents.enabled=true, but it leaks the world, see bug 1416296. +load 1418059.html diff --git a/layout/style/nsAnimationManager.cpp b/layout/style/nsAnimationManager.cpp index 75b0bda71d02..542c277b4f01 100644 --- a/layout/style/nsAnimationManager.cpp +++ b/layout/style/nsAnimationManager.cpp @@ -1068,10 +1068,17 @@ nsAnimationManager::UpdateAnimations( "Should not update animations that are not attached to the " "document tree"); - if (!aStyleContext) { + const nsStyleDisplay* disp = aStyleContext + ? aStyleContext->ComputedData()->GetStyleDisplay() + : nullptr; + + if (!disp || disp->mDisplay == StyleDisplay::None) { // If we are in a display:none subtree we will have no computed values. - // Since CSS animations should not run in display:none subtrees we should - // stop (actually, destroy) any animations on this element here. + // However, if we are on the root of display:none subtree, the computed + // values might not have been cleared yet. + // In either case, since CSS animations should not run in display:none + // subtrees we should stop (actually, destroy) any animations on this + // element here. StopAnimationsForElement(aElement, aPseudoType); return; } @@ -1079,8 +1086,6 @@ nsAnimationManager::UpdateAnimations( NonOwningAnimationTarget target(aElement, aPseudoType); ServoCSSAnimationBuilder builder(aStyleContext); - const nsStyleDisplay *disp = - aStyleContext->ComputedData()->GetStyleDisplay(); DoUpdateAnimations(target, *disp, builder); }