Bug 1025709 part 4 - Move EnsureStyleRuleFor from ElementTransitions and ElementAnimations to CommonElementAnimationData; r=heycam

Both ElementAnimations and ElementTransitions have an EnsureStyleRuleFor method.
The ElementAnimations version is a more general of the ElementTransitions one
with the exception that the ElementTransitions version checks for finished
transitions. This patch moves the code from ElementAnimations to
CommonElementAnimationData with one minor change: adding the checks for finished
transitions. The ElementTransitions version is removed.

Since the ElementAnimations version contains a second parameter, aIsThrottled,
callers of ElementTransitions must include this extra parameter. In
a subsequent patch we add an enum for this parameter to make call sites easier
to read.

The ElementAnimations version also sets the mNeedsRefreshes member so at the
same time we move mNeedsRefreshes to CommonElementAnimationData. Furthermore,
since the ElementAnimations version which we have adopted returns early if
mNeedsRefreshes is false, this patch ensures that when we call
EnsureStyleRuleFor from ElementTransitions::WalkTransitionRule, we set
mNeedsRefreshes to true first.

Another difference to account for is that the ElementTransitions version of
EnsureStyleRuleFor *always* sets mStyleRule (even if it doesn't add anything to
it) where as the ElementAnimations version only creates the rule when necessary
so we need to add a check to ElementTransitions::WalkTransitionRule that
mStyleRule is actually set before using it.

layout/style/AnimationCommon.cpp

@@ -9,6 +9,7 @@
 
 #include "gfxPlatform.h"
 #include "nsRuleData.h"
+#include "nsCSSPropertySet.h"
 #include "nsCSSValue.h"
 #include "nsStyleContext.h"
 #include "nsIFrame.h"
@@ -241,7 +242,8 @@ CommonAnimationManager::UpdateThrottledStyle(dom::Element* aElement,
       NS_ASSERTION(et,
         "Rule has level eTransitionSheet without transition on manager");
 
-      et->EnsureStyleRuleFor(mPresContext->RefreshDriver()->MostRecentRefresh());
+      et->EnsureStyleRuleFor(
+        mPresContext->RefreshDriver()->MostRecentRefresh(), false);
       curRule.mRule = et->mStyleRule;
     } else {
       curRule.mRule = ruleNode->GetRule();
@@ -612,6 +614,169 @@ CommonElementAnimationData::LogAsyncAnimationFailure(nsCString& aMessage,
   printf_stderr(aMessage.get());
 }
 
+void
+CommonElementAnimationData::EnsureStyleRuleFor(TimeStamp aRefreshTime,
+                                               bool aIsThrottled)
+{
+  if (!mNeedsRefreshes) {
+    mStyleRuleRefreshTime = aRefreshTime;
+    return;
+  }
+
+  // If we're performing animations on the compositor thread, then we can skip
+  // most of the work in this method. But even if we are throttled, then we
+  // have to do the work if an animation is ending in order to get correct end
+  // of animation behaviour (the styles of the animation disappear, or the fill
+  // mode behaviour). This loop checks for any finishing animations and forces
+  // the style recalculation if we find any.
+  if (aIsThrottled) {
+    for (uint32_t animIdx = mAnimations.Length(); animIdx-- != 0; ) {
+      ElementAnimation* anim = mAnimations[animIdx];
+
+      // Skip finished transitions or animations whose @keyframes rule
+      // is empty.
+      if (anim->IsFinishedTransition() || anim->mProperties.IsEmpty()) {
+        continue;
+      }
+
+      // The GetLocalTimeAt() call here handles pausing.  But:
+      // FIXME: avoid recalculating every time when paused.
+      TimeDuration localTime = anim->GetLocalTimeAt(aRefreshTime);
+      ComputedTiming computedTiming =
+        ElementAnimation::GetComputedTimingAt(localTime, anim->mTiming);
+
+      // XXX We shouldn't really be using mLastNotification as a general
+      // indicator that the animation has finished, it should be reserved for
+      // events. If we use it differently in the future this use might need
+      // changing.
+      if (!anim->mIsRunningOnCompositor ||
+          (computedTiming.mPhase == ComputedTiming::AnimationPhase_After &&
+           anim->mLastNotification != ElementAnimation::LAST_NOTIFICATION_END))
+      {
+        aIsThrottled = false;
+        break;
+      }
+    }
+  }
+
+  if (aIsThrottled) {
+    return;
+  }
+
+  // mStyleRule may be null and valid, if we have no style to apply.
+  if (mStyleRuleRefreshTime.IsNull() ||
+      mStyleRuleRefreshTime != aRefreshTime) {
+    mStyleRuleRefreshTime = aRefreshTime;
+    mStyleRule = nullptr;
+    // We'll set mNeedsRefreshes to true below in all cases where we need them.
+    mNeedsRefreshes = false;
+
+    // FIXME(spec): assume that properties in higher animations override
+    // those in lower ones.
+    // Therefore, we iterate from last animation to first.
+    nsCSSPropertySet properties;
+
+    for (uint32_t animIdx = mAnimations.Length(); animIdx-- != 0; ) {
+      ElementAnimation* anim = mAnimations[animIdx];
+
+      if (anim->IsFinishedTransition()) {
+        continue;
+      }
+
+      // The GetLocalTimeAt() call here handles pausing.  But:
+      // FIXME: avoid recalculating every time when paused.
+      TimeDuration localTime = anim->GetLocalTimeAt(aRefreshTime);
+      ComputedTiming computedTiming =
+        ElementAnimation::GetComputedTimingAt(localTime, anim->mTiming);
+
+      if ((computedTiming.mPhase == ComputedTiming::AnimationPhase_Before ||
+           computedTiming.mPhase == ComputedTiming::AnimationPhase_Active) &&
+          !anim->IsPaused()) {
+        mNeedsRefreshes = true;
+      }
+
+      // If the time fraction is null, we don't have fill data for the current
+      // time so we shouldn't animate.
+      if (computedTiming.mTimeFraction == ComputedTiming::kNullTimeFraction) {
+        continue;
+      }
+
+      NS_ABORT_IF_FALSE(0.0 <= computedTiming.mTimeFraction &&
+                        computedTiming.mTimeFraction <= 1.0,
+                        "timing fraction should be in [0-1]");
+
+      for (uint32_t propIdx = 0, propEnd = anim->mProperties.Length();
+           propIdx != propEnd; ++propIdx)
+      {
+        const AnimationProperty &prop = anim->mProperties[propIdx];
+
+        NS_ABORT_IF_FALSE(prop.mSegments[0].mFromKey == 0.0,
+                          "incorrect first from key");
+        NS_ABORT_IF_FALSE(prop.mSegments[prop.mSegments.Length() - 1].mToKey
+                            == 1.0,
+                          "incorrect last to key");
+
+        if (properties.HasProperty(prop.mProperty)) {
+          // A later animation already set this property.
+          continue;
+        }
+        properties.AddProperty(prop.mProperty);
+
+        NS_ABORT_IF_FALSE(prop.mSegments.Length() > 0,
+                          "property should not be in animations if it "
+                          "has no segments");
+
+        // FIXME: Maybe cache the current segment?
+        const AnimationPropertySegment *segment = prop.mSegments.Elements(),
+                               *segmentEnd = segment + prop.mSegments.Length();
+        while (segment->mToKey < computedTiming.mTimeFraction) {
+          NS_ABORT_IF_FALSE(segment->mFromKey < segment->mToKey,
+                            "incorrect keys");
+          ++segment;
+          if (segment == segmentEnd) {
+            NS_ABORT_IF_FALSE(false, "incorrect time fraction");
+            break; // in order to continue in outer loop (just below)
+          }
+          NS_ABORT_IF_FALSE(segment->mFromKey == (segment-1)->mToKey,
+                            "incorrect keys");
+        }
+        if (segment == segmentEnd) {
+          continue;
+        }
+        NS_ABORT_IF_FALSE(segment->mFromKey < segment->mToKey,
+                          "incorrect keys");
+        NS_ABORT_IF_FALSE(segment >= prop.mSegments.Elements() &&
+                          size_t(segment - prop.mSegments.Elements()) <
+                            prop.mSegments.Length(),
+                          "out of array bounds");
+
+        if (!mStyleRule) {
+          // Allocate the style rule now that we know we have animation data.
+          mStyleRule = new css::AnimValuesStyleRule();
+        }
+
+        double positionInSegment =
+          (computedTiming.mTimeFraction - segment->mFromKey) /
+          (segment->mToKey - segment->mFromKey);
+        double valuePosition =
+          segment->mTimingFunction.GetValue(positionInSegment);
+
+        nsStyleAnimation::Value *val =
+          mStyleRule->AddEmptyValue(prop.mProperty);
+
+#ifdef DEBUG
+        bool result =
+#endif
+          nsStyleAnimation::Interpolate(prop.mProperty,
+                                        segment->mFromValue, segment->mToValue,
+                                        valuePosition, *val);
+        NS_ABORT_IF_FALSE(result, "interpolate must succeed now");
+      }
+    }
+  }
+}
+
+
 bool
 CommonElementAnimationData::CanThrottleTransformChanges(TimeStamp aTime)
 {

layout/style/AnimationCommon.h

@@ -398,6 +398,7 @@ struct CommonElementAnimationData : public PRCList
     , mManager(aManager)
     , mAnimationGeneration(0)
     , mFlushGeneration(aNow)
+    , mNeedsRefreshes(true)
 #ifdef DEBUG
     , mCalledPropertyDtor(false)
 #endif
@@ -420,6 +421,12 @@ struct CommonElementAnimationData : public PRCList
     mElement->DeleteProperty(mElementProperty);
   }
 
+  // This updates mNeedsRefreshes so the caller may need to check
+  // for changes to values (for example, nsAnimationManager provides
+  // CheckNeedsRefresh to register or unregister from observing the refresh
+  // driver when this value changes).
+  void EnsureStyleRuleFor(TimeStamp aRefreshTime, bool aIsThrottled);
+
   bool CanThrottleTransformChanges(mozilla::TimeStamp aTime);
 
   bool CanThrottleAnimation(mozilla::TimeStamp aTime);
@@ -489,6 +496,11 @@ struct CommonElementAnimationData : public PRCList
   // UpdateAllThrottledStyles.
   TimeStamp mFlushGeneration;
 
+  // False when we know that our current style rule is valid
+  // indefinitely into the future (because all of our animations are
+  // either completed or paused).  May be invalidated by a style change.
+  bool mNeedsRefreshes;
+
 #ifdef DEBUG
   bool mCalledPropertyDtor;
 #endif

layout/style/nsAnimationManager.cpp

@@ -30,8 +30,7 @@ ElementAnimations::ElementAnimations(mozilla::dom::Element *aElement,
                                      nsAnimationManager *aAnimationManager,
                                      TimeStamp aNow)
   : CommonElementAnimationData(aElement, aElementProperty,
-                               aAnimationManager, aNow),
-    mNeedsRefreshes(true)
+                               aAnimationManager, aNow)
 {
 }
 
@@ -49,163 +48,6 @@ ElementAnimationsPropertyDtor(void           *aObject,
   delete ea;
 }
 
-void
-ElementAnimations::EnsureStyleRuleFor(TimeStamp aRefreshTime,
-                                      bool aIsThrottled)
-{
-  if (!mNeedsRefreshes) {
-    mStyleRuleRefreshTime = aRefreshTime;
-    return;
-  }
-
-  // If we're performing animations on the compositor thread, then we can skip
-  // most of the work in this method. But even if we are throttled, then we
-  // have to do the work if an animation is ending in order to get correct end
-  // of animation behaviour (the styles of the animation disappear, or the fill
-  // mode behaviour). This loop checks for any finishing animations and forces
-  // the style recalculation if we find any.
-  if (aIsThrottled) {
-    for (uint32_t animIdx = mAnimations.Length(); animIdx-- != 0; ) {
-      ElementAnimation* anim = mAnimations[animIdx];
-
-      if (anim->mProperties.IsEmpty()) {
-        // Empty @keyframes rule.
-        continue;
-      }
-
-      // The GetLocalTimeAt() call here handles pausing.  But:
-      // FIXME: avoid recalculating every time when paused.
-      TimeDuration localTime = anim->GetLocalTimeAt(aRefreshTime);
-      ComputedTiming computedTiming =
-        ElementAnimation::GetComputedTimingAt(localTime, anim->mTiming);
-
-      // XXX We shouldn't really be using mLastNotification as a general
-      // indicator that the animation has finished, it should be reserved for
-      // events. If we use it differently in the future this use might need
-      // changing.
-      if (!anim->mIsRunningOnCompositor ||
-          (computedTiming.mPhase == ComputedTiming::AnimationPhase_After &&
-           anim->mLastNotification != ElementAnimation::LAST_NOTIFICATION_END))
-      {
-        aIsThrottled = false;
-        break;
-      }
-    }
-  }
-
-  if (aIsThrottled) {
-    return;
-  }
-
-  // mStyleRule may be null and valid, if we have no style to apply.
-  if (mStyleRuleRefreshTime.IsNull() ||
-      mStyleRuleRefreshTime != aRefreshTime) {
-    mStyleRuleRefreshTime = aRefreshTime;
-    mStyleRule = nullptr;
-    // We'll set mNeedsRefreshes to true below in all cases where we need them.
-    mNeedsRefreshes = false;
-
-    // FIXME(spec): assume that properties in higher animations override
-    // those in lower ones.
-    // Therefore, we iterate from last animation to first.
-    nsCSSPropertySet properties;
-
-    for (uint32_t animIdx = mAnimations.Length(); animIdx-- != 0; ) {
-      ElementAnimation* anim = mAnimations[animIdx];
-
-      // The GetLocalTimeAt() call here handles pausing.  But:
-      // FIXME: avoid recalculating every time when paused.
-      TimeDuration localTime = anim->GetLocalTimeAt(aRefreshTime);
-      ComputedTiming computedTiming =
-        ElementAnimation::GetComputedTimingAt(localTime, anim->mTiming);
-
-      if ((computedTiming.mPhase == ComputedTiming::AnimationPhase_Before ||
-           computedTiming.mPhase == ComputedTiming::AnimationPhase_Active) &&
-          !anim->IsPaused()) {
-        mNeedsRefreshes = true;
-      }
-
-      // If the time fraction is null, we don't have fill data for the current
-      // time so we shouldn't animate.
-      if (computedTiming.mTimeFraction == ComputedTiming::kNullTimeFraction) {
-        continue;
-      }
-
-      NS_ABORT_IF_FALSE(0.0 <= computedTiming.mTimeFraction &&
-                        computedTiming.mTimeFraction <= 1.0,
-                        "timing fraction should be in [0-1]");
-
-      for (uint32_t propIdx = 0, propEnd = anim->mProperties.Length();
-           propIdx != propEnd; ++propIdx)
-      {
-        const AnimationProperty &prop = anim->mProperties[propIdx];
-
-        NS_ABORT_IF_FALSE(prop.mSegments[0].mFromKey == 0.0,
-                          "incorrect first from key");
-        NS_ABORT_IF_FALSE(prop.mSegments[prop.mSegments.Length() - 1].mToKey
-                            == 1.0,
-                          "incorrect last to key");
-
-        if (properties.HasProperty(prop.mProperty)) {
-          // A later animation already set this property.
-          continue;
-        }
-        properties.AddProperty(prop.mProperty);
-
-        NS_ABORT_IF_FALSE(prop.mSegments.Length() > 0,
-                          "property should not be in animations if it "
-                          "has no segments");
-
-        // FIXME: Maybe cache the current segment?
-        const AnimationPropertySegment *segment = prop.mSegments.Elements(),
-                               *segmentEnd = segment + prop.mSegments.Length();
-        while (segment->mToKey < computedTiming.mTimeFraction) {
-          NS_ABORT_IF_FALSE(segment->mFromKey < segment->mToKey,
-                            "incorrect keys");
-          ++segment;
-          if (segment == segmentEnd) {
-            NS_ABORT_IF_FALSE(false, "incorrect time fraction");
-            break; // in order to continue in outer loop (just below)
-          }
-          NS_ABORT_IF_FALSE(segment->mFromKey == (segment-1)->mToKey,
-                            "incorrect keys");
-        }
-        if (segment == segmentEnd) {
-          continue;
-        }
-        NS_ABORT_IF_FALSE(segment->mFromKey < segment->mToKey,
-                          "incorrect keys");
-        NS_ABORT_IF_FALSE(segment >= prop.mSegments.Elements() &&
-                          size_t(segment - prop.mSegments.Elements()) <
-                            prop.mSegments.Length(),
-                          "out of array bounds");
-
-        if (!mStyleRule) {
-          // Allocate the style rule now that we know we have animation data.
-          mStyleRule = new css::AnimValuesStyleRule();
-        }
-
-        double positionInSegment =
-          (computedTiming.mTimeFraction - segment->mFromKey) /
-          (segment->mToKey - segment->mFromKey);
-        double valuePosition =
-          segment->mTimingFunction.GetValue(positionInSegment);
-
-        nsStyleAnimation::Value *val =
-          mStyleRule->AddEmptyValue(prop.mProperty);
-
-#ifdef DEBUG
-        bool result =
-#endif
-          nsStyleAnimation::Interpolate(prop.mProperty,
-                                        segment->mFromValue, segment->mToValue,
-                                        valuePosition, *val);
-        NS_ABORT_IF_FALSE(result, "interpolate must succeed now");
-      }
-    }
-  }
-}
-
 void
 ElementAnimations::GetEventsAt(TimeStamp aRefreshTime,
                                EventArray& aEventsToDispatch)

layout/style/nsAnimationManager.h

@@ -60,9 +60,6 @@ struct ElementAnimations MOZ_FINAL
   ElementAnimations(mozilla::dom::Element *aElement, nsIAtom *aElementProperty,
                     nsAnimationManager *aAnimationManager, TimeStamp aNow);
 
-  // After calling this, be sure to call CheckNeedsRefresh on the animation
-  // manager afterwards.
-  void EnsureStyleRuleFor(TimeStamp aRefreshTime, bool aIsThrottled);
   void GetEventsAt(TimeStamp aRefreshTime, EventArray &aEventsToDispatch);
 
   bool IsForElement() const { // rather than for a pseudo-element
@@ -99,11 +96,6 @@ struct ElementAnimations MOZ_FINAL
   virtual bool CanPerformOnCompositorThread(CanAnimateFlags aFlags) const MOZ_OVERRIDE;
 
   virtual bool HasAnimationOfProperty(nsCSSProperty aProperty) const MOZ_OVERRIDE;
-
-  // False when we know that our current style rule is valid
-  // indefinitely into the future (because all of our animations are
-  // either completed or paused).  May be invalidated by a style change.
-  bool mNeedsRefreshes;
 };
 
 class nsAnimationManager MOZ_FINAL

layout/style/nsTransitionManager.cpp

@@ -97,42 +97,6 @@ ElementTransitionsPropertyDtor(void           *aObject,
   delete et;
 }
 
-void
-ElementTransitions::EnsureStyleRuleFor(TimeStamp aRefreshTime)
-{
-  if (!mStyleRule || mStyleRuleRefreshTime != aRefreshTime) {
-    mStyleRule = new css::AnimValuesStyleRule();
-    mStyleRuleRefreshTime = aRefreshTime;
-
-    for (uint32_t i = 0, i_end = mAnimations.Length(); i < i_end; ++i)
-    {
-      ElementPropertyTransition* pt = mAnimations[i]->AsTransition();
-      if (pt->IsFinishedTransition()) {
-        continue;
-      }
-
-      MOZ_ASSERT(pt->mProperties.Length() == 1,
-                 "Should have one animation property for a transition");
-      const AnimationProperty &prop = pt->mProperties[0];
-
-      nsStyleAnimation::Value *val = mStyleRule->AddEmptyValue(prop.mProperty);
-
-      double valuePortion = pt->ValuePortionFor(aRefreshTime);
-
-      MOZ_ASSERT(prop.mSegments.Length() == 1,
-                 "Animation property should have one segment for a transition");
-#ifdef DEBUG
-      bool ok =
-#endif
-        nsStyleAnimation::Interpolate(prop.mProperty,
-                                      prop.mSegments[0].mFromValue,
-                                      prop.mSegments[0].mToValue,
-                                      valuePortion, *val);
-      NS_ABORT_IF_FALSE(ok, "could not interpolate values");
-    }
-  }
-}
-
 bool
 ElementTransitions::HasAnimationOfProperty(nsCSSProperty aProperty) const
 {
@@ -826,10 +790,14 @@ nsTransitionManager::WalkTransitionRule(ElementDependentRuleProcessorData* aData
     return;
   }
 
+  et->mNeedsRefreshes = true;
   et->EnsureStyleRuleFor(
-    aData->mPresContext->RefreshDriver()->MostRecentRefresh());
+    aData->mPresContext->RefreshDriver()->MostRecentRefresh(),
+    false);
 
-  aData->mRuleWalker->Forward(et->mStyleRule);
+  if (et->mStyleRule) {
+    aData->mRuleWalker->Forward(et->mStyleRule);
+  }
 }
 
 /* virtual */ void

layout/style/nsTransitionManager.h

@@ -58,8 +58,6 @@ struct ElementTransitions MOZ_FINAL
                      nsTransitionManager *aTransitionManager,
                      mozilla::TimeStamp aNow);
 
-  void EnsureStyleRuleFor(mozilla::TimeStamp aRefreshTime);
-
   virtual bool HasAnimationOfProperty(nsCSSProperty aProperty) const MOZ_OVERRIDE;
 
   // If aFlags contains CanAnimate_AllowPartial, returns whether the