diff --git a/tests/browser/workspace-activity.spec.mjs b/tests/browser/workspace-activity.spec.mjs index f89c606..2c6b072 100644 --- a/tests/browser/workspace-activity.spec.mjs +++ b/tests/browser/workspace-activity.spec.mjs @@ -68,6 +68,8 @@ test.beforeEach(async ({ page, request, browserName }) => { }); test.afterEach(async ({ page, request, browserName }) => { + // The store's own reads can meet a route handler at teardown; that must not fail the test it outlived. + await page.unrouteAll({ behavior: 'ignoreErrors' }); await saveCoverage(page, browserName); expect(page.activityErrors).toEqual([]); await request.post('/__test__/jobs/restart'); diff --git a/tests/browser/workspace.spec.mjs b/tests/browser/workspace.spec.mjs index 3a71a3d..e353121 100644 --- a/tests/browser/workspace.spec.mjs +++ b/tests/browser/workspace.spec.mjs @@ -48,6 +48,8 @@ test.beforeEach(async ({ page, request, browserName }) => { }); test.afterEach(async ({ page, browserName }) => { + // A route handler still running at teardown must not fail the test it outlived. + await page.unrouteAll({ behavior: 'ignoreErrors' }); await saveCoverage(page, browserName); expect(page.workspaceErrors).toEqual([]); }); @@ -688,8 +690,14 @@ async function serveUnderProxyMount(page) { await page.route(url => url.pathname.startsWith(`${PROXY_MOUNT}/`), async route => { const url = new URL(route.request().url()); url.pathname = url.pathname.slice(PROXY_MOUNT.length); - const response = await route.fetch({ url: url.href }); - await route.fulfill({ response }); + // A request still in flight when the test ends (a thumbnail the last route started) meets a page that is + // closing: its fetched response is disposed under it. Only that teardown error is ignored. + try { + const response = await route.fetch({ url: url.href }); + await route.fulfill({ response }); + } catch (error) { + if (!/has been disposed|has been closed|Target page, context or browser has been closed/.test(String(error?.message))) throw error; + } }); return escaped; } diff --git a/tests/js/workspace-activity-store.test.js b/tests/js/workspace-activity-store.test.js index 5ded899..cd6e367 100644 --- a/tests/js/workspace-activity-store.test.js +++ b/tests/js/workspace-activity-store.test.js @@ -281,3 +281,46 @@ test('summaries are validated and the copy follows the spec', async () => { assert.equal(outcomeLine(other), ''); assert.deepEqual([-1, 0, 1, 4, 5, 6, 1e9, 0.5].map(backoffDelay), [1000, 1000, 2000, 16000, 30000, 30000, 30000, 1000]); }); + +test('pagehide stops polling, closes streams and aborts the in-flight read; pageshow resumes', async () => { + const { createActivityStore } = await source; + const { world, api, ...deps } = harness(); + const lifecycle = new EventTarget(); + const signals = []; + let answer = null; + const pendingApi = { ...api, get: (url, options) => { + signals.push(options.signal); + world.gets += 1; + if (world.gets === 1) return Promise.resolve({ success: true, jobs: [summary()] }); + return new Promise((resolve, reject) => { + answer = resolve; + options.signal.addEventListener('abort', () => reject(Object.assign(new Error('aborted'), { name: 'AbortError' }))); + }); + } }; + const store = createActivityStore({ ...deps, api: pendingApi, pollMs: 15000, lifecycle }); + await store.load(); + assert.equal(world.open().length, 1); + const poll = world.timers.find(timer => !timer.cleared && timer.delay === 15000); + assert.ok(poll, 'a visible tab polls'); + // A read in flight when the page goes away (a closed tab, a test teardown) is aborted, nothing is scheduled again. + const read = store.load(); + lifecycle.dispatchEvent(new Event('pagehide')); + await read; + assert.equal(signals.at(-1).aborted, true); + assert.equal(world.open().length, 0); + assert.ok(world.timers.every(timer => timer.cleared || timer.fired)); + assert.equal(store.find(ID_A).state, 'running', 'a hidden page marks nothing lost'); + await store.load(); + assert.equal(world.gets, 2, 'a suspended store reads nothing'); + // Back from the cache, it reads the list again and follows the job. + const resumed = Object.assign(new Event('pageshow'), { persisted: true }); + lifecycle.dispatchEvent(resumed); + await new Promise(resolve => setImmediate(resolve)); + assert.equal(world.gets, 3); + answer({ success: true, jobs: [summary()] }); + await new Promise(resolve => setImmediate(resolve)); + assert.equal(world.open().length, 1); + world.dispose(); + assert.equal(signals.at(-1).aborted, true, 'disposing aborts the store reads'); + assert.equal(world.open().length, 0); +}); diff --git a/web/js/workspace/activity-store.mjs b/web/js/workspace/activity-store.mjs index 259cc8d..3a248a1 100644 --- a/web/js/workspace/activity-store.mjs +++ b/web/js/workspace/activity-store.mjs @@ -101,10 +101,15 @@ function defaultConnect(url) { * @param {{api: {get: Function, post: Function}, scope: {signal: AbortSignal, cleanup: Function}, * connect?: (url: string) => {onmessage: Function|null, onerror: Function|null, close: Function}, * notify?: (message: string) => void, timers?: {setTimeout: Function, clearTimeout: Function}, - * pollMs?: number, isVisible?: () => boolean}} options + * pollMs?: number, isVisible?: () => boolean, lifecycle?: EventTarget}} options + * + * `lifecycle` (the window) ends the store's work on `pagehide`: the periodic and backoff reads stop, the streams close + * and a read in flight is aborted, so nothing answers into a page that is going away (a closed tab, a test teardown). + * A `pageshow` from the back-forward cache reads the list again. */ export function createActivityStore({ api, scope, connect = defaultConnect, notify = showWorkspaceToast, - timers = globalThis, pollMs = POLL_MS, isVisible = () => globalThis.document?.visibilityState !== 'hidden' } = {}) { + timers = globalThis, pollMs = POLL_MS, isVisible = () => globalThis.document?.visibilityState !== 'hidden', + lifecycle = globalThis.window ?? null } = {}) { let jobs = []; let status = 'idle'; let attempt = 0; @@ -112,6 +117,8 @@ export function createActivityStore({ api, scope, connect = defaultConnect, noti let pollTimer = null; let reads = 0; let disposed = false; + let suspended = false; + let reading = null; const streams = new Map(); const listeners = new Set(); @@ -181,7 +188,7 @@ export function createActivityStore({ api, scope, connect = defaultConnect, noti } function scheduleResync() { - if (disposed || retryTimer !== null) return; + if (disposed || suspended || retryTimer !== null) return; const delay = backoffDelay(attempt); attempt += 1; retryTimer = timers.setTimeout(() => { @@ -192,7 +199,7 @@ export function createActivityStore({ api, scope, connect = defaultConnect, noti /** The next periodic read; a hidden tab skips its turn. */ function schedulePoll() { - if (disposed || pollMs <= 0) return; + if (disposed || suspended || pollMs <= 0) return; if (pollTimer !== null) timers.clearTimeout(pollTimer); pollTimer = timers.setTimeout(() => { pollTimer = null; @@ -203,18 +210,22 @@ export function createActivityStore({ api, scope, connect = defaultConnect, noti /** Read this session's jobs. A failed read marks running jobs ended and tries again after the backoff. */ async function load() { - if (disposed) return; + if (disposed || suspended) return; const read = ++reads; + // Only the newest read applies, so an older one still in flight is aborted. + reading?.abort(); + const controller = new AbortController(); + reading = controller; try { - const result = await api.get(JOBS_URL, { signal: scope.signal, cache: 'no-store' }); - // Only the newest read applies; an older one answering late is dropped. - if (disposed || read !== reads) return; + const result = await api.get(JOBS_URL, { signal: controller.signal, cache: 'no-store' }); + // An older read answering late is dropped. + if (disposed || suspended || read !== reads) return; if (!Array.isArray(result?.jobs)) throw new TypeError('Invalid job list'); merge(result.jobs); status = 'ready'; schedulePoll(); } catch (error) { - if (disposed || scope.signal?.aborted || read !== reads) return; + if (disposed || suspended || scope.signal?.aborted || controller.signal.aborted || read !== reads) return; markLost(); status = 'error'; scheduleResync(); @@ -254,13 +265,30 @@ export function createActivityStore({ api, scope, connect = defaultConnect, noti }, }; - scope.cleanup(() => { - disposed = true; + /** Stop every timer, stream and read in flight. */ + function halt() { if (retryTimer !== null) timers.clearTimeout(retryTimer); if (pollTimer !== null) timers.clearTimeout(pollTimer); retryTimer = null; pollTimer = null; + reading?.abort(); + reading = null; for (const id of [...streams.keys()]) closeStream(id); + } + const onPageHide = () => { suspended = true; halt(); }; + const onPageShow = event => { + if (!suspended || !event.persisted) return; + suspended = false; + load(); + }; + lifecycle?.addEventListener('pagehide', onPageHide); + lifecycle?.addEventListener('pageshow', onPageShow); + + scope.cleanup(() => { + disposed = true; + halt(); + lifecycle?.removeEventListener('pagehide', onPageHide); + lifecycle?.removeEventListener('pageshow', onPageShow); listeners.clear(); }); return store;