Bug 1202333: AnimationEvent elapsedTime should reflect playbackRate (without tests)

Issue #55

2 files changed, 93 insertions, 73 deletions

diff --git a/layout/style/nsAnimationManager.cpp b/layout/style/nsAnimationManager.cpp
index ed2b5afc7..8d4e8fcee 100644
--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@@ -33,11 +33,15 @@ using mozilla::dom::AnimationPlayState;
 using mozilla::dom::KeyframeEffectReadOnly;
 using mozilla::dom::CSSAnimation;
 
+typedef mozilla::ComputedTiming::AnimationPhase AnimationPhase;
+
 namespace {
 
-// Pair of an event message and elapsed time used when determining the set of
-// events to queue.
-typedef Pair<EventMessage, StickyTimeDuration> EventPair;
+struct AnimationEventParams {
+  EventMessage mMessage;
+  StickyTimeDuration mElapsedTime;
+  TimeStamp mTimeStamp;
+};
 
 } // anonymous namespace
 
@@ -196,75 +200,93 @@ CSSAnimation::QueueEvents()
 
   ComputedTiming computedTiming = mEffect->GetComputedTiming();
 
-  if (computedTiming.mPhase == ComputedTiming::AnimationPhase::Null) {
-    return; // do nothing
+  ComputedTiming::AnimationPhase currentPhase = computedTiming.mPhase;
+  uint64_t currentIteration  = computedTiming.mCurrentIteration;
+  if (currentPhase == mPreviousPhase &&
+      currentIteration == mPreviousIteration) {
+    return;
   }
 
-  // Note that script can change the start time, so we have to handle moving
-  // backwards through the animation as well as forwards. An 'animationstart'
-  // is dispatched if we enter the active phase (regardless if that is from
-  // before or after the animation's active phase). An 'animationend' is
-  // dispatched if we leave the active phase (regardless if that is to before
-  // or after the animation's active phase).
-
-  bool wasActive = mPreviousPhaseOrIteration != PREVIOUS_PHASE_BEFORE &&
-                   mPreviousPhaseOrIteration != PREVIOUS_PHASE_AFTER;
-  bool isActive =
-    computedTiming.mPhase == ComputedTiming::AnimationPhase::Active;
-  bool isSameIteration =
-         computedTiming.mCurrentIteration == mPreviousPhaseOrIteration;
-  bool skippedActivePhase =
-    (mPreviousPhaseOrIteration == PREVIOUS_PHASE_BEFORE &&
-     computedTiming.mPhase == ComputedTiming::AnimationPhase::After) ||
-    (mPreviousPhaseOrIteration == PREVIOUS_PHASE_AFTER &&
-     computedTiming.mPhase == ComputedTiming::AnimationPhase::Before);
-  bool skippedFirstIteration =
-    isActive &&
-    mPreviousPhaseOrIteration == PREVIOUS_PHASE_BEFORE &&
-    computedTiming.mCurrentIteration > 0;
-
-  MOZ_ASSERT(!skippedActivePhase || (!isActive && !wasActive),
-             "skippedActivePhase only makes sense if we were & are inactive");
-
-  if (computedTiming.mPhase == ComputedTiming::AnimationPhase::Before) {
-    mPreviousPhaseOrIteration = PREVIOUS_PHASE_BEFORE;
-  } else if (computedTiming.mPhase == ComputedTiming::AnimationPhase::Active) {
-    mPreviousPhaseOrIteration = computedTiming.mCurrentIteration;
-  } else if (computedTiming.mPhase == ComputedTiming::AnimationPhase::After) {
-    mPreviousPhaseOrIteration = PREVIOUS_PHASE_AFTER;
+  const StickyTimeDuration zeroDuration;
+  StickyTimeDuration intervalStartTime =
+    std::max(std::min(StickyTimeDuration(-mEffect->SpecifiedTiming().mDelay),
+                      computedTiming.mActiveDuration),
+             zeroDuration);
+  StickyTimeDuration intervalEndTime =
+    std::max(std::min((EffectEnd() - mEffect->SpecifiedTiming().mDelay),
+                      computedTiming.mActiveDuration),
+             zeroDuration);
+
+  uint64_t iterationBoundary = mPreviousIteration > currentIteration
+                               ? currentIteration + 1
+                               : currentIteration;
+  StickyTimeDuration iterationStartTime  =
+    computedTiming.mDuration.MultDouble(
+      (iterationBoundary - computedTiming.mIterationStart));
+
+  TimeStamp startTimeStamp     = ElapsedTimeToTimeStamp(intervalStartTime);
+  TimeStamp endTimeStamp       = ElapsedTimeToTimeStamp(intervalEndTime);
+  TimeStamp iterationTimeStamp = ElapsedTimeToTimeStamp(iterationStartTime);
+
+  AutoTArray<AnimationEventParams, 2> events;
+  switch (mPreviousPhase) {
+    case AnimationPhase::Null:
+    case AnimationPhase::Before:
+      if (currentPhase == AnimationPhase::Active) {
+        events.AppendElement(AnimationEventParams{ eAnimationStart,
+                                                   intervalStartTime,
+                                                   startTimeStamp });
+      } else if (currentPhase == AnimationPhase::After) {
+        events.AppendElement(AnimationEventParams{ eAnimationStart,
+                                                   intervalStartTime,
+                                                   startTimeStamp });
+        events.AppendElement(AnimationEventParams{ eAnimationEnd,
+                                                   intervalEndTime,
+                                                   endTimeStamp });
+      }
+      break;
+    case AnimationPhase::Active:
+      if (currentPhase == AnimationPhase::Before) {
+        events.AppendElement(AnimationEventParams{ eAnimationEnd,
+                                                   intervalStartTime,
+                                                   startTimeStamp });
+      } else if (currentPhase == AnimationPhase::Active) {
+        // The currentIteration must have changed or element we would have
+        // returned early above.
+        MOZ_ASSERT(currentIteration != mPreviousIteration);
+        events.AppendElement(AnimationEventParams{ eAnimationIteration,
+                                                   iterationStartTime,
+                                                   iterationTimeStamp });
+      } else if (currentPhase == AnimationPhase::After) {
+        events.AppendElement(AnimationEventParams{ eAnimationEnd,
+                                                   intervalEndTime,
+                                                   endTimeStamp });
+      }
+      break;
+    case AnimationPhase::After:
+      if (currentPhase == AnimationPhase::Before) {
+        events.AppendElement(AnimationEventParams{ eAnimationStart,
+                                                   intervalEndTime,
+                                                   startTimeStamp});
+        events.AppendElement(AnimationEventParams{ eAnimationEnd,
+                                                   intervalStartTime,
+                                                   endTimeStamp });
+      } else if (currentPhase == AnimationPhase::Active) {
+        events.AppendElement(AnimationEventParams{ eAnimationStart,
+                                                   intervalEndTime,
+                                                   endTimeStamp });
+      }
+      break;
   }
 
-  AutoTArray<EventPair, 2> events;
-  StickyTimeDuration initialAdvance = StickyTimeDuration(InitialAdvance());
-  StickyTimeDuration iterationStart = computedTiming.mDuration *
-                                      computedTiming.mCurrentIteration;
-  const StickyTimeDuration& activeDuration = computedTiming.mActiveDuration;
-
-  if (skippedFirstIteration) {
-    // Notify animationstart and animationiteration in same tick.
-    events.AppendElement(EventPair(eAnimationStart, initialAdvance));
-    events.AppendElement(EventPair(eAnimationIteration,
-                                   std::max(iterationStart, initialAdvance)));
-  } else if (!wasActive && isActive) {
-    events.AppendElement(EventPair(eAnimationStart, initialAdvance));
-  } else if (wasActive && !isActive) {
-    events.AppendElement(EventPair(eAnimationEnd, activeDuration));
-  } else if (wasActive && isActive && !isSameIteration) {
-    events.AppendElement(EventPair(eAnimationIteration, iterationStart));
-  } else if (skippedActivePhase) {
-    events.AppendElement(EventPair(eAnimationStart,
-                                   std::min(initialAdvance, activeDuration)));
-    events.AppendElement(EventPair(eAnimationEnd, activeDuration));
-  } else {
-    return; // No events need to be sent
-  }
+  mPreviousPhase = currentPhase;
+  mPreviousIteration = currentIteration;
 
-  for (const EventPair& pair : events){
+  for (const AnimationEventParams& event : events){
     manager->QueueEvent(
                AnimationEventInfo(owningElement, owningPseudoType,
-                                  pair.first(), mAnimationName,
-                                  pair.second(),
-                                  ElapsedTimeToTimeStamp(pair.second()),
+                                  event.mMessage, mAnimationName,
+                                  event.mElapsedTime, event.mTimeStamp,
                                   this));
   }
 }
diff --git a/layout/style/nsAnimationManager.h b/layout/style/nsAnimationManager.h
index abe3aeeb8..868d4bb42 100644
--- a/layout/style/nsAnimationManager.h
+++ b/layout/style/nsAnimationManager.h
@@ -76,7 +76,8 @@ public:
     , mIsStylePaused(false)
     , mPauseShouldStick(false)
     , mNeedsNewAnimationIndexWhenRun(false)
-    , mPreviousPhaseOrIteration(PREVIOUS_PHASE_BEFORE)
+    , mPreviousPhase(ComputedTiming::AnimationPhase::Null)
+    , mPreviousIteration(0)
   {
     // We might need to drop this assertion once we add a script-accessible
     // constructor but for animations generated from CSS markup the
@@ -257,13 +258,10 @@ protected:
   // its animation index should be updated.
   bool mNeedsNewAnimationIndexWhenRun;
 
-  enum {
-    PREVIOUS_PHASE_BEFORE = uint64_t(-1),
-    PREVIOUS_PHASE_AFTER = uint64_t(-2)
-  };
-  // One of the PREVIOUS_PHASE_* constants, or an integer for the iteration
-  // whose start we last notified on.
-  uint64_t mPreviousPhaseOrIteration;
+  // Phase and current iteration from the previous time we queued events.
+  // This is used to determine what new events to dispatch.
+  ComputedTiming::AnimationPhase mPreviousPhase;
+  uint64_t mPreviousIteration;
 };
 
 } /* namespace dom */