Bug 1134163 - Part1.Modify animationstart event timing in order to fire event after end of pending task. r?birtles draft
authorMantaroh Yoshinaga <mantaroh@gmail.com>
Thu, 28 Apr 2016 09:51:07 +0900
changeset 357107 1ed7fe135570524f0d3f848eb6d68daee483d63c
parent 356965 86730d0a82093d705e44f33a34973d28b269f1ea
child 357108 f8d8e08e8bd6b0c0c976eeacf38d1acb2810a474
push id16701
push usermantaroh@gmail.com
push dateThu, 28 Apr 2016 00:51:42 +0000
reviewersbirtles
bugs1134163
milestone49.0a1
Bug 1134163 - Part1.Modify animationstart event timing in order to fire event after end of pending task. r?birtles MozReview-Commit-ID: DJtHDzhmS54
layout/base/nsPresShell.cpp
layout/style/nsAnimationManager.cpp
layout/style/nsAnimationManager.h
--- a/layout/base/nsPresShell.cpp
+++ b/layout/base/nsPresShell.cpp
@@ -4054,22 +4054,16 @@ PresShell::FlushPendingNotifications(moz
 
       // The FlushResampleRequests() above flushed style changes.
       if (!mIsDestroying) {
         nsAutoScriptBlocker scriptBlocker;
         mPresContext->RestyleManager()->ProcessPendingRestyles();
       }
     }
 
-    // Dispatch any 'animationstart' events those (or earlier) restyles
-    // queued up.
-    if (!mIsDestroying) {
-      mPresContext->AnimationManager()->DispatchEvents();
-    }
-
     // Process whatever XBL constructors those restyles queued up.  This
     // ensures that onload doesn't fire too early and that we won't do extra
     // reflows after those constructors run.
     if (!mIsDestroying) {
       mDocument->BindingManager()->ProcessAttachedQueue();
     }
 
     // Now those constructors or events might have posted restyle
--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@@ -147,16 +147,22 @@ CSSAnimation::HasLowerCompositeOrderThan
 
 void
 CSSAnimation::QueueEvents()
 {
   if (!mEffect) {
     return;
   }
 
+  // If the animation is pending, we ignore animation events until we finish
+  // pending.
+  if (mPendingState != PendingState::NotPending) {
+    return;
+  }
+
   // CSS animations dispatch events at their owning element. This allows
   // script to repurpose a CSS animation to target a different element,
   // to use a group effect (which has no obvious "target element"), or
   // to remove the animation effect altogether whilst still getting
   // animation events.
   //
   // It does mean, however, that for a CSS animation that has no owning
   // element (e.g. it was created using the CSSAnimation constructor or
@@ -191,76 +197,82 @@ CSSAnimation::QueueEvents()
   // 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;
+    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;
   }
 
-  EventMessage message;
+  AutoTArray<EventPair, 2> events;
+  StickyTimeDuration initialAdvanceTime = StickyTimeDuration(InitialAdvance());
+  StickyTimeDuration iterationStart = computedTiming.mDuration *
+                                      computedTiming.mCurrentIteration;
+
+  if (skippedFirstIteration) {
+    // Notify animationstart and animationiteration in same tick.
+    events.AppendElement(EventPair(eAnimationStart,
+                                   std::min(initialAdvanceTime,
+                                            computedTiming.mActiveDuration)));
 
-  if (!wasActive && isActive) {
-    message = eAnimationStart;
+    events.AppendElement(EventPair(eAnimationIteration,
+                                   std::max(iterationStart, initialAdvanceTime)));
+  } else if (!wasActive && isActive) {
+    events.AppendElement(EventPair(eAnimationStart,
+                                   std::max(iterationStart, initialAdvanceTime)));
   } else if (wasActive && !isActive) {
-    message = eAnimationEnd;
+    events.AppendElement(EventPair(eAnimationEnd, computedTiming.mActiveDuration));
   } else if (wasActive && isActive && !isSameIteration) {
-    message = eAnimationIteration;
+    events.AppendElement(EventPair(eAnimationIteration,
+                                   std::max(iterationStart, initialAdvanceTime)));
   } else if (skippedActivePhase) {
     // First notifying for start of 0th iteration by appending an
     // 'animationstart':
-    StickyTimeDuration elapsedTime =
-      std::min(StickyTimeDuration(InitialAdvance()),
-               computedTiming.mActiveDuration);
-    manager->QueueEvent(AnimationEventInfo(owningElement, owningPseudoType,
-                                           eAnimationStart, mAnimationName,
-                                           elapsedTime,
-                                           ElapsedTimeToTimeStamp(elapsedTime),
-                                           this));
+    events.AppendElement(EventPair(eAnimationStart,
+                                   std::min(initialAdvanceTime,
+                                            computedTiming.mActiveDuration)));
+
     // Then have the shared code below append an 'animationend':
-    message = eAnimationEnd;
+    events.AppendElement(EventPair(eAnimationEnd,
+                                   computedTiming.mActiveDuration));
   } else {
     return; // No events need to be sent
   }
 
-  StickyTimeDuration elapsedTime;
-
-  if (message == eAnimationStart || message == eAnimationIteration) {
-    StickyTimeDuration iterationStart = computedTiming.mDuration *
-                                          computedTiming.mCurrentIteration;
-    elapsedTime = std::max(iterationStart, StickyTimeDuration(InitialAdvance()));
-  } else {
-    MOZ_ASSERT(message == eAnimationEnd);
-    elapsedTime = computedTiming.mActiveDuration;
+  for (const EventPair pair : events){
+    manager->QueueEvent(AnimationEventInfo(owningElement, owningPseudoType,
+                                           pair.first(), mAnimationName,
+                                           pair.second(),
+                                           ElapsedTimeToTimeStamp(pair.second()),
+                                           this));
   }
-
-  manager->QueueEvent(AnimationEventInfo(owningElement, owningPseudoType,
-                                         message, mAnimationName, elapsedTime,
-                                         ElapsedTimeToTimeStamp(elapsedTime),
-                                         this));
 }
 
 void
 CSSAnimation::UpdateTiming(SeekFlag aSeekFlag, SyncNotifyFlag aSyncNotifyFlag)
 {
   if (mNeedsNewAnimationIndexWhenRun &&
       PlayState() != AnimationPlayState::Idle) {
     mAnimationIndex = sNextAnimationIndex++;
@@ -269,35 +281,18 @@ CSSAnimation::UpdateTiming(SeekFlag aSee
 
   Animation::UpdateTiming(aSeekFlag, aSyncNotifyFlag);
 }
 
 TimeStamp
 CSSAnimation::ElapsedTimeToTimeStamp(const StickyTimeDuration&
                                        aElapsedTime) const
 {
-  // Initializes to null. We always return this object to benefit from
-  // return-value-optimization.
-  TimeStamp result;
-
-  // Currently we may dispatch animationstart events before resolving
-  // mStartTime if we have a delay <= 0. This will change in bug 1134163
-  // but until then we should just use the latest refresh driver time as
-  // the event timestamp in that case.
-  if (!mEffect || mStartTime.IsNull()) {
-    nsPresContext* presContext = GetPresContext();
-    if (presContext) {
-      result = presContext->RefreshDriver()->MostRecentRefresh();
-    }
-    return result;
-  }
-
-  result = AnimationTimeToTimeStamp(aElapsedTime +
-                                    mEffect->SpecifiedTiming().mDelay);
-  return result;
+  return AnimationTimeToTimeStamp(aElapsedTime +
+                                  mEffect->SpecifiedTiming().mDelay);
 }
 
 ////////////////////////// nsAnimationManager ////////////////////////////
 
 NS_IMPL_CYCLE_COLLECTION(nsAnimationManager, mEventDispatcher)
 
 NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE(nsAnimationManager, AddRef)
 NS_IMPL_CYCLE_COLLECTION_UNROOT_NATIVE(nsAnimationManager, Release)
@@ -658,23 +653,16 @@ CSSAnimationBuilder::Build(nsPresContext
   animation->SetTimeline(mTimeline);
   animation->SetEffect(effect);
 
   if (isStylePaused) {
     animation->PauseFromStyle();
   } else {
     animation->PlayFromStyle();
   }
-  // FIXME: Bug 1134163 - We shouldn't queue animationstart events
-  // until the animation is actually ready to run. However, we
-  // currently have some tests that assume that these events are
-  // dispatched within the same tick as the animation is added
-  // so we need to queue up any animationstart events from newly-created
-  // animations.
-  animation->QueueEvents();
 
   return animation.forget();
 }
 
 nsTArray<Keyframe>
 CSSAnimationBuilder::BuildAnimationFrames(nsPresContext* aPresContext,
                                           const StyleAnimation& aSrc,
                                           const nsCSSKeyframesRule* aRule)
--- a/layout/style/nsAnimationManager.h
+++ b/layout/style/nsAnimationManager.h
@@ -258,16 +258,18 @@ protected:
 
   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;
+
+  typedef Pair<EventMessage, StickyTimeDuration> EventPair;
 };
 
 } /* namespace dom */
 
 template <>
 struct AnimationTypeTraits<dom::CSSAnimation>
 {
   static nsIAtom* ElementPropertyAtom()