From 57a7e68c3878652770a62b00fee93984006771c8 Mon Sep 17 00:00:00 2001 From: Pat O'Neill Date: Mon, 23 May 2022 17:01:17 -0400 Subject: [PATCH 1/3] refactor: remove internal Map, Set, and WeakMap shams --- src/js/component.js | 23 --------------- src/js/utils/dom-data.js | 58 +------------------------------------ src/js/utils/map.js | 28 ------------------ src/js/utils/set.js | 28 ------------------ test/unit/component.test.js | 43 --------------------------- 5 files changed, 1 insertion(+), 179 deletions(-) delete mode 100644 src/js/utils/map.js delete mode 100644 src/js/utils/set.js diff --git a/src/js/component.js b/src/js/component.js index fcd210c887..8b3c951241 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -11,8 +11,6 @@ import * as Fn from './utils/fn.js'; import * as Guid from './utils/guid.js'; import {toTitleCase, toLowerCase} from './utils/str.js'; import {merge} from './utils/obj.js'; -import Map from './utils/map.js'; -import Set from './utils/set.js'; import keycode from 'keycode'; /** @@ -1508,11 +1506,6 @@ class Component { * @see [Similar to]{@link https://developer.mozilla.org/en-US/docs/Web/API/window/requestAnimationFrame} */ requestAnimationFrame(fn) { - // Fall back to using a timer. - if (!this.supportsRaf_) { - return this.setTimeout(fn, 1000 / 60); - } - this.clearTimersOnDispose_(); // declare as variables so they are properly available in rAF function @@ -1595,11 +1588,6 @@ class Component { * @see [Similar to]{@link https://developer.mozilla.org/en-US/docs/Web/API/window/cancelAnimationFrame} */ cancelAnimationFrame(id) { - // Fall back to using a timer. - if (!this.supportsRaf_) { - return this.clearTimeout(id); - } - if (this.rafIds_.has(id)) { this.rafIds_.delete(id); window.cancelAnimationFrame(id); @@ -1732,17 +1720,6 @@ class Component { } } -/** - * Whether or not this component supports `requestAnimationFrame`. - * - * This is exposed primarily for testing purposes. - * - * @private - * @type {Boolean} - */ -Component.prototype.supportsRaf_ = typeof window.requestAnimationFrame === 'function' && - typeof window.cancelAnimationFrame === 'function'; - Component.registerComponent('Component', Component); export default Component; diff --git a/src/js/utils/dom-data.js b/src/js/utils/dom-data.js index cd077ebfa8..43cf1a915e 100644 --- a/src/js/utils/dom-data.js +++ b/src/js/utils/dom-data.js @@ -3,62 +3,6 @@ * @module dom-data */ -import log from './log.js'; -import * as Guid from './guid.js'; -import window from 'global/window'; - -let FakeWeakMap; - -if (!window.WeakMap) { - FakeWeakMap = class { - constructor() { - this.vdata = 'vdata' + Math.floor(window.performance && window.performance.now() || Date.now()); - this.data = {}; - } - - set(key, value) { - const access = key[this.vdata] || Guid.newGUID(); - - if (!key[this.vdata]) { - key[this.vdata] = access; - } - - this.data[access] = value; - - return this; - } - - get(key) { - const access = key[this.vdata]; - - // we have data, return it - if (access) { - return this.data[access]; - } - - // we don't have data, return nothing. - // return undefined explicitly as that's the contract for this method - log('We have no data for this element', key); - return undefined; - } - - has(key) { - const access = key[this.vdata]; - - return access in this.data; - } - - delete(key) { - const access = key[this.vdata]; - - if (access) { - delete this.data[access]; - delete key[this.vdata]; - } - } - }; -} - /** * Element Data Store. * @@ -69,4 +13,4 @@ if (!window.WeakMap) { * @type {Object} * @private */ -export default window.WeakMap ? new WeakMap() : new FakeWeakMap(); +export default new WeakMap(); diff --git a/src/js/utils/map.js b/src/js/utils/map.js deleted file mode 100644 index f12cdf7cdd..0000000000 --- a/src/js/utils/map.js +++ /dev/null @@ -1,28 +0,0 @@ -import window from 'global/window'; - -class MapSham { - constructor() { - this.map_ = {}; - } - has(key) { - return key in this.map_; - } - delete(key) { - const has = this.has(key); - - delete this.map_[key]; - - return has; - } - set(key, value) { - this.map_[key] = value; - return this; - } - forEach(callback, thisArg) { - for (const key in this.map_) { - callback.call(thisArg, this.map_[key], key, this); - } - } -} - -export default window.Map ? window.Map : MapSham; diff --git a/src/js/utils/set.js b/src/js/utils/set.js deleted file mode 100644 index 8d2767e2b6..0000000000 --- a/src/js/utils/set.js +++ /dev/null @@ -1,28 +0,0 @@ -import window from 'global/window'; - -class SetSham { - constructor() { - this.set_ = {}; - } - has(key) { - return key in this.set_; - } - delete(key) { - const has = this.has(key); - - delete this.set_[key]; - - return has; - } - add(key) { - this.set_[key] = 1; - return this; - } - forEach(callback, thisArg) { - for (const key in this.set_) { - callback.call(thisArg, key, key, this); - } - } -} - -export default window.Set ? window.Set : SetSham; diff --git a/test/unit/component.test.js b/test/unit/component.test.js index a7c3dd1b1d..169d1f7a65 100644 --- a/test/unit/component.test.js +++ b/test/unit/component.test.js @@ -969,9 +969,6 @@ QUnit.test('should provide a requestAnimationFrame method that is cleared on dis window.requestAnimationFrame = (fn) => window.setTimeout(fn, 1); window.cancelAnimationFrame = (id) => window.clearTimeout(id); - // Make sure the component thinks it supports rAF. - comp.supportsRaf_ = true; - const spyRAF = sinon.spy(); comp.requestNamedAnimationFrame('testing', spyRAF); @@ -1005,9 +1002,6 @@ QUnit.test('should provide a requestNamedAnimationFrame method that is cleared o window.requestAnimationFrame = (fn) => window.setTimeout(fn, 1); window.cancelAnimationFrame = (id) => window.clearTimeout(id); - // Make sure the component thinks it supports rAF. - comp.supportsRaf_ = true; - const spyRAF = sinon.spy(); comp.requestNamedAnimationFrame('testing', spyRAF); @@ -1031,34 +1025,6 @@ QUnit.test('should provide a requestNamedAnimationFrame method that is cleared o window.cancelAnimationFrame = oldCAF; }); -QUnit.test('requestAnimationFrame falls back to timers if rAF not supported', function(assert) { - const comp = new Component(this.player); - const oldRAF = window.requestAnimationFrame; - const oldCAF = window.cancelAnimationFrame; - - // Stub the window.*AnimationFrame methods with window.setTimeout methods - // so we can control when the callbacks are called via sinon's timer stubs. - const rAF = window.requestAnimationFrame = sinon.spy(); - const cAF = window.cancelAnimationFrame = sinon.spy(); - - // Make sure the component thinks it does not support rAF. - comp.supportsRaf_ = false; - - sinon.spy(comp, 'setTimeout'); - sinon.spy(comp, 'clearTimeout'); - - comp.cancelAnimationFrame(comp.requestAnimationFrame(() => {})); - - assert.strictEqual(rAF.callCount, 0, 'window.requestAnimationFrame was not called'); - assert.strictEqual(cAF.callCount, 0, 'window.cancelAnimationFrame was not called'); - assert.strictEqual(comp.setTimeout.callCount, 1, 'Component#setTimeout was called'); - assert.strictEqual(comp.clearTimeout.callCount, 1, 'Component#clearTimeout was called'); - - comp.dispose(); - window.requestAnimationFrame = oldRAF; - window.cancelAnimationFrame = oldCAF; -}); - QUnit.test('setTimeout should remove dispose handler on trigger', function(assert) { const comp = new Component(this.player); @@ -1083,9 +1049,6 @@ QUnit.test('requestNamedAnimationFrame should remove dispose handler on trigger' window.requestAnimationFrame = (fn) => window.setTimeout(fn, 1); window.cancelAnimationFrame = (id) => window.clearTimeout(id); - // Make sure the component thinks it supports rAF. - comp.supportsRaf_ = true; - const spyRAF = sinon.spy(); comp.requestNamedAnimationFrame('testFrame', spyRAF); @@ -1114,9 +1077,6 @@ QUnit.test('requestAnimationFrame should remove dispose handler on trigger', fun window.requestAnimationFrame = (fn) => window.setTimeout(fn, 1); window.cancelAnimationFrame = (id) => window.clearTimeout(id); - // Make sure the component thinks it supports rAF. - comp.supportsRaf_ = true; - const spyRAF = sinon.spy(); comp.requestAnimationFrame(spyRAF); @@ -1272,9 +1232,6 @@ QUnit.test('requestNamedAnimationFrame should only allow one raf of a specific n const oldRAF = window.requestAnimationFrame; const oldCAF = window.cancelAnimationFrame; - // Make sure the component thinks it supports rAF. - comp.supportsRaf_ = true; - // Stub the window.*AnimationFrame methods with window.setTimeout methods // so we can control when the callbacks are called via sinon's timer stubs. window.requestAnimationFrame = (fn) => window.setTimeout(fn, 1); From da608d3b48c3bdba147a1ef15c703afe3d7256c2 Mon Sep 17 00:00:00 2001 From: Pat O'Neill Date: Mon, 23 May 2022 17:15:46 -0400 Subject: [PATCH 2/3] remove stray options_.Promise reference introduced by merges/rebases --- src/js/player.js | 87 +++++++++++++++--------------------------------- 1 file changed, 26 insertions(+), 61 deletions(-) diff --git a/src/js/player.js b/src/js/player.js index 923b70a09e..cbd295941d 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -4313,46 +4313,28 @@ class Player extends Component { this.audioOnlyMode_ = value; - const PromiseClass = this.options_.Promise || window.Promise; - - if (PromiseClass) { - // Enable Audio Only Mode - if (value) { - const exitPromises = []; - - // Fullscreen and PiP are not supported in audioOnlyMode, so exit if we need to. - if (this.isInPictureInPicture()) { - exitPromises.push(this.exitPictureInPicture()); - } - - if (this.isFullscreen()) { - exitPromises.push(this.exitFullscreen()); - } - - if (this.audioPosterMode()) { - exitPromises.push(this.audioPosterMode(false)); - } - - return PromiseClass.all(exitPromises).then(() => this.enableAudioOnlyUI_()); - } - - // Disable Audio Only Mode - return PromiseClass.resolve().then(() => this.disableAudioOnlyUI_()); - } - + // Enable Audio Only Mode if (value) { + const exitPromises = []; + + // Fullscreen and PiP are not supported in audioOnlyMode, so exit if we need to. if (this.isInPictureInPicture()) { - this.exitPictureInPicture(); + exitPromises.push(this.exitPictureInPicture()); } if (this.isFullscreen()) { - this.exitFullscreen(); + exitPromises.push(this.exitFullscreen()); } - this.enableAudioOnlyUI_(); - } else { - this.disableAudioOnlyUI_(); + if (this.audioPosterMode()) { + exitPromises.push(this.audioPosterMode(false)); + } + + return Promise.all(exitPromises).then(() => this.enableAudioOnlyUI_()); } + + // Disable Audio Only Mode + return Promise.resolve().then(() => this.disableAudioOnlyUI_()); } enablePosterModeUI_() { @@ -4391,44 +4373,27 @@ class Player extends Component { this.audioPosterMode_ = value; - const PromiseClass = this.options_.Promise || window.Promise; - - if (PromiseClass) { - - if (value) { - - if (this.audioOnlyMode()) { - const audioOnlyModePromise = this.audioOnlyMode(false); + if (value) { - return audioOnlyModePromise.then(() => { - // enable audio poster mode after audio only mode is disabled - this.enablePosterModeUI_(); - }); - } + if (this.audioOnlyMode()) { + const audioOnlyModePromise = this.audioOnlyMode(false); - return PromiseClass.resolve().then(() => { - // enable audio poster mode + return audioOnlyModePromise.then(() => { + // enable audio poster mode after audio only mode is disabled this.enablePosterModeUI_(); }); } - return PromiseClass.resolve().then(() => { - // disable audio poster mode - this.disablePosterModeUI_(); + return Promise.resolve().then(() => { + // enable audio poster mode + this.enablePosterModeUI_(); }); } - if (value) { - - if (this.audioOnlyMode()) { - this.audioOnlyMode(false); - } - - this.enablePosterModeUI_(); - return; - } - - this.disablePosterModeUI_(); + return Promise.resolve().then(() => { + // disable audio poster mode + this.disablePosterModeUI_(); + }); } /** From e124304f0da008048c5efc7c7355973873ece8bd Mon Sep 17 00:00:00 2001 From: Pat O'Neill Date: Mon, 23 May 2022 19:40:27 -0400 Subject: [PATCH 3/3] assume window.performance.now support, test cleanup --- src/js/tech/html5.js | 7 +------ test/api/api.js | 18 +++++++++------- test/unit/player.test.js | 40 ------------------------------------ test/unit/tech/html5.test.js | 35 +------------------------------ test/unit/tech/tech.test.js | 30 +++++++++++++-------------- 5 files changed, 28 insertions(+), 102 deletions(-) diff --git a/src/js/tech/html5.js b/src/js/tech/html5.js index 90f93ee7fe..33b30a3fa8 100644 --- a/src/js/tech/html5.js +++ b/src/js/tech/html5.js @@ -965,13 +965,8 @@ class Html5 extends Tech { videoPlaybackQuality.totalVideoFrames = this.el().webkitDecodedFrameCount; } - if (window.performance && typeof window.performance.now === 'function') { + if (window.performance) { videoPlaybackQuality.creationTime = window.performance.now(); - } else if (window.performance && - window.performance.timing && - typeof window.performance.timing.navigationStart === 'number') { - videoPlaybackQuality.creationTime = - window.Date.now() - window.performance.timing.navigationStart; } return videoPlaybackQuality; diff --git a/test/api/api.js b/test/api/api.js index 167cd9dead..fc9774deb6 100644 --- a/test/api/api.js +++ b/test/api/api.js @@ -249,15 +249,17 @@ QUnit.test('component can be subclassed externally', function(assert) { const Component = videojs.getComponent('Component'); const ControlBar = videojs.getComponent('ControlBar'); - const player = new (videojs.extend(Component, { - reportUserActivity() {}, + class TestComponent extends Component { + reportUserActivity() {} textTracks() { return { addEventListener: Function.prototype, removeEventListener: Function.prototype }; } - }))({ + } + + const player = new TestComponent({ id() {}, reportUserActivity() {} }); @@ -275,14 +277,15 @@ function testHelperMakeTag() { QUnit.test('should extend Component', function(assert) { const Component = videojs.getComponent('Component'); - const MyComponent = videojs.extend(Component, { + + class MyComponent extends Component { constructor() { this.bar = true; - }, + } foo() { return true; } - }); + } const myComponent = new MyComponent(); @@ -291,7 +294,8 @@ QUnit.test('should extend Component', function(assert) { assert.ok(myComponent.bar, 'the constructor function is used'); assert.ok(myComponent.foo(), 'instance methods are applied'); - const NoMethods = videojs.extend(Component); + class NoMethods extends Component {} + const noMethods = new NoMethods({}); assert.ok(noMethods.on, 'should extend component with no methods or constructor'); diff --git a/test/unit/player.test.js b/test/unit/player.test.js index 523d64ac5e..2e1d45146b 100644 --- a/test/unit/player.test.js +++ b/test/unit/player.test.js @@ -1591,26 +1591,6 @@ QUnit.test('setting audioPosterMode() should trigger audiopostermodechange event }); }); -QUnit.test('audioPosterMode is synchronous and returns undefined when promises are unsupported', function(assert) { - const originalPromise = window.Promise; - const player = TestHelpers.makePlayer({}); - - player.trigger('ready'); - window.Promise = undefined; - - const returnValTrue = player.audioPosterMode(true); - - assert.equal(returnValTrue, undefined, 'return value is undefined'); - assert.ok(player.audioPosterMode(), 'state synchronously set to true'); - - const returnValFalse = player.audioPosterMode(false); - - assert.equal(returnValFalse, undefined, 'return value is undefined'); - assert.notOk(player.audioPosterMode(), 'state synchronously set to false'); - - window.Promise = originalPromise; -}); - QUnit.test('should setScrubbing when seeking or not seeking', function(assert) { const player = TestHelpers.makePlayer(); let isScrubbing; @@ -2877,26 +2857,6 @@ QUnit.test('audioOnlyMode() getter returns Boolean', function(assert) { assert.ok(typeof player.audioOnlyMode() === 'boolean', 'getter correctly returns boolean'); }); -QUnit.test('audioOnlyMode(true/false) is synchronous and returns undefined when promises are unsupported', function(assert) { - const originalPromise = window.Promise; - const player = TestHelpers.makePlayer({}); - - player.trigger('ready'); - window.Promise = undefined; - - const returnValTrue = player.audioOnlyMode(true); - - assert.equal(returnValTrue, undefined, 'return value is undefined'); - assert.ok(player.audioOnlyMode(), 'state synchronously set to true'); - - const returnValFalse = player.audioOnlyMode(false); - - assert.equal(returnValFalse, undefined, 'return value is undefined'); - assert.notOk(player.audioOnlyMode(), 'state synchronously set to false'); - - window.Promise = originalPromise; -}); - QUnit.test('audioOnlyMode() gets the correct audioOnlyMode state', function(assert) { const player = TestHelpers.makePlayer({}); diff --git a/test/unit/tech/html5.test.js b/test/unit/tech/html5.test.js index a7863d160c..0dead552e9 100644 --- a/test/unit/tech/html5.test.js +++ b/test/unit/tech/html5.test.js @@ -943,7 +943,6 @@ QUnit.test('When Android Chrome reports Infinity duration with currentTime 0, re QUnit.test('supports getting available media playback quality metrics', function(assert) { const origPerformance = window.performance; - const origDate = window.Date; const oldEl = tech.el_; const videoPlaybackQuality = { creationTime: 1, @@ -960,7 +959,6 @@ QUnit.test('supports getting available media playback quality metrics', function videoPlaybackQuality, 'uses native implementation when supported' ); - tech.el_ = { webkitDroppedFrameCount: 1, webkitDecodedFrameCount: 2 @@ -974,42 +972,12 @@ QUnit.test('supports getting available media playback quality metrics', function 'uses webkit prefixed metrics and performance.now when supported' ); - tech.el_ = { - webkitDroppedFrameCount: 1, - webkitDecodedFrameCount: 2 - }; - window.Date = { - now: () => 10 - }; - window.performance = { - timing: { - navigationStart: 3 - } - }; - assert.deepEqual( - tech.getVideoPlaybackQuality(), - { droppedVideoFrames: 1, totalVideoFrames: 2, creationTime: 7 }, - 'uses webkit prefixed metrics and Date.now() - navigationStart when ' + - 'supported' - ); - tech.el_ = {}; window.performance = void 0; assert.deepEqual(tech.getVideoPlaybackQuality(), {}, 'empty object when not supported'); window.performance = { - now: () => 5 - }; - assert.deepEqual( - tech.getVideoPlaybackQuality(), - { creationTime: 5 }, - 'only creation time when it\'s the only piece available' - ); - - window.performance = { - timing: { - navigationStart: 3 - } + now: () => 7 }; assert.deepEqual( tech.getVideoPlaybackQuality(), @@ -1030,5 +998,4 @@ QUnit.test('supports getting available media playback quality metrics', function tech.el_ = oldEl; window.performance = origPerformance; - window.Date = origDate; }); diff --git a/test/unit/tech/tech.test.js b/test/unit/tech/tech.test.js index 7f3113e39a..65984cb28f 100644 --- a/test/unit/tech/tech.test.js +++ b/test/unit/tech/tech.test.js @@ -3,7 +3,6 @@ import Tech from '../../../src/js/tech/tech.js'; import Html5 from '../../../src/js/tech/html5.js'; import Button from '../../../src/js/button.js'; import { createTimeRange } from '../../../src/js/utils/time.js'; -import extend from '../../../src/js/extend.js'; import MediaError from '../../../src/js/media-error.js'; import AudioTrack from '../../../src/js/tracks/audio-track'; import VideoTrack from '../../../src/js/tracks/video-track'; @@ -43,7 +42,7 @@ QUnit.module('Media Tech', { }); QUnit.test('Tech.registerTech and Tech.getTech', function(assert) { - const MyTech = extend(Tech); + class MyTech extends Tech {} const oldTechs = Tech.techs_; const oldDefaultTechOrder = Tech.defaultTechOrder_; @@ -223,7 +222,7 @@ QUnit.test('switching sources should clear all remote tracks that are added with const oldLogWarn = log.warn; // Define a new tech class - const MyTech = extend(Tech); + class MyTech extends Tech {} // Create source handler const handler = { @@ -301,7 +300,7 @@ QUnit.test('should add the source handler interface to a tech', function(assert) const sourceB = { src: 'no-support', type: 'no-support' }; // Define a new tech class - const MyTech = extend(Tech); + class MyTech extends Tech {} // Extend Tech with source handlers Tech.withSourceHandlers(MyTech); @@ -501,7 +500,7 @@ QUnit.test('should add the source handler interface to a tech', function(assert) QUnit.test('should handle unsupported sources with the source handler API', function(assert) { // Define a new tech class - const MyTech = extend(Tech); + class MyTech extends Tech {} // Extend Tech with source handlers Tech.withSourceHandlers(MyTech); @@ -553,17 +552,17 @@ QUnit.test('should track whether a video has played', function(assert) { }); QUnit.test('delegates deferrables to the source handler', function(assert) { - const MyTech = extend(Tech, { + class MyTech extends Tech { seekable() { throw new Error('You should not be calling me!'); - }, + } seeking() { throw new Error('You should not be calling me!'); - }, + } duration() { throw new Error('You should not be calling me!'); } - }); + } Tech.withSourceHandlers(MyTech); @@ -604,18 +603,19 @@ QUnit.test('delegates deferrables to the source handler', function(assert) { QUnit.test('delegates only deferred deferrables to the source handler', function(assert) { let seekingCount = 0; - const MyTech = extend(Tech, { + + class MyTech extends Tech { seekable() { throw new Error('You should not be calling me!'); - }, + } seeking() { seekingCount++; return false; - }, + } duration() { throw new Error('You should not be calling me!'); } - }); + } Tech.withSourceHandlers(MyTech); @@ -668,7 +668,7 @@ QUnit.test('Tech.isTech returns correct answers for techs and components', funct }); QUnit.test('setSource after tech dispose should dispose source handler once', function(assert) { - const MyTech = extend(Tech); + class MyTech extends Tech {} Tech.withSourceHandlers(MyTech); @@ -713,7 +713,7 @@ QUnit.test('setSource after tech dispose should dispose source handler once', fu }); QUnit.test('setSource after previous setSource should dispose source handler once', function(assert) { - const MyTech = extend(Tech); + class MyTech extends Tech {} Tech.withSourceHandlers(MyTech);