Bug 1025709 part 2 - Add IsFinished() to ElementAnimation; r=heycam

One of the main differences in handling a list of transitions vs a list of
regular animations is that when we are dealing with a list of transitions we
need to check for transitions that have finished and are about to be discarded
but need to be retained temporarily to provide correct triggering of subsequent
transitions. Such transitions are marked as "removed sentinels" and are ignored
for most operations.

This patch moves the methods for setting and checking such transitions to the
base class ElementAnimation so that we can treat animations and transitions
alike without having to downcast or do obscure checks for mStartTime.IsNull()
(which equates to checking if the animation is a "removed sentinel" but is not
particularly clear).

In the process, this patch renames said methods to Is/SetFinishedTransition
since hopefully that is a little easier to understand at a glance.

layout/base/nsLayoutUtils.cpp

@@ -437,18 +437,18 @@ nsLayoutUtils::ComputeSuitableScaleForAnimation(nsIContent* aContent)
   if (transitions) {
     for (uint32_t i = 0, i_end = transitions->mAnimations.Length();
          i < i_end; ++i){
-      ElementPropertyTransition* pt =
-        transitions->mAnimations[i]->AsTransition();
-      if (pt->IsRemovedSentinel()) {
+      ElementAnimation* anim = transitions->mAnimations[i];
+      if (anim->IsFinishedTransition()) {
         continue;
       }
-      MOZ_ASSERT(pt->mProperties.Length() == 1,
+      MOZ_ASSERT(anim->mProperties.Length() == 1,
         "Should have one animation property for a transition");
-      MOZ_ASSERT(pt->mProperties[0].mSegments.Length() == 1,
+      MOZ_ASSERT(anim->mProperties[0].mSegments.Length() == 1,
         "Animation property should have one segment for a transition");
-      const AnimationPropertySegment& segment = pt->mProperties[0].mSegments[0];
+      const AnimationPropertySegment& segment =
+        anim->mProperties[0].mSegments[0];
 
-      if (pt->mProperties[0].mProperty == eCSSProperty_transform) {
+      if (anim->mProperties[0].mProperty == eCSSProperty_transform) {
         gfxSize start = GetScaleForValue(segment.mFromValue,
                                          aContent->GetPrimaryFrame());
         maxScale.width = std::max<float>(maxScale.width, start.width);

layout/style/AnimationCommon.cpp

@@ -371,7 +371,7 @@ bool
 ElementAnimation::IsRunningAt(TimeStamp aTime) const
 {
   if (IsPaused() || mTiming.mIterationDuration.ToMilliseconds() <= 0.0 ||
-      mStartTime.IsNull()) {
+      IsFinishedTransition()) {
     return false;
   }
 
@@ -384,7 +384,7 @@ ElementAnimation::IsRunningAt(TimeStamp aTime) const
 bool
 ElementAnimation::IsCurrentAt(TimeStamp aTime) const
 {
-  if (!mStartTime.IsNull()) {
+  if (!IsFinishedTransition()) {
     TimeDuration elapsedDuration = ElapsedDurationAt(aTime);
     ComputedTiming computedTiming =
       ElementAnimation::GetComputedTimingAt(elapsedDuration, mTiming);

layout/style/AnimationCommon.h

@@ -305,6 +305,18 @@ public:
     return mPlayState == NS_STYLE_ANIMATION_PLAY_STATE_PAUSED;
   }
 
+  // After transitions finish they need to be retained for one throttle-able
+  // cycle (for reasons see explanation in nsTransitionManager.cpp). In the
+  // meantime, however, they should be ignored.
+  bool IsFinishedTransition() const {
+    return mStartTime.IsNull();
+  }
+  void SetFinishedTransition() {
+    MOZ_ASSERT(AsTransition(),
+               "Calling SetFinishedTransition but it's not a transition");
+    mStartTime = mozilla::TimeStamp();
+  }
+
   bool HasAnimationOfProperty(nsCSSProperty aProperty) const;
   bool IsRunningAt(mozilla::TimeStamp aTime) const;
   bool IsCurrentAt(mozilla::TimeStamp aTime) const;
@@ -350,9 +362,9 @@ public:
 
   nsString mName; // empty string for 'none'
   AnimationTiming mTiming;
-  // The beginning of the delay period.  This is also used by
-  // ElementPropertyTransition in its IsRemovedSentinel and
-  // SetRemovedSentinel methods.
+  // The beginning of the delay period.  This is also set to a null
+  // timestamp to mark transitions that have finished and are due to
+  // be removed on the next throttle-able cycle.
   mozilla::TimeStamp mStartTime;
   mozilla::TimeStamp mPauseStart;
   uint8_t mPlayState;

layout/style/nsTransitionManager.cpp

@@ -55,7 +55,7 @@ ElementPropertyTransition::ValuePortionFor(TimeStamp aRefreshTime) const
   double duration = mTiming.mIterationDuration.ToSeconds();
   NS_ABORT_IF_FALSE(duration >= 0.0, "negative duration forbidden");
   double timePortion;
-  if (IsRemovedSentinel()) {
+  if (IsFinishedTransition()) {
     // The transition is being removed, but we still want an update so that any
     // new transitions start in the right place.
     timePortion = 1.0;
@@ -107,7 +107,7 @@ ElementTransitions::EnsureStyleRuleFor(TimeStamp aRefreshTime)
     for (uint32_t i = 0, i_end = mAnimations.Length(); i < i_end; ++i)
     {
       ElementPropertyTransition* pt = mAnimations[i]->AsTransition();
-      if (pt->IsRemovedSentinel()) {
+      if (pt->IsFinishedTransition()) {
         continue;
       }
 
@@ -137,8 +137,9 @@ bool
 ElementTransitions::HasAnimationOfProperty(nsCSSProperty aProperty) const
 {
   for (uint32_t animIdx = mAnimations.Length(); animIdx-- != 0; ) {
-    const ElementPropertyTransition* pt = mAnimations[animIdx]->AsTransition();
-    if (pt->HasAnimationOfProperty(aProperty) && !pt->IsRemovedSentinel()) {
+    const ElementAnimation* anim = mAnimations[animIdx];
+    if (anim->HasAnimationOfProperty(aProperty) &&
+        !anim->IsFinishedTransition()) {
       return true;
     }
   }
@@ -649,7 +650,7 @@ nsTransitionManager::ConsiderStartingTransition(nsCSSProperty aProperty,
   // If the new transition reverses an existing one, we'll need to
   // handle the timing differently.
   if (haveCurrentTransition &&
-      !oldPT->IsRemovedSentinel() &&
+      !oldPT->IsFinishedTransition() &&
       oldPT->mStartForReversingTest == endValue) {
     // Compute the appropriate negative transition-delay such that right
     // now we'd end up at the current position.
@@ -954,8 +955,8 @@ nsTransitionManager::FlushTransitions(FlushFlags aFlags)
       bool transitionStartedOrEnded = false;
       do {
         --i;
-        ElementPropertyTransition* pt = et->mAnimations[i]->AsTransition();
-        if (pt->IsRemovedSentinel()) {
+        ElementAnimation* anim = et->mAnimations[i];
+        if (anim->IsFinishedTransition()) {
           // Actually remove transitions one throttle-able cycle after their
           // completion. We only clear on a throttle-able cycle because that
           // means it is a regular restyle tick and thus it is safe to discard
@@ -964,11 +965,11 @@ nsTransitionManager::FlushTransitions(FlushFlags aFlags)
           if (aFlags == Can_Throttle) {
             et->mAnimations.RemoveElementAt(i);
           }
-        } else if (pt->mStartTime + pt->mTiming.mDelay +
-                   pt->mTiming.mIterationDuration <= now) {
-          MOZ_ASSERT(pt->mProperties.Length() == 1,
+        } else if (anim->mStartTime + anim->mTiming.mDelay +
+                   anim->mTiming.mIterationDuration <= now) {
+          MOZ_ASSERT(anim->mProperties.Length() == 1,
                      "Should have one animation property for a transition");
-          nsCSSProperty prop = pt->mProperties[0].mProperty;
+          nsCSSProperty prop = anim->mProperties[0].mProperty;
           if (nsCSSProps::PropHasFlags(prop, CSS_PROPERTY_REPORT_OTHER_NAME))
           {
             prop = nsCSSProps::OtherNameFor(prop);
@@ -978,7 +979,7 @@ nsTransitionManager::FlushTransitions(FlushFlags aFlags)
           NS_NAMED_LITERAL_STRING(after, "::after");
           events.AppendElement(
             TransitionEventInfo(et->mElement, prop,
-                                pt->mTiming.mIterationDuration,
+                                anim->mTiming.mIterationDuration,
                                 ep == nsGkAtoms::transitionsProperty ?
                                   EmptyString() :
                                   ep == nsGkAtoms::transitionsOfBeforeProperty ?
@@ -992,12 +993,12 @@ nsTransitionManager::FlushTransitions(FlushFlags aFlags)
           // a non-animation style change that would affect it, we need
           // to know not to start a new transition for the transition
           // from the almost-completed value to the final value.
-          pt->SetRemovedSentinel();
+          anim->SetFinishedTransition();
           et->UpdateAnimationGeneration(mPresContext);
           transitionStartedOrEnded = true;
-        } else if (pt->mStartTime + pt->mTiming.mDelay <= now &&
+        } else if (anim->mStartTime + anim->mTiming.mDelay <= now &&
                    canThrottleTick &&
-                   !pt->mIsRunningOnCompositor) {
+                   !anim->mIsRunningOnCompositor) {
           // Start a transition with a delay where we should start the
           // transition proper.
           et->UpdateAnimationGeneration(mPresContext);

layout/style/nsTransitionManager.h

@@ -49,19 +49,6 @@ struct ElementPropertyTransition : public mozilla::ElementAnimation
   // at the given time.  (The input to the transition timing function
   // has time units, the output has value units.)
   double ValuePortionFor(mozilla::TimeStamp aRefreshTime) const;
-
-  bool IsRemovedSentinel() const
-  {
-    // Note that mozilla::ElementAnimation::IsRunningAt depends on removed
-    // sentinels being represented by a null mStartTime.
-    return mStartTime.IsNull();
-  }
-
-  void SetRemovedSentinel()
-  {
-    // assign the null time stamp
-    mStartTime = mozilla::TimeStamp();
-  }
 };
 
 struct ElementTransitions MOZ_FINAL