web/video.js
1
0
Fork 0
mirror of https://github.com/videojs/video.js.git synced 2025-07-15 01:34:23 +02:00
Code Releases Activity

perf: Use WeakMap for dom data (#6103)

Browse Source
This commit is contained in:
Brandon Casey
2019-08-01 14:26:59 -04:00
committed by Gary Katsevman
parent 1d2b20636c
commit 8610f99673
7 changed files with 30 additions and 208 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/js/component.js
View File

@ -7,7 +7,7 @@ import window from 'global/window';
import evented from './mixins/evented';
import stateful from './mixins/stateful';
import * as Dom from './utils/dom.js';
import * as DomData from './utils/dom-data';
import DomData from './utils/dom-data';
import * as Fn from './utils/fn.js';
import * as Guid from './utils/guid.js';
import toTitleCase from './utils/to-title-case.js';
@ -153,7 +153,9 @@ class Component {
this.el_.parentNode.removeChild(this.el_);
}
DomData.removeData(this.el_);
if (DomData.has(this.el_)) {
DomData.delete(this.el_);
}
this.el_ = null;
}

85
src/js/utils/dom-data.js
View File

@ -2,8 +2,6 @@
* @file dom-data.js
* @module dom-data
*/
import * as Guid from './guid.js';
import window from 'global/window';
/**
* Element Data Store.
@ -15,85 +13,4 @@ import window from 'global/window';
* @type {Object}
* @private
*/
export const elData = {};
/*
* Unique attribute name to store an element's guid in
*
* @type {String}
* @constant
* @private
*/
const elIdAttr = 'vdata' + Math.floor(window.performance && window.performance.now() || Date.now());
/**
* Returns the cache object where data for an element is stored
*
* @param {Element} el
* Element to store data for.
*
* @return {Object}
* The cache object for that el that was passed in.
*/
export function getData(el) {
let id = el[elIdAttr];
if (!id) {
id = el[elIdAttr] = Guid.newGUID();
}
if (!elData[id]) {
elData[id] = {};
}
return elData[id];
}
/**
* Returns whether or not an element has cached data
*
* @param {Element} el
* Check if this element has cached data.
*
* @return {boolean}
* - True if the DOM element has cached data.
* - False otherwise.
*/
export function hasData(el) {
const id = el[elIdAttr];
if (!id) {
return false;
}
return !!Object.getOwnPropertyNames(elData[id]).length;
}
/**
* Delete data for the element from the cache and the guid attr from getElementById
*
* @param {Element} el
* Remove cached data for this element.
*/
export function removeData(el) {
const id = el[elIdAttr];
if (!id) {
return;
}
// Remove all stored data
delete elData[id];
// Remove the elIdAttr property from the DOM node
try {
delete el[elIdAttr];
} catch (e) {
if (el.removeAttribute) {
el.removeAttribute(elIdAttr);
} else {
// IE doesn't appear to support removeAttribute on the document element
el[elIdAttr] = null;
}
}
}
export default new WeakMap();

26
src/js/utils/events.js
View File

@ -7,7 +7,7 @@
* @file events.js
* @module events
*/
import * as DomData from './dom-data';
import DomData from './dom-data';
import * as Guid from './guid.js';
import log from './log.js';
import window from 'global/window';
@ -23,7 +23,10 @@ import document from 'global/document';
* Type of event to clean up
*/
function _cleanUpEvents(elem, type) {
const data = DomData.getData(elem);
if (!DomData.has(elem)) {
return;
}
const data = DomData.get(elem);
// Remove the events of a particular type if there are none left
if (data.handlers[type].length === 0) {
@ -48,7 +51,7 @@ function _cleanUpEvents(elem, type) {
// Finally remove the element data if there is no data left
if (Object.getOwnPropertyNames(data).length === 0) {
DomData.removeData(elem);
DomData.delete(elem);
}
}
@ -250,7 +253,11 @@ export function on(elem, type, fn) {
return _handleMultipleEvents(on, elem, type, fn);
}
const data = DomData.getData(elem);
if (!DomData.has(elem)) {
DomData.set(elem, {});
}
const data = DomData.get(elem);
// We need a place to store all our handler data
if (!data.handlers) {
@ -329,11 +336,11 @@ export function on(elem, type, fn) {
*/
export function off(elem, type, fn) {
// Don't want to add a cache object through getElData if not needed
if (!DomData.hasData(elem)) {
if (!DomData.has(elem)) {
return;
}
const data = DomData.getData(elem);
const data = DomData.get(elem);
// If no events exist, nothing to unbind
if (!data.handlers) {
@ -405,7 +412,7 @@ export function trigger(elem, event, hash) {
// Fetches element data and a reference to the parent (for bubbling).
// Don't want to add a data object to cache for every parent,
// so checking hasElData first.
const elemData = (DomData.hasData(elem)) ? DomData.getData(elem) : {};
const elemData = DomData.has(elem) ? DomData.get(elem) : {};
const parent = elem.parentNode || elem.ownerDocument;
// type = event.type || event,
// handler;
@ -432,7 +439,10 @@ export function trigger(elem, event, hash) {
// If at the top of the DOM, triggers the default action unless disabled.
} else if (!parent && !event.defaultPrevented && event.target && event.target[event.type]) {
const targetData = DomData.getData(event.target);
if (!DomData.has(event.target)) {
DomData.set(event.target, {});
}
const targetData = DomData.get(event.target);
// Checks if the target has a default action for this event.
if (event.target[event.type]) {

10
test/unit/component.test.js
View File

@ -2,7 +2,7 @@
import window from 'global/window';
import Component from '../../src/js/component.js';
import * as Dom from '../../src/js/utils/dom.js';
import * as DomData from '../../src/js/utils/dom-data';
import DomData from '../../src/js/utils/dom-data';
import * as Events from '../../src/js/utils/events.js';
import * as Obj from '../../src/js/utils/obj';
import * as browser from '../../src/js/utils/browser.js';
@ -346,7 +346,7 @@ QUnit.test('should dispose of component and children', function(assert) {
return true;
});
const el = comp.el();
const data = DomData.getData(el);
const data = DomData.get(el);
let hasDisposed = false;
let bubbles = null;
@ -365,7 +365,7 @@ QUnit.test('should dispose of component and children', function(assert) {
assert.ok(!comp.el(), 'component element was deleted');
assert.ok(!child.children(), 'child children were deleted');
assert.ok(!child.el(), 'child element was deleted');
assert.ok(!DomData.hasData(el), 'listener data nulled');
assert.ok(!DomData.has(el), 'listener data nulled');
assert.ok(
!Object.getOwnPropertyNames(data).length,
'original listener data object was emptied'
@ -979,7 +979,7 @@ QUnit.test('*AnimationFrame methods fall back to timers if rAF not supported', f
QUnit.test('setTimeout should remove dispose handler on trigger', function(assert) {
const comp = new Component(getFakePlayer());
const el = comp.el();
const data = DomData.getData(el);
const data = DomData.get(el);
comp.setTimeout(() => {}, 1);
@ -996,7 +996,7 @@ QUnit.test('setTimeout should remove dispose handler on trigger', function(asser
QUnit.test('requestAnimationFrame should remove dispose handler on trigger', function(assert) {
const comp = new Component(getFakePlayer());
const el = comp.el();
const data = DomData.getData(el);
const data = DomData.get(el);
const oldRAF = window.requestAnimationFrame;
const oldCAF = window.cancelAnimationFrame;

4
test/unit/menu.test.js
View File

@ -1,5 +1,5 @@
/* eslint-env qunit */
import * as DomData from '../../src/js/utils/dom-data';
import DomData from '../../src/js/utils/dom-data';
import MenuButton from '../../src/js/menu/menu-button.js';
import Menu from '../../src/js/menu/menu.js';
import CaptionSettingsMenuItem from '../../src/js/control-bar/text-track-controls/caption-settings-menu-item';
@ -137,7 +137,7 @@ QUnit.test('should remove old event listeners when the menu item adds to the new
* A reusable collection of assertions.
*/
function validateMenuEventListeners(watchedMenu) {
const eventData = DomData.getData(menuItem.eventBusEl_);
const eventData = DomData.get(menuItem.eventBusEl_);
// `MenuButton`.`unpressButton` will be called when triggering click event on the menu item.
const unpressButtonSpy = sinon.spy(menuButton, 'unpressButton');
// `MenuButton`.`focus` will be called when triggering click event on the menu item.

63
test/unit/utils/dom-data.test.js
View File

@ -1,63 +0,0 @@
/* eslint-env qunit */
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');
QUnit.test('should get and remove data from an element', function(assert) {
const el = document.createElement('div');
const data = DomData.getData(el);
assert.strictEqual(typeof data, 'object', 'data object created');
// Add data
const testData = {asdf: 'fdsa'};
data.test = testData;
assert.strictEqual(DomData.getData(el).test, testData, 'data added');
// Remove all data
DomData.removeData(el);
assert.notOk(DomData.hasData(el), 'cached item emptied');
});
let memoryTestRun = false;
QUnit.done(function(details) {
// don't run the extra dom data test on failures, there will likely be
// memory leaks
if (details.failed || memoryTestRun) {
return;
}
memoryTestRun = true;
QUnit.module('dom-data memory');
/**
* If this test fails you will want to add a debug statement
* in DomData.getData with the `id`. For instance if DomData.elData
* had 2 objects in it {5: {...}, 2003: {...} you would add:
*
* ```js
* if (id === 5) {
* debugger;
* }
* ```
* to the tests to see what test. Then re-run the tests to see
* what leaking and where.
*
* > Note that the id can be off by 1-2 in either direction
* for larger guids, so you may have to account for that.
*/
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');
});
});

44
test/unit/videojs-integration.test.js
View File

@ -2,7 +2,6 @@
import videojs from '../../src/js/video.js';
import window from 'global/window';
import document from 'global/document';
import * as DomData from '../../src/js/utils/dom-data';
import * as Fn from '../../src/js/utils/fn';
/**
@ -70,54 +69,11 @@ QUnit.test('create a real player and dispose', function(assert) {
player.muted(true);
const checkDomData = function() {
Object.keys(DomData.elData).forEach(function(elId) {
const data = DomData.elData[elId] || {};
Object.keys(data.handlers || {}).forEach(function(eventName) {
const listeners = data.handlers[eventName];
const uniqueList = [];
(listeners || []).forEach(function(listener) {
let add = true;
for (let i = 0; i < uniqueList.length; i++) {
const obj = uniqueList[i];
if (listener.og_ && listener.cx_ && obj.fn === listener.og_ && obj.cx === listener.cx_) {
add = false;
break;
}
if (listener.og_ && !listener.cx_ && obj.fn === listener.og_) {
add = false;
break;
}
if (!listener.og_ && !listener.cx_ && obj.fn === listener) {
add = false;
break;
}
}
const obj = {fn: listener.og_ || listener, cx: listener.cx_};
if (add) {
uniqueList.push(obj);
assert.ok(true, `${elId}/${eventName}/${obj.fn.name} is unique`);
} else {
assert.ok(false, `${elId}/${eventName}/${obj.fn.name} is not unique`);
}
});
});
});
};
player.addTextTrack('captions', 'foo', 'en');
player.ready(function() {
assert.ok(player.tech_, 'tech exists');
assert.equal(player.textTracks().length, 1, 'should have one text track');
checkDomData();
player.dispose();
Object.keys(old).forEach(function(k) {