Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/renderer/services/dialog.service-shard.float.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ vi.mock('@renderer/services/web-view.service-shard', () => ({
// about the float layout, not either of those. See `dialog.service-shard.layout-load.test.ts`.
throwIfWindowIsClosing: vi.fn(),
waitForLayoutLoadToSettle: vi.fn(async () => {}),
// No layout load happens in this file; the subscription just has to exist to start the shard.
onLayoutLoadTabIds: vi.fn(() => () => {}),
}));

vi.mock('@shared/services/localization.service', () => ({
Expand Down Expand Up @@ -161,6 +163,8 @@ describe('dialog.service-shard float layout', () => {
expect(mockAddTab).toHaveBeenCalledWith(
expect.objectContaining({ tabType: 'platform.alert' }),
{ type: 'float', position: 'center' },
true,
expect.any(Function),
);

resolveDialogRequest('mock-guid', undefined);
Expand Down
57 changes: 57 additions & 0 deletions src/renderer/services/dialog.service-shard.layout-load.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -461,3 +461,60 @@ describe('a dialog arriving while this window has an emptiness report in flight'
releaseEmptiedReport({ action: 'stay' });
});
});

describe('a docked dialog dropped by a whole-layout load', () => {
test('a layout load settles a dialog request whose tab the new layout drops', async () => {
// A Simple/Power mode switch replaces the whole dock via `loadLayout`, which never runs
// rc-dock's per-tab remove callback — the same mechanism `dialog.service-shard.layout-load.test.ts`'s
// other describes exercise for a load still in flight. Here the load actually lands, and the
// dialog's tab is not part of what it lands on, so nothing but the requestor would ever learn
// its request can no longer be answered.
let interfaceModeCallback: ((newMode: unknown) => Promise<void>) | undefined;
mocks.settingsSubscribe.mockImplementation(
async (_key: string, callback: (newMode: unknown) => Promise<void>) => {
interfaceModeCallback = callback;
return async () => true;
},
);
let interfaceMode = 'power';
mocks.settingsGet.mockImplementation(async (key: string) =>
key === 'platform.interfaceMode' ? interfaceMode : false,
);
mocks.networkRequest.mockImplementation(async (requestType: string) => {
if (requestType === 'windowLayout:emptied') return { action: 'stay' };
if (requestType === 'windowLayout:get') return { kind: 'empty' };
return undefined;
});

const webViewModule = await import('@renderer/services/web-view.service-shard');
const { dockLayout, loadedLayouts, dockedTabs } = makeLiveDockLayout();
webViewModule.registerDockLayout(dockLayout);
await webViewModule.startWebViewServiceShard();
await primeProvider();
await vi.waitFor(() => expect(loadedLayouts.length).toBe(1));

const dialogModule = await import('./dialog.service-shard');
await dialogModule.startDialogServiceShard();
const dialogShard = findPublishedShard<DialogShard>('showDialog');

const showing = dialogShard.showDialog(ALERT_DIALOG_TYPE, { prompt: 'Alert message' });
await vi.waitFor(() =>
expect(dockedTabs.map((tab) => tab.tabType)).toContain(ALERT_DIALOG_TYPE),
);
const dialogTab = dockedTabs.find((tab) => tab.tabType === ALERT_DIALOG_TYPE);
if (!dialogTab) throw new Error('the dialog tab was not docked');
expect(dialogModule.hasDialogRequest(dialogTab.id)).toBe(true);

if (!interfaceModeCallback) throw new Error('interface mode subscription never registered');
interfaceMode = 'simple';
await interfaceModeCallback('simple');

await expect(showing).resolves.toBeUndefined();
expect(dialogModule.hasDialogRequest(dialogTab.id)).toBe(false);
});
// The control — a layout load that keeps the dialog's tab must leave its request alone — is
// covered in `dialog.service-shard.test.ts`, which drives the dialog shard's `onLayoutLoadTabIds`
// subscriber directly with a chosen surviving-tab set. None of this file's real layouts (baked
// Simple/Power defaults, `windowLayout:get` answers) ever include an arbitrary already-open
// non-web-view tab, so there is no realistic "kept" layout to load here.
});
155 changes: 155 additions & 0 deletions src/renderer/services/dialog.service-shard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ vi.mock('@renderer/services/overlays/overlay-store', () => ({

// Mock web-view service (needed by dialog service initialize)
const mockCloseTab = vi.fn();
const mockOnLayoutLoadTabIds = vi.fn();
vi.mock('@renderer/services/web-view.service-shard', () => ({
initialize: vi.fn().mockResolvedValue(undefined),
addTab: vi.fn(),
Expand All @@ -23,6 +24,10 @@ vi.mock('@renderer/services/web-view.service-shard', () => ({
// covered against the real module in `dialog.service-shard.layout-load.test.ts`.
throwIfWindowIsClosing: vi.fn(),
waitForLayoutLoadToSettle: vi.fn(async () => {}),
// Captured so a test can invoke the shard's own subscriber directly with a chosen surviving-tab
// set, the same way the real event would deliver one. Real end-to-end coverage (a live layout
// load actually dropping a docked dialog's tab) lives in `dialog.service-shard.layout-load.test.ts`.
onLayoutLoadTabIds: mockOnLayoutLoadTabIds,
}));

// Mock localization service
Expand Down Expand Up @@ -288,4 +293,154 @@ describe('dialog.service-shard', () => {
await expect(dialogPromise).rejects.toBe('something went wrong');
});
});

describe('a layout load that keeps the dialog tab', () => {
it('leaves the dialog request alone', async () => {
const { hasDialogRequest } = await import('./dialog.service-shard');

const { addTab } = await import('@renderer/services/web-view.service-shard');
vi.mocked(addTab).mockResolvedValue(undefined);

const dialogPromise = capturedShowDialog('platform.selectProject', {});
// Never resolved by this test; a wrongly-settled promise would otherwise report as an
// unhandled rejection instead of failing the assertion below.
dialogPromise.catch(() => {});

await vi.waitFor(() => {
expect(hasDialogRequest('mock-guid')).toBe(true);
});

expect(mockOnLayoutLoadTabIds).toHaveBeenCalledTimes(1);
const [layoutLoadHandler] = mockOnLayoutLoadTabIds.mock.calls[0];
// The loaded layout still contains this dialog's tab id, so its request must survive.
layoutLoadHandler(new Set(['mock-guid', 'some-other-tab']));

expect(hasDialogRequest('mock-guid')).toBe(true);

// Clean up: resolve the dialog request so it doesn't leak into subsequent tests
const { resolveDialogRequest } = await import('./dialog.service-shard');
resolveDialogRequest('mock-guid', undefined);
await dialogPromise;
});
});

describe('a layout load that arrives before the dialog tab is placed', () => {
it('leaves an unplaced dialog request alone', async () => {
const { hasDialogRequest, resolveDialogRequest } = await import('./dialog.service-shard');

const { addTab } = await import('@renderer/services/web-view.service-shard');
// Never resolves during this test: standing in for a request registered synchronously at
// showDialog's start whose tab has not yet reached the dock when a layout load's sweep runs.
vi.mocked(addTab).mockReturnValue(new Promise(() => {}));

const dialogPromise = capturedShowDialog('platform.selectProject', {});
// A wrongly-settled promise would otherwise report as an unhandled rejection instead of
// failing the assertion below.
dialogPromise.catch(() => {});

await vi.waitFor(() => {
expect(hasDialogRequest('mock-guid')).toBe(true);
});

expect(mockOnLayoutLoadTabIds).toHaveBeenCalledTimes(1);
const [layoutLoadHandler] = mockOnLayoutLoadTabIds.mock.calls[0];
// The loaded layout does not report this id, but the request's tab was never placed in the
// dock for this load to have dropped, so the sweep must leave it alone.
layoutLoadHandler(new Set(['some-other-tab']));

expect(hasDialogRequest('mock-guid')).toBe(true);

// Clean up: resolve the request directly so it doesn't leak into subsequent tests (addTab
// never resolves in this test, so showDialog's own request/tab setup never completes).
resolveDialogRequest('mock-guid', undefined, false);
});
});

describe('a layout load that drops a docked dialog tab', () => {
it('settles the request once its tab has been placed in the dock', async () => {
const { hasDialogRequest } = await import('./dialog.service-shard');

const { addTab } = await import('@renderer/services/web-view.service-shard');
let resolveAddTab: () => void = () => {};
const addTabPromise = new Promise<undefined>((resolve) => {
resolveAddTab = () => resolve(undefined);
});
// The real `addTab` invokes its fourth argument synchronously, at the moment it places the tab
// in the dock, strictly before its own returned promise resolves — simulate that ordering here
// rather than marking the request docked as a side effect of the promise alone.
let onDocked: (() => void) | undefined;
vi.mocked(addTab).mockImplementation(
(_tabInfo, _layout, _shouldBringToFront, onDockedArg) => {
onDocked = onDockedArg;
return addTabPromise;
},
);

const dialogPromise = capturedShowDialog('platform.selectProject', {});

await vi.waitFor(() => {
expect(hasDialogRequest('mock-guid')).toBe(true);
});

// Placed, then addTab resolves and showDialog's own continuation runs.
onDocked?.();
resolveAddTab();
await addTabPromise;

expect(mockOnLayoutLoadTabIds).toHaveBeenCalledTimes(1);
const [layoutLoadHandler] = mockOnLayoutLoadTabIds.mock.calls[0];
layoutLoadHandler(new Set(['some-other-tab']));

expect(hasDialogRequest('mock-guid')).toBe(false);
await expect(dialogPromise).resolves.toBeUndefined();
});
});

describe('a layout load that lands between tab placement and addTab resolving', () => {
it('still settles the request, even though addTab has not resolved yet', async () => {
const { hasDialogRequest } = await import('./dialog.service-shard');

const { addTab } = await import('@renderer/services/web-view.service-shard');
let resolveAddTab: () => void = () => {};
const addTabPromise = new Promise<undefined>((resolve) => {
resolveAddTab = () => resolve(undefined);
});
// The real `addTab` places the tab in the dock and runs whatever it was given as its fourth
// argument synchronously, at that exact instant — strictly before its own promise resolves.
// Standing in for that split here lets the test trigger "placed" and "addTab resolved" as two
// independently-timed events, the same way production can have a competing layout load's wipe
// land in the gap between them.
let onDocked: (() => void) | undefined;
vi.mocked(addTab).mockImplementation(
(_tabInfo, _layout, _shouldBringToFront, onDockedArg) => {
onDocked = onDockedArg;
return addTabPromise;
},
);

const dialogPromise = capturedShowDialog('platform.selectProject', {});
// A wrongly-unsettled promise would otherwise report as a test timeout instead of failing the
// assertion below.
dialogPromise.catch(() => {});

await vi.waitFor(() => {
expect(hasDialogRequest('mock-guid')).toBe(true);
});

// The tab has been placed in the dock, but `addTab`'s own promise — and so `showDialog`'s
// continuation that used to do this marking — has not resolved yet.
onDocked?.();

expect(mockOnLayoutLoadTabIds).toHaveBeenCalledTimes(1);
const [layoutLoadHandler] = mockOnLayoutLoadTabIds.mock.calls[0];
// A layout load's wipe lands in exactly this gap.
layoutLoadHandler(new Set(['some-other-tab']));

expect(hasDialogRequest('mock-guid')).toBe(false);

// Clean up: let addTab resolve so showDialog's own continuation does not hang past this test.
resolveAddTab();
await addTabPromise;
});
});
});
36 changes: 36 additions & 0 deletions src/renderer/services/dialog.service-shard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,13 @@ type DialogRequest<DialogTabType extends DialogTabTypes> = {
| PromiseLike<DialogTypes[DialogTabType]['responseType'] | undefined>,
) => void;
reject: (reason?: unknown) => void;
/**
* Whether this request's tab has actually been placed in the dock. False from registration until
* `addTab` places it — marked from `addTab`'s own synchronous callback rather than after its
* returned promise resolves, since a layout load can land in that window before the tab exists
* for it to drop, or in the gap between placement and this request's continuation resuming.
*/
isTabDocked: boolean;
};

/** Map of all live dialog requests */
Expand Down Expand Up @@ -309,6 +316,7 @@ async function showDialog<DialogTabType extends DialogTabTypes>(
id: dialogId,
resolve,
reject,
isTabDocked: false,
};
dialogRequests.set(dialogId, dialogRequest);
},
Expand All @@ -326,6 +334,19 @@ async function showDialog<DialogTabType extends DialogTabTypes>(
type: 'float',
position: 'center',
},
true,
() => {
// Marked from `addTab`'s own synchronous callback, in the same tick the tab is actually
// placed in the dock, rather than after `addTab`'s promise resolves: the sweep below runs
// synchronously inside a whole-layout load, so a load landing in the one-tick gap between
// placement and this request's continuation resuming would otherwise find the tab already
// gone but this request still reading as undocked, and never settle it. Fresh map lookup
// rather than the `dialogRequest` closure variable: the id may already have been removed by
// the time this runs (e.g. the window closed while addTab was in flight), and a fresh lookup
// naturally skips marking a request that is no longer there.
const dockedRequest = dialogRequests.get(dialogId);
if (dockedRequest) dockedRequest.isTabDocked = true;
},
);

// TODO: preserve requests between refreshes - add keepalive messages to indicate to the
Expand Down Expand Up @@ -376,6 +397,21 @@ export async function startDialogServiceShard(): Promise<void> {
if (globalThis.windowId === undefined)
throw new Error('Cannot start DialogService: windowId is not set');

// A whole-layout load (e.g. a Simple/Power mode switch) replaces the dock without running
// rc-dock's per-tab remove callback, so a docked dialog's tab can vanish with nothing telling this
// shard its request is now unanswerable — the requestor would then await a promise that never
// settles. Settle it as though the user canceled; `false` because the tab this would try to close
// is already gone. Only a request whose tab actually reached the dock qualifies: a request can be
// registered here before `addTab` places its tab, and a load landing in that window has no tab of
// this request's to have dropped, so settling it would be indistinguishable from a user
// cancellation the user never made.
webViewService.onLayoutLoadTabIds((survivingTabIds) => {
dialogRequests.forEach((dialogRequest, id) => {
if (dialogRequest.isTabDocked && !survivingTabIds.has(id))
resolveDialogRequest(id, undefined, false);
});
});

// Registered under this window's scoped name (e.g.
// `DialogService-f81d4fae-7dec-11d0-a765-00a0c91e6bf6`) so every window can own its own dialogs.
// The object type and window id are how the main process's dialog service router finds this
Expand Down
2 changes: 2 additions & 0 deletions src/renderer/services/dialog.service-shard.unload.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ vi.mock('@renderer/services/web-view.service-shard', () => ({
closeTab: vi.fn(async () => true),
throwIfWindowIsClosing: vi.fn(),
waitForLayoutLoadToSettle: vi.fn(async () => {}),
// No layout load happens in this file; the subscription just has to exist to start the shard.
onLayoutLoadTabIds: vi.fn(() => () => {}),
}));
vi.mock('@shared/services/logger.service', () => ({
logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() },
Expand Down
45 changes: 45 additions & 0 deletions src/renderer/services/web-view.service-shard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import type {
} from '@shared/models/docking-framework.model';
import { SCRIPTURE_EDITOR_WEBVIEW_TYPE } from '@shared/models/web-view.model';
import {
EVENT_NAME_ON_DID_CLOSE_WEB_VIEW,
EVENT_NAME_ON_DID_OPEN_WEB_VIEW,
EVENT_NAME_ON_DID_UPDATE_WEB_VIEW,
} from '@shared/services/web-view.service-model';
Expand Down Expand Up @@ -3369,3 +3370,47 @@ describe('content zoom wiring', () => {
);
});
});

describe('a layout load that drops a web view', () => {
test('still emits its close event', async () => {
const TARGET_WEB_VIEW_ID = 'target-web-view';
let interfaceMode = 'simple';
settingsGetMock.mockImplementation(async (key: string) =>
key === 'platform.interfaceMode' ? interfaceMode : false,
);
let interfaceModeCallback: ((newMode: unknown) => Promise<void>) | undefined;
settingsSubscribeMock.mockImplementation(
async (_key: string, callback: (newMode: unknown) => Promise<void>) => {
interfaceModeCallback = callback;
return async () => true;
},
);

const module = await primeWebViewOpenPath();
const { dockLayout, loadedLayouts, getCurrentWebViewId } = makeDockLayoutTrackingOneWebView(
layoutWithTab(TARGET_WEB_VIEW_ID),
);
module.registerDockLayout(dockLayout);
// Simple mode's initial load is a baked default, so the target tab lands under a freshly
// minted id rather than the `TARGET_WEB_VIEW_ID` literal - wait for that mint to land, then
// read back the live id the rest of this test operates on.
await vi.waitFor(() => expect(loadedLayouts.length).toBeGreaterThan(0));
const mintedTargetWebViewId = getCurrentWebViewId();
if (!mintedTargetWebViewId) throw new Error('expected the target tab to receive a minted id');
if (!interfaceModeCallback) throw new Error('interface mode subscription never registered');

// The mode switch loads a persisted layout that never mentions the target's minted id, so the
// load drops it — the same shape a real Simple/Power switch takes.
interfaceMode = 'power';
respondToGetLayout({ kind: 'entry', layout: layoutWithTab('other-tab') });
await interfaceModeCallback('power');

const closeEmitter = mocks.bufferedEmitters.get(EVENT_NAME_ON_DID_CLOSE_WEB_VIEW);
if (!closeEmitter) throw new Error('close emitter was never created');
expect(closeEmitter.emit).toHaveBeenCalledWith(
expect.objectContaining({
webView: expect.objectContaining({ id: mintedTargetWebViewId }),
}),
);
});
});
Loading
Loading