Merge pull request #247 from dstockwell/newidle

Players should be constructed in idle state

Author: dstoc

src/animation-constructor.js

@@ -38,7 +38,6 @@
     else
       this.effect = new KeyframeEffect(effect);
     this._effect = effect;
-    this._internalPlayer = null;
     this.activeDuration = shared.calculateActiveDuration(this._timing);
     return this;
   };

src/group-constructors.js

@@ -21,7 +21,6 @@
 
     if (this._timing.duration === 'auto')
       this._timing.duration = this.activeDuration;
-    this._internalPlayer = null;
   }
 
   window.AnimationSequence = function() {

src/maxifill-player.js

@@ -24,6 +24,8 @@
     this._childPlayers = [];
     this._callback = null;
     this._rebuildUnderlyingPlayer();
+    // Players are constructed in the idle state.
+    this._player.cancel();
   };
 
   // TODO: add a source getter/setter
@@ -34,18 +36,16 @@
         this._player = null;
       }
 
-      if (!this.source) {
-        return;
-      }
-
-      if (this.source instanceof window.Animation) {
+      if (!this.source || this.source instanceof window.Animation) {
         this._player = scope.newUnderlyingPlayerForAnimation(this.source);
         scope.bindPlayerForAnimation(this);
       }
       if (this.source instanceof window.AnimationSequence || this.source instanceof window.AnimationGroup) {
         this._player = scope.newUnderlyingPlayerForGroup(this.source);
         scope.bindPlayerForGroup(this);
       }
+
+      // FIXME: move existing currentTime/startTime/playState to new player
     },
     get paused() {
       return this._player.paused;
@@ -118,11 +118,7 @@
     },
     cancel: function() {
       this._player.cancel();
-      if (this._callback) {
-        this._register();
-        this._callback._player = null;
-      }
-      this.source = null;
+      this._register();
       this._removePlayers();
     },
     reverse: function() {

src/player.js

@@ -71,8 +71,7 @@
       newTime = +newTime;
       if (isNaN(newTime))
         return;
-      if (scope.restart())
-        this._startTime = null;
+      scope.restart();
       if (!this.paused && this._startTime != null) {
         this._startTime = this._timeline.currentTime - newTime / this._playbackRate;
       }
@@ -116,14 +115,11 @@
       this.paused = false;
       if (this.finished || this._idle) {
         this._currentTime = this._playbackRate > 0 ? 0 : this._totalDuration;
+        this._startTime = null;
         scope.invalidateEffects();
       }
       this._finishedFlag = false;
-      if (!scope.restart()) {
-        this._startTime = this._timeline.currentTime - this._currentTime / this._playbackRate;
-      }
-      else
-        this._startTime = null;
+      scope.restart();
       this._idle = false;
       this._ensureAlive();
     },
@@ -149,6 +145,7 @@
     },
     reverse: function() {
       this._playbackRate *= -1;
+      this._startTime = null;
       this.play();
     },
     addEventListener: function(type, handler) {

src/timeline.js

@@ -25,10 +25,10 @@
     // TODO: Does this need to be sorted?
     // TODO: Do we need to consider needsRetick?
     getAnimationPlayers: function() {
-      this.filterPlayers();
+      this._discardPlayers();
       return this._players.slice();
     },
-    filterPlayers: function() {
+    _discardPlayers: function() {
       this._players = this._players.filter(function(player) {
         return player.playState != 'finished' && player.playState != 'idle';
       });
@@ -37,6 +37,7 @@
       var player = new scope.Player(source);
       this._players.push(player);
       scope.restartMaxifillTick();
+      player.play();
       return player;
     },
   };
@@ -53,7 +54,7 @@
   function maxifillTick(t) {
     var timeline = window.document.timeline;
     timeline.currentTime = t;
-    timeline.filterPlayers();
+    timeline._discardPlayers();
     if (timeline._players.length == 0)
       ticking = false;
     else

test/js/animation-constructor.js

@@ -1,6 +1,8 @@
 suite('animation-constructor', function() {
   setup(function() {
-    document.timeline._players = [];
+    document.timeline.getAnimationPlayers().forEach(function(player) {
+      player.cancel();
+    });
   });
 
   test('Playing an Animation makes a Player', function() {

test/js/group-constructors.js

@@ -15,6 +15,7 @@ suite('group-constructors', function() {
 
   test('player getter for children in groups, and __internalPlayer, work as expected', function() {
     var p = document.timeline.play(simpleAnimationGroup());
+    tick(0);
     tick(100);
     assert.equal(p.source.player, p);
     assert.equal(p._childPlayers[0].source.player, p);

test/js/group-player-finish-event.js

@@ -41,13 +41,14 @@ suite('group-player-finish-event', function() {
   test('fire when reversed player completes', function(done) {
     this.player.onfinish = function(event) {
       assert.equal(event.currentTime, 0);
-      assert.equal(event.timelineTime, 1000);
+      assert.equal(event.timelineTime, 1001);
       done();
     };
     tick(0);
     tick(500);
     this.player.reverse();
-    tick(1000);
+    tick(501);
+    tick(1001);
   });
 
   test('fire after player is cancelled', function(done) {

test/js/player-finish-event.js

@@ -33,13 +33,14 @@ suite('player-finish-event', function() {
   test('fire when reversed player completes', function(done) {
     this.player.onfinish = function(event) {
       assert.equal(event.currentTime, 0);
-      assert.equal(event.timelineTime, 1000);
+      assert.equal(event.timelineTime, 1001);
       done();
     };
     tick(0);
     tick(500);
     this.player.reverse();
-    tick(1000);
+    tick(501);
+    tick(1001);
   });
 
   test('fire after player is cancelled', function(done) {

test/js/player.js

@@ -38,6 +38,7 @@ suite('player', function() {
     assert.equal(p.currentTime, null);
     tick(700);
     p.play();
+    tick(700);
     assert.equal(p.currentTime, 1000);
     tick(800);
     assert.equal(p.currentTime, 1100);
@@ -75,6 +76,7 @@ suite('player', function() {
     assert.equal(p.currentTime, 300);
     assert.equal(p.playbackRate, 1);
     p.reverse();
+    tick(600);
     assert.equal(p.startTime, 900);
     assert.equal(p.currentTime, 300);
     assert.equal(p.playbackRate, -1);
@@ -90,8 +92,8 @@ suite('player', function() {
     p.reverse();
     tick(601);
     tick(700);
-    assert.equal(p.startTime, 1100);
-    assert.equal(p.currentTime, 400);
+    assert.equal(p.startTime, 1101);
+    assert.equal(p.currentTime, 401);
   });
   test('reversing after finishing works as expected', function() {
     tick(90);
@@ -172,7 +174,8 @@ suite('player', function() {
     assert.equal(p.playbackRate, 1);
     setTicking(true);
     p.play();
-    assert.equal(p.startTime, 2600);
+    tick(2700);
+    assert.equal(p.startTime, 2700);
     assert.equal(p.currentTime, 0);
     assert.equal(p.finished, false);
     assert.equal(p.playbackRate, 1);
@@ -183,14 +186,16 @@ suite('player', function() {
     tick(600);
     tick(700);
     p.reverse();
+    tick(701);
     tick(900);
-    assert.equal(p.startTime, 800);
+    assert.equal(p.startTime, 801);
     assert.equal(p.currentTime, 0);
     assert.equal(p.finished, true);
     assert.equal(p.playbackRate, -1);
     setTicking(true);
     p.play();
-    assert.equal(p.startTime, 3900);
+    tick(1000);
+    assert.equal(p.startTime, 4000);
     assert.equal(p.currentTime, 3000);
     assert.equal(p.finished, false);
     assert.equal(p.playbackRate, -1);
@@ -205,10 +210,11 @@ suite('player', function() {
     assert.equal(p.currentTime, 600);
     assert.equal(p.startTime, 300);
     p.reverse();
-    assert.equal(p.startTime, 1500);
+    tick(1000);
+    assert.equal(p.startTime, 1600);
     p.currentTime = 300;
     assert.equal(p.currentTime, 300);
-    assert.equal(p.startTime, 1200);
+    assert.equal(p.startTime, 1300);
   });
   test('seeking while paused works as expected', function() {
     tick(790);
@@ -434,4 +440,22 @@ suite('player', function() {
     p.currentTime = undefined;
     assert.equal(p.currentTime, 10);
   });
+  test('play() should not set a start time', function() {
+    var p = document.body.animate([], 1000);
+    p.cancel();
+    assert.equal(p.startTime, null);
+    assert.equal(p.playState, 'idle');
+    p.play();
+    assert.equal(p.startTime, null);
+    assert.equal(p.playState, 'pending');
+  });
+  test('reverse() should not set a start time', function() {
+    var p = document.body.animate([], 1000);
+    p.cancel();
+    assert.equal(p.startTime, null);
+    assert.equal(p.playState, 'idle');
+    p.reverse();
+    assert.equal(p.startTime, null);
+    assert.equal(p.playState, 'pending');
+  });
 });