From 914d7d6bf4969c68a82987532e69ec3e9d0447a5 Mon Sep 17 00:00:00 2001
From: Alan Cutter <alancutter@chromium.org>
Date: Tue, 26 Jul 2016 17:07:49 +1000
Subject: [PATCH 1/2] Fix finish() play state issues

---
 src/animation.js                        | 12 ++++++++----
 test/js/animation.js                    | 16 ++++++++--------
 test/web-platform-tests-expectations.js |  9 +++------
 3 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/src/animation.js b/src/animation.js
index e6ac863..a33f90d 100644
--- a/src/animation.js
+++ b/src/animation.js
@@ -172,11 +172,15 @@
       this._paused = true;
     },
     finish: function() {
-      if (this._idle)
-        return;
-      this.currentTime = this._playbackRate > 0 ? this._totalDuration : 0;
-      this._startTime = this._totalDuration - this.currentTime;
+      var end = this._playbackRate > 0 ? this._totalDuration : 0;
+      this.currentTime = end;
+      if (this.startTime === null) {
+        this._startTime = this._timeline.currentTime - end / this._playbackRate;
+      }
       this._currentTimePending = false;
+      this._idle = false;
+      this._paused = false;
+      this._finished = true;
       scope.invalidateEffects();
     },
     cancel: function() {
diff --git a/test/js/animation.js b/test/js/animation.js
index 8c67ca6..4b22835 100644
--- a/test/js/animation.js
+++ b/test/js/animation.js
@@ -318,12 +318,12 @@ suite('animation', function() {
     tick(1000);
     var a = document.body.animate([], 2000);
     a.finish();
-    assert.equal(a.startTime, 0);
+    assert.equal(a.startTime, -1000);
     assert.equal(a.currentTime, 2000);
     a.reverse();
     a.finish();
     assert.equal(a.currentTime, 0);
-    assert.equal(a.startTime, 2000);
+    assert.equal(a.startTime, 1000);
     tick(2000);
   });
   test('cancelling clears all effects', function() {
@@ -494,13 +494,13 @@ suite('animation', function() {
     assert.equal(a.startTime, null);
     tick(1);
     a.finish();
-    assert.equal(a.playState, 'idle');
-    assert.equal(a.currentTime, null);
-    assert.equal(a.startTime, null);
+    assert.equal(a.playState, 'finished');
+    assert.equal(a.currentTime, 300);
+    assert.equal(a.startTime, -299);
     tick(2);
-    assert.equal(a.playState, 'idle');
-    assert.equal(a.currentTime, null);
-    assert.equal(a.startTime, null);
+    assert.equal(a.playState, 'finished');
+    assert.equal(a.currentTime, 300);
+    assert.equal(a.startTime, -299);
   });
   test('Pause after cancel', function() {
     var a = document.body.animate([], 300);
diff --git a/test/web-platform-tests-expectations.js b/test/web-platform-tests-expectations.js
index 38fd47a..34280b3 100644
--- a/test/web-platform-tests-expectations.js
+++ b/test/web-platform-tests-expectations.js
@@ -167,16 +167,13 @@ module.exports = {
           'KeyframeEffectReadOnly is not defined',
 
       'Test finish() while pause-pending with negative playbackRate':
-          'assert_equals: The play state of a pause-pending animation should become "finished" after finish() is called expected "finished" but got "paused"',
+          'FLAKY_TEST_RESULT',
 
       'Test finish() while pause-pending with positive playbackRate':
-          'assert_equals: The play state of a pause-pending animation should become "finished" after finish() is called expected "finished" but got "paused"',
-
-      'Test finish() while paused':
-          'assert_equals: The play state of a paused animation should become "finished" after finish() is called expected "finished" but got "paused"',
+          'FLAKY_TEST_RESULT',
 
       'Test finish() while play-pending':
-          'assert_approx_equals: The start time of a play-pending animation should be set after calling finish() expected NaN +/- 0.0005 but got 0',
+          'FLAKY_TEST_RESULT',
 
       'Test finishing of animation with a current time past the effect end':
           'animation.effect.getComputedTiming is not a function',

From e3855d2df82824c0878717a69f54abb39fef4b41 Mon Sep 17 00:00:00 2001
From: Alan Cutter <alancutter@chromium.org>
Date: Wed, 27 Jul 2016 11:39:17 +1000
Subject: [PATCH 2/2] Rename to limit to match spec

---
 src/animation.js | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/animation.js b/src/animation.js
index a33f90d..faf3611 100644
--- a/src/animation.js
+++ b/src/animation.js
@@ -172,10 +172,10 @@
       this._paused = true;
     },
     finish: function() {
-      var end = this._playbackRate > 0 ? this._totalDuration : 0;
-      this.currentTime = end;
+      var limit = this._playbackRate > 0 ? this._totalDuration : 0;
+      this.currentTime = limit;
       if (this.startTime === null) {
-        this._startTime = this._timeline.currentTime - end / this._playbackRate;
+        this._startTime = this._timeline.currentTime - limit / this._playbackRate;
       }
       this._currentTimePending = false;
       this._idle = false;