Skip to content

Commit fe2bfb4

Browse files
Bil0000Bil0000
committed
fix(desktop): preserve static preview deliveries
1 parent 1279cf8 commit fe2bfb4

2 files changed

Lines changed: 104 additions & 33 deletions

File tree

‎apps/desktop/src/preview/Manager.test.ts‎

Lines changed: 82 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,7 @@ const settle = function* (until: () => boolean) {
288288

289289
const makeTestPictureInPictureWindow = (loadURL: () => Promise<void> = async () => undefined) => {
290290
const listeners = new Map<string, () => void>();
291+
const webContentsListeners = new Map<string, () => void>();
291292
const send = vi.fn();
292293
let destroyed = false;
293294
const pictureInPictureWindow = {
@@ -310,10 +311,16 @@ const makeTestPictureInPictureWindow = (loadURL: () => Promise<void> = async ()
310311
listeners.get("closed")?.();
311312
}),
312313
webContents: {
314+
on: vi.fn((event: string, listener: () => void) => {
315+
webContentsListeners.set(event, listener);
316+
}),
317+
off: vi.fn((event: string) => {
318+
webContentsListeners.delete(event);
319+
}),
313320
send,
314321
},
315322
};
316-
return { pictureInPictureWindow, send };
323+
return { pictureInPictureWindow, send, webContentsListeners };
317324
};
318325

319326
describe("PreviewManager", () => {
@@ -2369,6 +2376,8 @@ describe("PreviewManager", () => {
23692376
pictureInPictureListeners.get("closed")?.();
23702377
}),
23712378
webContents: {
2379+
on: vi.fn(),
2380+
off: vi.fn(),
23722381
send: pictureInPictureSend,
23732382
},
23742383
};
@@ -2467,7 +2476,7 @@ describe("PreviewManager", () => {
24672476
),
24682477
);
24692478

2470-
effectIt.effect("delivers unchanged frames only to a new picture-in-picture consumer", () =>
2479+
effectIt.effect("starts picture-in-picture without an extra recording capture", () =>
24712480
withManager((manager) =>
24722481
Effect.gen(function* () {
24732482
const jpeg = Buffer.from("shared-preview-frame");
@@ -2499,7 +2508,7 @@ describe("PreviewManager", () => {
24992508
yield* TestClock.adjust(100);
25002509

25012510
expect(capturePage).toHaveBeenCalledTimes(2);
2502-
expect(recordingFrames).toHaveLength(1);
2511+
expect(recordingFrames).toHaveLength(2);
25032512
expect(send).toHaveBeenCalledOnce();
25042513
yield* manager.closePictureInPicture("tab_recording_then_pip");
25052514
yield* manager.stopRecording("tab_recording_then_pip");
@@ -2538,20 +2547,43 @@ describe("PreviewManager", () => {
25382547
),
25392548
);
25402549

2541-
effectIt.effect("delivers recording frames only when pixels change", () =>
2550+
effectIt.effect("replays an unchanged picture-in-picture frame after its renderer reloads", () =>
25422551
withManager((manager) =>
25432552
Effect.gen(function* () {
2544-
const unchanged = Buffer.from("unchanged-preview-frame");
2545-
const changed = Buffer.from("changed-preview-frame");
2546-
let captureIndex = 0;
2547-
const capturePage = vi.fn(async () => {
2548-
const jpeg = captureIndex < 2 ? unchanged : changed;
2549-
captureIndex += 1;
2550-
return {
2551-
toJPEG: () => jpeg,
2552-
getSize: () => ({ width: 1280, height: 720 }),
2553-
};
2553+
const jpeg = Buffer.from("reloaded-preview-frame");
2554+
const capturePage = vi.fn(async () => ({
2555+
toJPEG: () => jpeg,
2556+
getSize: () => ({ width: 1280, height: 720 }),
2557+
}));
2558+
fromId.mockReturnValue(makeTestPreviewWebContents(capturePage));
2559+
const { pictureInPictureWindow, send, webContentsListeners } =
2560+
makeTestPictureInPictureWindow();
2561+
browserWindowConstructor.mockImplementation(function () {
2562+
return pictureInPictureWindow;
25542563
});
2564+
2565+
yield* manager.createTab("tab_pip_reload");
2566+
yield* manager.registerWebview("tab_pip_reload", 42);
2567+
yield* manager.openPictureInPicture("tab_pip_reload");
2568+
expect(send).toHaveBeenCalledOnce();
2569+
2570+
webContentsListeners.get("did-finish-load")?.();
2571+
yield* TestClock.adjust(100);
2572+
2573+
expect(send).toHaveBeenCalledTimes(2);
2574+
yield* manager.closePictureInPicture("tab_pip_reload");
2575+
}),
2576+
),
2577+
);
2578+
2579+
effectIt.effect("keeps recording cadence when pixels remain unchanged", () =>
2580+
withManager((manager) =>
2581+
Effect.gen(function* () {
2582+
const unchanged = Buffer.from("unchanged-preview-frame");
2583+
const capturePage = vi.fn(async () => ({
2584+
toJPEG: () => unchanged,
2585+
getSize: () => ({ width: 1280, height: 720 }),
2586+
}));
25552587
fromId.mockReturnValue(makeTestPreviewWebContents(capturePage));
25562588
const frames: DesktopPreviewRecordingFrame[] = [];
25572589

@@ -2569,20 +2601,54 @@ describe("PreviewManager", () => {
25692601
yield* TestClock.adjust(100);
25702602

25712603
expect(capturePage).toHaveBeenCalledTimes(2);
2572-
expect(frames.map((frame) => frame.data)).toEqual([unchanged.toString("base64")]);
2604+
expect(frames.map((frame) => frame.data)).toEqual([
2605+
unchanged.toString("base64"),
2606+
unchanged.toString("base64"),
2607+
]);
25732608

25742609
yield* TestClock.adjust(100);
25752610

25762611
expect(capturePage).toHaveBeenCalledTimes(3);
25772612
expect(frames.map((frame) => frame.data)).toEqual([
25782613
unchanged.toString("base64"),
2579-
changed.toString("base64"),
2614+
unchanged.toString("base64"),
2615+
unchanged.toString("base64"),
25802616
]);
25812617
yield* manager.stopRecording("tab_unchanged_frame");
25822618
}),
25832619
),
25842620
);
25852621

2622+
effectIt.effect("retries recording delivery when pixels remain unchanged", () =>
2623+
withManager((manager) =>
2624+
Effect.gen(function* () {
2625+
const jpeg = Buffer.from("retry-recording-frame");
2626+
const capturePage = vi.fn(async () => ({
2627+
toJPEG: () => jpeg,
2628+
getSize: () => ({ width: 1280, height: 720 }),
2629+
}));
2630+
fromId.mockReturnValue(makeTestPreviewWebContents(capturePage));
2631+
let deliveries = 0;
2632+
2633+
yield* manager.subscribeRecordingFrames(() =>
2634+
Effect.sync(() => {
2635+
deliveries += 1;
2636+
if (deliveries === 1) throw new Error("recording delivery failed");
2637+
}),
2638+
);
2639+
yield* manager.createTab("tab_recording_delivery_retry");
2640+
yield* manager.registerWebview("tab_recording_delivery_retry", 42);
2641+
yield* manager.startRecording("tab_recording_delivery_retry");
2642+
expect(deliveries).toBe(1);
2643+
2644+
yield* TestClock.adjust(100);
2645+
2646+
expect(deliveries).toBe(2);
2647+
yield* manager.stopRecording("tab_recording_delivery_retry");
2648+
}),
2649+
),
2650+
);
2651+
25862652
effectIt.effect("retries a cold hidden-tab capture without dropping recording", () =>
25872653
withManager((manager) =>
25882654
Effect.gen(function* () {

‎apps/desktop/src/preview/Manager.ts‎

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,6 @@ interface FrameCaptureSession {
382382
readonly scope: Scope.Closeable;
383383
readonly consumers: ReadonlySet<FrameCaptureConsumer>;
384384
readonly lastPictureInPictureFrame: Buffer | null;
385-
readonly lastRecordingFrame: Buffer | null;
386385
}
387386

388387
interface PictureInPictureSession {
@@ -623,7 +622,6 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
623622
consumers,
624623
lastPictureInPictureFrame:
625624
consumer === "picture-in-picture" ? null : current.lastPictureInPictureFrame,
626-
lastRecordingFrame: consumer === "recording" ? null : current.lastRecordingFrame,
627625
});
628626
}),
629627
] as const;
@@ -2564,23 +2562,13 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
25642562
if (current?.scope !== captureSession.scope) {
25652563
return [undefined, sessions] as const;
25662564
}
2567-
const recording =
2568-
current.consumers.has("recording") && current.lastRecordingFrame?.equals(encoded) !== true;
2565+
const recording = current.consumers.has("recording");
25692566
const pictureInPicture =
25702567
current.consumers.has("picture-in-picture") &&
25712568
current.lastPictureInPictureFrame?.equals(encoded) !== true;
2572-
if (!recording && !pictureInPicture) {
2573-
return [undefined, sessions] as const;
2574-
}
2575-
const next = recording ? { ...current, lastRecordingFrame: encoded } : current;
2576-
return [
2577-
{ pictureInPicture, recording, session: next },
2578-
recording
2579-
? replaceMap(sessions, (copy) => {
2580-
copy.set(tabId, next);
2581-
})
2582-
: sessions,
2583-
] as const;
2569+
return recording || pictureInPicture
2570+
? [{ pictureInPicture, recording, session: current }, sessions]
2571+
: [undefined, sessions];
25842572
});
25852573
if (!frameConsumers) return;
25862574
const receivedAt = yield* currentIso;
@@ -2729,7 +2717,6 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
27292717
scope,
27302718
consumers: new Set([consumer]),
27312719
lastPictureInPictureFrame: null,
2732-
lastRecordingFrame: null,
27332720
});
27342721
}),
27352722
] as const;
@@ -2884,6 +2871,17 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
28842871
),
28852872
);
28862873
};
2874+
const onDidFinishLoad = () => {
2875+
runFork(
2876+
SynchronizedRef.update(frameCaptureSessionsRef, (sessions) => {
2877+
const current = sessions.get(tabId);
2878+
if (!current?.consumers.has("picture-in-picture")) return sessions;
2879+
return replaceMap(sessions, (copy) => {
2880+
copy.set(tabId, { ...current, lastPictureInPictureFrame: null });
2881+
});
2882+
}),
2883+
);
2884+
};
28872885
yield* attempt(
28882886
{
28892887
operation: "pictureInPicture.configure",
@@ -2904,6 +2902,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
29042902
skipTransformProcessType: true,
29052903
});
29062904
}
2905+
pictureInPictureWindow.webContents.on("did-finish-load", onDidFinishLoad);
29072906
},
29082907
).pipe(
29092908
Effect.onError(() =>
@@ -2918,6 +2917,12 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
29182917
),
29192918
),
29202919
);
2920+
yield* Scope.addFinalizer(
2921+
initializationScope,
2922+
Effect.sync(() => {
2923+
pictureInPictureWindow.webContents.off("did-finish-load", onDidFinishLoad);
2924+
}).pipe(Effect.ignore),
2925+
);
29212926
yield* SynchronizedRef.update(pictureInPictureSessionsRef, (sessions) =>
29222927
replaceMap(sessions, (copy) => {
29232928
copy.set(tabId, session);

0 commit comments

Comments
 (0)