perf: null out els on dispose to minimize detached els (#4745)

A we retained a lot of references to DOM elements in various components. Here we clear it up. Also, make sure that we remove unused listeners as they can retain objects as well.
Update evented mixin to null out the eventBusEl_ after the component is disposed.
Add a feature for components, to tell it not to auto-initialize the evented mixin.
Re-enable the tests that were removed in #4640.

src/js/clickable-component.js

@@ -77,6 +77,13 @@ class ClickableComponent extends Component {
     return el;
   }
 
+  dispose() {
+    // remove controlTextEl_ on dipose
+    this.controlTextEl_ = null;
+
+    super.dispose();
+  }
+
   /**
    * Create a control text element on this `Component`
    *

src/js/component.js

@@ -84,8 +84,11 @@ class Component {
       this.el_ = this.createEl();
     }
 
-    // Make this an evented object and use `el_`, if available, as its event bus
-    evented(this, {eventBusKey: this.el_ ? 'el_' : null});
+    // if evented is anything except false, we want to mixin in evented
+    if (options.evented !== false) {
+      // Make this an evented object and use `el_`, if available, as its event bus
+      evented(this, {eventBusKey: this.el_ ? 'el_' : null});
+    }
     stateful(this, this.constructor.defaultState);
 
     this.children_ = [];
@@ -148,6 +151,9 @@ class Component {
       DomData.removeData(this.el_);
       this.el_ = null;
     }
+
+    // remove reference to the player after disposing of the element
+    this.player_ = null;
   }
 
   /**

src/js/control-bar/live-display.js

@@ -51,6 +51,12 @@ class LiveDisplay extends Component {
     return el;
   }
 
+  dispose() {
+    this.contentEl_ = null;
+
+    super.dispose();
+  }
+
   /**
    * Check the duration to see if the LiveDisplay should be showing or not. Then show/hide
    * it accordingly

src/js/control-bar/playback-rate-menu/playback-rate-menu-button.js

@@ -52,6 +52,12 @@ class PlaybackRateMenuButton extends MenuButton {
     return el;
   }
 
+  dispose() {
+    this.labelEl_ = null;
+
+    super.dispose();
+  }
+
   /**
    * Builds the default DOM `className`.
    *

src/js/control-bar/progress-control/load-progress-bar.js

@@ -39,6 +39,12 @@ class LoadProgressBar extends Component {
     });
   }
 
+  dispose() {
+    this.partEls_ = null;
+
+    super.dispose();
+  }
+
   /**
    * Update progress bar
    *

src/js/control-bar/text-track-controls/text-track-menu-item.js

@@ -3,7 +3,6 @@
  */
 import MenuItem from '../../menu/menu-item.js';
 import Component from '../../component.js';
-import * as Fn from '../../utils/fn.js';
 import window from 'global/window';
 import document from 'global/document';
 
@@ -34,13 +33,18 @@ class TextTrackMenuItem extends MenuItem {
     super(player, options);
 
     this.track = track;
-    const changeHandler = Fn.bind(this, this.handleTracksChange);
-    const selectedLanguageChangeHandler = Fn.bind(this, this.handleSelectedLanguageChange);
+    const changeHandler = (...args) => {
+      this.handleTracksChange.apply(this, args);
+    };
+    const selectedLanguageChangeHandler = (...args) => {
+      this.handleSelectedLanguageChange.apply(this, args);
+    };
 
     player.on(['loadstart', 'texttrackchange'], changeHandler);
     tracks.addEventListener('change', changeHandler);
     tracks.addEventListener('selectedlanguagechange', selectedLanguageChangeHandler);
     this.on('dispose', function() {
+      player.off(['loadstart', 'texttrackchange'], changeHandler);
       tracks.removeEventListener('change', changeHandler);
       tracks.removeEventListener('selectedlanguagechange', selectedLanguageChangeHandler);
     });
@@ -144,6 +148,13 @@ class TextTrackMenuItem extends MenuItem {
     }
   }
 
+  dispose() {
+    // remove reference to track object on dispose
+    this.track = null;
+
+    super.dispose();
+  }
+
 }
 
 Component.registerComponent('TextTrackMenuItem', TextTrackMenuItem);

src/js/control-bar/time-controls/time-display.js

@@ -56,6 +56,13 @@ class TimeDisplay extends Component {
     return el;
   }
 
+  dispose() {
+    this.contentEl_ = null;
+    this.textNode_ = null;
+
+    super.dispose();
+  }
+
   /**
    * Updates the "remaining time" text node with new content using the
    * contents of the `formattedTime_` property.

src/js/menu/menu-button.js

@@ -62,6 +62,7 @@ class MenuButton extends Component {
     const menu = this.createMenu();
 
     if (this.menu) {
+      this.menu.dispose();
       this.removeChild(this.menu);
     }
 

src/js/menu/menu.js

@@ -91,6 +91,12 @@ class Menu extends Component {
     return el;
   }
 
+  dispose() {
+    this.contentEl_ = null;
+
+    super.dispose();
+  }
+
   /**
    * Handle a `keydown` event on this menu. This listener is added in the constructor.
    *

src/js/mixins/evented.js

@@ -2,6 +2,7 @@
  * @file mixins/evented.js
  * @module evented
  */
+import window from 'global/window';
 import * as Dom from '../utils/dom';
 import * as Events from '../utils/events';
 import * as Fn from '../utils/fn';
@@ -366,7 +367,12 @@ function evented(target, options = {}) {
   Obj.assign(target, EventedMixin);
 
   // When any evented object is disposed, it removes all its listeners.
-  target.on('dispose', () => target.off());
+  target.on('dispose', () => {
+    target.off();
+    window.setTimeout(() => {
+      target.eventBusEl_ = null;
+    }, 0);
+  });
 
   return target;
 }

src/js/modal-dialog.js

@@ -97,6 +97,14 @@ class ModalDialog extends Component {
     });
   }
 
+  dispose() {
+    this.contentEl_ = null;
+    this.descEl_ = null;
+    this.previouslyActiveEl_ = null;
+
+    super.dispose();
+  }
+
   /**
    * Builds the default DOM `className`.
    *

src/js/player.js

@@ -284,6 +284,9 @@ class Player extends Component {
     // Same with creating the element
     options.createEl = false;
 
+    // don't auto mixin the evented mixin
+    options.evented = false;
+
     // we don't want the player to report touch activity on itself
     // see enableTouchActivity in Component
     options.reportTouchActivity = false;
@@ -485,6 +488,7 @@ class Player extends Component {
 
     if (this.styleEl_ && this.styleEl_.parentNode) {
       this.styleEl_.parentNode.removeChild(this.styleEl_);
+      this.styleEl_ = null;
     }
 
     // Kill reference to this player
@@ -502,6 +506,15 @@ class Player extends Component {
       this.tech_.dispose();
     }
 
+    if (this.playerElIngest_) {
+      this.playerElIngest_ = null;
+    }
+
+    if (this.tag) {
+      this.tag = null;
+    }
+
+    // the actual .el_ is removed here
     super.dispose();
   }
 

src/js/popup/popup.js

@@ -56,6 +56,12 @@ class Popup extends Component {
 
     return el;
   }
+
+  dispose() {
+    this.contentEl_ = null;
+
+    super.dispose();
+  }
 }
 
 Component.registerComponent('Popup', Popup);

src/js/tech/html5.js

@@ -113,6 +113,8 @@ class Html5 extends Tech {
    */
   dispose() {
     Html5.disposeMediaElement(this.el_);
+    this.options_ = null;
+
     // tech will handle clearing of the emulated track list
     super.dispose();
   }
@@ -150,6 +152,8 @@ class Html5 extends Tech {
     takeMetadataTrackSnapshot();
     textTracks.addEventListener('change', takeMetadataTrackSnapshot);
 
+    this.on('dispose', () => textTracks.removeEventListener('change', takeMetadataTrackSnapshot));
+
     const restoreTrackMode = () => {
       for (let i = 0; i < metadataTracksPreFullscreenState.length; i++) {
         const storedTrack = metadataTracksPreFullscreenState[i];

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

@@ -293,6 +293,12 @@ class TextTrackSettings extends ModalDialog {
     }
   }
 
+  dispose() {
+    this.endDialog = null;
+
+    super.dispose();
+  }
+
   /**
    * Create a <select> element with configured options.
    *

src/js/utils/events.js

@@ -344,16 +344,16 @@ export function off(elem, type, fn) {
   }
 
   // Utility function
-  const removeType = function(t) {
+  const removeType = function(el, t) {
     data.handlers[t] = [];
-    _cleanUpEvents(elem, t);
+    _cleanUpEvents(el, t);
   };
 
   // Are we removing all bound events?
   if (type === undefined) {
     for (const t in data.handlers) {
       if (Object.prototype.hasOwnProperty.call(data.handlers || {}, t)) {
-        removeType(t);
+        removeType(elem, t);
       }
     }
     return;
@@ -368,7 +368,7 @@ export function off(elem, type, fn) {
 
   // If no listener was provided, remove all listeners for type
   if (!fn) {
-    removeType(type);
+    removeType(elem, type);
     return;
   }
 

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

@@ -293,14 +293,14 @@ if (!Html5.supportsNativeTextTracks()) {
       src: 'en.vtt'
     };
     const captionsButton = player.controlBar.getChild('SubsCapsButton');
-    // we know the postition of the OffTextTrackMenuItem
-    const offMenuItem = captionsButton.items[1];
 
     player.src({type: 'video/mp4', src: 'http://google.com'});
     // manualCleanUp = true by default
     const englishTrack = player.addRemoteTextTrack(track1, true).track;
     // Keep track of menu items
     const enCaptionMenuItem = getMenuItemByLanguage(captionsButton.items, 'en');
+    // we know the postition of the OffTextTrackMenuItem
+    const offMenuItem = captionsButton.items[1];
 
     // Select English initially
     player.play();

test/unit/video.test.js

@@ -1,7 +1,6 @@
 /* eslint-env qunit */
 import videojs from '../../src/js/video.js';
 import * as Dom from '../../src/js/utils/dom.js';
-import * as browser from '../../src/js/utils/browser.js';
 import log from '../../src/js/utils/log.js';
 import document from 'global/document';
 import sinon from 'sinon';
@@ -368,153 +367,151 @@ QUnit.test('should create a new tag for movingMediaElementInDOM', function(asser
 /* **************************************************** *
  * div embed tests copied from video emebed tests above *
  * **************************************************** */
-if (!browser.IS_IE8) {
-  QUnit.module('video.js video-js embed', {
-    beforeEach() {
-      this.clock = sinon.useFakeTimers();
-    },
-    afterEach() {
-      this.clock.restore();
-    }
-  });
-  QUnit.test('should return a video player instance', function(assert) {
-    const fixture = document.getElementById('qunit-fixture');
+QUnit.module('video.js video-js embed', {
+  beforeEach() {
+    this.clock = sinon.useFakeTimers();
+  },
+  afterEach() {
+    this.clock.restore();
+  }
+});
+QUnit.test('should return a video player instance', function(assert) {
+  const fixture = document.getElementById('qunit-fixture');
 
-    fixture.innerHTML += '<video-js id="test_vid_id"></video-js>' +
-                         '<video-js id="test_vid_id2"></video-js>';
+  fixture.innerHTML += '<video-js id="test_vid_id"></video-js>' +
+                       '<video-js id="test_vid_id2"></video-js>';
 
-    const player = videojs('test_vid_id', { techOrder: ['techFaker'] });
+  const player = videojs('test_vid_id', { techOrder: ['techFaker'] });
 
-    assert.ok(player, 'created player from tag');
-    assert.ok(player.id() === 'test_vid_id');
-    assert.ok(videojs.getPlayers().test_vid_id === player,
-             'added player to global reference');
+  assert.ok(player, 'created player from tag');
+  assert.ok(player.id() === 'test_vid_id');
+  assert.ok(videojs.getPlayers().test_vid_id === player,
+           'added player to global reference');
 
-    const playerAgain = videojs('test_vid_id');
+  const playerAgain = videojs('test_vid_id');
 
-    assert.ok(player === playerAgain, 'did not create a second player from same tag');
+  assert.ok(player === playerAgain, 'did not create a second player from same tag');
 
-    assert.equal(player, playerAgain, 'we did not make a new player');
+  assert.equal(player, playerAgain, 'we did not make a new player');
 
-    const tag2 = document.getElementById('test_vid_id2');
-    const player2 = videojs(tag2, { techOrder: ['techFaker'] });
+  const tag2 = document.getElementById('test_vid_id2');
+  const player2 = videojs(tag2, { techOrder: ['techFaker'] });
 
-    assert.ok(player2.id() === 'test_vid_id2', 'created player from element');
+  assert.ok(player2.id() === 'test_vid_id2', 'created player from element');
 
-    player.dispose();
-    player2.dispose();
-  });
+  player.dispose();
+  player2.dispose();
+});
 
-  QUnit.test('should log about already initalized players if options already passed',
-  function(assert) {
-    const origWarnLog = log.warn;
-    const fixture = document.getElementById('qunit-fixture');
-    const warnLogs = [];
+QUnit.test('should log about already initalized players if options already passed',
+function(assert) {
+  const origWarnLog = log.warn;
+  const fixture = document.getElementById('qunit-fixture');
+  const warnLogs = [];
 
-    log.warn = (args) => {
-      warnLogs.push(args);
-    };
+  log.warn = (args) => {
+    warnLogs.push(args);
+  };
 
-    fixture.innerHTML += '<video-js id="test_vid_id"></video-js>';
+  fixture.innerHTML += '<video-js id="test_vid_id"></video-js>';
 
-    const player = videojs('test_vid_id', { techOrder: ['techFaker'] });
+  const player = videojs('test_vid_id', { techOrder: ['techFaker'] });
 
-    assert.ok(player, 'created player from tag');
-    assert.equal(player.id(), 'test_vid_id', 'player has the right ID');
-    assert.equal(warnLogs.length, 0, 'no warn logs');
+  assert.ok(player, 'created player from tag');
+  assert.equal(player.id(), 'test_vid_id', 'player has the right ID');
+  assert.equal(warnLogs.length, 0, 'no warn logs');
 
-    const playerAgain = videojs('test_vid_id');
+  const playerAgain = videojs('test_vid_id');
 
-    assert.equal(player, playerAgain, 'did not create a second player from same tag');
-    assert.equal(warnLogs.length, 0, 'no warn logs');
+  assert.equal(player, playerAgain, 'did not create a second player from same tag');
+  assert.equal(warnLogs.length, 0, 'no warn logs');
 
-    const playerAgainWithOptions = videojs('test_vid_id', { techOrder: ['techFaker'] });
+  const playerAgainWithOptions = videojs('test_vid_id', { techOrder: ['techFaker'] });
 
-    assert.equal(player,
-                 playerAgainWithOptions,
-                 'did not create a second player from same tag');
-    assert.equal(warnLogs.length, 1, 'logged a warning');
-    assert.equal(warnLogs[0],
-                 'Player "test_vid_id" is already initialised. Options will not be applied.',
-                 'logged the right message');
+  assert.equal(player,
+               playerAgainWithOptions,
+               'did not create a second player from same tag');
+  assert.equal(warnLogs.length, 1, 'logged a warning');
+  assert.equal(warnLogs[0],
+               'Player "test_vid_id" is already initialised. Options will not be applied.',
+               'logged the right message');
 
-    log.warn = origWarnLog;
+  log.warn = origWarnLog;
 
-    player.dispose();
-  });
+  player.dispose();
+});
 
-  QUnit.test('should return a video player instance from el html5 tech', function(assert) {
-    const fixture = document.getElementById('qunit-fixture');
+QUnit.test('should return a video player instance from el html5 tech', function(assert) {
+  const fixture = document.getElementById('qunit-fixture');
 
-    fixture.innerHTML += '<video-js id="test_vid_id"></video-js>' +
-                         '<video-js id="test_vid_id2"></video-js>';
+  fixture.innerHTML += '<video-js id="test_vid_id"></video-js>' +
+                       '<video-js id="test_vid_id2"></video-js>';
 
-    const vid = document.querySelector('#test_vid_id');
+  const vid = document.querySelector('#test_vid_id');
 
-    const player = videojs(vid);
+  const player = videojs(vid);
 
-    assert.ok(player, 'created player from tag');
-    assert.ok(player.id() === 'test_vid_id');
-    assert.ok(videojs.getPlayers().test_vid_id === player,
-             'added player to global reference');
+  assert.ok(player, 'created player from tag');
+  assert.ok(player.id() === 'test_vid_id');
+  assert.ok(videojs.getPlayers().test_vid_id === player,
+           'added player to global reference');
 
-    const playerAgain = videojs(vid);
+  const playerAgain = videojs(vid);
 
-    assert.ok(player === playerAgain, 'did not create a second player from same tag');
-    assert.equal(player, playerAgain, 'we did not make a new player');
+  assert.ok(player === playerAgain, 'did not create a second player from same tag');
+  assert.equal(player, playerAgain, 'we did not make a new player');
 
-    const tag2 = document.getElementById('test_vid_id2');
-    const player2 = videojs(tag2, { techOrder: ['techFaker'] });
+  const tag2 = document.getElementById('test_vid_id2');
+  const player2 = videojs(tag2, { techOrder: ['techFaker'] });
 
-    assert.ok(player2.id() === 'test_vid_id2', 'created player from element');
+  assert.ok(player2.id() === 'test_vid_id2', 'created player from element');
 
-    player.dispose();
-    player2.dispose();
-  });
+  player.dispose();
+  player2.dispose();
+});
 
-  QUnit.test('should return a video player instance from el techfaker', function(assert) {
-    const fixture = document.getElementById('qunit-fixture');
+QUnit.test('should return a video player instance from el techfaker', function(assert) {
+  const fixture = document.getElementById('qunit-fixture');
 
-    fixture.innerHTML += '<video-js id="test_vid_id"></video-js>' +
-                         '<video-js id="test_vid_id2"></video-js>';
+  fixture.innerHTML += '<video-js id="test_vid_id"></video-js>' +
+                       '<video-js id="test_vid_id2"></video-js>';
 
-    const vid = document.querySelector('#test_vid_id');
-    const player = videojs(vid, {techOrder: ['techFaker']});
+  const vid = document.querySelector('#test_vid_id');
+  const player = videojs(vid, {techOrder: ['techFaker']});
 
-    assert.ok(player, 'created player from tag');
-    assert.ok(player.id() === 'test_vid_id');
-    assert.ok(videojs.getPlayers().test_vid_id === player,
-             'added player to global reference');
+  assert.ok(player, 'created player from tag');
+  assert.ok(player.id() === 'test_vid_id');
+  assert.ok(videojs.getPlayers().test_vid_id === player,
+           'added player to global reference');
 
-    const playerAgain = videojs(vid);
+  const playerAgain = videojs(vid);
 
-    assert.ok(player === playerAgain, 'did not create a second player from same tag');
-    assert.equal(player, playerAgain, 'we did not make a new player');
+  assert.ok(player === playerAgain, 'did not create a second player from same tag');
+  assert.equal(player, playerAgain, 'we did not make a new player');
 
-    const tag2 = document.getElementById('test_vid_id2');
-    const player2 = videojs(tag2, { techOrder: ['techFaker'] });
+  const tag2 = document.getElementById('test_vid_id2');
+  const player2 = videojs(tag2, { techOrder: ['techFaker'] });
 
-    assert.ok(player2.id() === 'test_vid_id2', 'created player from element');
+  assert.ok(player2.id() === 'test_vid_id2', 'created player from element');
 
-    player.dispose();
-    player2.dispose();
-  });
+  player.dispose();
+  player2.dispose();
+});
 
-  QUnit.test('adds video-js class name with the video-js embed', function(assert) {
-    const fixture = document.getElementById('qunit-fixture');
+QUnit.test('adds video-js class name with the video-js embed', function(assert) {
+  const fixture = document.getElementById('qunit-fixture');
 
-    fixture.innerHTML += '<video-js id="test_vid_id"></video-js>' +
-                         '<video-js class="video-js" id="test_vid_id2"></video-js>';
+  fixture.innerHTML += '<video-js id="test_vid_id"></video-js>' +
+                       '<video-js class="video-js" id="test_vid_id2"></video-js>';
 
-    const vid = document.querySelector('#test_vid_id');
-    const player = videojs(vid, {techOrder: ['techFaker']});
-    const tag2 = document.getElementById('test_vid_id2');
-    const player2 = videojs(tag2, { techOrder: ['techFaker'] });
+  const vid = document.querySelector('#test_vid_id');
+  const player = videojs(vid, {techOrder: ['techFaker']});
+  const tag2 = document.getElementById('test_vid_id2');
+  const player2 = videojs(tag2, { techOrder: ['techFaker'] });
 
-    assert.ok(player.hasClass('video-js'), 'video-js class was added to the first embed');
-    assert.ok(player2.hasClass('video-js'), 'video-js class was preserved to the second embed');
+  assert.ok(player.hasClass('video-js'), 'video-js class was added to the first embed');
+  assert.ok(player2.hasClass('video-js'), 'video-js class was preserved to the second embed');
 
-    player.dispose();
-    player2.dispose();
-  });
-}
+  player.dispose();
+  player2.dispose();
+});