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
authorBrian Birtles <birtles@gmail.com>
Fri, 15 Jan 2016 15:15:47 +0900
changeset 322094 1d51246e9c8f72a71a33ba4ba16d745b5eff9d8c
parent 322093 de9a0fc7828dbc3cbe6a921e796a64971cf78d3a
child 322095 7fdf99692385affe2fa3170d918c7a6ea008c0e7
push id9530
push userdmitchell@mozilla.com
push dateFri, 15 Jan 2016 19:42:24 +0000
reviewersheycam
bugs1237467
milestone46.0a1
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,32 +231,54 @@ 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;
   }
 
+  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;
+  }
+
+  MaybeUpdateAnimationRule(aElement, aPseudoType, aCascadeLevel);
+
+#ifdef DEBUG
+  {
+    auto& elementsToRestyle = mElementsToRestyle[aCascadeLevel];
+    PseudoElementHashKey key = { aElement, aPseudoType };
+    MOZ_ASSERT(!elementsToRestyle.Contains(key),
+               "Element should no longer require a restyle after its "
+               "animation rule has been updated");
+  }
+#endif
+
   EffectSet* effectSet = EffectSet::GetEffectSet(aElement, aPseudoType);
   if (!effectSet) {
     return nullptr;
   }
 
-  if (mPresContext->RestyleManager()->SkipAnimationRules()) {
-    return nullptr;
-  }
-
-  MaybeUpdateAnimationRule(aElement, aPseudoType, aCascadeLevel);
-
   return effectSet->AnimationRule(aCascadeLevel);
 }
 
 /* static */ dom::Element*
 EffectCompositor::GetElementToRestyle(dom::Element* aElement,
                                       nsCSSPseudoElements::Type aPseudoType)
 {
   if (aPseudoType == nsCSSPseudoElements::ePseudo_NotPseudoElement) {