From 6b3a271c939056a3a5e7503a35c2ecf4f1c7b8cc Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 22 Jan 2020 06:33:58 +0100 Subject: [PATCH] fix(youtube-player): memory leak if player is destroyed before it is done initializing (#18046) The way the YouTube player is set up is that a player object is only assigned once it is done initializing (its `onReady` event has fired) and the cleanup logic only applies to the assigned player. The problem is that if the player is swapped out, its initialization won't finish and we'll end up leaking it since it still has some pending listeners that are referring to the `YouTubePlayer` component. These changes add an extra callback that is invoked when loading was interrupted and which destroys the in-progress player. (cherry picked from commit 2c8dca897bdb4ebc75a9522feee04696248fb3b9) --- src/youtube-player/youtube-player.ts | 62 +++++++++++++++++----------- 1 file changed, 38 insertions(+), 24 deletions(-) diff --git a/src/youtube-player/youtube-player.ts b/src/youtube-player/youtube-player.ts index 3f20bd7058f8..071a372e4492 100644 --- a/src/youtube-player/youtube-player.ts +++ b/src/youtube-player/youtube-player.ts @@ -210,7 +210,12 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit { this._height, this.createEventsBoundInZone(), this._ngZone - ).pipe(waitUntilReady(), takeUntil(this._destroyed), publish()); + ).pipe(waitUntilReady(player => { + // Destroy the player if loading was aborted so that we don't end up leaking memory. + if (!playerIsReady(player)) { + player.destroy(); + } + }), takeUntil(this._destroyed), publish()); // Set up side effects to bind inputs to the player. playerObs.subscribe(player => this._player = player); @@ -427,39 +432,43 @@ function bindSuggestedQualityToPlayer( /** * Returns an observable that emits the loaded player once it's ready. Certain properties/methods * won't be available until the iframe finishes loading. + * @param onAbort Callback function that will be invoked if the player loading was aborted before + * it was able to complete. Can be used to clean up any loose references. */ -function waitUntilReady(): OperatorFunction { +function waitUntilReady(onAbort: (player: UninitializedPlayer) => void): + OperatorFunction { return flatMap(player => { if (!player) { return observableOf(undefined); } - if ('getPlayerStatus' in player) { + if (playerIsReady(player)) { return observableOf(player as Player); } + + // Since removeEventListener is not on Player when it's initialized, we can't use fromEvent. // The player is not initialized fully until the ready is called. - return fromPlayerOnReady(player) - .pipe(take(1), startWith(undefined)); - }); -} + return new Observable(emitter => { + let aborted = false; + let resolved = false; + const onReady = (event: YT.PlayerEvent) => { + resolved = true; + + if (!aborted) { + event.target.removeEventListener('onReady', onReady); + emitter.next(event.target); + } + }; -/** Since removeEventListener is not on Player when it's initialized, we can't use fromEvent. */ -function fromPlayerOnReady(player: UninitializedPlayer): Observable { - return new Observable(emitter => { - let aborted = false; + player.addEventListener('onReady', onReady); - const onReady = (event: YT.PlayerEvent) => { - if (aborted) { - return; - } - event.target.removeEventListener('onReady', onReady); - emitter.next(event.target); - }; - - player.addEventListener('onReady', onReady); + return () => { + aborted = true; - return () => { - aborted = true; - }; + if (!resolved) { + onAbort(player); + } + }; + }).pipe(take(1), startWith(undefined)); }); } @@ -573,7 +582,12 @@ function bindCueVideoCall( } function hasPlayerStarted(player: YT.Player): boolean { - return [YT.PlayerState.UNSTARTED, YT.PlayerState.CUED].indexOf(player.getPlayerState()) === -1; + const state = player.getPlayerState(); + return state !== YT.PlayerState.UNSTARTED && state !== YT.PlayerState.CUED; +} + +function playerIsReady(player: UninitializedPlayer): player is Player { + return 'getPlayerStatus' in player; } /** Combines the two observables temporarily for the filter function. */