diff --git a/docs/architecture/embedded-mpv-native.md b/docs/architecture/embedded-mpv-native.md index 08ba44f80..e2af29d4e 100644 --- a/docs/architecture/embedded-mpv-native.md +++ b/docs/architecture/embedded-mpv-native.md @@ -79,7 +79,7 @@ The renderer never gets direct native-module access. It can only call the preloa - dispose session - subscribe to session updates -Settings uses the preload support API only as a lightweight availability check. That check verifies platform, experiment gating, addon presence, and bundled platform runtime presence without `require()`-loading `embedded_mpv.node`. This avoids blocking Settings navigation on synchronous native-addon loading and code-signing or dynamic-linker work. +Settings uses the preload support API as an availability and capability check. Unsupported paths return before loading the addon when platform, experiment gating, addon presence, bundled runtime presence, or the Linux `mpv` executable check fails. Supported paths load `embedded_mpv.node` so the renderer can receive capability flags from the actual addon binary. Avoid calling this support API from global workspace startup paths; use an explicit user action or idle preparation path when a renderer surface only needs to reveal optional Embedded MPV UI. When `embedded-mpv` is the saved player, the settings store schedules an idle `prepareEmbeddedMpv()` call. This intentionally moves the first native addon load away from the click-to-play path. It can still block the Electron main process briefly because Node native addon loading is synchronous, but doing it during idle is less visible than doing it when the user clicks a video. Actual MPV session creation still happens on playback because it needs the current Electron window handle and viewport bounds. diff --git a/docs/architecture/workspace-shell.md b/docs/architecture/workspace-shell.md index 20d053125..f5a7d14a8 100644 --- a/docs/architecture/workspace-shell.md +++ b/docs/architecture/workspace-shell.md @@ -189,11 +189,14 @@ Command palette behavior is shell-owned but view-extensible: MPV, MPV, VLC). Each command carries a `requires` flag gating its visibility: the MPV/VLC ("managed-external") entries are visible only when `RuntimeCapabilitiesService.supportsManagedExternalPlayers` is true, and the - Embedded MPV ("embedded-mpv") entry is visible only after an async - `window.electron.getEmbeddedMpvSupport()` check resolves to `supported` - (mirroring the Settings dropdown gate). The entry matching the current - `SettingsStore.player()` value is disabled. The new player setting applies - to the next playback session; an existing stream is not re-mounted. + Embedded MPV ("embedded-mpv") entry is visible only after the command + palette lazily preloads an async `window.electron.getEmbeddedMpvSupport()` + check and it resolves to `supported` (mirroring the Settings dropdown gate). + Do not run this Embedded MPV support check from workspace shell bootstrap: + supported desktop builds may load the native addon while resolving + capabilities. The entry matching the current `SettingsStore.player()` value + is disabled. The new player setting applies to the next playback session; an + existing stream is not re-mounted. Keyboard shortcut help is shell-owned: diff --git a/libs/workspace/shell/feature/src/lib/workspace-player-commands/workspace-player-commands.contributor.spec.ts b/libs/workspace/shell/feature/src/lib/workspace-player-commands/workspace-player-commands.contributor.spec.ts index 1da95ea1f..e3ea56261 100644 --- a/libs/workspace/shell/feature/src/lib/workspace-player-commands/workspace-player-commands.contributor.spec.ts +++ b/libs/workspace/shell/feature/src/lib/workspace-player-commands/workspace-player-commands.contributor.spec.ts @@ -50,6 +50,14 @@ describe('WorkspacePlayerCommandsContributor', () => { supportsManagedExternalPlayers: boolean; supportsEmbeddedMpv: boolean; }; + let electronStub: + | { + getEmbeddedMpvSupport: jest.Mock< + Promise<{ supported: boolean }>, + [] + >; + } + | undefined; let translate: { instant: jest.Mock; onLangChange: ReturnType }; function bootstrap(options: { @@ -72,7 +80,7 @@ describe('WorkspacePlayerCommandsContributor', () => { supportsEmbeddedMpv: options.supportsEmbeddedMpv ?? false, }; - const electronStub = runtime.supportsEmbeddedMpv + electronStub = runtime.supportsEmbeddedMpv ? { getEmbeddedMpvSupport: jest.fn().mockResolvedValue( options.embeddedMpvSupportResult ?? { @@ -153,15 +161,23 @@ describe('WorkspacePlayerCommandsContributor', () => { expect(resolveBoolean(embedded?.visible)).toBe(false); }); - it('shows embedded MPV once support resolves to supported', async () => { + it('does not verify embedded MPV support during contributor bootstrap', () => { bootstrap({ supportsManagedExternalPlayers: true, supportsEmbeddedMpv: true, + }); + + expect(electronStub?.getEmbeddedMpvSupport).not.toHaveBeenCalled(); + }); + + it('shows embedded MPV once support resolves to supported', async () => { + const contributor = bootstrap({ + supportsManagedExternalPlayers: true, + supportsEmbeddedMpv: true, embeddedMpvSupportResult: { supported: true }, }); - await Promise.resolve(); - await Promise.resolve(); + await contributor.ensureEmbeddedMpvSupportLoaded(); const embedded = getRegistered(viewCommands).find( (c) => c.id === 'switch-player-embedded-mpv' @@ -170,14 +186,13 @@ describe('WorkspacePlayerCommandsContributor', () => { }); it('keeps embedded MPV hidden when support resolves to unsupported', async () => { - bootstrap({ + const contributor = bootstrap({ supportsManagedExternalPlayers: true, supportsEmbeddedMpv: true, embeddedMpvSupportResult: { supported: false }, }); - await Promise.resolve(); - await Promise.resolve(); + await contributor.ensureEmbeddedMpvSupportLoaded(); const embedded = getRegistered(viewCommands).find( (c) => c.id === 'switch-player-embedded-mpv' diff --git a/libs/workspace/shell/feature/src/lib/workspace-player-commands/workspace-player-commands.contributor.ts b/libs/workspace/shell/feature/src/lib/workspace-player-commands/workspace-player-commands.contributor.ts index 17dbfb7cd..4d0893e39 100644 --- a/libs/workspace/shell/feature/src/lib/workspace-player-commands/workspace-player-commands.contributor.ts +++ b/libs/workspace/shell/feature/src/lib/workspace-player-commands/workspace-player-commands.contributor.ts @@ -86,14 +86,14 @@ export class WorkspacePlayerCommandsContributor { private readonly destroyRef = inject(DestroyRef); private readonly runtime = inject(RuntimeCapabilitiesService); private readonly embeddedMpvSupported = signal(false); + private embeddedMpvSupportChecked = false; + private embeddedMpvSupportLoad: Promise | null = null; constructor() { const unregisters = PLAYER_COMMAND_DEFS.map((def) => this.viewCommands.registerCommand(this.toContribution(def)) ); - void this.loadEmbeddedMpvSupport(); - this.destroyRef.onDestroy(() => { for (const unregister of unregisters) { unregister(); @@ -101,23 +101,36 @@ export class WorkspacePlayerCommandsContributor { }); } - private async loadEmbeddedMpvSupport(): Promise { + ensureEmbeddedMpvSupportLoaded(): Promise | undefined { + if (this.embeddedMpvSupportChecked) { + return undefined; + } + if ( !this.runtime.supportsEmbeddedMpv || typeof window === 'undefined' || !window.electron?.getEmbeddedMpvSupport ) { - return; + this.embeddedMpvSupportChecked = true; + return undefined; } + this.embeddedMpvSupportLoad ??= this.loadEmbeddedMpvSupport(); + return this.embeddedMpvSupportLoad; + } + + private async loadEmbeddedMpvSupport(): Promise { try { - const support = await window.electron.getEmbeddedMpvSupport(); + const support = await window.electron?.getEmbeddedMpvSupport?.(); this.embeddedMpvSupported.set(!!support?.supported); } catch (error) { console.warn( 'Failed to verify embedded MPV support for the command palette.', error ); + } finally { + this.embeddedMpvSupportChecked = true; + this.embeddedMpvSupportLoad = null; } } diff --git a/libs/workspace/shell/feature/src/lib/workspace-shell/services/workspace-shell-command-palette.service.ts b/libs/workspace/shell/feature/src/lib/workspace-shell/services/workspace-shell-command-palette.service.ts index 421cf79ae..8e4bd4b48 100644 --- a/libs/workspace/shell/feature/src/lib/workspace-shell/services/workspace-shell-command-palette.service.ts +++ b/libs/workspace/shell/feature/src/lib/workspace-shell/services/workspace-shell-command-palette.service.ts @@ -20,8 +20,7 @@ export class WorkspaceShellCommandPaletteService { private readonly viewCommands = inject(WorkspaceViewCommandService); private readonly recentCommands = inject(RecentCommandsService); private readonly destroyRef = inject(DestroyRef); - // Eager construction registers the player-switch commands via WorkspaceViewCommandService. - private readonly _playerCommandsBootstrap = inject( + private readonly playerCommands = inject( WorkspacePlayerCommandsContributor ); @@ -29,6 +28,7 @@ export class WorkspaceShellCommandPaletteService { WorkspaceCommandPaletteComponent, WorkspaceCommandSelection | undefined > | null = null; + private commandPaletteOpening = false; buildPaletteCommands( ctx: CommandBuilderContext @@ -42,8 +42,36 @@ export class WorkspaceShellCommandPaletteService { return; } + if (this.commandPaletteOpening) { + return; + } + + const embeddedMpvSupportLoad = + this.playerCommands.ensureEmbeddedMpvSupportLoaded(); + if (embeddedMpvSupportLoad) { + this.commandPaletteOpening = true; + void embeddedMpvSupportLoad.finally(() => { + this.commandPaletteOpening = false; + this.openResolvedCommandPalette(ctx, initialQuery); + }); + return; + } + + this.openResolvedCommandPalette(ctx, initialQuery); + } + + private openResolvedCommandPalette( + ctx: CommandBuilderContext, + initialQuery: string + ): void { + if (this.commandPaletteRef) { + return; + } + const commands = this.buildPaletteCommands(ctx); - const recentIds = this.recentCommands.entries().map((entry) => entry.id); + const recentIds = this.recentCommands + .entries() + .map((entry) => entry.id); const dialogRef = this.dialog.open< WorkspaceCommandPaletteComponent, { diff --git a/libs/workspace/shell/feature/src/lib/workspace-shell/services/workspace-shell.facade.spec.ts b/libs/workspace/shell/feature/src/lib/workspace-shell/services/workspace-shell.facade.spec.ts index 4148ad06d..32aad8089 100644 --- a/libs/workspace/shell/feature/src/lib/workspace-shell/services/workspace-shell.facade.spec.ts +++ b/libs/workspace/shell/feature/src/lib/workspace-shell/services/workspace-shell.facade.spec.ts @@ -111,6 +111,9 @@ describe('WorkspaceShellFacade', () => { record: jest.Mock; prune: jest.Mock; }; + let playerCommands: { + ensureEmbeddedMpvSupportLoaded: jest.Mock; + }; let router: { url: string; events: ReturnType; @@ -218,6 +221,9 @@ describe('WorkspaceShellFacade', () => { record: jest.fn(), prune: jest.fn(), }; + playerCommands = { + ensureEmbeddedMpvSupportLoaded: jest.fn(), + }; const selectSignal = jest.fn().mockReturnValue(playlistsSignal); @@ -349,7 +355,7 @@ describe('WorkspaceShellFacade', () => { }, { provide: WorkspacePlayerCommandsContributor, - useValue: {}, + useValue: playerCommands, }, ], }); @@ -880,6 +886,27 @@ describe('WorkspaceShellFacade', () => { expect(recentCommands.record).toHaveBeenCalledWith('open-settings'); }); + it('waits for embedded MPV support preload before opening the palette', async () => { + const dialog = TestBed.inject(MatDialog) as unknown as { + open: jest.Mock; + }; + let resolveSupport!: () => void; + playerCommands.ensureEmbeddedMpvSupportLoaded.mockReturnValueOnce( + new Promise((resolve) => { + resolveSupport = resolve; + }) + ); + + facade.openCommandPalette(); + + expect(dialog.open).not.toHaveBeenCalled(); + + resolveSupport(); + await Promise.resolve(); + + expect(dialog.open).toHaveBeenCalledTimes(1); + }); + it('does not record when the palette closes without a selection', () => { const dialog = TestBed.inject(MatDialog) as unknown as { open: jest.Mock;