perf: Fix memory leaks in safari, edge, and ie (#5880)

1. We were not always able to clean up `resize` on the `ResizeManager`, as the iframe
contentWindow can disappear before dispose
2. Native Tracks on Safari do not have an `off` so we have to deal with
event listeners manually

Fixes #5878

src/js/resize-manager.js

@@ -71,7 +71,18 @@ class ResizeManager extends Component {
           return;
         }
 
-        Events.on(this.el_.contentWindow, 'resize', this.debouncedHandler_);
+        const debouncedHandler_ = this.debouncedHandler_;
+        let unloadListener_ = this.unloadListener_ = function() {
+          Events.off(this, 'resize', debouncedHandler_);
+          Events.off(this, 'unload', unloadListener_);
+
+          unloadListener_ = null;
+        };
+
+        // safari and edge can unload the iframe before resizemanager dispose
+        // we have to dispose of event handlers correctly before that happens
+        Events.on(this.el_.contentWindow, 'unload', unloadListener_);
+        Events.on(this.el_.contentWindow, 'resize', debouncedHandler_);
       };
 
       this.one('load', this.loadListener_);
@@ -120,14 +131,14 @@ class ResizeManager extends Component {
       this.resizeObserver_.disconnect();
     }
 
-    if (this.el_ && this.el_.contentWindow) {
-      Events.off(this.el_.contentWindow, 'resize', this.debouncedHandler_);
-    }
-
     if (this.loadListener_) {
       this.off('load', this.loadListener_);
     }
 
+    if (this.el_ && this.el_.contentWindow && this.unloadListener_) {
+      this.unloadListener_.call(this.el_.contentWindow);
+    }
+
     this.ResizeObserver = null;
     this.resizeObserver = null;
     this.debouncedHandler_ = null;

src/js/tracks/audio-track-list.js

@@ -72,22 +72,34 @@ class AudioTrackList extends TrackList {
       return;
     }
 
+    if (!this.enabledChange_) {
+      this.enabledChange_ = () => {
+        // when we are disabling other tracks (since we don't support
+        // more than one track at a time) we will set changing_
+        // to true so that we don't trigger additional change events
+        if (this.changing_) {
+          return;
+        }
+        this.changing_ = true;
+        disableOthers(this, track);
+        this.changing_ = false;
+        this.trigger('change');
+      };
+    }
+
     /**
      * @listens AudioTrack#enabledchange
      * @fires TrackList#change
      */
-    track.addEventListener('enabledchange', () => {
-      // when we are disabling other tracks (since we don't support
-      // more than one track at a time) we will set changing_
-      // to true so that we don't trigger additional change events
-      if (this.changing_) {
-        return;
-      }
-      this.changing_ = true;
-      disableOthers(this, track);
-      this.changing_ = false;
-      this.trigger('change');
-    });
+    track.addEventListener('enabledchange', this.enabledChange_);
+  }
+
+  removeTrack(rtrack) {
+    super.removeTrack(rtrack);
+
+    if (rtrack.removeEventListener && this.enabledChange_) {
+      rtrack.removeEventListener('enabledchange', this.enabledChange_);
+    }
   }
 }
 

src/js/tracks/text-track-list.js

@@ -2,7 +2,6 @@
  * @file text-track-list.js
  */
 import TrackList from './track-list';
-import * as Fn from '../utils/fn.js';
 
 /**
  * The current list of {@link TextTrack} for a media file.
@@ -23,20 +22,36 @@ class TextTrackList extends TrackList {
   addTrack(track) {
     super.addTrack(track);
 
+    if (!this.queueChange_) {
+      this.queueChange_ = () => this.queueTrigger('change');
+    }
+    if (!this.triggerSelectedlanguagechange) {
+      this.triggerSelectedlanguagechange_ = () => this.trigger('selectedlanguagechange');
+    }
+
     /**
      * @listens TextTrack#modechange
      * @fires TrackList#change
      */
-    track.addEventListener('modechange', Fn.bind(this, function() {
-      this.queueTrigger('change');
-    }));
-
+    track.addEventListener('modechange', this.queueChange_);
     const nonLanguageTextTrackKind = ['metadata', 'chapters'];
 
     if (nonLanguageTextTrackKind.indexOf(track.kind) === -1) {
-      track.addEventListener('modechange', Fn.bind(this, function() {
-        this.trigger('selectedlanguagechange');
-      }));
+      track.addEventListener('modechange', this.triggerSelectedlanguagechange_);
+    }
+  }
+
+  removeTrack(rtrack) {
+    super.removeTrack(rtrack);
+
+    // manually remove the event handlers we added
+    if (rtrack.removeEventListener) {
+      if (this.queueChange_) {
+        rtrack.removeEventListener('modechange', this.queueChange_);
+      }
+      if (this.selectedlanguagechange_) {
+        rtrack.removeEventListener('modechange', this.triggerSelectedlanguagechange_);
+      }
     }
   }
 }

src/js/tracks/video-track-list.js

@@ -87,19 +87,31 @@ class VideoTrackList extends TrackList {
       return;
     }
 
+    if (!this.selectedChange_) {
+      this.selectedChange_ = () => {
+        if (this.changing_) {
+          return;
+        }
+        this.changing_ = true;
+        disableOthers(this, track);
+        this.changing_ = false;
+        this.trigger('change');
+      };
+    }
+
     /**
      * @listens VideoTrack#selectedchange
      * @fires TrackList#change
      */
-    track.addEventListener('selectedchange', () => {
-      if (this.changing_) {
-        return;
-      }
-      this.changing_ = true;
-      disableOthers(this, track);
-      this.changing_ = false;
-      this.trigger('change');
-    });
+    track.addEventListener('selectedchange', this.selectedChange_);
+  }
+
+  removeTrack(rtrack) {
+    super.removeTrack(rtrack);
+
+    if (rtrack.removeEventListener && this.selectedChange_) {
+      rtrack.removeEventListener('selectedchange', this.selectedChange_);
+    }
   }
 }
 

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

@@ -140,6 +140,9 @@ if (Html5.supportsNativeTextTracks()) {
     c.jsonToTextTracks(cleanup(c.textTracksToJson(tech)), tech);
 
     assert.equal(addRemotes, 2, 'we added two text tracks');
+
+    tt.removeTrack(nativeTrack.track);
+    tt.removeTrack(emulatedTrack);
   });
 }
 

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

@@ -272,12 +272,11 @@ if (Html5.supportsNativeTextTracks()) {
       assert.equal(emulatedTt.length, tt.length, 'we have matching tracks length');
       assert.equal(emulatedTt.length, 1, 'we have one text track');
 
-      emulatedTt.off('addtrack', addtrack);
       el.removeChild(track);
     };
 
-    emulatedTt.on('addtrack', addtrack);
-    emulatedTt.on('removetrack', function() {
+    emulatedTt.one('addtrack', addtrack);
+    emulatedTt.one('removetrack', function() {
       assert.equal(emulatedTt.length, tt.length, 'we have matching tracks length');
       assert.equal(emulatedTt.length, 0, 'we have no more text tracks');
 

test/unit/utils/dom-data.test.js

@@ -2,6 +2,7 @@
 import document from 'global/document';
 import * as DomData from '../../../src/js/utils/dom-data';
 import videojs from '../../../src/js/video.js';
+import window from 'global/window';
 
 QUnit.module('dom-data');
 
@@ -32,11 +33,6 @@ QUnit.done(function(details) {
     return;
   }
 
-  // TODO: fix memory leaks on the following
-  if (videojs.browser.IS_SAFARI || videojs.browser.IS_EDGE || videojs.browser.IE_VERSION) {
-    return;
-  }
-
   memoryTestRun = true;
 
   QUnit.module('dom-data memory');
@@ -60,6 +56,7 @@ QUnit.done(function(details) {
   QUnit.test('Memory is not leaking', function(assert) {
     if (Object.keys(DomData.elData).length > 0) {
       videojs.domData = DomData;
+      window.videojs = videojs;
     }
     assert.equal(Object.keys(DomData.elData).length, 0, 'no leaks, check videojs.domData.elData if failure');
   });