fix(settings): follow Embedded MPV support for search only while the settings page is open

The settings search service is provided in the root injector, so the
watch it started on its first use had no owner: with a login shell that
never answers it kept asking every 30 s until the app quit, long after
the settings page or the command palette that needed the answer was
closed. A failed recheck also ended the watch as if it were a final
answer, hiding the Embedded MPV rows for the rest of the session.

The service now separates the two uses. `ensureEmbeddedMpvSupportLoaded()`
is a single request again, for the command palette. The settings page
calls `followEmbeddedMpvSupport()` and ends it when the page is destroyed.
Only a final answer is kept: after an inconclusive one or a failed
request the next use asks again, for the palette's player commands too.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Fable 5.1 committed 2026-10-04 09:17:29 +02:00
1 parent 98eb267daa
commit cb8c289310
7 files changed
+178 -65

No files matched your search

@@ -66,7 +66,9 @@ export class SettingsSearchFacade {
});
constructor() {
void this.settingsSearch.ensureEmbeddedMpvSupportLoaded();
inject(DestroyRef).onDestroy(
this.settingsSearch.followEmbeddedMpvSupport()
);
effect(() => {
if (!this.isSearching()) {
@@ -95,6 +95,21 @@ describe('SettingsComponent search', () => {
expect(query('app-settings-general-section')).not.toBeNull();
});
it('follows Embedded MPV support only while the page is open', () => {
const stopFollowing = jest.fn();
const follow = jest
.spyOn(settingsSearch, 'followEmbeddedMpvSupport')
.mockReturnValue(stopFollowing);
const page = TestBed.createComponent(SettingsComponent);
expect(follow).toHaveBeenCalledTimes(1);
expect(stopFollowing).not.toHaveBeenCalled();
// Closing the page ends it: nothing shows these rows any more.
page.destroy();
expect(stopFollowing).toHaveBeenCalledTimes(1);
});
it('shows an empty state when nothing matches', () => {
setSettingsSearchQuery('zzzz-no-such-setting');
fixture.detectChanges();
+10 -7
View File
@@ -206,14 +206,17 @@ Whatever holds on to one answer follows it through `watchEmbeddedMpvSupport()`
mounted on one answer: a player mounted in that window starts playback by
itself once `mpv` is found, and the option appears without reopening the
page.
- The settings search (`SettingsSearchService`) serves both the open settings
page and the command palette, so it follows the answer as well: the Embedded
MPV rows become searchable while the page stays open, and a call made while
the answer is still inconclusive asks again at once.
- The settings page also follows the answer for its search
(`SettingsSearchService.followEmbeddedMpvSupport()`) and ends that when it
closes: the Embedded MPV rows become searchable while the page stays open,
and nothing keeps asking once no surface shows them.
The command palette's player commands ask on demand, on every palette open;
they keep a final answer for the session, but probe again on the next open
after an inconclusive one.
The command palette asks on demand, on every open, for its player commands and
its settings rows (`ensureEmbeddedMpvSupportLoaded()`). Only a final answer is
kept for the session; after an inconclusive one, or a failed request, the next
open asks again. An open palette is a snapshot of that moment: it does not wait
for a final answer, because a login shell that never answers would then keep
it from opening.
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.
@@ -247,6 +247,33 @@ describe('WorkspacePlayerCommandsContributor', () => {
expect(contributor.ensureEmbeddedMpvSupportLoaded()).toBeUndefined();
});
it('asks again after a failed request instead of giving up for the session', async () => {
const warn = jest.spyOn(console, 'warn').mockImplementation();
const contributor = bootstrap({
supportsManagedExternalPlayers: true,
supportsEmbeddedMpv: true,
});
const embedded = getRegistered(viewCommands).find(
(c) => c.id === 'switch-player-embedded-mpv'
);
electronStub?.getEmbeddedMpvSupport.mockRejectedValueOnce(
new Error('bridge failed')
);
try {
await contributor.ensureEmbeddedMpvSupportLoaded();
expect(resolveBoolean(embedded?.visible)).toBe(false);
await contributor.ensureEmbeddedMpvSupportLoaded();
expect(resolveBoolean(embedded?.visible)).toBe(true);
expect(electronStub?.getEmbeddedMpvSupport).toHaveBeenCalledTimes(
2
);
} finally {
warn.mockRestore();
}
});
it('switches to embedded MPV on run', () => {
bootstrap({
supportsManagedExternalPlayers: true,
@@ -120,9 +120,9 @@ export class WorkspacePlayerCommandsContributor {
}
private async loadEmbeddedMpvSupport(): Promise<void> {
// An inconclusive answer hides the command for now, but is not kept:
// the next palette open asks again.
let final = true;
// Only a final answer is kept. An inconclusive one, or a failed
// request, hides the command for now: the next palette open asks again.
let final = false;
try {
const support = await window.electron?.getEmbeddedMpvSupport?.();
this.embeddedMpvSupported.set(!!support?.supported);
@@ -214,24 +214,42 @@ describe('SettingsSearchService', () => {
expect(getEmbeddedMpvSupport).toHaveBeenCalledTimes(2);
});
it('asks again after a failed request instead of giving up for the session', async () => {
const getEmbeddedMpvSupport = probeWith({ supported: true });
getEmbeddedMpvSupport.mockRejectedValueOnce(
new Error('bridge failed')
);
const { service } = setup({
...DESKTOP,
supportsEmbeddedMpv: true,
});
const entries = () => service.visibleEntries().map(({ id }) => id);
await service.ensureEmbeddedMpvSupportLoaded();
expect(entries()).not.toContain('embedded-mpv-extra-options');
await service.ensureEmbeddedMpvSupportLoaded();
expect(entries()).toContain('embedded-mpv-extra-options');
expect(getEmbeddedMpvSupport).toHaveBeenCalledTimes(2);
});
describe('while the settings page stays open', () => {
const inconclusive = { supported: false, inconclusive: true };
const mountedSetup = () =>
setup({ ...DESKTOP, supportsEmbeddedMpv: true });
beforeEach(() => jest.useFakeTimers());
afterEach(() => jest.useRealTimers());
it('follows an inconclusive answer without being asked again', async () => {
const getEmbeddedMpvSupport = probeWith({
supported: false,
inconclusive: true,
});
const { service } = setup({
...DESKTOP,
supportsEmbeddedMpv: true,
});
const getEmbeddedMpvSupport = probeWith(inconclusive);
const { service } = mountedSetup();
const entries = () =>
service.visibleEntries().map(({ id }) => id);
// The page asks once, when it is created.
await service.ensureEmbeddedMpvSupportLoaded();
// The page starts following once, when it is created.
service.followEmbeddedMpvSupport();
await jest.advanceTimersByTimeAsync(0);
expect(entries()).not.toContain('embedded-mpv-extra-options');
getEmbeddedMpvSupport.mockResolvedValue({
@@ -252,18 +270,54 @@ describe('SettingsSearchService', () => {
expect(getEmbeddedMpvSupport).toHaveBeenCalledTimes(2);
});
it('stops following once the service is destroyed', async () => {
const getEmbeddedMpvSupport = probeWith({
supported: false,
inconclusive: true,
});
const { service } = setup({
...DESKTOP,
supportsEmbeddedMpv: true,
it('stops asking once the page is closed, and asks again on the next use', async () => {
const getEmbeddedMpvSupport = probeWith(inconclusive);
const { service } = mountedSetup();
const stopFollowing = service.followEmbeddedMpvSupport();
await jest.advanceTimersByTimeAsync(0);
// Nothing shows these rows any more: no polling is left.
stopFollowing();
await jest.advanceTimersByTimeAsync(
EMBEDDED_MPV_SUPPORT_RECHECK_MS * 20
);
expect(getEmbeddedMpvSupport).toHaveBeenCalledTimes(1);
await service.ensureEmbeddedMpvSupportLoaded();
expect(getEmbeddedMpvSupport).toHaveBeenCalledTimes(2);
});
it('ends on a failed recheck, and the next use asks again', async () => {
const getEmbeddedMpvSupport = probeWith(inconclusive);
const { service } = mountedSetup();
service.followEmbeddedMpvSupport();
await jest.advanceTimersByTimeAsync(0);
getEmbeddedMpvSupport.mockRejectedValueOnce(
new Error('bridge failed')
);
await jest.advanceTimersByTimeAsync(
EMBEDDED_MPV_SUPPORT_RECHECK_MS * 20
);
expect(getEmbeddedMpvSupport).toHaveBeenCalledTimes(2);
getEmbeddedMpvSupport.mockResolvedValue({
platform: 'linux',
supported: true,
});
await service.ensureEmbeddedMpvSupportLoaded();
expect(service.visibleEntries().map(({ id }) => id)).toContain(
'embedded-mpv-extra-options'
);
expect(getEmbeddedMpvSupport).toHaveBeenCalledTimes(3);
});
TestBed.resetTestingModule();
it('has nothing to follow once a final answer is in hand', async () => {
const getEmbeddedMpvSupport = probeWith({ supported: true });
const { service } = mountedSetup();
await service.ensureEmbeddedMpvSupportLoaded();
service.followEmbeddedMpvSupport();
await jest.advanceTimersByTimeAsync(
EMBEDDED_MPV_SUPPORT_RECHECK_MS * 20
);
@@ -1,4 +1,4 @@
import { DestroyRef, inject, Injectable, signal } from '@angular/core';
import { inject, Injectable, signal } from '@angular/core';
import { Router } from '@angular/router';
import { TranslateService } from '@ngx-translate/core';
import { VodSourceDiscoveryService } from '@iptvnator/portal/shared/data-access';
@@ -42,13 +42,6 @@ export class SettingsSearchService {
);
private embeddedMpvSupportChecked = false;
private embeddedMpvSupportLoad: Promise<void> | undefined;
private stopEmbeddedMpvSupportWatch: (() => void) | undefined;
constructor() {
inject(DestroyRef).onDestroy(() =>
this.stopEmbeddedMpvSupportWatch?.()
);
}
/** Row the settings page should scroll to and highlight next. */
readonly pendingReveal = this.revealRequest.asReadonly();
@@ -74,13 +67,49 @@ export class SettingsSearchService {
/**
* Probes embedded MPV support so rows that need it become searchable.
* Returns the pending probe, or `undefined` when there is nothing to
* wait for. Call it lazily (palette open, settings page), never from
* shell bootstrap: supported desktop builds may load the native addon
* while answering. A final answer is kept. An inconclusive one keeps
* being followed, so an open settings page updates by itself, and the
* next call asks again at once.
* wait for. Call it lazily (palette open), never from shell bootstrap:
* supported desktop builds may load the native addon while answering.
* Only a final answer is kept; after an inconclusive one, or a failed
* request, the next call asks again.
*/
ensureEmbeddedMpvSupportLoaded(): Promise<void> | undefined {
const getSupport = this.embeddedMpvSupportProbe();
if (!getSupport) {
return undefined;
}
this.embeddedMpvSupportLoad ??= getSupport()
.then((support) => this.takeEmbeddedMpvSupport(support))
.catch(() => this.takeEmbeddedMpvSupport(null))
.finally(() => {
this.embeddedMpvSupportLoad = undefined;
});
return this.embeddedMpvSupportLoad;
}
/**
* For a surface that keeps showing these rows (the settings page): probes
* like `ensureEmbeddedMpvSupportLoaded()` and follows an inconclusive
* answer until it is final, so the rows appear by themselves. Returns
* the function that ends it; call it when the surface goes away, so
* nothing keeps asking for an answer no one shows.
*/
followEmbeddedMpvSupport(): () => void {
const getSupport = this.embeddedMpvSupportProbe();
if (!getSupport) {
return () => undefined;
}
return watchEmbeddedMpvSupport(
getSupport,
(support) => this.takeEmbeddedMpvSupport(support),
() => this.takeEmbeddedMpvSupport(null)
);
}
/** The support request, or `undefined` when there is nothing to ask. */
private embeddedMpvSupportProbe():
(() => Promise<EmbeddedMpvSupport>) | undefined {
if (this.embeddedMpvSupportChecked) {
return undefined;
}
@@ -95,30 +124,13 @@ export class SettingsSearchService {
return undefined;
}
this.embeddedMpvSupportLoad ??= this.followEmbeddedMpvSupport(() =>
electron.getEmbeddedMpvSupport()
);
return this.embeddedMpvSupportLoad;
return () => electron.getEmbeddedMpvSupport();
}
/** Follows the answer afresh; resolves with its first one. */
private followEmbeddedMpvSupport(
getSupport: () => Promise<EmbeddedMpvSupport>
): Promise<void> {
this.stopEmbeddedMpvSupportWatch?.();
return new Promise<void>((answered) => {
const take = (support: EmbeddedMpvSupport | null) => {
this.embeddedMpvSupport.set(support);
this.embeddedMpvSupportChecked = !support?.inconclusive;
this.embeddedMpvSupportLoad = undefined;
answered();
};
this.stopEmbeddedMpvSupportWatch = watchEmbeddedMpvSupport(
getSupport,
take,
() => take(null)
);
});
private takeEmbeddedMpvSupport(support: EmbeddedMpvSupport | null): void {
this.embeddedMpvSupport.set(support);
this.embeddedMpvSupportChecked =
support !== null && !support.inconclusive;
}
/**