fix: prevent dispose error and text track duplicate listeners (#6984)

Author: brandonocasey

src/js/tracks/text-track.js

@@ -184,7 +184,10 @@ class TextTrack extends Track {
     const activeCues = new TextTrackCueList(this.activeCues_);
     let changed = false;
     const timeupdateHandler = Fn.bind(this, function() {
+      if (!this.tech_.isReady_ || this.tech_.isDisposed()) {
+        return;
 
+      }
       // Accessing this.activeCues for the side-effects of updating itself
       // due to its nature as a getter function. Do not remove or cues will
       // stop updating!
@@ -196,10 +199,13 @@ class TextTrack extends Track {
       }
     });
 
+    const disposeHandler = () => {
+      this.tech_.off('timeupdate', timeupdateHandler);
+    };
+
+    this.tech_.one('dispose', disposeHandler);
     if (mode !== 'disabled') {
-      this.tech_.ready(() => {
-        this.tech_.on('timeupdate', timeupdateHandler);
-      }, true);
+      this.tech_.on('timeupdate', timeupdateHandler);
     }
 
     Object.defineProperties(this, {
@@ -236,17 +242,19 @@ class TextTrack extends Track {
           if (!TextTrackMode[newMode]) {
             return;
           }
+          if (mode === newMode) {
+            return;
+          }
+
           mode = newMode;
           if (!this.preload_ && mode !== 'disabled' && this.cues.length === 0) {
             // On-demand load.
             loadTrack(this.src, this);
           }
+          this.tech_.off('timeupdate', timeupdateHandler);
+
           if (mode !== 'disabled') {
-            this.tech_.ready(() => {
-              this.tech_.on('timeupdate', timeupdateHandler);
-            }, true);
-          } else {
-            this.tech_.off('timeupdate', timeupdateHandler);
+            this.tech_.on('timeupdate', timeupdateHandler);
           }
           /**
            * An event that fires when mode changes on this track. This allows

test/unit/tech/tech.test.js

@@ -142,8 +142,9 @@ QUnit.test('dispose() should stop time tracking', function(assert) {
 QUnit.test('dispose() should clear all tracks that are passed when its created', function(assert) {
   const audioTracks = new AudioTrackList([new AudioTrack(), new AudioTrack()]);
   const videoTracks = new VideoTrackList([new VideoTrack(), new VideoTrack()]);
-  const textTracks = new TextTrackList([new TextTrack({tech: {}}),
-    new TextTrack({tech: {}})]);
+  const pretech = new Tech();
+  const textTracks = new TextTrackList([new TextTrack({tech: pretech}),
+    new TextTrack({tech: pretech})]);
 
   assert.equal(audioTracks.length, 2, 'should have two audio tracks at the start');
   assert.equal(videoTracks.length, 2, 'should have two video tracks at the start');
@@ -167,6 +168,7 @@ QUnit.test('dispose() should clear all tracks that are passed when its created',
     'should hold text tracks that we passed'
   );
 
+  pretech.dispose();
   tech.dispose();
 
   assert.equal(audioTracks.length, 0, 'should have zero audio tracks after dispose');

test/unit/tracks/html-track-element-list.test.js

@@ -6,6 +6,7 @@ const defaultTech = {
   textTracks() {},
   on() {},
   off() {},
+  one() {},
   currentTime() {}
 };
 

test/unit/tracks/html-track-element.test.js

@@ -65,7 +65,7 @@ QUnit.test('fires loadeddata when track cues become populated', function(assert)
     changes++;
   };
   const htmlTrackElement = new HTMLTrackElement({
-    tech() {}
+    tech: this.tech
   });
 
   htmlTrackElement.addEventListener('load', loadHandler);

test/unit/tracks/text-track-list-converter.test.js

@@ -48,7 +48,9 @@ if (Html5.supportsNativeTextTracks()) {
       tech: {
         crossOrigin() {
           return null;
-        }
+        },
+        on() {},
+        one() {}
       }
     });
 
@@ -75,6 +77,7 @@ if (Html5.supportsNativeTextTracks()) {
           }
         };
       },
+      on() {},
       crossOrigin() {
         return null;
       },
@@ -108,7 +111,9 @@ if (Html5.supportsNativeTextTracks()) {
       tech: {
         crossOrigin() {
           return null;
-        }
+        },
+        on() {},
+        one() {}
       }
     });
 
@@ -140,6 +145,7 @@ if (Html5.supportsNativeTextTracks()) {
       crossOrigin() {
         return null;
       },
+      on() {},
       textTracks() {
         return tt;
       },
@@ -169,7 +175,9 @@ QUnit.test('trackToJson_ produces correct representation for emulated track obje
     tech: {
       crossOrigin() {
         return null;
-      }
+      },
+      on() {},
+      one() {}
     }
   });
 
@@ -191,7 +199,9 @@ QUnit.test('textTracksToJson produces good json output for emulated only', funct
     tech: {
       crossOrigin() {
         return null;
-      }
+      },
+      on() {},
+      one() {}
     }
   });
 
@@ -203,7 +213,9 @@ QUnit.test('textTracksToJson produces good json output for emulated only', funct
     tech: {
       crossOrigin() {
         return null;
-      }
+      },
+      on() {},
+      one() {}
     }
   });
 
@@ -227,6 +239,8 @@ QUnit.test('textTracksToJson produces good json output for emulated only', funct
     crossOrigin() {
       return null;
     },
+    on() {},
+    one() {},
     textTracks() {
       return tt;
     }
@@ -259,7 +273,9 @@ QUnit.test('jsonToTextTracks calls addRemoteTextTrack on the tech with emulated
     tech: {
       crossOrigin() {
         return null;
-      }
+      },
+      on() {},
+      one() {}
     }
   });
 
@@ -271,7 +287,9 @@ QUnit.test('jsonToTextTracks calls addRemoteTextTrack on the tech with emulated
     tech: {
       crossOrigin() {
         return null;
-      }
+      },
+      on() {},
+      one() {}
     }
   });
 
@@ -296,6 +314,8 @@ QUnit.test('jsonToTextTracks calls addRemoteTextTrack on the tech with emulated
     crossOrigin() {
       return null;
     },
+    on() {},
+    one() {},
     textTracks() {
       return tt;
     },

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

@@ -343,7 +343,7 @@ QUnit.test('removes cuechange event when text track is hidden for emulated track
     numTextTrackChanges++;
   });
 
-  tt.mode = 'showing';
+  tt.mode = 'disabled';
   this.clock.tick(1);
   assert.equal(
     numTextTrackChanges, 1,