From b2f87a094d21abac857e58e9847cf3c102862ab0 Mon Sep 17 00:00:00 2001 From: Gregor MacLennan Date: Thu, 17 Sep 2026 10:06:55 +0100 Subject: [PATCH] feat(download): report when the browser has read the whole download MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The page's writes complete when the service worker has queued them, which on iOS is well before the file is written. The worker now announces when the browser has read the last chunk (download protocol 2), exportMap waits for it, and the form shows "Writing file…" in the meantime. --- docs/architecture.md | 3 ++- e2e/export.test.ts | 20 +++++++++++++++++++ public/sw.js | 39 ++++++++++++++++++++++++++++++++------ src/lib/download/README.md | 18 ++++++++++++------ src/lib/download/index.ts | 35 +++++++++++++++++++++++++++++++--- src/lib/export/README.md | 8 +++++--- src/lib/export/index.ts | 10 +++++++++- src/settings-form.ts | 5 +++++ 8 files changed, 118 insertions(+), 20 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index 6fd7d7a..b2f0bb0 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -193,7 +193,8 @@ Port of map-downloader's service-worker streaming download ```ts registerDownloadWorker(): void // at startup; also startMessages() downloadReady(): Promise // resolves once a worker controls the page -startDownload(opts: { filename: string; contentType: string }): Promise<{ writable: WritableStream; cleanup(): void }> +startDownload(opts: { filename: string; contentType: string }): Promise<{ writable: WritableStream; complete: Promise; cleanup(): void }> + // complete: the worker has seen the browser read the last chunk (protocol 2) ``` `downloadReady()` rejects if registering the worker failed, so the UI is not diff --git a/e2e/export.test.ts b/e2e/export.test.ts index 1411960..f06e702 100644 --- a/e2e/export.test.ts +++ b/e2e/export.test.ts @@ -321,4 +321,24 @@ describeEngines((engine) => { if (download) expect(await download.failure()).not.toBeNull(); }, ); + + test("startDownload's complete resolves once the browser has read the file", async () => { + const [download, waitedMs] = await Promise.all([ + page.waitForEvent("download", { timeout: 60_000 }), + page.evaluate(async () => { + const { writable, complete } = await window.mapPrinter.startDownload({ + filename: "complete.bin", + contentType: "application/octet-stream", + }); + const writer = writable.getWriter(); + for (let i = 0; i < 16; i++) await writer.write(new Uint8Array(65536)); + await writer.close(); + const t0 = performance.now(); + await complete; + return performance.now() - t0; + }), + ]); + expect(waitedMs).toBeLessThan(10_000); + expect(await download.failure()).toBeNull(); + }); }); diff --git a/public/sw.js b/public/sw.js index 1d1b199..72e5d80 100644 --- a/public/sw.js +++ b/public/sw.js @@ -50,7 +50,7 @@ class MessagePortSource { // path existed ignores them — the page checks DOWNLOAD_PROTOCOL to notice. const DOWNLOAD_PATH = "/_download/"; const DOWNLOAD_TIMEOUT = 30_000; -const DOWNLOAD_PROTOCOL = 1; +const DOWNLOAD_PROTOCOL = 2; /** Streams whose fetch has not arrived yet, keyed by URL */ const pending = new Map(); @@ -68,7 +68,7 @@ self.addEventListener("message", (evt) => { new MessagePortSource(data.readablePort), new CountQueuingStrategy({ highWaterMark: 4 }), ); - const download = { rs, headers: data.headers }; + const download = { rs: announceWhenRead(rs, data.url), headers: data.headers }; const waiter = waiting.get(data.url); if (waiter) { waiting.delete(data.url); @@ -80,11 +80,38 @@ self.addEventListener("message", (evt) => { // A navigation does not always reach the service worker, and the page can only // tell by being told the request arrived. -async function announceDownload(url) { +function announceDownload(url) { + return announce({ type: "downloadStarted", url }); +} + +async function announce(message) { const clients = await self.clients.matchAll({ includeUncontrolled: true }); - for (const client of clients) { - client.postMessage({ type: "downloadStarted", url }); - } + for (const client of clients) client.postMessage(message); +} + +// The page's writes complete when this worker has queued them, long before +// the browser has read them out of the response, so the browser's read of the +// last chunk is announced separately. pull() only runs once the browser has +// taken the previous chunk, so `done` here means it has read everything. +function announceWhenRead(rs, url) { + const reader = rs.getReader(); + return new ReadableStream( + { + async pull(controller) { + const { value, done } = await reader.read(); + if (done) { + controller.close(); + await announce({ type: "downloadComplete", url }); + } else { + controller.enqueue(value); + } + }, + cancel(reason) { + return reader.cancel(reason); + }, + }, + new CountQueuingStrategy({ highWaterMark: 1 }), + ); } self.addEventListener("fetch", (event) => { diff --git a/src/lib/download/README.md b/src/lib/download/README.md index 6d65302..154f1f6 100644 --- a/src/lib/download/README.md +++ b/src/lib/download/README.md @@ -9,7 +9,7 @@ verbatim, and this module is the page side of the same handshake. registerDownloadWorker(): void downloadReady(): Promise startDownload(opts: { filename: string; contentType: string }): - Promise<{ writable: WritableStream; cleanup(): void }> + Promise<{ writable: WritableStream; complete: Promise; cleanup(): void }> ``` ## How it works @@ -25,11 +25,17 @@ activation from the click is still live. The worker cannot see a navigation that never reached it, so the page waits for its `downloadStarted` message before handing over the `MessagePort` and returning the writable. -The iframe outlives the writable. The worker's `PULL`s only mean the -browser's network layer has read the bytes, not that they are on disk, and -iOS Safari finalises a download the moment its initiating frame is removed, -saving only what had been flushed. So `cleanup()` is for failures; after -success the iframe stays until the next `startDownload` replaces it. +The iframe outlives the writable. The worker's `PULL`s only mean the worker +has queued the bytes, not that they are on disk, and iOS Safari finalises a +download the moment its initiating frame is removed, saving only what had +been flushed. So `cleanup()` is for failures; after success the iframe stays +until the next `startDownload` replaces it. + +`complete` (protocol 2) resolves when the worker announces +`downloadComplete`, which it does once the browser has read the last chunk +out of the response. That is later than the writable closing, though still +short of the bytes being on disk. It never rejects, so callers race it with +a timeout in case an older worker is still active. ## Backpressure and errors diff --git a/src/lib/download/index.ts b/src/lib/download/index.ts index edec206..71897d4 100644 --- a/src/lib/download/index.ts +++ b/src/lib/download/index.ts @@ -9,7 +9,7 @@ const CLOSE = 2; // Kept in step with public/sw.js: an older worker ignores /_download/ requests. const DOWNLOAD_PATH = "/_download/"; -const DOWNLOAD_PROTOCOL = 1; +const DOWNLOAD_PROTOCOL = 2; const DOWNLOAD_START_TIMEOUT_MS = 5_000; const NO_WORKER_MESSAGE = @@ -24,6 +24,10 @@ export interface DownloadOptions { export interface Download { writable: WritableStream; + /** Resolves once the browser has read the last byte out of the worker's + * response, which is later than the writable closing: writes complete when + * the worker has queued them. Never rejects. */ + complete: Promise; /** Removes the iframe that started the download. Call it only after a * failure: iOS Safari finalises a download when its initiating frame goes * away, even while bytes it has already accepted are still being flushed @@ -71,16 +75,38 @@ export function downloadReady(): Promise { export async function startDownload(opts: DownloadOptions): Promise { // Nothing may be awaited before this call: it navigates an iframe, which // Safari only turns into a download while the user activation is still live. - const { workerPort, cleanup } = await prepareSwDownload( + const { workerPort, complete, cleanup } = await prepareSwDownload( opts.filename, opts.contentType, ); return { writable: new WritableStream(new MessagePortSink(workerPort)), + complete, cleanup, }; } +/** Resolves when the worker announces that the browser has read the whole + * response for `url`; `stop` gives up listening (the promise then never + * settles, which is fine for a download that failed). */ +function waitForDownloadComplete(url: string): { + complete: Promise; + stop: () => void; +} { + let stop = () => {}; + const complete = new Promise((resolve) => { + const onMessage = (event: MessageEvent) => { + if (event.data?.type === "downloadComplete" && event.data.url === url) { + stop(); + resolve(); + } + }; + stop = () => navigator.serviceWorker.removeEventListener("message", onMessage); + navigator.serviceWorker.addEventListener("message", onMessage); + }); + return { complete, stop }; +} + /** Ask the active service worker which download protocol it speaks (0 = none) */ function downloadProtocol(registration: ServiceWorkerRegistration) { return new Promise((resolve) => { @@ -126,6 +152,7 @@ function waitForDownloadStart(url: string, timeout: number): Promise { interface SwDownloadChannel { workerPort: MessagePort; + complete: Promise; cleanup: () => void; } @@ -154,7 +181,9 @@ async function prepareSwDownload( iframe.src = url; document.body.appendChild(iframe); activeIframe = iframe; + const { complete, stop } = waitForDownloadComplete(url); const cleanup = () => { + stop(); iframe.remove(); if (activeIframe === iframe) activeIframe = undefined; }; @@ -185,7 +214,7 @@ async function prepareSwDownload( const channel = new MessageChannel(); sw.postMessage({ url, headers, readablePort: channel.port1 }, [channel.port1]); - return { workerPort: channel.port2, cleanup }; + return { workerPort: channel.port2, complete, cleanup }; } class MessagePortSink implements UnderlyingSink { diff --git a/src/lib/export/README.md b/src/lib/export/README.md index 552894a..6a2c871 100644 --- a/src/lib/export/README.md +++ b/src/lib/export/README.md @@ -17,9 +17,11 @@ exportMap(opts: { }): Promise<{ bbox: Bbox; zoom: number }> ``` -Resolves once the last byte has been written to the download, with the bbox -the exported image actually covers (grown from `bbox` on the non-limiting -axis) and the zoom it was rendered at. +Resolves once the browser has read the last byte of the download out of the +service worker (or after 60 s if a pre-protocol-2 worker never says so), with +the bbox the exported image actually covers (grown from `bbox` on the +non-limiting axis) and the zoom it was rendered at. On iOS the file may still +be flushing to disk for a moment after that. ## Sizes diff --git a/src/lib/export/index.ts b/src/lib/export/index.ts index b5c8e6f..ea8c9c6 100644 --- a/src/lib/export/index.ts +++ b/src/lib/export/index.ts @@ -46,6 +46,12 @@ const CHANNELS = 3; * tile the renderer just returned and its RGBA readPixels scratch. */ const MEMORY_BUDGET_BYTES = 64 * 1024 * 1024; +const COMPLETE_TIMEOUT_MS = 60_000; + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + type ExportTile = { col: number; row: number; @@ -87,7 +93,7 @@ export async function exportMap(opts: ExportOptions): Promise { css, })); - const { writable, cleanup } = await startDownload({ + const { writable, complete, cleanup } = await startDownload({ filename: opts.filename, contentType: "image/png", }); @@ -121,6 +127,8 @@ export async function exportMap(opts: ExportOptions): Promise { }), ) .pipeTo(writable, { signal }); + // A worker from before protocol 2 never announces, so do not wait forever. + await Promise.race([complete, sleep(COMPLETE_TIMEOUT_MS)]); } catch (err) { // The response only errors if the worker is told; a failure before or // outside `pipeTo` would otherwise leave a truncated file on disk. diff --git a/src/settings-form.ts b/src/settings-form.ts index 0f3dd0f..e18d137 100644 --- a/src/settings-form.ts +++ b/src/settings-form.ts @@ -371,6 +371,11 @@ export class SettingsForm extends LightElement { style="width: ${Math.round(this.progress * 100)}%" > + ${this.progress >= 1 + ? html`

+ Writing file… +

` + : nothing}