Bug 1032573 part 1 - Don't overload start time for marking finished transitions; r=dbaron

When transitions finish, we keep them around for one additional throttle-able
tick to provide correct behavior for subsequent transitions that may be
triggered. Prior to this patch we did this by overloading the start time of the
animation and setting it to null in this case.

However, if we begin returning ElementAnimation objects to script, script can
hold on to those objects and query their start time even after they are
finished. Therefore we need another means of marking finished transitions that
doesn't clobber the start time field.

This patch introduces a new boolean member for marking such transitions.

While we're touching IsFinishedTransition we also take the chance to tidy up one
of the call sites, namely IsCurrentAt, to make the logic a little easier to
follow.

layout/style/AnimationCommon.cpp

@@ -421,16 +421,14 @@ ElementAnimation::IsRunningAt(TimeStamp aTime) const
 bool
 ElementAnimation::IsCurrentAt(TimeStamp aTime) const
 {
-  if (!IsFinishedTransition()) {
-    ComputedTiming computedTiming =
-      GetComputedTimingAt(GetLocalTimeAt(aTime), mTiming);
-    if (computedTiming.mPhase == ComputedTiming::AnimationPhase_Before ||
-        computedTiming.mPhase == ComputedTiming::AnimationPhase_Active) {
-      return true;
-    }
+  if (IsFinishedTransition()) {
+    return false;
   }
 
-  return false;
+  ComputedTiming computedTiming =
+    GetComputedTimingAt(GetLocalTimeAt(aTime), mTiming);
+  return computedTiming.mPhase == ComputedTiming::AnimationPhase_Before ||
+         computedTiming.mPhase == ComputedTiming::AnimationPhase_Active;
 }
 
 bool

layout/style/AnimationCommon.h

@@ -313,6 +313,7 @@ protected:
 public:
   ElementAnimation()
     : mIsRunningOnCompositor(false)
+    , mIsFinishedTransition(false)
     , mLastNotification(LAST_NOTIFICATION_NONE)
   {
   }
@@ -333,12 +334,12 @@ public:
   // cycle (for reasons see explanation in nsTransitionManager.cpp). In the
   // meantime, however, they should be ignored.
   bool IsFinishedTransition() const {
-    return mStartTime.IsNull();
+    return mIsFinishedTransition;
   }
   void SetFinishedTransition() {
     MOZ_ASSERT(AsTransition(),
                "Calling SetFinishedTransition but it's not a transition");
-    mStartTime = mozilla::TimeStamp();
+    mIsFinishedTransition = true;
   }
 
   bool HasAnimationOfProperty(nsCSSProperty aProperty) const;
@@ -377,13 +378,14 @@ public:
 
   nsString mName;
   AnimationTiming mTiming;
-  // 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.
+  // The beginning of the delay period.
   mozilla::TimeStamp mStartTime;
   mozilla::TimeStamp mPauseStart;
   uint8_t mPlayState;
   bool mIsRunningOnCompositor;
+  // A flag to mark transitions that have finished and are due to
+  // be removed on the next throttle-able cycle.
+  bool mIsFinishedTransition;
 
   enum {
     LAST_NOTIFICATION_NONE = uint64_t(-1),