Bug 1166164 part 3 - Resolve start time on finish(); r=jwatt

We have already resolved to make calling Finish() clear the pause state (see
https://lists.w3.org/Archives/Public/public-fx/2015AprJun/0038.html, item 2).
Doing that involves resolving the start time when the animation is paused.

Furthermore, as a separate change, we resolved to make the finished promise not
resolve when the animation is paused. That suggests making UpdateFinishedState()
only resolve the finished promise when PlayState() == Finished rather than using
IsFinished() which returns true even if the animation is paused.

However, if we compare PlayState() == Finished in UpdateFinishedState() then we
will *not* resolve the finished promise when the animation is play-pending since
PlayState() == Pending in that case (pause-pending is ok since the call to
SetCurrentTime will cause a transition to the Paused state). Furthermore, the
existing call to cancel the pending play task will effectively leave this
animation forever pending. Hence, in this patch we unconditionally fill in the
start time.

--HG--
extra : rebase_source : 499ad0530eac0ee62c8ed2df41360c45abc34816

dom/animation/Animation.cpp

@@ -230,7 +230,26 @@ Animation::Finish(ErrorResult& aRv)
 
   SetCurrentTime(limit);
 
-  if (mPendingState == PendingState::PlayPending) {
+  // If we are paused or play-pending we need to fill in the start time in
+  // order to transition to the finished state.
+  //
+  // We only do this, however, if we have an active timeline. If we have an
+  // inactive timeline we can't transition into the finished state just like
+  // we can't transition to the running state (this finished state is really
+  // a substate of the running state).
+  if (mStartTime.IsNull() &&
+      mTimeline &&
+      !mTimeline->GetCurrentTime().IsNull()) {
+    mStartTime.SetValue(mTimeline->GetCurrentTime().Value() -
+                        limit.MultDouble(1.0 / mPlaybackRate));
+  }
+
+  // If we just resolved the start time for a pause-pending animation, we need
+  // to clear the task. We don't do this as a branch of the above however since
+  // we can have a play-pending animation with a resolved start time if we
+  // aborted a pause operation.
+  if (mPendingState == PendingState::PlayPending &&
+      !mStartTime.IsNull()) {
     CancelPendingTasks();
     if (mReady) {
       mReady->MaybeResolve(this);

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

@@ -121,24 +121,108 @@ async_test(function(t) {
                   'should be set back to zero');
     t.done();
   }));
-}, 'Test finishing of reversed animation with with a current time less ' +
-   'than zero');
+}, 'Test finishing of reversed animation with a current time less than zero');
+
+async_test(function(t) {
+  var div = addDiv(t);
+  div.style.animation = ANIM_PROP_VAL + ' paused';
+  var animation = div.getAnimations()[0];
+
+  animation.ready.then(t.step_func(function() {
+    animation.finish();
+    assert_equals(animation.playState, 'finished',
+                  'The play state of a paused animation should become ' +
+                  '"finished" after finish() is called');
+    assert_approx_equals(animation.startTime,
+                         animation.timeline.currentTime - ANIM_DURATION,
+                         0.0001,
+                         'The start time of a paused animation should be set ' +
+                         'after calling finish()');
+    t.done();
+  }));
+}, 'Test finish() while paused');
+
+test(function(t) {
+  var div = addDiv(t);
+  div.style.animation = ANIM_PROP_VAL + ' paused';
+  var animation = div.getAnimations()[0];
+
+  // Update playbackRate so we can test that the calculated startTime
+  // respects it
+  animation.playbackRate = 2;
+
+  // While animation is still pause-pending call finish()
+  animation.finish();
+
+  assert_equals(animation.playState, 'finished',
+                'The play state of a pause-pending animation should become ' +
+                '"finished" after finish() is called');
+  assert_approx_equals(animation.startTime,
+                       animation.timeline.currentTime - ANIM_DURATION / 2,
+                       0.0001,
+                       'The start time of a pause-pending animation should ' +
+                       'be set after calling finish()');
+}, 'Test finish() while pause-pending with positive playbackRate');
+
+test(function(t) {
+  var div = addDiv(t);
+  div.style.animation = ANIM_PROP_VAL + ' paused';
+  var animation = div.getAnimations()[0];
+
+  animation.playbackRate = -2;
+  animation.finish();
+
+  assert_equals(animation.playState, 'finished',
+                'The play state of a pause-pending animation should become ' +
+                '"finished" after finish() is called');
+  assert_equals(animation.startTime, animation.timeline.currentTime,
+                'The start time of a pause-pending animation should be ' +
+                'set after calling finish()');
+}, 'Test finish() while pause-pending with negative playbackRate');
+
+test(function(t) {
+  var div = addDiv(t);
+  div.style.animation = ANIM_PROP_VAL;
+  var animation = div.getAnimations()[0];
+
+  animation.playbackRate = 0.5;
+  animation.finish();
+
+  assert_equals(animation.playState, 'finished',
+                'The play state of a play-pending animation should become ' +
+                '"finished" after finish() is called');
+  assert_approx_equals(animation.startTime,
+                       animation.timeline.currentTime - ANIM_DURATION / 0.5,
+                       0.0001,
+                       'The start time of a play-pending animation should ' +
+                       'be set after calling finish()');
+}, 'Test finish() while play-pending');
+
+// FIXME: Add a test for when we are play-pending without an active timeline.
+// - In that case even after calling finish() we should still be pending but
+//   the current time should be updated
 
 async_test(function(t) {
   var div = addDiv(t);
   div.style.animation = ANIM_PROP_VAL;
   var animation = div.getAnimations()[0];
 
-  animation.pause();
-
   animation.ready.then(t.step_func(function() {
+    // Trigger pause
+    animation.pause();
+    // Then abort
+    animation.play();
+
+    // We are now in the unusual situation of being play-pending whilst having
+    // a resolved start time. Check that finish() still triggers a transition
+    // to the finished state immediately.
     animation.finish();
-    assert_equals(animation.playState, 'paused',
-                  'The play state of a paused animation should remain ' +
-                  '"paused" even after finish() is called');
+    assert_equals(animation.playState, 'finished',
+                  'After aborting a pause then calling finish() the play ' +
+                  'state of an animation should become "finished" immediately');
     t.done();
   }));
-}, 'Test paused state after finishing of animation');
+}, 'Test finish() during aborted pause');
 
 async_test(function(t) {
   var div = addDiv(t, {'class': 'animated-div'});

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

@@ -48,7 +48,7 @@ async_test(function(t) {
 
   var previousFinishedPromise = animation.finished;
 
-  animation.currentTime = ANIM_DURATION;
+  animation.finish();
 
   animation.finished.then(t.step_func(function() {
     assert_equals(animation.finished, previousFinishedPromise,
@@ -73,7 +73,7 @@ async_test(function(t) {
 
   var previousFinishedPromise;
 
-  animation.currentTime = ANIM_DURATION;
+  animation.finish();
 
   animation.finished.then(t.step_func(function() {
     previousFinishedPromise = animation.finished;
@@ -100,7 +100,7 @@ async_test(function(t) {
 
   var previousFinishedPromise = animation.finished;
 
-  animation.currentTime = ANIM_DURATION;
+  animation.finish();
 
   animation.finished.then(t.step_func(function() {
     animation.currentTime = ANIM_DURATION + 1000;