Bug 1302648 part 4 - Call UpdateTiming() after removing the animation from the timeline. r?birtles draft
authorMantaroh Yoshinaga <mantaroh@gmail.com>
Fri, 10 Feb 2017 12:32:44 +0900
changeset 481636 8b7ba1a464a19d9f66b2a253e036647eaa2a534e
parent 481635 bb2941d84a7c1b5db170030cd9ee4f57160df40c
child 481637 e15baf78be03d20bffd1113f2a7129bcb33a2929
push id44889
push usermantaroh@gmail.com
push dateFri, 10 Feb 2017 08:41:14 +0000
reviewersbirtles
bugs1302648, 1264125
milestone54.0a1
Bug 1302648 part 4 - Call UpdateTiming() after removing the animation from the timeline. r?birtles We will need to remove animation from timeline before calling Animation::UpdateTiming() in order to fire the cancel event. In bug 1264125, we request one more tick after calling Animation::Cancel(), however we won't need to call this request if we apply this changeset. MozReview-Commit-ID: h0dxUdtgkl
dom/animation/Animation.cpp
layout/style/nsAnimationManager.h
layout/style/nsTransitionManager.h
--- a/dom/animation/Animation.cpp
+++ b/dom/animation/Animation.cpp
@@ -782,22 +782,31 @@ Animation::CancelNoUpdate()
 
   StickyTimeDuration activeTime = mEffect
                                   ? mEffect->GetComputedTiming().mActiveTime
                                   : StickyTimeDuration();
 
   mHoldTime.SetNull();
   mStartTime.SetNull();
 
-  UpdateTiming(SeekFlag::NoSeek, SyncNotifyFlag::Async);
-
   if (mTimeline) {
     mTimeline->RemoveAnimation(this);
   }
   MaybeQueueCancelEvent(activeTime);
+
+  // When an animation is cancelled it no longer needs further ticks from the
+  // timeline. However, if we queued a cancel event and this was the last
+  // animation attached to the timeline, the timeline will stop observing the
+  // refresh driver and there may be no subsequent refresh driver tick for
+  // dispatching the queued event.
+  //
+  // By calling UpdateTiming *after* removing ourselves from our timeline, we
+  // ensure the timeline will register with the refresh driver for at least one
+  // more tick.
+  UpdateTiming(SeekFlag::NoSeek, SyncNotifyFlag::Async);
 }
 
 bool
 Animation::ShouldBeSynchronizedWithMainThread(
   nsCSSPropertyID aProperty,
   const nsIFrame* aFrame,
   AnimationPerformanceWarning::Type& aPerformanceWarning) const
 {
--- a/layout/style/nsAnimationManager.h
+++ b/layout/style/nsAnimationManager.h
@@ -105,17 +105,16 @@ public:
 
   virtual AnimationPlayState PlayStateFromJS() const override;
   virtual void PlayFromJS(ErrorResult& aRv) override;
 
   void PlayFromStyle();
   void PauseFromStyle();
   void CancelFromStyle() override
   {
-
     // When an animation is disassociated with style it enters an odd state
     // where its composite order is undefined until it first transitions
     // out of the idle state.
     //
     // Even if the composite order isn't defined we don't want it to be random
     // in case we need to determine the order to dispatch events associated
     // with an animation in this state. To solve this we treat the animation as
     // if it had been added to the end of the global animation list so that
--- a/layout/style/nsTransitionManager.h
+++ b/layout/style/nsTransitionManager.h
@@ -157,28 +157,16 @@ public:
     // defined until that moment.
     //
     // See longer explanation in CSSAnimation::CancelFromStyle.
     mAnimationIndex = sNextAnimationIndex++;
     mNeedsNewAnimationIndexWhenRun = true;
 
     Animation::CancelFromStyle();
 
-    // The above call to Animation::CancelFromStyle may cause a transitioncancel
-    // event to be queued. However, it will also remove the transition from its
-    // timeline. If this transition was the last animation attached to
-    // the timeline, the timeline will stop observing the refresh driver and
-    // there may be no subsequent tick fro dispatching animation events.
-    //
-    // To ensure the cancel event is dispatched we tell the timeline it needs to
-    // observe the refresh driver for at least one more tick.
-    if (mTimeline) {
-      mTimeline->NotifyAnimationUpdated(*this);
-    }
-
     // It is important we do this *after* calling CancelFromStyle().
     // This is because CancelFromStyle() will end up posting a restyle and
     // that restyle should target the *transitions* level of the cascade.
     // However, once we clear the owning element, CascadeLevel() will begin
     // returning CascadeLevel::Animations.
     mOwningElement = OwningElementRef();
   }