feat: option to have remoteTextTracks automatically 'garbage-collected' when sources change (#3736)

Tech#addRemoteTextTrack now accepts a second parameter - a boolean named manualCleanup which defaults to true preserving backwards compatibility. When that value is set to false the text track will be removed from the video element whenever a source change occurs.

src/js/player.js

@@ -2521,13 +2521,20 @@ class Player extends Component {
   }
 
   /**
-   * Add a remote text track
+   * Creates a remote text track object and returns an html track element.
    *
-   * @param {Object} options    Options for remote text track
+   * @param {Object} options The object should contain values for
+   * kind, language, label, and src (location of the WebVTT file)
+   * @param {Boolean} [manualCleanup=true] if set to false, the TextTrack will be
+   * automatically removed from the video element whenever the source changes
+   * @return {HTMLTrackElement} An Html Track Element.
+   * This can be an emulated {@link HTMLTrackElement} or a native one.
+   * @deprecated The default value of the "manualCleanup" parameter will default
+   * to "false" in upcoming versions of Video.js
    */
-  addRemoteTextTrack(options) {
+  addRemoteTextTrack(options, manualCleanup) {
     if (this.tech_) {
-      return this.tech_.addRemoteTextTrack(options);
+      return this.tech_.addRemoteTextTrack(options, manualCleanup);
     }
   }
 

src/js/tech/html5.js

@@ -608,17 +608,16 @@ class Html5 extends Tech {
   }
 
   /**
-   * Creates a remote text track object and returns a html track element
+   * Creates either native TextTrack or an emulated TextTrack depending
+   * on the value of `featuresNativeTextTracks`
    *
    * @param {Object} options The object should contain values for
    * kind, language, label and src (location of the WebVTT file)
-   * @return {HTMLTrackElement}
    */
-  addRemoteTextTrack(options = {}) {
+  createRemoteTextTrack(options) {
     if (!this.featuresNativeTextTracks) {
-      return super.addRemoteTextTrack(options);
+      return super.createRemoteTextTrack(options);
     }
-
     const htmlTrackElement = document.createElement('track');
 
     if (options.kind) {
@@ -640,11 +639,25 @@ class Html5 extends Tech {
       htmlTrackElement.src = options.src;
     }
 
-    this.el().appendChild(htmlTrackElement);
+    return htmlTrackElement;
+  }
 
-    // store HTMLTrackElement and TextTrack to remote list
-    this.remoteTextTrackEls().addTrackElement_(htmlTrackElement);
-    this.remoteTextTracks().addTrack_(htmlTrackElement.track);
+  /**
+   * Creates a remote text track object and returns an html track element.
+   *
+   * @param {Object} options The object should contain values for
+   * kind, language, label, and src (location of the WebVTT file)
+   * @param {Boolean} [manualCleanup=true] if set to false, the TextTrack will be
+   * automatically removed from the video element whenever the source changes
+   * @return {HTMLTrackElement} An Html Track Element.
+   * This can be an emulated {@link HTMLTrackElement} or a native one.
+   * @deprecated The default value of the "manualCleanup" parameter will default
+   * to "false" in upcoming versions of Video.js
+   */
+  addRemoteTextTrack(options, manualCleanup) {
+    const htmlTrackElement = super.addRemoteTextTrack(options, manualCleanup);
+
+    this.el().appendChild(htmlTrackElement);
 
     return htmlTrackElement;
   }
@@ -655,15 +668,7 @@ class Html5 extends Tech {
    * @param {TextTrackObject} track Texttrack object to remove
    */
   removeRemoteTextTrack(track) {
-    if (!this.featuresNativeTextTracks) {
-      return super.removeRemoteTextTrack(track);
-    }
-
-    const trackElement = this.remoteTextTrackEls().getTrackElementByTrack_(track);
-
-    // remove HTMLTrackElement and TextTrack from remote list
-    this.remoteTextTrackEls().removeTrackElement_(trackElement);
-    this.remoteTextTracks().removeTrack_(track);
+    super.removeRemoteTextTrack(track);
 
     const tracks = this.$$('track');
 

src/js/tech/tech.js

@@ -85,9 +85,11 @@ class Tech extends Component {
     }
 
     if (!this.featuresNativeTextTracks) {
-      this.on('ready', this.emulateTextTracks);
+      this.emulateTextTracks();
     }
 
+    this.autoRemoteTextTracks_ = new TextTrackList();
+
     this.initTextTrackListeners();
     this.initTrackListeners();
 
@@ -291,6 +293,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.
    *
@@ -394,17 +413,11 @@ class Tech extends Component {
   }
 
   /**
-   * Emulate texttracks
+   * Add vtt.js if necessary
    *
-   * @method emulateTextTracks
+   * @private
    */
-  emulateTextTracks() {
-    const tracks = this.textTracks();
-
-    if (!tracks) {
-      return;
-    }
-
+  addWebVttScript_() {
     if (!window.WebVTT && this.el().parentNode !== null && this.el().parentNode !== undefined) {
       const script = document.createElement('script');
 
@@ -424,6 +437,32 @@ class Tech extends Component {
       window.WebVTT = true;
       this.el().parentNode.appendChild(script);
     }
+  }
+
+  /**
+   * Emulate texttracks
+   *
+   * @method emulateTextTracks
+   */
+  emulateTextTracks() {
+    const tracks = this.textTracks();
+
+    if (!tracks) {
+      return;
+    }
+
+    this.remoteTextTracks().on('addtrack', (e) => {
+      this.textTracks().addTrack_(e.track);
+    });
+
+    this.remoteTextTracks().on('removetrack', (e) => {
+      this.textTracks().removeTrack_(e.track);
+    });
+
+    // Initially, Tech.el_ is a child of a dummy-div wait until the Component system
+    // signals that the Tech is ready at which point Tech.el_ is part of the DOM
+    // before inserting the WebVTT script
+    this.on('ready', this.addWebVttScript_);
 
     const updateDisplay = () => this.trigger('texttrackchange');
     const textTracksChanges = () => {
@@ -527,26 +566,51 @@ class Tech extends Component {
   }
 
   /**
-   * Creates a remote text track object and returns a emulated html track element
+   * Create an emulated TextTrack for use by addRemoteTextTrack
+   *
+   * This is intended to be overridden by classes that inherit from
+   * Tech in order to create native or custom TextTracks.
    *
    * @param {Object} options The object should contain values for
    * kind, language, label and src (location of the WebVTT file)
-   * @return {HTMLTrackElement}
-   * @method addRemoteTextTrack
    */
-  addRemoteTextTrack(options) {
+  createRemoteTextTrack(options) {
     const track = mergeOptions(options, {
       tech: this
     });
 
-    const htmlTrackElement = new HTMLTrackElement(track);
+    return new HTMLTrackElement(track);
+  }
+
+  /**
+   * Creates a remote text track object and returns an html track element.
+   *
+   * @param {Object} options The object should contain values for
+   * kind, language, label, and src (location of the WebVTT file)
+   * @param {Boolean} [manualCleanup=true] if set to false, the TextTrack will be
+   * automatically removed from the video element whenever the source changes
+   * @return {HTMLTrackElement} An Html Track Element.
+   * This can be an emulated {@link HTMLTrackElement} or a native one.
+   * @deprecated The default value of the "manualCleanup" parameter will default
+   * to "false" in upcoming versions of Video.js
+   */
+  addRemoteTextTrack(options = {}, manualCleanup) {
+    const htmlTrackElement = this.createRemoteTextTrack(options);
+
+    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);
 
-    // must come after remoteTextTracks()
-    this.textTracks().addTrack_(htmlTrackElement.track);
+    if (manualCleanup !== true) {
+      // create the TextTrackList if it doesn't exist
+      this.autoRemoteTextTracks_.addTrack_(htmlTrackElement.track);
+    }
 
     return htmlTrackElement;
   }
@@ -558,13 +622,12 @@ class Tech extends Component {
    * @method removeRemoteTextTrack
    */
   removeRemoteTextTrack(track) {
-    this.textTracks().removeTrack_(track);
-
     const trackElement = this.remoteTextTrackEls().getTrackElementByTrack_(track);
 
     // remove HTMLTrackElement and TextTrack from remote list
     this.remoteTextTrackEls().removeTrackElement_(trackElement);
     this.remoteTextTracks().removeTrack_(track);
+    this.autoRemoteTextTracks_.removeTrack_(track);
   }
 
   /**
@@ -685,7 +748,7 @@ Tech.prototype.featuresNativeTextTracks = false;
  *
  * ##### EXAMPLE:
  *
- *   Tech.withSourceHandlers.call(MyTech);
+ *   Tech.withSourceHandlers(MyTech);
  *
  */
 Tech.withSourceHandlers = function(_Tech) {
@@ -820,17 +883,7 @@ 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.
@@ -838,7 +891,6 @@ Tech.withSourceHandlers = function(_Tech) {
       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 +906,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 +914,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' };

test/unit/tracks/text-tracks.test.js

@@ -377,7 +377,6 @@ QUnit.test('removes cuechange event when text track is hidden for emulated track
     endTime: 5
   });
   player.tech_.textTracks().addTrack_(tt);
-  player.tech_.emulateTextTracks();
 
   let numTextTrackChanges = 0;