Permalink
Browse files

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.
  • Loading branch information...
gkatsev committed Nov 16, 2017
1 parent d8aadd5 commit 2da7af1137e3c8f3b14bd603f17fef3173fe2da0
@@ -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`
*
View
@@ -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;
}
/**
@@ -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
@@ -52,6 +52,12 @@ class PlaybackRateMenuButton extends MenuButton {
return el;
}
dispose() {
this.labelEl_ = null;
super.dispose();
}
/**
* Builds the default DOM `className`.
*
@@ -39,6 +39,12 @@ class LoadProgressBar extends Component {
});
}
dispose() {
this.partEls_ = null;
super.dispose();
}
/**
* Update progress bar
*
@@ -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);
@@ -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.
@@ -62,6 +62,7 @@ class MenuButton extends Component {
const menu = this.createMenu();
if (this.menu) {
this.menu.dispose();
this.removeChild(this.menu);
}
View
@@ -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.
*
View
@@ -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;
}
View
@@ -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`.
*
View
@@ -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();
}
View
@@ -56,6 +56,12 @@ class Popup extends Component {
return el;
}
dispose() {
this.contentEl_ = null;
super.dispose();
}
}
Component.registerComponent('Popup', Popup);
View
@@ -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];
@@ -293,6 +293,12 @@ class TextTrackSettings extends ModalDialog {
}
}
dispose() {
this.endDialog = null;
super.dispose();
}
/**
* Create a <select> element with configured options.
*
View
@@ -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;
}
@@ -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();
Oops, something went wrong.

0 comments on commit 2da7af1

Please sign in to comment.