From a158a48084f586e32494fe694f3a1dd429afd3b4 Mon Sep 17 00:00:00 2001 From: Mattias Buelens Date: Tue, 16 Apr 2024 16:00:23 +0200 Subject: [PATCH 01/12] Remove player listeners on destroy --- src/UIContainer.ts | 59 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 45 insertions(+), 14 deletions(-) diff --git a/src/UIContainer.ts b/src/UIContainer.ts index e5d038c..1c7302e 100644 --- a/src/UIContainer.ts +++ b/src/UIContainer.ts @@ -448,20 +448,7 @@ export class UIContainer extends HTMLElement { this._updateError(); this._updatePausedAndEnded(); this._updateCasting(); - this._player.addEventListener('resize', this._updateAspectRatio); - this._player.addEventListener(['error', 'sourcechange', 'emptied'], this._updateError); - this._player.addEventListener('volumechange', this._updateMuted); - this._player.addEventListener('play', this._onPlay); - this._player.addEventListener('pause', this._onPause); - this._player.addEventListener(['ended', 'emptied'], this._updatePausedAndEnded); - this._player.addEventListener(['durationchange', 'sourcechange', 'emptied'], this._updateStreamType); - this._player.addEventListener('ratechange', this._updatePlaybackRate); - this._player.addEventListener('sourcechange', this._onSourceChange); - this._player.theoLive?.addEventListener('publicationloadstart', this._onSourceChange); - this._player.videoTracks.addEventListener(['addtrack', 'removetrack', 'change'], this._updateActiveVideoTrack); - this._player.cast?.addEventListener('castingchange', this._updateCasting); - this._player.addEventListener(['durationchange', 'sourcechange', 'emptied'], this._updatePlayingAd); - this._player.ads?.addEventListener(['adbreakbegin', 'adbreakend', 'adbegin', 'adend', 'adskip'], this._updatePlayingAd); + this._addPlayerListeners(this._player); this.dispatchEvent(createCustomEvent(READY_EVENT)); } @@ -491,6 +478,7 @@ export class UIContainer extends HTMLElement { this.removeEventListener('mouseleave', this._onMouseLeave); if (this._player) { + this._removePlayerListeners(this._player); this._player.destroy(); this._player = undefined; } @@ -1065,6 +1053,49 @@ export class UIContainer extends HTMLElement { } } }; + + private _addPlayerListeners(player: ChromelessPlayer): void { + player.addEventListener('destroy', this._onDestroy); + player.addEventListener('resize', this._updateAspectRatio); + player.addEventListener(['error', 'sourcechange', 'emptied'], this._updateError); + player.addEventListener('volumechange', this._updateMuted); + player.addEventListener('play', this._onPlay); + player.addEventListener('pause', this._onPause); + player.addEventListener(['ended', 'emptied'], this._updatePausedAndEnded); + player.addEventListener(['durationchange', 'sourcechange', 'emptied'], this._updateStreamType); + player.addEventListener('ratechange', this._updatePlaybackRate); + player.addEventListener('sourcechange', this._onSourceChange); + player.theoLive?.addEventListener('publicationloadstart', this._onSourceChange); + player.videoTracks.addEventListener(['addtrack', 'removetrack', 'change'], this._updateActiveVideoTrack); + player.cast?.addEventListener('castingchange', this._updateCasting); + player.addEventListener(['durationchange', 'sourcechange', 'emptied'], this._updatePlayingAd); + player.ads?.addEventListener(['adbreakbegin', 'adbreakend', 'adbegin', 'adend', 'adskip'], this._updatePlayingAd); + } + + private _removePlayerListeners(player: ChromelessPlayer): void { + player.removeEventListener('destroy', this._onDestroy); + player.removeEventListener('resize', this._updateAspectRatio); + player.removeEventListener(['error', 'sourcechange', 'emptied'], this._updateError); + player.removeEventListener('volumechange', this._updateMuted); + player.removeEventListener('play', this._onPlay); + player.removeEventListener('pause', this._onPause); + player.removeEventListener(['ended', 'emptied'], this._updatePausedAndEnded); + player.removeEventListener(['durationchange', 'sourcechange', 'emptied'], this._updateStreamType); + player.removeEventListener('ratechange', this._updatePlaybackRate); + player.removeEventListener('sourcechange', this._onSourceChange); + player.theoLive?.removeEventListener('publicationloadstart', this._onSourceChange); + player.videoTracks.removeEventListener(['addtrack', 'removetrack', 'change'], this._updateActiveVideoTrack); + player.cast?.removeEventListener('castingchange', this._updateCasting); + player.removeEventListener(['durationchange', 'sourcechange', 'emptied'], this._updatePlayingAd); + player.ads?.removeEventListener(['adbreakbegin', 'adbreakend', 'adbegin', 'adend', 'adskip'], this._updatePlayingAd); + } + + private readonly _onDestroy = (): void => { + if (this._player) { + this._removePlayerListeners(this._player); + this._player = undefined; + } + }; } customElements.define('theoplayer-ui', UIContainer); From 0f27e7990b310c9da7f449bb8254c5d33e8f472c Mon Sep 17 00:00:00 2001 From: Mattias Buelens Date: Tue, 16 Apr 2024 16:06:03 +0200 Subject: [PATCH 02/12] Remove player from state receivers on destroy --- src/UIContainer.ts | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/UIContainer.ts b/src/UIContainer.ts index 1c7302e..8b85f06 100644 --- a/src/UIContainer.ts +++ b/src/UIContainer.ts @@ -438,17 +438,12 @@ export class UIContainer extends HTMLElement { this._player.muted = this.muted; this._player.autoplay = this.autoplay; - for (const receiver of this._stateReceivers) { - if (receiver[StateReceiverProps].indexOf('player') >= 0) { - receiver.player = this._player; - } - } - this._updateAspectRatio(); this._updateError(); this._updatePausedAndEnded(); this._updateCasting(); this._addPlayerListeners(this._player); + this.propagatePlayerToAllReceivers_(); this.dispatchEvent(createCustomEvent(READY_EVENT)); } @@ -457,10 +452,6 @@ export class UIContainer extends HTMLElement { this._resizeObserver?.disconnect(); this._mutationObserver.disconnect(); this.shadowRoot!.removeEventListener('slotchange', this._onSlotChange); - for (const receiver of this._stateReceivers) { - this.removeStateFromReceiver_(receiver); - } - this._stateReceivers.length = 0; this._menuGroup.removeEventListener(CLOSE_MENU_EVENT, this._onCloseMenu); this._menuGroup.removeEventListener(MENU_CHANGE_EVENT, this._onMenuChange); @@ -481,7 +472,10 @@ export class UIContainer extends HTMLElement { this._removePlayerListeners(this._player); this._player.destroy(); this._player = undefined; + this.propagatePlayerToAllReceivers_(); } + + this._stateReceivers.length = 0; } attributeChangedCallback(attrName: string, oldValue: any, newValue: any): void { @@ -616,6 +610,15 @@ export class UIContainer extends HTMLElement { } } + private propagatePlayerToAllReceivers_(): void { + for (const receiver of this._stateReceivers) { + const receiverProps = receiver[StateReceiverProps]; + if (receiverProps.indexOf('player') >= 0) { + receiver.player = this._player; + } + } + } + private removeStateFromReceiver_(receiver: StateReceiverElement): void { const receiverProps = receiver[StateReceiverProps]; if (receiverProps.indexOf('player') >= 0) { @@ -1094,6 +1097,7 @@ export class UIContainer extends HTMLElement { if (this._player) { this._removePlayerListeners(this._player); this._player = undefined; + this.propagatePlayerToAllReceivers_(); } }; } From 566df4a5624a7d46c129839c6bafa58daeb8f944 Mon Sep 17 00:00:00 2001 From: Mattias Buelens Date: Tue, 16 Apr 2024 16:06:49 +0200 Subject: [PATCH 03/12] Rename --- src/UIContainer.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/UIContainer.ts b/src/UIContainer.ts index 8b85f06..9e533d2 100644 --- a/src/UIContainer.ts +++ b/src/UIContainer.ts @@ -442,7 +442,7 @@ export class UIContainer extends HTMLElement { this._updateError(); this._updatePausedAndEnded(); this._updateCasting(); - this._addPlayerListeners(this._player); + this.addPlayerListeners_(this._player); this.propagatePlayerToAllReceivers_(); this.dispatchEvent(createCustomEvent(READY_EVENT)); @@ -469,7 +469,7 @@ export class UIContainer extends HTMLElement { this.removeEventListener('mouseleave', this._onMouseLeave); if (this._player) { - this._removePlayerListeners(this._player); + this.removePlayerListeners_(this._player); this._player.destroy(); this._player = undefined; this.propagatePlayerToAllReceivers_(); @@ -1057,7 +1057,7 @@ export class UIContainer extends HTMLElement { } }; - private _addPlayerListeners(player: ChromelessPlayer): void { + private addPlayerListeners_(player: ChromelessPlayer): void { player.addEventListener('destroy', this._onDestroy); player.addEventListener('resize', this._updateAspectRatio); player.addEventListener(['error', 'sourcechange', 'emptied'], this._updateError); @@ -1075,7 +1075,7 @@ export class UIContainer extends HTMLElement { player.ads?.addEventListener(['adbreakbegin', 'adbreakend', 'adbegin', 'adend', 'adskip'], this._updatePlayingAd); } - private _removePlayerListeners(player: ChromelessPlayer): void { + private removePlayerListeners_(player: ChromelessPlayer): void { player.removeEventListener('destroy', this._onDestroy); player.removeEventListener('resize', this._updateAspectRatio); player.removeEventListener(['error', 'sourcechange', 'emptied'], this._updateError); @@ -1095,7 +1095,7 @@ export class UIContainer extends HTMLElement { private readonly _onDestroy = (): void => { if (this._player) { - this._removePlayerListeners(this._player); + this.removePlayerListeners_(this._player); this._player = undefined; this.propagatePlayerToAllReceivers_(); } From 44061b71fc7c5c0dace357364e40166c64fdd046 Mon Sep 17 00:00:00 2001 From: Mattias Buelens Date: Tue, 16 Apr 2024 16:14:30 +0200 Subject: [PATCH 04/12] Ignore errors when the player is already destroyed --- src/UIContainer.ts | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/UIContainer.ts b/src/UIContainer.ts index 9e533d2..e1bb2e3 100644 --- a/src/UIContainer.ts +++ b/src/UIContainer.ts @@ -1068,10 +1068,11 @@ export class UIContainer extends HTMLElement { player.addEventListener(['durationchange', 'sourcechange', 'emptied'], this._updateStreamType); player.addEventListener('ratechange', this._updatePlaybackRate); player.addEventListener('sourcechange', this._onSourceChange); + player.addEventListener(['durationchange', 'sourcechange', 'emptied'], this._updatePlayingAd); + player.theoLive?.addEventListener('publicationloadstart', this._onSourceChange); player.videoTracks.addEventListener(['addtrack', 'removetrack', 'change'], this._updateActiveVideoTrack); player.cast?.addEventListener('castingchange', this._updateCasting); - player.addEventListener(['durationchange', 'sourcechange', 'emptied'], this._updatePlayingAd); player.ads?.addEventListener(['adbreakbegin', 'adbreakend', 'adbegin', 'adend', 'adskip'], this._updatePlayingAd); } @@ -1086,11 +1087,16 @@ export class UIContainer extends HTMLElement { player.removeEventListener(['durationchange', 'sourcechange', 'emptied'], this._updateStreamType); player.removeEventListener('ratechange', this._updatePlaybackRate); player.removeEventListener('sourcechange', this._onSourceChange); - player.theoLive?.removeEventListener('publicationloadstart', this._onSourceChange); - player.videoTracks.removeEventListener(['addtrack', 'removetrack', 'change'], this._updateActiveVideoTrack); - player.cast?.removeEventListener('castingchange', this._updateCasting); player.removeEventListener(['durationchange', 'sourcechange', 'emptied'], this._updatePlayingAd); - player.ads?.removeEventListener(['adbreakbegin', 'adbreakend', 'adbegin', 'adend', 'adskip'], this._updatePlayingAd); + + try { + player.theoLive?.removeEventListener('publicationloadstart', this._onSourceChange); + player.videoTracks.removeEventListener(['addtrack', 'removetrack', 'change'], this._updateActiveVideoTrack); + player.cast?.removeEventListener('castingchange', this._updateCasting); + player.ads?.removeEventListener(['adbreakbegin', 'adbreakend', 'adbegin', 'adend', 'adskip'], this._updatePlayingAd); + } catch { + // Ignore errors from accessing player.ads when the player is already destroyed. + } } private readonly _onDestroy = (): void => { From 173fc7df6dd06a6f6ed71c1b5840688ce98cb096 Mon Sep 17 00:00:00 2001 From: Mattias Buelens Date: Tue, 16 Apr 2024 17:10:13 +0200 Subject: [PATCH 05/12] Fix removing event listeners when destroyed --- src/components/ChromecastDisplay.ts | 12 +++++++++--- src/components/GestureReceiver.ts | 14 ++++++++------ src/components/LanguageMenu.ts | 18 +++++++++--------- src/components/LanguageMenuButton.ts | 18 +++++++++--------- src/components/PreviewThumbnail.ts | 12 +++++------- src/components/QualityRadioGroup.ts | 12 +++++------- src/components/TextTrackStyleDisplay.ts | 12 +++++------- src/components/TextTrackStyleRadioGroup.ts | 12 +++++------- src/components/TimeRange.ts | 8 +++++--- src/components/ads/AdClickThroughButton.ts | 12 +++++------- src/components/ads/AdCountdown.ts | 14 ++++++-------- src/components/ads/AdDisplay.ts | 12 +++++------- src/components/ads/AdSkipButton.ts | 14 ++++++-------- .../quality/AbstractQualitySelector.ts | 19 ++++++++++--------- .../theolive/quality/BadNetworkModeButton.ts | 16 +++++++++------- 15 files changed, 101 insertions(+), 104 deletions(-) diff --git a/src/components/ChromecastDisplay.ts b/src/components/ChromecastDisplay.ts index dc4d042..893f28c 100644 --- a/src/components/ChromecastDisplay.ts +++ b/src/components/ChromecastDisplay.ts @@ -2,7 +2,7 @@ import * as shadyCss from '@webcomponents/shadycss'; import chromecastDisplayCss from './ChromecastDisplay.css'; import chromecastIcon from '../icons/chromecast-48px.svg'; import { StateReceiverMixin } from './StateReceiverMixin'; -import type { ChromelessPlayer } from 'theoplayer/chromeless'; +import type { Chromecast, ChromelessPlayer } from 'theoplayer/chromeless'; import { setTextContent } from '../util/CommonUtils'; import { Attribute } from '../util/Attribute'; import { createTemplate } from '../util/TemplateUtils'; @@ -27,6 +27,7 @@ const CAST_EVENTS = ['statechange'] as const; export class ChromecastDisplay extends StateReceiverMixin(HTMLElement, ['player']) { private readonly _receiverNameEl: HTMLElement; private _player: ChromelessPlayer | undefined; + private _castApi: Chromecast | undefined; constructor() { super(); @@ -61,10 +62,15 @@ export class ChromecastDisplay extends StateReceiverMixin(HTMLElement, ['player' if (this._player === player) { return; } - this._player?.cast?.chromecast?.removeEventListener(CAST_EVENTS, this._updateFromPlayer); + if (this._castApi !== undefined) { + this._castApi.removeEventListener(CAST_EVENTS, this._updateFromPlayer); + } this._player = player; + this._castApi = player?.cast?.chromecast; this._updateFromPlayer(); - this._player?.cast?.chromecast?.addEventListener(CAST_EVENTS, this._updateFromPlayer); + if (this._castApi !== undefined) { + this._castApi.addEventListener(CAST_EVENTS, this._updateFromPlayer); + } } private readonly _updateFromPlayer = () => { diff --git a/src/components/GestureReceiver.ts b/src/components/GestureReceiver.ts index dfc48ef..81fbfca 100644 --- a/src/components/GestureReceiver.ts +++ b/src/components/GestureReceiver.ts @@ -16,6 +16,7 @@ const template = createTemplate('theoplayer-gesture-receiver', `