Added the ability to flag remoteTextTracks to be automatically 'garba…

…ge-collected' when the source changes

src/js/tech/tech.js

@@ -291,6 +291,23 @@ class Tech extends Component {
     });
   }
 
+  /**
+   * Remove any TextTracks added via addRemoteTextTrack that are
+   * flagged for automatic garbage collection
+   *
+   * @method cleanupAutoTextTracks
+   */
+  cleanupAutoTextTracks() {
+    const list = this.autoRemoteTextTracks_ || [];
+    let i = list.length;
+
+    while (i--) {
+      const track = list[i];
+
+      this.removeRemoteTextTrack(track);
+    }
+  }
+
   /**
    * Reset the tech. Removes all sources and resets readyState.
    *
@@ -531,20 +548,33 @@ class Tech extends Component {
    *
    * @param {Object} options The object should contain values for
    * kind, language, label and src (location of the WebVTT file)
+   * @param {Boolean} manualCleanup if set to false, the TextTrack will be
+   * automatically removed from the video element whenever the source changes
    * @return {HTMLTrackElement}
    * @method addRemoteTextTrack
    */
-  addRemoteTextTrack(options) {
+  addRemoteTextTrack(options, manualCleanup) {
     const track = mergeOptions(options, {
       tech: this
     });
-
     const htmlTrackElement = new HTMLTrackElement(track);
 
+    if (manualCleanup !== true && manualCleanup !== false) {
+      // deprecation warning
+      log.warn('Calling addRemoteTextTrack without explicitly setting the "manualCleanup" parameter to `true` is deprecated and default to `false` in future version of video.js');
+      manualCleanup = true;
+    }
+
     // store HTMLTrackElement and TextTrack to remote list
     this.remoteTextTrackEls().addTrackElement_(htmlTrackElement);
     this.remoteTextTracks().addTrack_(htmlTrackElement.track);
 
+    if (manualCleanup !== true) {
+      // create the TextTrackList if it doesn't exist
+      this.autoRemoteTextTracks_ = this.autoRemoteTextTracks_ || new TextTrackList();
+      this.autoRemoteTextTracks_.addTrack_(htmlTrackElement.track);
+    }
+
     // must come after remoteTextTracks()
     this.textTracks().addTrack_(htmlTrackElement.track);
 
@@ -565,6 +595,9 @@ class Tech extends Component {
     // remove HTMLTrackElement and TextTrack from remote list
     this.remoteTextTrackEls().removeTrackElement_(trackElement);
     this.remoteTextTracks().removeTrack_(track);
+    if (this.autoRemoteTextTracks_) {
+      this.autoRemoteTextTracks_.removeTrack_(track);
+    }
   }
 
   /**
@@ -820,25 +853,14 @@ Tech.withSourceHandlers = function(_Tech) {
     this.disposeSourceHandler();
     this.off('dispose', this.disposeSourceHandler);
 
-    // if we have a source and get another one
-    // then we are loading something new
-    // than clear all of our current tracks
-    if (this.currentSource_) {
-      this.clearTracks(['audio', 'video']);
-
-      this.currentSource_ = null;
-    }
-
     if (sh !== _Tech.nativeSourceHandler) {
-
       this.currentSource_ = source;
 
       // Catch if someone replaced the src without calling setSource.
       // If they do, set currentSource_ to null and dispose our source handler.
       this.off(this.el_, 'loadstart', _Tech.prototype.firstLoadStartListener_);
       this.off(this.el_, 'loadstart', _Tech.prototype.successiveLoadStartListener_);
       this.one(this.el_, 'loadstart', _Tech.prototype.firstLoadStartListener_);
-
     }
 
     this.sourceHandler_ = sh.handleSource(source, this, this.options_);
@@ -854,7 +876,6 @@ Tech.withSourceHandlers = function(_Tech) {
 
   // On successive loadstarts when setSource has not been called again
   _Tech.prototype.successiveLoadStartListener_ = function() {
-    this.currentSource_ = null;
     this.disposeSourceHandler();
     this.one(this.el_, 'loadstart', _Tech.prototype.successiveLoadStartListener_);
   };
@@ -863,10 +884,25 @@ Tech.withSourceHandlers = function(_Tech) {
    * Clean up any existing source handler
    */
   _Tech.prototype.disposeSourceHandler = function() {
-    if (this.sourceHandler_ && this.sourceHandler_.dispose) {
+    // if we have a source and get another one
+    // then we are loading something new
+    // than clear all of our current tracks
+    if (this.currentSource_) {
+      this.clearTracks(['audio', 'video']);
+      this.currentSource_ = null;
+    }
+
+    // always clean up auto-text tracks
+    this.cleanupAutoTextTracks();
+
+    if (this.sourceHandler_) {
       this.off(this.el_, 'loadstart', _Tech.prototype.firstLoadStartListener_);
       this.off(this.el_, 'loadstart', _Tech.prototype.successiveLoadStartListener_);
-      this.sourceHandler_.dispose();
+
+      if (this.sourceHandler_.dispose) {
+        this.sourceHandler_.dispose();
+      }
+
       this.sourceHandler_ = null;
     }
   };

test/unit/tech/tech.test.js

@@ -146,7 +146,7 @@ QUnit.test('dispose() should clear all tracks that are added after creation', fu
 
   assert.equal(tech.audioTracks().length, 2, 'should have two audio tracks at the start');
   assert.equal(tech.videoTracks().length, 2, 'should have two video tracks at the start');
-  assert.equal(tech.textTracks().length, 2, 'should have two video tracks at the start');
+  assert.equal(tech.textTracks().length, 2, 'should have two text tracks at the start');
   assert.equal(tech.remoteTextTrackEls().length,
               2,
               'should have two remote text tracks els');
@@ -167,6 +167,62 @@ QUnit.test('dispose() should clear all tracks that are added after creation', fu
   assert.equal(tech.textTracks().length, 0, 'should have zero video tracks after dispose');
 });
 
+QUnit.test('switching sources should clear all remote tracks that are added with manualCleanup = false', function(assert) {
+  // Define a new tech class
+  const MyTech = extendFn(Tech);
+
+  // Create source handler
+  const handler = {
+    canPlayType: () => 'probably',
+    canHandleSource: () => 'probably',
+    handleSource: () => {
+      return {
+        dispose: () => {}
+      };
+    }
+  };
+
+  // Extend Tech with source handlers
+  Tech.withSourceHandlers(MyTech);
+
+  MyTech.registerSourceHandler(handler);
+
+  const tech = new MyTech();
+
+  // set the initial source
+  tech.setSource({src: 'foo.mp4', type: 'mp4'});
+
+  // default value for manualCleanup is true
+  tech.addRemoteTextTrack({});
+  // should be automatically cleaned up when source changes
+  tech.addRemoteTextTrack({}, false);
+
+  assert.equal(tech.textTracks().length, 2, 'should have two text tracks at the start');
+  assert.equal(tech.remoteTextTrackEls().length,
+              2,
+              'should have two remote text tracks els');
+  assert.equal(tech.remoteTextTracks().length, 2, 'should have two remote text tracks');
+  assert.equal(tech.autoRemoteTextTracks_.length,
+               1,
+               'should have one auto-cleanup remote text track');
+
+  // change source to force cleanup of auto remote text tracks
+  tech.setSource({src: 'bar.mp4', type: 'mp4'});
+
+  assert.equal(tech.textTracks().length,
+               1,
+               'should have one text track after source change');
+  assert.equal(tech.remoteTextTrackEls().length,
+              1,
+              'should have one remote remote text track els after source change');
+  assert.equal(tech.remoteTextTracks().length,
+               1,
+               'should have one remote text track after source change');
+  assert.equal(tech.autoRemoteTextTracks_.length,
+               0,
+               'should have zero auto-cleanup remote text tracks');
+});
+
 QUnit.test('should add the source handler interface to a tech', function(assert) {
   const sourceA = { src: 'foo.mp4', type: 'video/mp4' };
   const sourceB = { src: 'no-support', type: 'no-support' };