From 878ef1c1b59c48480110683b4fecf583edbec292 Mon Sep 17 00:00:00 2001 From: Taehoon Moon Date: Sun, 18 Oct 2026 08:19:20 +0900 Subject: [PATCH] Fix app updates downloading every file after the browser drops its cache A new build's service worker filled its shell cache with cache.addAll, so an unchanged file was kept only while the browser's HTTP cache still held it; once Chrome dropped those copies, the next update downloaded all 26 MB again. It now asks about each file with the tag of the copy in an earlier build's cache and keeps that copy when the server answers that it is unchanged. --- docs/architecture/frontend.md | 13 +- frontend/service-worker.js | 56 ++++++- frontend/service-worker.test.js | 152 +++++++++++++++++- .../tests/contracts/service-worker.test.mjs | 14 +- frontend/tests/e2e/app-shell-update.spec.js | 68 +++++++- 5 files changed, 280 insertions(+), 23 deletions(-) diff --git a/docs/architecture/frontend.md b/docs/architecture/frontend.md index d7e1e6fe..30bdde02 100644 --- a/docs/architecture/frontend.md +++ b/docs/architecture/frontend.md @@ -283,11 +283,14 @@ to the Tasks page's `startTaskCreation`, as Start Task does. A replacement is `ready` only after its complete shell cache is available, and Reload explicitly transitions to that prepared generation. The server sends every `/assets/` file except the manifest, which carries the server's name, -with `Cache-Control: no-cache` and an `ETag` taken from the file's contents, so -filling a replacement's shell cache revalidates the files the browser already -holds and downloads only those that changed. The viewport-fixed -`caffold-build-mismatch-alert` remains a separate exceptional diagnostic and -appears only when UI and server builds differ after the lifecycle is `settled`. +with `Cache-Control: no-cache` and an `ETag` taken from the file's contents. +While filling a replacement's shell cache, the service worker asks about each +file an earlier build's shell cache holds with that copy's tag and keeps the +copy when the server answers that it is unchanged, so an update downloads only +the files that changed whether or not the browser kept its own copies. The +viewport-fixed `caffold-build-mismatch-alert` remains a separate exceptional +diagnostic and appears only when UI and server builds differ after the +lifecycle is `settled`. The private update-handoff graph contains `detached`, `idle`, `activating`, `claiming`, and `applying`. Prepared-worker metadata, registration and diff --git a/frontend/service-worker.js b/frontend/service-worker.js index 65cf4fb5..1d02e511 100644 --- a/frontend/service-worker.js +++ b/frontend/service-worker.js @@ -427,11 +427,7 @@ const APP_SHELL_ASSETS = [ const APP_SHELL_ASSET_PATHS = new Set(APP_SHELL_ASSETS); self.addEventListener("install", (event) => { - event.waitUntil( - caches - .open(CACHE_NAME) - .then((cache) => cache.addAll(APP_SHELL_ASSETS)), - ); + event.waitUntil(installShell()); }); self.addEventListener("activate", (event) => { @@ -494,6 +490,56 @@ self.addEventListener("fetch", (event) => { } }); +// Fills this build's cache. A file an earlier build's cache holds is asked +// about by its tag, and that copy goes in when the server says it is +// unchanged, so an update downloads only what changed even when the browser +// no longer keeps its own copies. Each answer is stored as it arrives, since +// unread bodies hold their connections and would leave later requests waiting. +// A file that cannot be fetched discards the cache, leaving nothing half made. +async function installShell() { + const cache = await caches.open(CACHE_NAME); + const previousCaches = await Promise.all( + (await caches.keys()) + .filter((key) => key.startsWith(CACHE_PREFIX) && key !== CACHE_NAME) + .reverse() + .map((key) => caches.open(key)), + ); + try { + await Promise.all( + APP_SHELL_ASSETS.map(async (path) => { + await cache.put(path, await shellAssetResponse(path, previousCaches)); + }), + ); + } catch (error) { + await caches.delete(CACHE_NAME); + throw error; + } +} + +async function shellAssetResponse(path, previousCaches) { + let previous = null; + for (const previousCache of previousCaches) { + const copy = await previousCache.match(path); + if (copy?.headers.get("ETag")) { + previous = copy; + break; + } + } + const response = previous + ? await fetch(path, { + cache: "no-store", + headers: { "If-None-Match": previous.headers.get("ETag") }, + }) + : await fetch(path); + if (previous && response.status === 304) { + return previous; + } + if (!response.ok) { + throw new Error(`The app shell could not fetch ${path}: ${response.status}`); + } + return response; +} + async function activeShellFirst(request, cachePath) { const cache = await caches.open(CACHE_NAME); const cached = await cache.match(cachePath, { ignoreSearch: true }); diff --git a/frontend/service-worker.test.js b/frontend/service-worker.test.js index eca73c4d..9305583c 100644 --- a/frontend/service-worker.test.js +++ b/frontend/service-worker.test.js @@ -27,9 +27,20 @@ function createHarness(options = {}) { const claimCalls = []; const notifications = []; const openedWindows = []; - const cache = { - addAll: options.addAll ?? (async () => {}), - match: options.match ?? (async () => undefined), + const puts = []; + const cacheEntries = options.cacheEntries ?? { + "caffold-shell-test-build": new Map(), + }; + const openCache = (key) => { + cacheEntries[key] ??= new Map(); + const entries = cacheEntries[key]; + return { + match: options.match ?? (async (path) => entries.get(path)), + async put(path, response) { + puts.push({ cache: key, path, response }); + entries.set(path, response); + }, + }; }; const controlledClients = options.controlledClients ?? []; const allClients = options.allClients ?? controlledClients; @@ -44,10 +55,10 @@ function createHarness(options = {}) { deletedCaches.push(key); return true; }, - keys: async () => options.cacheKeys ?? ["caffold-shell-test-build"], + keys: async () => options.cacheKeys ?? Object.keys(cacheEntries), open: async (key) => { openedCaches.push(key); - return cache; + return openCache(key); }, }, clearTimeout: options.clearTimeout ?? clearTimeout, @@ -88,6 +99,7 @@ function createHarness(options = {}) { openedCaches, openedWindows, notifications, + puts, skipWaitingCalls, async dispatchExtendable(type, properties = {}) { let completion = Promise.resolve(); @@ -114,11 +126,12 @@ function createHarness(options = {}) { test("installs the complete shell without replacing the current worker", async () => { const gate = deferred(); - let installedAssets; + const fetched = []; const harness = createHarness({ - addAll: async (assets) => { - installedAssets = [...assets]; + fetch: async (path) => { + fetched.push(path); await gate.promise; + return new Response(`network ${path}`); }, }); @@ -129,6 +142,11 @@ test("installs the complete shell without replacing the current worker", async ( await completion; assert.equal(harness.skipWaitingCalls.length, 0); + const installedAssets = harness.puts.map(({ path }) => path); + assert.deepEqual([...installedAssets].sort(), [...fetched].sort()); + assert.ok( + harness.puts.every(({ cache }) => cache === "caffold-shell-test-build"), + ); assert.ok(installedAssets.includes("/assets/file-status.js")); assert.ok(installedAssets.includes("/assets/pages/foreground-recovery.js")); assert.ok( @@ -165,6 +183,124 @@ test("installs the complete shell without replacing the current worker", async ( ), ); }); + +function taggedResponse(body, tag) { + return new Response(body, { headers: { ETag: tag } }); +} + +function installedResponse(harness, path) { + return harness.puts.find( + (put) => put.cache === "caffold-shell-test-build" && put.path === path, + )?.response; +} + +test("copies a file the server calls unchanged from an earlier build's cache", async () => { + const requests = []; + const harness = createHarness({ + cacheEntries: { + "caffold-shell-older-build": new Map([ + ["/assets/app.js", taggedResponse("older app", '"app-older"')], + ]), + "caffold-shell-current-build": new Map([ + ["/assets/app.js", taggedResponse("current app", '"app-current"')], + ]), + "caffold-shell-test-build": new Map(), + }, + fetch: async (path, init) => { + requests.push({ path, init }); + return path === "/assets/app.js" + ? new Response(null, { status: 304 }) + : new Response(`network ${path}`); + }, + }); + + await harness.dispatchExtendable("install"); + + assert.equal(await installedResponse(harness, "/assets/app.js").text(), "current app"); + // The worker builds the request options in its own context. + assert.deepEqual( + JSON.parse(JSON.stringify(requests.filter(({ path }) => path === "/assets/app.js"))), + [{ + path: "/assets/app.js", + init: { cache: "no-store", headers: { "If-None-Match": '"app-current"' } }, + }], + ); + assert.equal( + await installedResponse(harness, "/assets/styles.css").text(), + "network /assets/styles.css", + ); +}); + +test("downloads a file the server says changed since the earlier build", async () => { + const harness = createHarness({ + cacheEntries: { + "caffold-shell-current-build": new Map([ + ["/assets/app.js", taggedResponse("current app", '"app-current"')], + ]), + "caffold-shell-test-build": new Map(), + }, + fetch: async (path) => + new Response(path === "/assets/app.js" ? "replacement app" : `network ${path}`), + }); + + await harness.dispatchExtendable("install"); + + assert.equal( + await installedResponse(harness, "/assets/app.js").text(), + "replacement app", + ); +}); + +test("downloads a file without asking when no earlier copy carries a tag", async () => { + const requests = []; + const harness = createHarness({ + cacheEntries: { + "caffold-shell-current-build": new Map([["/", new Response("current shell")]]), + "other-cache": new Map([ + ["/assets/app.js", taggedResponse("other app", '"app-other"')], + ]), + "caffold-shell-test-build": new Map([ + ["/assets/styles.css", taggedResponse("partial styles", '"styles-partial"')], + ]), + }, + fetch: async (path, init) => { + requests.push({ path, init }); + return new Response(`network ${path}`); + }, + }); + + await harness.dispatchExtendable("install"); + + for (const path of ["/", "/assets/app.js", "/assets/styles.css"]) { + assert.deepEqual(requests.filter((request) => request.path === path), [ + { path, init: undefined }, + ]); + assert.equal(await installedResponse(harness, path).text(), `network ${path}`); + } +}); + +test("discards the build's cache when any shell file cannot be fetched", async () => { + const harness = createHarness({ + cacheEntries: { + "caffold-shell-current-build": new Map([ + ["/assets/app.js", taggedResponse("current app", '"app-current"')], + ]), + "caffold-shell-test-build": new Map(), + }, + fetch: async (path) => { + if (path === "/assets/styles.css") { + return new Response("unavailable", { status: 503 }); + } + return path === "/assets/app.js" + ? new Response(null, { status: 304 }) + : new Response(`network ${path}`); + }, + }); + + await assert.rejects(harness.dispatchExtendable("install"), /\/assets\/styles\.css: 503/); + assert.deepEqual(harness.deletedCaches, ["caffold-shell-test-build"]); +}); + for (const [name, url, cachePath] of [ ["navigation", "https://caffold.test/tasks/83", "/"], ["asset", "https://caffold.test/assets/app.js", "/assets/app.js"], diff --git a/frontend/tests/contracts/service-worker.test.mjs b/frontend/tests/contracts/service-worker.test.mjs index 7228887c..3430fec9 100644 --- a/frontend/tests/contracts/service-worker.test.mjs +++ b/frontend/tests/contracts/service-worker.test.mjs @@ -16,7 +16,7 @@ const source = readFileSync( test("precaches every bundled runtime asset", async () => { let installListener; - let installedAssets; + const installedAssets = new Set(); const context = { AbortController, Promise, @@ -24,16 +24,22 @@ test("precaches every bundled runtime asset", async () => { Set, URL, caches: { + async keys() { + return []; + }, async open() { return { - async addAll(assets) { - installedAssets = new Set(assets); + async match() { + return undefined; + }, + async put(asset) { + installedAssets.add(asset); }, }; }, }, clearTimeout, - fetch, + fetch: async () => new Response(""), self: { addEventListener(type, listener) { if (type === "install") { diff --git a/frontend/tests/e2e/app-shell-update.spec.js b/frontend/tests/e2e/app-shell-update.spec.js index 6d952db3..6e85b635 100644 --- a/frontend/tests/e2e/app-shell-update.spec.js +++ b/frontend/tests/e2e/app-shell-update.spec.js @@ -12,6 +12,11 @@ const serviceWorkerSource = readFileSync( new URL("../../service-worker.js", import.meta.url), "utf8", ); +const appShellAssets = JSON.parse( + serviceWorkerSource + .match(/const APP_SHELL_ASSETS = (\[[\s\S]*?\]);/)[1] + .replace(/,\s*\]$/, "]"), +); const TEST_ACTIVATION_GATE_MESSAGE = "caffold:test-activation-gate"; const TEST_ACTIVATION_REQUEST_OBSERVED_MESSAGE = "caffold:test-activation-request-observed"; @@ -989,6 +994,52 @@ test("prepares and reloads the latest consecutive replacement through the real b } }); +test("fills a replacement from the current build's cache after the browser dropped its copies", { tag: "@viewport-independent" }, async ({ + page, +}, testInfo) => { + test.setTimeout(60_000); + const fixture = await startBuildLifecycleServer(testInfo.project.use.baseURL); + try { + await page.goto(fixture.origin); + await page.evaluate(() => navigator.serviceWorker.ready); + await page.reload(); + await expect.poll(() => activeControllerState(page)).toBe("activated"); + + const cdp = await page.context().newCDPSession(page); + await cdp.send("Network.clearBrowserCache"); + fixture.takeFileRequests(); + fixture.setBuild("browser-build-b", 2); + await openAboutWithoutReload(page); + await expect.poll(() => serviceWorkerDiagnostics(page), { timeout: 20_000 }).toMatchObject({ + waitingState: "installed", + }); + await expect.poll(() => cachedShellBuild(page, "browser-build-b")).toBe( + "browser-build-b", + ); + + const shellPaths = new Set(appShellAssets); + const filled = fixture.takeFileRequests().filter(({ path }) => shellPaths.has(path)); + expect(filled.map(({ path }) => path).sort()).toEqual([...shellPaths].sort()); + // Only the files that carry no tag are downloaded again. + expect( + filled.filter(({ status }) => status !== 304).map(({ path }) => path).sort(), + ).toEqual(["/", "/assets/build-info.js", "/assets/manifest.webmanifest"]); + expect( + filled.filter(({ status }) => status === 304).every(({ ifNoneMatch }) => ifNoneMatch), + ).toBe(true); + + const font = "/assets/fonts/Pretendard-Variable.woff2"; + const copiedBytes = await page.evaluate(async (path) => { + const cache = await caches.open("caffold-shell-browser-build-b"); + return (await (await cache.match(path)).arrayBuffer()).byteLength; + }, font); + const served = await fetch(new URL(font, testInfo.project.use.baseURL)); + expect(copiedBytes).toBe((await served.arrayBuffer()).byteLength); + } finally { + await fixture.close(); + } +}); + test("reloads through controllerchange without a custom acknowledgement or loop", { tag: "@all-viewports" }, async ({ page, }, testInfo) => { @@ -2044,6 +2095,7 @@ async function startBuildLifecycleServer(upstreamOrigin) { number: 1, omitControlledAck: false, }; + let fileRequests = []; const server = createServer(async (request, response) => { try { const url = new URL(request.url, "http://caffold.test"); @@ -2103,6 +2155,11 @@ async function startBuildLifecycleServer(upstreamOrigin) { return; } if (url.pathname === "/assets/build-info.js") { + fileRequests.push({ + path: url.pathname, + ifNoneMatch: request.headers["if-none-match"] ?? null, + status: 200, + }); response.writeHead(200, { "Content-Type": "text/javascript; charset=utf-8", }); @@ -2125,7 +2182,11 @@ async function startBuildLifecycleServer(upstreamOrigin) { return; } - const upstream = await fetch(new URL(request.url, upstreamOrigin)); + const ifNoneMatch = request.headers["if-none-match"] ?? null; + const upstream = await fetch(new URL(request.url, upstreamOrigin), { + headers: ifNoneMatch ? { "If-None-Match": ifNoneMatch } : {}, + }); + fileRequests.push({ path: url.pathname, ifNoneMatch, status: upstream.status }); const headers = Object.fromEntries(upstream.headers); delete headers["content-encoding"]; delete headers["content-length"]; @@ -2158,6 +2219,11 @@ async function startBuildLifecycleServer(upstreamOrigin) { setBuild(id, number, { omitControlledAck = false } = {}) { currentBuild = { id, number, omitControlledAck }; }, + takeFileRequests() { + const taken = fileRequests; + fileRequests = []; + return taken; + }, close() { return new Promise((resolve, reject) => { server.close((error) => error ? reject(error) : resolve());