From 0c324158fb35e56de4f3e5ccac09788ff7d01b9e Mon Sep 17 00:00:00 2001 From: Jason Walton Date: Thu, 1 Apr 2021 15:22:46 -0400 Subject: [PATCH 1/2] fix: stopLoad() after destroy() becomes no-op, startLoad() after destroy() throws nicer error message. --- src/hls.ts | 16 ++++++++++------ tests/unit/hls.js | 17 +++++++++++++++++ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/hls.ts b/src/hls.ts index db32379ae2d..1996c466121 100644 --- a/src/hls.ts +++ b/src/hls.ts @@ -40,7 +40,7 @@ export default class Hls implements HlsEventEmitter { public readonly userConfig: Partial; private coreComponents: ComponentAPI[]; - private networkControllers: NetworkComponentAPI[]; + private networkControllers: NetworkComponentAPI[] | undefined; private _emitter: HlsEventEmitter = new EventEmitter(); private _autoLevelCapping: number; @@ -284,8 +284,7 @@ export default class Hls implements HlsEventEmitter { this.url = null; if (this.networkControllers) { this.networkControllers.forEach((component) => component.destroy()); - // @ts-ignore - this.networkControllers = null; + this.networkControllers = undefined; } if (this.coreComponents) { this.coreComponents.forEach((component) => component.destroy()); @@ -342,6 +341,9 @@ export default class Hls implements HlsEventEmitter { */ startLoad(startPosition: number = -1) { logger.log(`startLoad(${startPosition})`); + if (!this.networkControllers) { + throw new Error('Cannot call `startLoad()` on destroyed instance of hls'); + } this.networkControllers.forEach((controller) => { controller.startLoad(startPosition); }); @@ -352,9 +354,11 @@ export default class Hls implements HlsEventEmitter { */ stopLoad() { logger.log('stopLoad'); - this.networkControllers.forEach((controller) => { - controller.stopLoad(); - }); + if (this.networkControllers) { + this.networkControllers.forEach((controller) => { + controller.stopLoad(); + }); + } } /** diff --git a/tests/unit/hls.js b/tests/unit/hls.js index b6dcadbca7d..f742065d9fc 100644 --- a/tests/unit/hls.js +++ b/tests/unit/hls.js @@ -1,5 +1,6 @@ import Hls from '../../src/hls'; import { hlsDefaultConfig } from '../../src/config'; +import { expect } from 'chai'; describe('Hls', function () { describe('bandwidthEstimate', function () { @@ -21,4 +22,20 @@ describe('Hls', function () { ); }); }); + + describe('destroy', function () { + it('should allow stopLoad() after destroy()', function () { + const hls = new Hls(); + hls.destroy(); + expect(() => hls.stopLoad()).to.not.throw(); + }); + + it('should not allow startLoad() after destroy()', function () { + const hls = new Hls(); + hls.destroy(); + expect(() => hls.startLoad()).to.throw( + 'Cannot call `startLoad()` on destroyed instance of hls' + ); + }); + }); }); From cd070b556ee8030a976638f60f0a33453dad12c0 Mon Sep 17 00:00:00 2001 From: Jason Walton Date: Tue, 6 Apr 2021 09:07:17 -0400 Subject: [PATCH 2/2] refactor: Update fix to networkControllers and coreComponents based on feedback from review. re: #3710 --- src/hls.ts | 28 ++++++++++------------------ tests/unit/hls.js | 8 +++----- 2 files changed, 13 insertions(+), 23 deletions(-) diff --git a/src/hls.ts b/src/hls.ts index 1996c466121..643672669f6 100644 --- a/src/hls.ts +++ b/src/hls.ts @@ -40,7 +40,7 @@ export default class Hls implements HlsEventEmitter { public readonly userConfig: Partial; private coreComponents: ComponentAPI[]; - private networkControllers: NetworkComponentAPI[] | undefined; + private networkControllers: NetworkComponentAPI[]; private _emitter: HlsEventEmitter = new EventEmitter(); private _autoLevelCapping: number; @@ -282,15 +282,12 @@ export default class Hls implements HlsEventEmitter { this.removeAllListeners(); this._autoLevelCapping = -1; this.url = null; - if (this.networkControllers) { - this.networkControllers.forEach((component) => component.destroy()); - this.networkControllers = undefined; - } - if (this.coreComponents) { - this.coreComponents.forEach((component) => component.destroy()); - // @ts-ignore - this.coreComponents = null; - } + + this.networkControllers.forEach((component) => component.destroy()); + this.networkControllers.length = 0; + + this.coreComponents.forEach((component) => component.destroy()); + this.coreComponents.length = 0; } /** @@ -341,9 +338,6 @@ export default class Hls implements HlsEventEmitter { */ startLoad(startPosition: number = -1) { logger.log(`startLoad(${startPosition})`); - if (!this.networkControllers) { - throw new Error('Cannot call `startLoad()` on destroyed instance of hls'); - } this.networkControllers.forEach((controller) => { controller.startLoad(startPosition); }); @@ -354,11 +348,9 @@ export default class Hls implements HlsEventEmitter { */ stopLoad() { logger.log('stopLoad'); - if (this.networkControllers) { - this.networkControllers.forEach((controller) => { - controller.stopLoad(); - }); - } + this.networkControllers.forEach((controller) => { + controller.stopLoad(); + }); } /** diff --git a/tests/unit/hls.js b/tests/unit/hls.js index f742065d9fc..235750badc5 100644 --- a/tests/unit/hls.js +++ b/tests/unit/hls.js @@ -24,18 +24,16 @@ describe('Hls', function () { }); describe('destroy', function () { - it('should allow stopLoad() after destroy()', function () { + it('should not crash on stopLoad() after destroy()', function () { const hls = new Hls(); hls.destroy(); expect(() => hls.stopLoad()).to.not.throw(); }); - it('should not allow startLoad() after destroy()', function () { + it('should not crash on startLoad() after destroy()', function () { const hls = new Hls(); hls.destroy(); - expect(() => hls.startLoad()).to.throw( - 'Cannot call `startLoad()` on destroyed instance of hls' - ); + expect(() => hls.startLoad()).to.not.throw(); }); }); });