Bug 1178665 - Part 3: Make finish notifications asynchronously in most cases. r=bbirtles, r=smaug

dom/animation/Animation.cpp

@@ -13,6 +13,7 @@
 #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 {
@@ -70,7 +71,7 @@ Animation::SetTimeline(AnimationTimeline* 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).
@@ -107,7 +108,7 @@ Animation::SetStartTime(const Nullable<TimeDuration>& aNewStartTime)
     mReady->MaybeResolve(this);
   }
 
-  UpdateTiming(SeekFlag::NoSeek);
+  UpdateTiming(SeekFlag::NoSeek, SyncNotifyFlag::Async);
   PostUpdate();
 }
 
@@ -148,7 +149,7 @@ Animation::SetCurrentTime(const TimeDuration& aSeekTime)
     CancelPendingTasks();
   }
 
-  UpdateTiming(SeekFlag::DidSeek);
+  UpdateTiming(SeekFlag::DidSeek, SyncNotifyFlag::Async);
   PostUpdate();
 }
 
@@ -208,8 +209,8 @@ Animation::GetFinished(ErrorResult& aRv)
   }
   if (!mFinished) {
     aRv.Throw(NS_ERROR_FAILURE);
-  } else if (PlayState() == AnimationPlayState::Finished) {
-    mFinished->MaybeResolve(this);
+  } else if (mFinishedIsResolved) {
+    MaybeResolveFinishedPromise();
   }
   return mFinished;
 }
@@ -265,7 +266,7 @@ Animation::Finish(ErrorResult& aRv)
       mReady->MaybeResolve(this);
     }
   }
-  UpdateTiming(SeekFlag::DidSeek);
+  UpdateTiming(SeekFlag::DidSeek, SyncNotifyFlag::Sync);
   PostUpdate();
 }
 
@@ -361,7 +362,7 @@ Animation::Tick()
     FinishPendingAt(mTimeline->GetCurrentTime().Value());
   }
 
-  UpdateTiming(SeekFlag::NoSeek);
+  UpdateTiming(SeekFlag::NoSeek, SyncNotifyFlag::Async);
 }
 
 void
@@ -468,13 +469,12 @@ Animation::DoCancel()
   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
@@ -606,7 +606,7 @@ Animation::ComposeStyle(nsRefPtr<css::AnimValuesStyleRule>& aStyleRule,
     mEffect->ComposeStyle(aStyleRule, aSetProperties);
 
     if (updatedHoldTime) {
-      UpdateTiming(SeekFlag::NoSeek);
+      UpdateTiming(SeekFlag::NoSeek, SyncNotifyFlag::Async);
     }
 
     mFinishedAtLastComposeStyle = (playState == AnimationPlayState::Finished);
@@ -685,7 +685,7 @@ Animation::DoPlay(ErrorResult& aRv, LimitBehavior aLimitBehavior)
     TriggerOnNextTick(Nullable<TimeDuration>());
   }
 
-  UpdateTiming(SeekFlag::NoSeek);
+  UpdateTiming(SeekFlag::NoSeek, SyncNotifyFlag::Async);
 }
 
 // http://w3c.github.io/web-animations/#pause-an-animation
@@ -736,7 +736,7 @@ Animation::DoPause(ErrorResult& aRv)
     TriggerOnNextTick(Nullable<TimeDuration>());
   }
 
-  UpdateTiming(SeekFlag::NoSeek);
+  UpdateTiming(SeekFlag::NoSeek, SyncNotifyFlag::Async);
 }
 
 void
@@ -764,7 +764,7 @@ Animation::ResumeAt(const TimeDuration& aReadyTime)
   }
   mPendingState = PendingState::NotPending;
 
-  UpdateTiming(SeekFlag::NoSeek);
+  UpdateTiming(SeekFlag::NoSeek, SyncNotifyFlag::Async);
 
   if (mReady) {
     mReady->MaybeResolve(this);
@@ -784,7 +784,7 @@ Animation::PauseAt(const TimeDuration& aReadyTime)
   mStartTime.SetNull();
   mPendingState = PendingState::NotPending;
 
-  UpdateTiming(SeekFlag::NoSeek);
+  UpdateTiming(SeekFlag::NoSeek, SyncNotifyFlag::Async);
 
   if (mReady) {
     mReady->MaybeResolve(this);
@@ -792,7 +792,7 @@ Animation::PauseAt(const TimeDuration& aReadyTime)
 }
 
 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
@@ -806,7 +806,7 @@ Animation::UpdateTiming(SeekFlag aSeekFlag)
 
   // 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
@@ -846,7 +846,8 @@ Animation::UpdateTiming(SeekFlag aSeekFlag)
 }
 
 void
-Animation::UpdateFinishedState(SeekFlag aSeekFlag)
+Animation::UpdateFinishedState(SeekFlag aSeekFlag,
+                               SyncNotifyFlag aSyncNotifyFlag)
 {
   Nullable<TimeDuration> currentTime = GetCurrentTime();
   TimeDuration effectEnd = TimeDuration(EffectEnd());
@@ -884,18 +885,14 @@ Animation::UpdateFinishedState(SeekFlag aSeekFlag)
   }
 
   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();
@@ -1066,5 +1063,40 @@ Animation::GetCollection() const
   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

dom/animation/Animation.h

@@ -60,9 +60,9 @@ public:
     , mPendingState(PendingState::NotPending)
     , mSequenceNum(kUnsequenced)
     , mIsRunningOnCompositor(false)
-    , mIsPreviousStateFinished(false)
     , mFinishedAtLastComposeStyle(false)
     , mIsRelevant(false)
+    , mFinishedIsResolved(false)
   {
   }
 
@@ -323,11 +323,21 @@ protected:
     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
@@ -385,13 +395,17 @@ protected:
   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

dom/animation/test/css-animations/file_animation-finish.html

@@ -239,6 +239,27 @@ async_test(function(t) {
   }));
 }, '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>

dom/animation/test/css-animations/file_animation-finished.html

@@ -405,6 +405,142 @@ async_test(function(t) {
   }));
 }, '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>