Bug 1237467 part 1 - No longer mark element as needing an animation restyle if we go to restyle it and it no longer has an effect set; r=heycam
☠☠ backed out by d353d4310837 ☠ ☠
authorBrian Birtles <birtles@gmail.com>
Thu, 14 Jan 2016 08:02:39 +0900
changeset 279822 5a3d7e16b1e02e1f04c9769f2d5c5557770663f6
parent 279821 b8be495bdde2fe5583ff805ea6a858389073a352
child 279823 d63ca2677bd52b3ee47210cc9b3dd04a825b90fc
push id70233
push userbbirtles@mozilla.com
push dateWed, 13 Jan 2016 23:03:11 +0000
treeherdermozilla-inbound@ac21baf87df2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1237467
milestone46.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1237467 part 1 - No longer mark element as needing an animation restyle if we go to restyle it and it no longer has an effect set; r=heycam
dom/animation/EffectCompositor.cpp
--- a/dom/animation/EffectCompositor.cpp
+++ b/dom/animation/EffectCompositor.cpp
@@ -231,27 +231,46 @@ EffectCompositor::MaybeUpdateAnimationRu
   elementsToRestyle.Remove(key);
 }
 
 nsIStyleRule*
 EffectCompositor::GetAnimationRule(dom::Element* aElement,
                                    nsCSSPseudoElements::Type aPseudoType,
                                    CascadeLevel aCascadeLevel)
 {
+  // NOTE: We need to be careful about early returns in this method where
+  // we *don't* update mElementsToRestyle. When we get a call to
+  // RequestRestyle that results in a call to PostRestyleForAnimation, we
+  // will set a bool flag in mElementsToRestyle indicating that we've
+  // called PostRestyleForAnimation so we don't need to call it again
+  // until that restyle happens. During that restyle, if we arrive here
+  // and *don't* update mElementsToRestyle we'll continue to skip calling
+  // PostRestyleForAnimation from RequestRestyle.
+
   if (!mPresContext || !mPresContext->IsDynamic()) {
     // For print or print preview, ignore animations.
     return nullptr;
   }
 
-  EffectSet* effectSet = EffectSet::GetEffectSet(aElement, aPseudoType);
-  if (!effectSet) {
+  if (mPresContext->RestyleManager()->SkipAnimationRules()) {
+    // We don't need to worry about updating mElementsToRestyle in this case
+    // since this is not the animation restyle we requested when we called
+    // PostRestyleForAnimation (see comment at start of this method).
     return nullptr;
   }
 
-  if (mPresContext->RestyleManager()->SkipAnimationRules()) {
+  EffectSet* effectSet = EffectSet::GetEffectSet(aElement, aPseudoType);
+  if (!effectSet) {
+    // If the EffectSet has been removed but the element has been marked as
+    // needing an animation restyle, we should act as if that animation
+    // restyle has now been completed.
+    auto& elementsToRestyle = mElementsToRestyle[aCascadeLevel];
+    PseudoElementHashKey key = { aElement, aPseudoType };
+    elementsToRestyle.Remove(key);
+
     return nullptr;
   }
 
   MaybeUpdateAnimationRule(aElement, aPseudoType, aCascadeLevel);
 
   return effectSet->AnimationRule(aCascadeLevel);
 }