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

2015-07-29 23:21:00 +02:00 · 2015-07-29 23:21:00 +02:00 · 77aabfc80f
--- a/dom/animation/Animation.cpp
+++ b/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
--- a/dom/animation/Animation.h
+++ b/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
--- a/dom/animation/test/css-animations/file_animation-finish.html
+++ b/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>
--- a/dom/animation/test/css-animations/file_animation-finished.html
+++ b/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>