Skip to content

Commit 9028a57

Browse files
committed
fix(desktop): closed popups no longer block download setup
1 parent 662bec1 commit 9028a57

2 files changed

Lines changed: 54 additions & 14 deletions

File tree

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

Lines changed: 49 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -644,23 +644,35 @@ describe("CDP relay downloads", () => {
644644
expect(written).toEqual([]);
645645
});
646646

647-
it.each([false, true])(
648-
"keeps the previous download configuration (events enabled: %s) when configuration fails",
649-
async (enabled) => {
650-
const send = vi
651-
.fn(async () => ({}))
652-
.mockResolvedValueOnce({})
653-
.mockRejectedValueOnce(new Error("Page command rejected"));
647+
it.each(
648+
[false, true].flatMap((enabled) =>
649+
["page", "popup", "removed popup"].map((failure) => ({ enabled, failure })),
650+
),
651+
)(
652+
"keeps download configuration consistent when $failure rejects (events enabled: $enabled)",
653+
async ({ enabled, failure }) => {
654+
const send = vi.fn(async () => ({}));
655+
const popupSend = vi.fn(async () => ({}));
654656
const written: Array<Record<string, unknown>> = [];
655657
let directory: string | null = null;
658+
let popupDirectory: string | null = null;
656659
const relay = makeRelay(
657660
makeTarget(send, (value) => {
658661
directory = value;
659662
}),
660663
(raw) => written.push(JSON.parse(raw)),
661664
);
665+
const popup = relay.addPopup({
666+
...makeTarget(popupSend, (value) => {
667+
popupDirectory = value;
668+
}),
669+
targetId: async () => "POPUP",
670+
});
662671
relay.receive(JSON.stringify({ id: 1, method: "Target.setAutoAttach", params: {} }));
663672
for (const [id, eventsEnabled] of [enabled, !enabled].entries()) {
673+
const pending = Promise.withResolvers<object>();
674+
if (id === 1)
675+
(failure === "page" ? send : popupSend).mockImplementationOnce(() => pending.promise);
664676
relay.receive(
665677
JSON.stringify({
666678
id: id + 2,
@@ -672,18 +684,42 @@ describe("CDP relay downloads", () => {
672684
},
673685
}),
674686
);
687+
if (id === 1) {
688+
if (failure === "removed popup") popup.remove();
689+
pending.reject(new Error("Page command rejected"));
690+
}
675691
await settle();
676692
}
677-
expect(written).toContainEqual({
678-
id: 3,
679-
error: { code: -32000, message: "Page command rejected" },
680-
});
681-
expect(directory).toBe("/srv/artifacts");
693+
const removed = failure === "removed popup";
694+
expect(written).toContainEqual(
695+
removed
696+
? { id: 3, result: {} }
697+
: { id: 3, error: { code: -32000, message: "Page command rejected" } },
698+
);
699+
const expectedDirectory = removed
700+
? enabled
701+
? null
702+
: "/srv/new-artifacts"
703+
: "/srv/artifacts";
704+
expect(directory).toBe(expectedDirectory);
705+
expect(popupDirectory).toBe("/srv/artifacts");
706+
if (removed) {
707+
const lateSend = vi.fn(async () => ({}));
708+
const lateDirectory = vi.fn();
709+
relay.addPopup(makeTarget(lateSend, lateDirectory));
710+
await settle();
711+
expect(lateDirectory).toHaveBeenCalledExactlyOnceWith(expectedDirectory);
712+
expect(lateSend).toHaveBeenCalledExactlyOnceWith(
713+
"Page.setDownloadBehavior",
714+
{ behavior: enabled ? "deny" : "default" },
715+
undefined,
716+
);
717+
}
682718
written.length = 0;
683719
relay.event("Page.downloadWillBegin", { guid: "g1" }, "");
684720
relay.event("Page.downloadProgress", { guid: "g1", state: "completed" }, "");
685721
expect(written).toEqual(
686-
enabled
722+
(removed ? !enabled : enabled)
687723
? [
688724
{ method: "Browser.downloadWillBegin", params: { guid: "g1" } },
689725
{ method: "Browser.downloadProgress", params: { guid: "g1", state: "completed" } },

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,11 @@ export function createCdpRelayConnection(
215215
// Browser-scoped behavior targets Electron's app session; preview paths belong to will-download.
216216
const [result] = await Promise.all(
217217
[...targets].map((relayed) =>
218-
relayed.target.send("Page.setDownloadBehavior", { behavior }, undefined),
218+
relayed.target
219+
.send("Page.setDownloadBehavior", { behavior }, undefined)
220+
.catch((cause: unknown) => {
221+
if (relayed === page || !relayed.removed) throw cause;
222+
}),
219223
),
220224
);
221225
downloads = { behavior, directory: allowed ? directory : null };

0 commit comments

Comments
 (0)