From 50124981a2d6942fecc9f50176ae9665e3487dbb Mon Sep 17 00:00:00 2001 From: absidue <48293849+absidue@users.noreply.github.com> Date: Mon, 16 Sep 2024 00:17:25 +0200 Subject: [PATCH] Fix a few memory leaks while tearing down the player (#5698) * Fix a few memory leaks while tearing down the player * Fix `ui` properties not getting cleared --- .../ft-shaka-video-player.js | 87 +++++++++++++++---- src/renderer/views/Watch/Watch.js | 14 ++- 2 files changed, 81 insertions(+), 20 deletions(-) diff --git a/src/renderer/components/ft-shaka-video-player/ft-shaka-video-player.js b/src/renderer/components/ft-shaka-video-player/ft-shaka-video-player.js index b075405aa0d2..8b49954cc0ca 100644 --- a/src/renderer/components/ft-shaka-video-player/ft-shaka-video-player.js +++ b/src/renderer/components/ft-shaka-video-player/ft-shaka-video-player.js @@ -1588,6 +1588,8 @@ export default defineComponent({ // #region custom player controls + const { ContextMenu: shakaContextMenu, Controls: shakaControls, OverflowMenu: shakaOverflowMenu } = shaka.ui + function registerAudioTrackSelection() { /** @implements {shaka.extern.IUIElement.Factory} */ class AudioTrackSelectionFactory { @@ -1596,8 +1598,8 @@ export default defineComponent({ } } - shaka.ui.Controls.registerElement('ft_audio_tracks', new AudioTrackSelectionFactory()) - shaka.ui.OverflowMenu.registerElement('ft_audio_tracks', new AudioTrackSelectionFactory()) + shakaControls.registerElement('ft_audio_tracks', new AudioTrackSelectionFactory()) + shakaOverflowMenu.registerElement('ft_audio_tracks', new AudioTrackSelectionFactory()) } function registerTheatreModeButton() { @@ -1614,8 +1616,8 @@ export default defineComponent({ } } - shaka.ui.Controls.registerElement('ft_theatre_mode', new TheatreModeButtonFactory()) - shaka.ui.OverflowMenu.registerElement('ft_theatre_mode', new TheatreModeButtonFactory()) + shakaControls.registerElement('ft_theatre_mode', new TheatreModeButtonFactory()) + shakaOverflowMenu.registerElement('ft_theatre_mode', new TheatreModeButtonFactory()) } function registerFullWindowButton() { @@ -1642,8 +1644,8 @@ export default defineComponent({ } } - shaka.ui.Controls.registerElement('ft_full_window', new FullWindowButtonFactory()) - shaka.ui.OverflowMenu.registerElement('ft_full_window', new FullWindowButtonFactory()) + shakaControls.registerElement('ft_full_window', new FullWindowButtonFactory()) + shakaOverflowMenu.registerElement('ft_full_window', new FullWindowButtonFactory()) } function registerLegacyQualitySelection() { @@ -1677,8 +1679,8 @@ export default defineComponent({ } } - shaka.ui.Controls.registerElement('ft_legacy_quality', new LegacyQualitySelectionFactory()) - shaka.ui.OverflowMenu.registerElement('ft_legacy_quality', new LegacyQualitySelectionFactory()) + shakaControls.registerElement('ft_legacy_quality', new LegacyQualitySelectionFactory()) + shakaOverflowMenu.registerElement('ft_legacy_quality', new LegacyQualitySelectionFactory()) } function registerStatsButton() { @@ -1699,7 +1701,7 @@ export default defineComponent({ } } - shaka.ui.ContextMenu.registerElement('ft_stats', new StatsButtonFactory()) + shakaContextMenu.registerElement('ft_stats', new StatsButtonFactory()) } function registerScreenshotButton() { @@ -1716,8 +1718,34 @@ export default defineComponent({ } } - shaka.ui.Controls.registerElement('ft_screenshot', new ScreenshotButtonFactory()) - shaka.ui.OverflowMenu.registerElement('ft_screenshot', new ScreenshotButtonFactory()) + shakaControls.registerElement('ft_screenshot', new ScreenshotButtonFactory()) + shakaOverflowMenu.registerElement('ft_screenshot', new ScreenshotButtonFactory()) + } + + /** + * As shaka-player doesn't let you unregister custom control factories, + * overwrite them with `null` instead so the referenced objects + * (e.g. {@linkcode events}, {@linkcode fullWindowEnabled}) can get gargabe collected + */ + function cleanUpCustomPlayerControls() { + shakaControls.registerElement('ft_audio_tracks', null) + shakaOverflowMenu.registerElement('ft_audio_tracks', null) + + shakaControls.registerElement('ft_theatre_mode', null) + shakaOverflowMenu.registerElement('ft_theatre_mode', null) + + shakaControls.registerElement('ft_full_window', null) + shakaOverflowMenu.registerElement('ft_full_window', null) + + shakaControls.registerElement('ft_legacy_quality', null) + shakaOverflowMenu.registerElement('ft_legacy_quality', null) + + shakaContextMenu.registerElement('ft_stats', null) + + if (process.env.IS_ELECTRON) { + shakaControls.registerElement('ft_screenshot', null) + shakaOverflowMenu.registerElement('ft_screenshot', null) + } } // #endregion custom player controls @@ -2636,12 +2664,7 @@ export default defineComponent({ resizeObserver = null } - if (ui) { - // destroying the ui also destroys the player - ui.destroy() - ui = null - player = null - } + cleanUpCustomPlayerControls() stopPowerSaveBlocker() window.removeEventListener('beforeunload', stopPowerSaveBlocker) @@ -2679,13 +2702,41 @@ export default defineComponent({ video.value.currentTime = time } + /** + * Vue's lifecycle hooks are synchonous, so if we destroy the player in {@linkcode onBeforeUnmount}, + * it won't be finished in time, as the player destruction is asynchronous. + * To workaround that we destroy the player first and wait for it to finish before we unmount this component. + */ + async function destroyPlayer() { + if (ui) { + // destroying the ui also destroys the player + await ui.destroy() + ui = null + player = null + } else if (player) { + await player.destroy() + player = null + } + + // shaka-player doesn't clear these itself, which prevents shaka.ui.Overlay from being garbage collected + // Should really be fixed in shaka-player but it's easier just to do it ourselves + if (container.value) { + container.value.ui = null + } + + if (video.value) { + video.value.ui = null + } + } + expose({ hasLoaded, isPaused, pause, getCurrentTime, - setCurrentTime + setCurrentTime, + destroyPlayer }) // #endregion functions used by the watch page diff --git a/src/renderer/views/Watch/Watch.js b/src/renderer/views/Watch/Watch.js index 30e87e82610c..fd8d6be8e6c5 100644 --- a/src/renderer/views/Watch/Watch.js +++ b/src/renderer/views/Watch/Watch.js @@ -54,9 +54,14 @@ export default defineComponent({ 'watch-video-recommendations': WatchVideoRecommendations, 'ft-age-restricted': FtAgeRestricted }, - beforeRouteLeave: function (to, from, next) { + beforeRouteLeave: async function (to, from, next) { this.handleRouteChange() window.removeEventListener('beforeunload', this.handleWatchProgress) + + if (this.$refs.player) { + await this.$refs.player.destroyPlayer() + } + next() }, data: function () { @@ -229,8 +234,13 @@ export default defineComponent({ }, }, watch: { - $route() { + async $route() { this.handleRouteChange() + + if (this.$refs.player) { + await this.$refs.player.destroyPlayer() + } + // react to route changes... this.videoId = this.$route.params.id