Skip to content

Commit 6b3a271

Browse files
crisbetojelbourn
authored andcommitted
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 2c8dca8)
1 parent 684ea65 commit 6b3a271

File tree

1 file changed

+38
-24
lines changed

1 file changed

+38
-24
lines changed

src/youtube-player/youtube-player.ts

Lines changed: 38 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,12 @@ export class YouTubePlayer implements AfterViewInit, OnDestroy, OnInit {
210210
this._height,
211211
this.createEventsBoundInZone(),
212212
this._ngZone
213-
).pipe(waitUntilReady(), takeUntil(this._destroyed), publish());
213+
).pipe(waitUntilReady(player => {
214+
// Destroy the player if loading was aborted so that we don't end up leaking memory.
215+
if (!playerIsReady(player)) {
216+
player.destroy();
217+
}
218+
}), takeUntil(this._destroyed), publish());
214219

215220
// Set up side effects to bind inputs to the player.
216221
playerObs.subscribe(player => this._player = player);
@@ -427,39 +432,43 @@ function bindSuggestedQualityToPlayer(
427432
/**
428433
* Returns an observable that emits the loaded player once it's ready. Certain properties/methods
429434
* won't be available until the iframe finishes loading.
435+
* @param onAbort Callback function that will be invoked if the player loading was aborted before
436+
* it was able to complete. Can be used to clean up any loose references.
430437
*/
431-
function waitUntilReady(): OperatorFunction<UninitializedPlayer | undefined, Player | undefined> {
438+
function waitUntilReady(onAbort: (player: UninitializedPlayer) => void):
439+
OperatorFunction<UninitializedPlayer | undefined, Player | undefined> {
432440
return flatMap(player => {
433441
if (!player) {
434442
return observableOf<Player|undefined>(undefined);
435443
}
436-
if ('getPlayerStatus' in player) {
444+
if (playerIsReady(player)) {
437445
return observableOf(player as Player);
438446
}
447+
448+
// Since removeEventListener is not on Player when it's initialized, we can't use fromEvent.
439449
// The player is not initialized fully until the ready is called.
440-
return fromPlayerOnReady(player)
441-
.pipe(take(1), startWith(undefined));
442-
});
443-
}
450+
return new Observable<Player>(emitter => {
451+
let aborted = false;
452+
let resolved = false;
453+
const onReady = (event: YT.PlayerEvent) => {
454+
resolved = true;
455+
456+
if (!aborted) {
457+
event.target.removeEventListener('onReady', onReady);
458+
emitter.next(event.target);
459+
}
460+
};
444461

445-
/** Since removeEventListener is not on Player when it's initialized, we can't use fromEvent. */
446-
function fromPlayerOnReady(player: UninitializedPlayer): Observable<Player> {
447-
return new Observable<Player>(emitter => {
448-
let aborted = false;
462+
player.addEventListener('onReady', onReady);
449463

450-
const onReady = (event: YT.PlayerEvent) => {
451-
if (aborted) {
452-
return;
453-
}
454-
event.target.removeEventListener('onReady', onReady);
455-
emitter.next(event.target);
456-
};
457-
458-
player.addEventListener('onReady', onReady);
464+
return () => {
465+
aborted = true;
459466

460-
return () => {
461-
aborted = true;
462-
};
467+
if (!resolved) {
468+
onAbort(player);
469+
}
470+
};
471+
}).pipe(take(1), startWith(undefined));
463472
});
464473
}
465474

@@ -573,7 +582,12 @@ function bindCueVideoCall(
573582
}
574583

575584
function hasPlayerStarted(player: YT.Player): boolean {
576-
return [YT.PlayerState.UNSTARTED, YT.PlayerState.CUED].indexOf(player.getPlayerState()) === -1;
585+
const state = player.getPlayerState();
586+
return state !== YT.PlayerState.UNSTARTED && state !== YT.PlayerState.CUED;
587+
}
588+
589+
function playerIsReady(player: UninitializedPlayer): player is Player {
590+
return 'getPlayerStatus' in player;
577591
}
578592

579593
/** Combines the two observables temporarily for the filter function. */

0 commit comments

Comments
 (0)