fix: make sure sources, volume, and playback rate are reset along with the player (videojs#5676)

misteroneill · gkatsev · commit 4c9e09d4602e · 2019-01-03T15:14:54.000-05:00
Clear out the player cache_ on reset. Fixes videojs#5675
diff --git a/src/js/player.js b/src/js/player.js
@@ -401,18 +401,14 @@ class Player extends Component {
       this.languages_ = Player.prototype.options_.languages;
     }
 
-    // Cache for video property values.
-    this.cache_ = {};
+    this.resetCache_();
 
     // Set poster
     this.poster_ = options.poster || '';
 
     // Set controls
     this.controls_ = !!options.controls;
 
-    // Set default values for lastVolume
-    this.cache_.lastVolume = 1;
-
     // Original tag settings stored in options
     // now remove immediately so native controls don't flash.
     // May be turned back on by HTML5 tech if nativeControlsForTouch is true
@@ -438,9 +434,6 @@ class Player extends Component {
 
     this.el_ = this.createEl();
 
-    // Set default value for lastPlaybackRate
-    this.cache_.lastPlaybackRate = this.defaultPlaybackRate();
-
     // Make this an evented object and use `el_` as its event bus.
     evented(this, {eventBusKey: 'el_'});
 
@@ -2011,6 +2004,33 @@ class Player extends Component {
     return this.cache_;
   }
 
+  /**
+   * Resets the internal cache object.
+   *
+   * Using this function outside the player constructor or reset method may
+   * have unintended side-effects.
+   *
+   * @private
+   */
+  resetCache_() {
+    this.cache_ = {
+
+      // Right now, the currentTime is not _really_ cached because it is always
+      // retrieved from the tech (see: currentTime). However, for completeness,
+      // we set it to zero here to ensure that if we do start actually caching
+      // it, we reset it along with everything else.
+      currentTime: 0,
+      duration: NaN,
+      lastVolume: 1,
+      lastPlaybackRate: this.defaultPlaybackRate(),
+      media: null,
+      src: '',
+      source: {},
+      sources: [],
+      volume: 1
+    };
+  }
+
   /**
    * Pass values to the playback tech
    *
@@ -2933,9 +2953,7 @@ class Player extends Component {
     if (this.tech_) {
       this.tech_.clearTracks('text');
     }
-    if (this.cache_) {
-      this.cache_.media = null;
-    }
+    this.resetCache_();
     this.poster('');
     this.loadTech_(this.options_.techOrder[0], null);
     this.techCall_('reset');
diff --git a/test/unit/player.test.js b/test/unit/player.test.js
@@ -1436,6 +1436,7 @@ QUnit.test('player#reset loads the Html5 tech and then techCalls reset', functio
     options_: {
       techOrder: ['html5', 'flash']
     },
+    resetCache_() {},
     loadTech_(tech, source) {
       loadedTech = tech;
       loadedSource = source;
@@ -1462,6 +1463,7 @@ QUnit.test('player#reset loads the first item in the techOrder and then techCall
     options_: {
       techOrder: ['flash', 'html5']
     },
+    resetCache_() {},
     loadTech_(tech, source) {
       loadedTech = tech;
       loadedSource = source;
@@ -1479,6 +1481,48 @@ QUnit.test('player#reset loads the first item in the techOrder and then techCall
   assert.equal(techCallMethod, 'reset', 'we then reset the tech');
 });
 
+QUnit.test('player#reset clears the player cache', function(assert) {
+  const player = TestHelpers.makePlayer();
+  const sources = [{
+    src: '//vjs.zencdn.net/v/oceans.mp4',
+    type: 'video/mp4'
+  }, {
+    src: '//vjs.zencdn.net/v/oceans.webm',
+    type: 'video/webm'
+  }];
+
+  this.clock.tick(1);
+
+  player.src(sources);
+  player.duration(10);
+  player.playbackRate(0.5);
+  player.volume(0.2);
+
+  assert.strictEqual(player.currentSrc(), sources[0].src, 'currentSrc is correct');
+  assert.deepEqual(player.currentSource(), sources[0], 'currentSource is correct');
+  assert.deepEqual(player.currentSources(), sources, 'currentSources is correct');
+  assert.strictEqual(player.duration(), 10, 'duration is correct');
+  assert.strictEqual(player.playbackRate(), 0.5, 'playbackRate is correct');
+  assert.strictEqual(player.volume(), 0.2, 'volume is correct');
+  assert.strictEqual(player.lastVolume_(), 0.2, 'lastVolume_ is correct');
+
+  player.reset();
+
+  assert.strictEqual(player.currentSrc(), '', 'currentSrc is correct');
+  assert.deepEqual(player.currentSource(), {}, 'currentSource is correct');
+  assert.deepEqual(player.currentSources(), [], 'currentSources is correct');
+
+  // Right now, the currentTime is not _really_ cached because it is always
+  // retrieved from the tech. However, for completeness, we set it to zero in
+  // the `resetCache_` method to ensure that if we do start actually caching it,
+  // we reset it along with everything else.
+  assert.strictEqual(player.getCache().currentTime, 0, 'currentTime is correct');
+  assert.ok(isNaN(player.duration()), 'duration is correct');
+  assert.strictEqual(player.playbackRate(), 1, 'playbackRate is correct');
+  assert.strictEqual(player.volume(), 1, 'volume is correct');
+  assert.strictEqual(player.lastVolume_(), 1, 'lastVolume_ is correct');
+});
+
 QUnit.test('player#reset removes the poster', function(assert) {
   const player = TestHelpers.makePlayer();
 
diff --git a/test/unit/tech/tech-faker.js b/test/unit/tech/tech-faker.js
@@ -10,6 +10,11 @@ class TechFaker extends Tech {
   constructor(options, handleReady) {
     super(options, handleReady);
 
+    this.featuresPlaybackRate = true;
+    this.defaultPlaybackRate_ = 1;
+    this.playbackRate_ = 1;
+    this.currentTime_ = 0;
+
     if (this.options_ && this.options_.sourceset) {
       this.fakeSourceset();
     }
@@ -37,7 +42,9 @@ class TechFaker extends Tech {
 
   setControls(val) {}
 
-  setVolume(newVolume) {}
+  setVolume(value) {
+    this.volume_ = value;
+  }
 
   setMuted() {}
 
@@ -49,9 +56,41 @@ class TechFaker extends Tech {
     this.options_.autoplay = true;
   }
 
+  defaultPlaybackRate(value) {
+    if (value !== undefined) {
+      this.defaultPlaybackRate_ = parseFloat(value);
+    }
+    return this.defaultPlaybackRate_;
+  }
+
+  setPlaybackRate(value) {
+    const last = this.playbackRate_;
+
+    this.playbackRate_ = parseFloat(value);
+
+    if (value !== last) {
+      this.trigger('ratechange');
+    }
+  }
+
+  playbackRate() {
+    return this.playbackRate_;
+  }
+
+  setCurrentTime(value) {
+    const last = this.currentTime_;
+
+    this.currentTime_ = parseFloat(value);
+
+    if (value !== last) {
+      this.trigger('timeupdate');
+    }
+  }
+
   currentTime() {
-    return 0;
+    return this.currentTime_;
   }
+
   seekable() {
     return createTimeRanges(0, 0);
   }
@@ -73,7 +112,7 @@ class TechFaker extends Tech {
     return 'movie.mp4';
   }
   volume() {
-    return 0;
+    return this.volume_ || 0;
   }
   muted() {
     return false;
