Bug 1178665 - Part 3: Make finish notifications asynchronously in most cases. r=bbirtles, r=smaug
authorHiroyuki Ikezoe <hiikezoe@mozilla-japan.org>
Wed, 29 Jul 2015 23:21:00 +0200
changeset 287099 a1cb2639c58c06954ced64ae1b260e6fc763447c
parent 287098 7e8106a7e30333c641e335258793db6b91aba8e8
child 287100 bce7955eaa1363cbc141596b0fa74276aa33b9fe
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbbirtles, smaug
bugs1178665
milestone42.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1178665 - Part 3: Make finish notifications asynchronously in most cases. r=bbirtles, r=smaug
dom/animation/Animation.cpp
dom/animation/Animation.h
dom/animation/test/css-animations/file_animation-finish.html
dom/animation/test/css-animations/file_animation-finished.html
--- a/dom/animation/Animation.cpp
+++ b/dom/animation/Animation.cpp
@@ -8,16 +8,17 @@
 #include "AnimationUtils.h"
 #include "mozilla/dom/AnimationBinding.h"
 #include "mozilla/AutoRestore.h"
 #include "AnimationCommon.h" // For AnimationCollection,
                              // CommonAnimationManager
 #include "nsIDocument.h" // For nsIDocument
 #include "nsIPresShell.h" // For nsIPresShell
 #include "nsLayoutUtils.h" // For PostRestyleEvent (remove after bug 1073336)
+#include "nsThreadUtils.h" // For nsRunnableMethod and nsRevocableEventPtr
 #include "PendingAnimationTracker.h" // For PendingAnimationTracker
 
 namespace mozilla {
 namespace dom {
 
 // Static members
 uint64_t Animation::sNextSequenceNum = 0;
 
@@ -65,17 +66,17 @@ Animation::SetTimeline(AnimationTimeline
   if (mTimeline) {
     mTimeline->RemoveAnimation(*this);
   }
 
   mTimeline = aTimeline;
 
   // FIXME(spec): Once we implement the seeking defined in the spec
   // surely this should be SeekFlag::DidSeek but the spec says otherwise.
-  UpdateTiming(SeekFlag::NoSeek);
+  UpdateTiming(SeekFlag::NoSeek, SyncNotifyFlag::Async);
 
   // FIXME: When we expose this method to script we'll need to call PostUpdate
   // (but *not* when this method gets called from style).
 }
 
 void
 Animation::SetStartTime(const Nullable<TimeDuration>& aNewStartTime)
 {
@@ -102,17 +103,17 @@ Animation::SetStartTime(const Nullable<T
 
   CancelPendingTasks();
   if (mReady) {
     // We may have already resolved mReady, but in that case calling
     // MaybeResolve is a no-op, so that's okay.
     mReady->MaybeResolve(this);
   }
 
-  UpdateTiming(SeekFlag::NoSeek);
+  UpdateTiming(SeekFlag::NoSeek, SyncNotifyFlag::Async);
   PostUpdate();
 }
 
 // http://w3c.github.io/web-animations/#current-time
 Nullable<TimeDuration>
 Animation::GetCurrentTime() const
 {
   Nullable<TimeDuration> result;
@@ -143,17 +144,17 @@ Animation::SetCurrentTime(const TimeDura
     mStartTime.SetNull();
 
     if (mReady) {
       mReady->MaybeResolve(this);
     }
     CancelPendingTasks();
   }
 
-  UpdateTiming(SeekFlag::DidSeek);
+  UpdateTiming(SeekFlag::DidSeek, SyncNotifyFlag::Async);
   PostUpdate();
 }
 
 void
 Animation::SetPlaybackRate(double aPlaybackRate)
 {
   Nullable<TimeDuration> previousTime = GetCurrentTime();
   mPlaybackRate = aPlaybackRate;
@@ -203,18 +204,18 @@ Animation::GetReady(ErrorResult& aRv)
 Promise*
 Animation::GetFinished(ErrorResult& aRv)
 {
   if (!mFinished && mGlobal) {
     mFinished = Promise::Create(mGlobal, aRv); // Lazily create on demand
   }
   if (!mFinished) {
     aRv.Throw(NS_ERROR_FAILURE);
-  } else if (PlayState() == AnimationPlayState::Finished) {
-    mFinished->MaybeResolve(this);
+  } else if (mFinishedIsResolved) {
+    MaybeResolveFinishedPromise();
   }
   return mFinished;
 }
 
 void
 Animation::Cancel()
 {
   DoCancel();
@@ -260,17 +261,17 @@ Animation::Finish(ErrorResult& aRv)
     if (mPendingState == PendingState::PausePending) {
       mHoldTime.SetNull();
     }
     CancelPendingTasks();
     if (mReady) {
       mReady->MaybeResolve(this);
     }
   }
-  UpdateTiming(SeekFlag::DidSeek);
+  UpdateTiming(SeekFlag::DidSeek, SyncNotifyFlag::Sync);
   PostUpdate();
 }
 
 void
 Animation::Play(ErrorResult& aRv, LimitBehavior aLimitBehavior)
 {
   DoPlay(aRv, aLimitBehavior);
   PostUpdate();
@@ -356,17 +357,17 @@ Animation::Tick()
   }
 
   if (IsPossiblyOrphanedPendingAnimation()) {
     MOZ_ASSERT(mTimeline && !mTimeline->GetCurrentTime().IsNull(),
                "Orphaned pending animtaions should have an active timeline");
     FinishPendingAt(mTimeline->GetCurrentTime().Value());
   }
 
-  UpdateTiming(SeekFlag::NoSeek);
+  UpdateTiming(SeekFlag::NoSeek, SyncNotifyFlag::Async);
 }
 
 void
 Animation::TriggerOnNextTick(const Nullable<TimeDuration>& aReadyTime)
 {
   // Normally we expect the play state to be pending but it's possible that,
   // due to the handling of possibly orphaned animations in Tick(), this
   // animation got started whilst still being in another document's pending
@@ -463,23 +464,22 @@ Animation::DoCancel()
     if (mReady) {
       mReady->MaybeReject(NS_ERROR_DOM_ABORT_ERR);
     }
   }
 
   if (mFinished) {
     mFinished->MaybeReject(NS_ERROR_DOM_ABORT_ERR);
   }
-  // Clear finished promise. We'll create a new one lazily.
-  mFinished = nullptr;
+  ResetFinishedPromise();
 
   mHoldTime.SetNull();
   mStartTime.SetNull();
 
-  UpdateTiming(SeekFlag::NoSeek);
+  UpdateTiming(SeekFlag::NoSeek, SyncNotifyFlag::Async);
 }
 
 void
 Animation::UpdateRelevance()
 {
   bool wasRelevant = mIsRelevant;
   mIsRelevant = HasCurrentEffect() || IsInEffect();
 
@@ -601,17 +601,17 @@ Animation::ComposeStyle(nsRefPtr<css::An
         UpdateEffect();
         updatedHoldTime = true;
       }
     }
 
     mEffect->ComposeStyle(aStyleRule, aSetProperties);
 
     if (updatedHoldTime) {
-      UpdateTiming(SeekFlag::NoSeek);
+      UpdateTiming(SeekFlag::NoSeek, SyncNotifyFlag::Async);
     }
 
     mFinishedAtLastComposeStyle = (playState == AnimationPlayState::Finished);
   }
 }
 
 // http://w3c.github.io/web-animations/#play-an-animation
 void
@@ -680,17 +680,17 @@ Animation::DoPlay(ErrorResult& aRv, Limi
   if (doc) {
     PendingAnimationTracker* tracker =
       doc->GetOrCreatePendingAnimationTracker();
     tracker->AddPlayPending(*this);
   } else {
     TriggerOnNextTick(Nullable<TimeDuration>());
   }
 
-  UpdateTiming(SeekFlag::NoSeek);
+  UpdateTiming(SeekFlag::NoSeek, SyncNotifyFlag::Async);
 }
 
 // http://w3c.github.io/web-animations/#pause-an-animation
 void
 Animation::DoPause(ErrorResult& aRv)
 {
   if (IsPausedOrPausing()) {
     return;
@@ -731,17 +731,17 @@ Animation::DoPause(ErrorResult& aRv)
   if (doc) {
     PendingAnimationTracker* tracker =
       doc->GetOrCreatePendingAnimationTracker();
     tracker->AddPausePending(*this);
   } else {
     TriggerOnNextTick(Nullable<TimeDuration>());
   }
 
-  UpdateTiming(SeekFlag::NoSeek);
+  UpdateTiming(SeekFlag::NoSeek, SyncNotifyFlag::Async);
 }
 
 void
 Animation::ResumeAt(const TimeDuration& aReadyTime)
 {
   // This method is only expected to be called for an animation that is
   // waiting to play. We can easily adapt it to handle other states
   // but it's currently not necessary.
@@ -759,17 +759,17 @@ Animation::ResumeAt(const TimeDuration& 
                           (mHoldTime.Value().MultDouble(1 / mPlaybackRate)));
       mHoldTime.SetNull();
     } else {
       mStartTime.SetValue(aReadyTime);
     }
   }
   mPendingState = PendingState::NotPending;
 
-  UpdateTiming(SeekFlag::NoSeek);
+  UpdateTiming(SeekFlag::NoSeek, SyncNotifyFlag::Async);
 
   if (mReady) {
     mReady->MaybeResolve(this);
   }
 }
 
 void
 Animation::PauseAt(const TimeDuration& aReadyTime)
@@ -779,39 +779,39 @@ Animation::PauseAt(const TimeDuration& a
 
   if (!mStartTime.IsNull()) {
     mHoldTime.SetValue((aReadyTime - mStartTime.Value())
                         .MultDouble(mPlaybackRate));
   }
   mStartTime.SetNull();
   mPendingState = PendingState::NotPending;
 
-  UpdateTiming(SeekFlag::NoSeek);
+  UpdateTiming(SeekFlag::NoSeek, SyncNotifyFlag::Async);
 
   if (mReady) {
     mReady->MaybeResolve(this);
   }
 }
 
 void
-Animation::UpdateTiming(SeekFlag aSeekFlag)
+Animation::UpdateTiming(SeekFlag aSeekFlag, SyncNotifyFlag aSyncNotifyFlag)
 {
   // Update the sequence number each time we transition in or out of the
   // idle state
   if (!IsUsingCustomCompositeOrder()) {
     if (PlayState() == AnimationPlayState::Idle) {
       mSequenceNum = kUnsequenced;
     } else if (mSequenceNum == kUnsequenced) {
       mSequenceNum = sNextSequenceNum++;
     }
   }
 
   // We call UpdateFinishedState before UpdateEffect because the former
   // can change the current time, which is used by the latter.
-  UpdateFinishedState(aSeekFlag);
+  UpdateFinishedState(aSeekFlag, aSyncNotifyFlag);
   UpdateEffect();
 
   // Unconditionally Add/Remove from the timeline. This is ok because if the
   // animation has already been added/removed (which will be true more often
   // than not) the work done by AnimationTimeline/DocumentTimeline is still
   // negligible and its easier than trying to detect whenever we are switching
   // to/from being relevant.
   //
@@ -841,17 +841,18 @@ Animation::UpdateTiming(SeekFlag aSeekFl
       mTimeline->AddAnimation(*this);
     } else {
       mTimeline->RemoveAnimation(*this);
     }
   }
 }
 
 void
-Animation::UpdateFinishedState(SeekFlag aSeekFlag)
+Animation::UpdateFinishedState(SeekFlag aSeekFlag,
+                               SyncNotifyFlag aSyncNotifyFlag)
 {
   Nullable<TimeDuration> currentTime = GetCurrentTime();
   TimeDuration effectEnd = TimeDuration(EffectEnd());
 
   if (!mStartTime.IsNull() &&
       mPendingState == PendingState::NotPending) {
     if (mPlaybackRate > 0.0 &&
         !currentTime.IsNull() &&
@@ -879,28 +880,24 @@ Animation::UpdateFinishedState(SeekFlag 
         mStartTime.SetValue(mTimeline->GetCurrentTime().Value() -
                              (mHoldTime.Value().MultDouble(1 / mPlaybackRate)));
       }
       mHoldTime.SetNull();
     }
   }
 
   bool currentFinishedState = PlayState() == AnimationPlayState::Finished;
-  if (currentFinishedState && !mIsPreviousStateFinished) {
-    if (mFinished) {
-      mFinished->MaybeResolve(this);
-    }
-  } else if (!currentFinishedState && mIsPreviousStateFinished) {
-    // Clear finished promise. We'll create a new one lazily.
-    mFinished = nullptr;
+  if (currentFinishedState && !mFinishedIsResolved) {
+    DoFinishNotification(aSyncNotifyFlag);
+  } else if (!currentFinishedState && mFinishedIsResolved) {
+    ResetFinishedPromise();
     if (mEffect->AsTransition()) {
       mEffect->SetIsFinishedTransition(false);
     }
   }
-  mIsPreviousStateFinished = currentFinishedState;
   // We must recalculate the current time to take account of any mHoldTime
   // changes the code above made.
   mPreviousCurrentTime = GetCurrentTime();
 }
 
 void
 Animation::UpdateEffect()
 {
@@ -1061,10 +1058,45 @@ Animation::GetCollection() const
   nsCSSPseudoElements::Type targetPseudoType;
   mEffect->GetTarget(targetElement, targetPseudoType);
   MOZ_ASSERT(targetElement,
              "An animation with an animation manager must have a target");
 
   return manager->GetAnimations(targetElement, targetPseudoType, false);
 }
 
+void
+Animation::DoFinishNotification(SyncNotifyFlag aSyncNotifyFlag)
+{
+  if (aSyncNotifyFlag == SyncNotifyFlag::Sync) {
+    MaybeResolveFinishedPromise();
+  } else if (!mFinishNotificationTask.IsPending()) {
+    nsRefPtr<nsRunnableMethod<Animation>> runnable =
+      NS_NewRunnableMethod(this, &Animation::MaybeResolveFinishedPromise);
+    Promise::DispatchToMicroTask(runnable);
+    mFinishNotificationTask = runnable;
+  }
+}
+
+void
+Animation::ResetFinishedPromise()
+{
+  mFinishedIsResolved = false;
+  mFinished = nullptr;
+}
+
+void
+Animation::MaybeResolveFinishedPromise()
+{
+  mFinishNotificationTask.Revoke();
+
+  if (PlayState() != AnimationPlayState::Finished) {
+    return;
+  }
+
+  if (mFinished) {
+    mFinished->MaybeResolve(this);
+  }
+  mFinishedIsResolved = true;
+}
+
 } // namespace dom
 } // namespace mozilla
--- a/dom/animation/Animation.h
+++ b/dom/animation/Animation.h
@@ -55,19 +55,19 @@ protected:
 
 public:
   explicit Animation(nsIGlobalObject* aGlobal)
     : mGlobal(aGlobal)
     , mPlaybackRate(1.0)
     , mPendingState(PendingState::NotPending)
     , mSequenceNum(kUnsequenced)
     , mIsRunningOnCompositor(false)
-    , mIsPreviousStateFinished(false)
     , mFinishedAtLastComposeStyle(false)
     , mIsRelevant(false)
+    , mFinishedIsResolved(false)
   {
   }
 
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(Animation)
 
   AnimationTimeline* GetParentObject() const { return mTimeline; }
   virtual JSObject* WrapObject(JSContext* aCx,
@@ -318,21 +318,31 @@ protected:
    * Finishing behavior depends on if changes to timing occurred due
    * to a seek or regular playback.
    */
   enum class SeekFlag {
     NoSeek,
     DidSeek
   };
 
-  void UpdateTiming(SeekFlag aSeekFlag);
-  void UpdateFinishedState(SeekFlag aSeekFlag);
+  enum class SyncNotifyFlag {
+    Sync,
+    Async
+  };
+
+  void UpdateTiming(SeekFlag aSeekFlag,
+                    SyncNotifyFlag aSyncNotifyFlag);
+  void UpdateFinishedState(SeekFlag aSeekFlag,
+                           SyncNotifyFlag aSyncNotifyFlag);
   void UpdateEffect();
   void FlushStyle() const;
   void PostUpdate();
+  void ResetFinishedPromise();
+  void MaybeResolveFinishedPromise();
+  void DoFinishNotification(SyncNotifyFlag aSyncNotifyFlag);
 
   /**
    * Remove this animation from the pending animation tracker and reset
    * mPendingState as necessary. The caller is responsible for resolving or
    * aborting the mReady promise as necessary.
    */
   void CancelPendingTasks();
 
@@ -380,21 +390,25 @@ protected:
   static const uint64_t kUnsequenced = UINT64_MAX;
 
   // The sequence number assigned to this animation. This is kUnsequenced
   // while the animation is in the idle state and is updated each time
   // the animation transitions out of the idle state.
   uint64_t mSequenceNum;
 
   bool mIsRunningOnCompositor;
-  // Indicates whether we were in the finished state during our
-  // most recent unthrottled sample (our last ComposeStyle call).
-  bool mIsPreviousStateFinished; // Spec calls this "previous finished state"
   bool mFinishedAtLastComposeStyle;
   // Indicates that the animation should be exposed in an element's
   // getAnimations() list.
   bool mIsRelevant;
+
+  nsRevocableEventPtr<nsRunnableMethod<Animation>> mFinishNotificationTask;
+  // True if mFinished is resolved or would be resolved if mFinished has
+  // yet to be created. This is not set when mFinished is rejected since
+  // in that case mFinished is immediately reset to represent a new current
+  // finished promise.
+  bool mFinishedIsResolved;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_Animation_h
--- a/dom/animation/test/css-animations/file_animation-finish.html
+++ b/dom/animation/test/css-animations/file_animation-finish.html
@@ -234,11 +234,32 @@ async_test(function(t) {
     var marginLeft = parseFloat(getComputedStyle(div).marginLeft);
     assert_equals(marginLeft, 10,
                   'The computed style should be reset when finish() is ' +
                   'called');
     t.done();
   }));
 }, 'Test resetting of computed style');
 
+async_test(function(t) {
+  var div = addDiv(t, {'class': 'animated-div'});
+  div.style.animation = ANIM_PROP_VAL;
+  var animation = div.getAnimations()[0];
+
+  var resolvedFinished = false;
+  animation.finished.then(function() {
+    resolvedFinished = true;
+  });
+
+  animation.ready.then(function() {
+    animation.finish();
+  }).then(t.step_func(function() {
+    assert_true(resolvedFinished,
+      'Animation.finished should be resolved soon after ' +
+      'Animation.finish()');
+    t.done();
+  }));
+
+}, 'Test finish() resolves finished promise synchronously');
+
 done();
 </script>
 </body>
--- a/dom/animation/test/css-animations/file_animation-finished.html
+++ b/dom/animation/test/css-animations/file_animation-finished.html
@@ -400,11 +400,147 @@ async_test(function(t) {
                   '"running" on finished animation');
     assert_equals(animation.currentTime, ANIM_DURATION,
                   'currentTime should not change when animation-play-state ' +
                   'changes to "running" on finished animation');
     t.done();
   }));
 }, 'Test finished promise changes when animationPlayState set to running');
 
+async_test(function(t) {
+  var div = addDiv(t);
+  div.style.animation = ANIM_PROP_VAL;
+  var animation = div.getAnimations()[0];
+
+  var previousFinishedPromise = animation.finished;
+
+  animation.currentTime = ANIM_DURATION;
+
+  animation.finished.then(t.step_func(function() {
+    animation.currentTime = 0;
+    assert_not_equals(animation.finished, previousFinishedPromise,
+                      'Finished promise should change once a prior ' +
+                      'finished promise resolved and the animation ' +
+                      'falls out finished state');
+    t.done();
+  }));
+}, 'Test finished promise changes when a prior finished promise resolved ' +
+   'and the animation falls out finished state');
+
+async_test(function(t) {
+  var div = addDiv(t);
+  div.style.animation = ANIM_PROP_VAL;
+  var animation = div.getAnimations()[0];
+
+  var previousFinishedPromise = animation.finished;
+
+  animation.currentTime = ANIM_DURATION;
+  animation.currentTime = ANIM_DURATION / 2;
+
+  assert_equals(animation.finished, previousFinishedPromise,
+                'No new finished promise generated when finished state ' +
+                'is checked asynchronously');
+  t.done();
+}, 'Test no new finished promise generated when finished state ' +
+   'is checked asynchronously');
+
+async_test(function(t) {
+  var div = addDiv(t);
+  div.style.animation = ANIM_PROP_VAL;
+  var animation = div.getAnimations()[0];
+
+  var previousFinishedPromise = animation.finished;
+
+  animation.finish();
+  animation.currentTime = ANIM_DURATION / 2;
+
+  assert_not_equals(animation.finished, previousFinishedPromise,
+                    'New finished promise generated when finished state ' +
+                    'is checked synchronously');
+  t.done();
+}, 'Test new finished promise generated when finished state ' +
+   'is checked synchronously');
+
+async_test(function(t) {
+  var div = addDiv(t);
+  div.style.animation = ANIM_PROP_VAL;
+  var animation = div.getAnimations()[0];
+
+  var resolvedFinished = false;
+  animation.finished.then(function() {
+    resolvedFinished = true;
+  });
+
+  animation.ready.then(function() {
+    animation.finish();
+    animation.currentTime = ANIM_DURATION / 2;
+  }).then(t.step_func(function() {
+    assert_true(resolvedFinished,
+      'Animation.finished should be resolved even if ' +
+      'the finished state is changed soon');
+    t.done();
+  }));
+
+}, 'Test synchronous finished promise resolved even if finished state ' +
+   'is changed soon');
+
+async_test(function(t) {
+  var div = addDiv(t);
+  div.style.animation = ANIM_PROP_VAL;
+  var animation = div.getAnimations()[0];
+
+  var resolvedFinished = false;
+  animation.finished.then(function() {
+    resolvedFinished = true;
+  });
+
+  animation.ready.then(t.step_func(function() {
+    animation.currentTime = ANIM_DURATION;
+    animation.finish();
+  })).then(t.step_func(function() {
+    assert_true(resolvedFinished,
+      'Animation.finished should be resolved soon after finish() is ' +
+      'called even if there are other asynchronous promises just before it');
+    t.done();
+  }));
+}, 'Test synchronous finished promise resolved even if asynchronous ' +
+   'finished promise happens just before synchronous promise');
+
+async_test(function(t) {
+  var div = addDiv(t);
+  div.style.animation = ANIM_PROP_VAL;
+  var animation = div.getAnimations()[0];
+
+  animation.finished.then(t.step_func(function() {
+    assert_unreached('Animation.finished should not be resolved');
+  }));
+
+  animation.ready.then(function() {
+    animation.currentTime = ANIM_DURATION;
+    animation.currentTime = ANIM_DURATION / 2;
+  }).then(t.step_func(function() {
+    t.done();
+  }));
+}, 'Test finished promise is not resolved when the animation ' +
+   'falls out finished state immediately');
+
+async_test(function(t) {
+  var div = addDiv(t);
+  div.style.animation = ANIM_PROP_VAL;
+  var animation = div.getAnimations()[0];
+
+  animation.ready.then(function() {
+    animation.currentTime = ANIM_DURATION;
+    animation.finished.then(t.step_func(function() {
+      assert_unreached('Animation.finished should not be resolved');
+    }));
+    animation.currentTime = 0;
+  }).then(t.step_func(function() {
+    t.done();
+  }));
+
+}, 'Test finished promise is not resolved once the animation ' +
+   'falls out finished state even though the current finished ' +
+   'promise is generated soon after animation state became finished');
+
 done();
 </script>
 </body>