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

Issue #55

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
+struct AnimationEventParams {
-// events to queue.
+  EventMessage mMessage;
-typedef Pair<EventMessage, StickyTimeDuration> EventPair;
+  StickyTimeDuration mElapsedTime;
+  TimeStamp mTimeStamp;
+};
 
 } // anonymous namespace
 
@@ -196,75 +200,93 @@ CSSAnimation::QueueEvents()
 
   ComputedTiming computedTiming = mEffect->GetComputedTiming();
 
-  if (computedTiming.mPhase == ComputedTiming::AnimationPhase::Null) {
+  ComputedTiming::AnimationPhase currentPhase = computedTiming.mPhase;
-    return; // do nothing
+  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
+  const StickyTimeDuration zeroDuration;
-  // backwards through the animation as well as forwards. An 'animationstart'
+  StickyTimeDuration intervalStartTime =
-  // is dispatched if we enter the active phase (regardless if that is from
+    std::max(std::min(StickyTimeDuration(-mEffect->SpecifiedTiming().mDelay),
-  // before or after the animation's active phase). An 'animationend' is
+                      computedTiming.mActiveDuration),
-  // dispatched if we leave the active phase (regardless if that is to before
+             zeroDuration);
-  // or after the animation's active phase).
+  StickyTimeDuration intervalEndTime =
+    std::max(std::min((EffectEnd() - mEffect->SpecifiedTiming().mDelay),
+                      computedTiming.mActiveDuration),
+             zeroDuration);
 
-  bool wasActive = mPreviousPhaseOrIteration != PREVIOUS_PHASE_BEFORE &&
+  uint64_t iterationBoundary = mPreviousIteration > currentIteration
-                   mPreviousPhaseOrIteration != PREVIOUS_PHASE_AFTER;
+                               ? currentIteration + 1
-  bool isActive =
+                               : currentIteration;
-    computedTiming.mPhase == ComputedTiming::AnimationPhase::Active;
+  StickyTimeDuration iterationStartTime  =
-  bool isSameIteration =
+    computedTiming.mDuration.MultDouble(
-         computedTiming.mCurrentIteration == mPreviousPhaseOrIteration;
+      (iterationBoundary - computedTiming.mIterationStart));
-  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),
+  TimeStamp startTimeStamp     = ElapsedTimeToTimeStamp(intervalStartTime);
-             "skippedActivePhase only makes sense if we were & are inactive");
+  TimeStamp endTimeStamp       = ElapsedTimeToTimeStamp(intervalEndTime);
+  TimeStamp iterationTimeStamp = ElapsedTimeToTimeStamp(iterationStartTime);
 
-  if (computedTiming.mPhase == ComputedTiming::AnimationPhase::Before) {
+  AutoTArray<AnimationEventParams, 2> events;
-    mPreviousPhaseOrIteration = PREVIOUS_PHASE_BEFORE;
+  switch (mPreviousPhase) {
-  } else if (computedTiming.mPhase == ComputedTiming::AnimationPhase::Active) {
+    case AnimationPhase::Null:
-    mPreviousPhaseOrIteration = computedTiming.mCurrentIteration;
+    case AnimationPhase::Before:
-  } else if (computedTiming.mPhase == ComputedTiming::AnimationPhase::After) {
+      if (currentPhase == AnimationPhase::Active) {
-    mPreviousPhaseOrIteration = PREVIOUS_PHASE_AFTER;
+        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;
+  mPreviousPhase = currentPhase;
-  StickyTimeDuration initialAdvance = StickyTimeDuration(InitialAdvance());
+  mPreviousIteration = currentIteration;
-  StickyTimeDuration iterationStart = computedTiming.mDuration *
-                                      computedTiming.mCurrentIteration;
-  const StickyTimeDuration& activeDuration = computedTiming.mActiveDuration;
 
-  if (skippedFirstIteration) {
+  for (const AnimationEventParams& event : events){
-    // 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
-  }
-
-  for (const EventPair& pair : events){
     manager->QueueEvent(
                AnimationEventInfo(owningElement, owningPseudoType,
-                                  pair.first(), mAnimationName,
+                                  event.mMessage, mAnimationName,
-                                  pair.second(),
+                                  event.mElapsedTime, event.mTimeStamp,
-                                  ElapsedTimeToTimeStamp(pair.second()),
                                   this));
   }
 }

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 {
+  // Phase and current iteration from the previous time we queued events.
-    PREVIOUS_PHASE_BEFORE = uint64_t(-1),
+  // This is used to determine what new events to dispatch.
-    PREVIOUS_PHASE_AFTER = uint64_t(-2)
+  ComputedTiming::AnimationPhase mPreviousPhase;
-  };
+  uint64_t mPreviousIteration;
-  // One of the PREVIOUS_PHASE_* constants, or an integer for the iteration
-  // whose start we last notified on.
-  uint64_t mPreviousPhaseOrIteration;
 };
 
 } /* namespace dom */