From 08480992f30bf8e839393fd9b588c5b6bea90254 Mon Sep 17 00:00:00 2001 From: Nawid Salehie <102778966+Nawid3333@users.noreply.github.com> Date: Sat, 3 Oct 2026 15:46:29 +0200 Subject: [PATCH 01/15] fix(storage): store fragments in OPFS at all, survive a write backlog, catch short writes - Every downloaded fragment's identifier is its URL and range ("https://host/seg.ts::0-100::arraybuffer"), and Firefox refuses an OPFS name with a "/" (IsValidName, dom/fs/shared/FileSystemHelpers.cpp). So no fragment was ever stored: each write failed, was logged, and the fragment stayed in RAM. OPFSManager now gives each identifier a short file name of its own (fileName) and the worker only sees those (#132). - The 30 s worker timeout ran from when a call was posted. The worker answers one call at a time, so a backlog of writes longer than that (a whole video downloading to a slow disk) counted as a crash and OPFS was dropped for the session. One watchdog now measures how long the worker has gone without answering any call, and no timer is left per call (#135). - FileSystemSyncAccessHandle.write() reports a write that failed part-way (disk or quota full) only through the byte count it returns; Gecko never throws for it. Nothing checked it, so a fragment could be stored cut short and a save go on with a hole. Every write now checks the count; a fragment that did not fit is removed and kept in RAM, and a save fails (#138). Proven in tests/unit/opfsStorage.test.mjs, which now runs the real worker on a stand-in OPFS that applies Firefox's name rule and can run out of space: with the old OPFSManager the URL-named fragment and the backlog cases fail, and with the count check removed the three short-write cases fail. storage.e2e.mjs round-trips a URL-named fragment in real Firefox. Co-Authored-By: Claude Opus 5.5 --- chrome/player/network/OPFSManager.mjs | 87 +++++++-- chrome/player/network/opfs-worker.mjs | 31 +++- tests/e2e/specs/storage.e2e.mjs | 77 ++++---- tests/unit/opfsStorage.test.mjs | 245 +++++++++++++++++++++++--- 4 files changed, 363 insertions(+), 77 deletions(-) diff --git a/chrome/player/network/OPFSManager.mjs b/chrome/player/network/OPFSManager.mjs index 1a540d616..40a65383c 100644 --- a/chrome/player/network/OPFSManager.mjs +++ b/chrome/player/network/OPFSManager.mjs @@ -12,7 +12,7 @@ export class OPFSManager { */ static live = new Set(); - /** How long a worker call may take before the worker counts as crashed. */ + /** How long the worker may go without answering while calls wait, before it counts as crashed. */ static CallTimeoutMs = 30000; constructor() { @@ -20,6 +20,29 @@ export class OPFSManager { this.pending = new Map(); this.nextId = 0; this.sessionName = null; + // identifier -> its name in the session directory (fileName). + this.fileNames = new Map(); + this.fileCount = 0; + this.watchdog = null; + } + + /** + * The name a stored identifier has in the session directory; the worker only ever sees + * these. A download's identifier is its URL with its range + * ('https://host/seg.ts::0-100::arraybuffer'), and an OPFS name must not hold a '/' (or a + * '\' on Windows): Firefox refused every such name with a TypeError, so no fragment was + * ever stored here, and every one stayed in RAM. + * @param {string} identifier + * @param {boolean} [create] - name an identifier that has no name yet + * @return {?string} null for an identifier with no name + */ + fileName(identifier, create = false) { + let name = this.fileNames.get(identifier); + if (name === undefined && create) { + name = 'f' + this.fileCount++; + this.fileNames.set(identifier, name); + } + return name ?? null; } /** @return {Array<{id: number, op: string, ms: number}>} Calls still waiting on the worker. */ @@ -71,6 +94,12 @@ export class OPFSManager { const pending = this.pending.get(msg.id); if (!pending) return; this.pending.delete(msg.id); + // An answer is progress: the calls still waiting get the full time again. + if (this.pending.size) { + this.armWatchdog(); + } else { + clearTimeout(this.watchdog); + } if (msg.ok) { pending.resolve(msg.result); } else { @@ -86,19 +115,22 @@ export class OPFSManager { * @return {Promise} */ async saveBegin(identifier) { - await this.call('saveBegin', {identifier}); + await this.call('saveBegin', {identifier: this.fileName(identifier, true)}); } async saveAppend(identifier, chunk) { - await this.call('saveAppend', {identifier, data: chunk}, [chunk.buffer]); + await this.call('saveAppend', {identifier: this.fileName(identifier), data: chunk}, [chunk.buffer]); } async saveEnd(identifier) { - await this.call('saveEnd', {identifier}); + await this.call('saveEnd', {identifier: this.fileName(identifier)}); } async saveAbort(identifier) { - await this.call('saveAbort', {identifier}); + const name = this.fileName(identifier); + if (name === null) return; + this.fileNames.delete(identifier); + await this.call('saveAbort', {identifier: name}); } /** @@ -112,10 +144,14 @@ export class OPFSManager { if (!this.sessionName) { throw new Error('OPFS session not initialized'); } + const name = this.fileName(identifier); + if (name === null) { + throw new Error('No OPFS file for ' + identifier); + } const root = await navigator.storage.getDirectory(); const fsblobRoot = await root.getDirectoryHandle('fsblob'); const sessionDir = await fsblobRoot.getDirectoryHandle(this.sessionName); - const fileHandle = await sessionDir.getFileHandle(identifier); + const fileHandle = await sessionDir.getFileHandle(name); return fileHandle.getFile(); } @@ -127,6 +163,7 @@ export class OPFSManager { * always mean the worker stopped running) and never replies. */ handleWorkerCrash(event) { + clearTimeout(this.watchdog); const error = new Error(event.message || 'OPFS worker crashed'); for (const {reject} of this.pending.values()) { reject(error); @@ -149,22 +186,34 @@ export class OPFSManager { } const id = this.nextId++; return new Promise((resolve, reject) => { + // The wait starts now only if the worker had nothing else to answer (armWatchdog). + if (this.pending.size === 0) this.armWatchdog(); this.pending.set(id, {resolve, reject, op, started: Date.now()}); this.worker.postMessage({id, op, ...payload}, transfer || []); - // A worker that never answers (wedged, or killed without an error event) held up - // setup and every save for good. Past the limit it counts as crashed: every call - // fails, and FSBlob moves on to its next backend. - setTimeout(() => { - if (this.pending.has(id)) { - this.handleWorkerCrash({message: `OPFS worker did not answer ${op} in ${OPFSManager.CallTimeoutMs / 1000} s`}); - } - }, OPFSManager.CallTimeoutMs); }); } + /** + * A worker that never answers (wedged, or killed without an error event) held up setup + * and every save for good. Past the limit it counts as crashed: every call fails, and + * FSBlob moves on to its next backend. The worker answers its calls one at a time, in + * order, so the limit is on how long it goes without answering any, not on how long one + * call has waited: that is the queue ahead of it, and a 30 s backlog of writes (a whole + * video downloading to a slow disk) counted as a crash, which dropped OPFS for the session. + */ + armWatchdog() { + clearTimeout(this.watchdog); + this.watchdog = setTimeout(() => { + const [oldest] = this.pending.values(); + if (oldest) { + this.handleWorkerCrash({message: `OPFS worker did not answer ${oldest.op} in ${OPFSManager.CallTimeoutMs / 1000} s`}); + } + }, OPFSManager.CallTimeoutMs); + } + async setFile(identifier, blob) { const buffer = await blob.arrayBuffer(); - await this.call('set', {identifier, data: buffer}, [buffer]); + await this.call('set', {identifier: this.fileName(identifier, true), data: buffer}, [buffer]); } /** @@ -178,10 +227,15 @@ export class OPFSManager { } async deleteFile(identifier) { - await this.call('delete', {identifier}); + const name = this.fileName(identifier); + if (name === null) return; + this.fileNames.delete(identifier); + await this.call('delete', {identifier: name}); } async clearStorage() { + // The worker deletes every file in the session. Names are not used again. + this.fileNames.clear(); await this.call('clear'); } @@ -203,6 +257,7 @@ export class OPFSManager { this.worker.terminate(); } this.worker = null; + clearTimeout(this.watchdog); OPFSManager.live.delete(this); for (const {reject} of this.pending.values()) { reject(new Error('OPFSManager closed')); diff --git a/chrome/player/network/opfs-worker.mjs b/chrome/player/network/opfs-worker.mjs index 1eefa596e..543d9e586 100644 --- a/chrome/player/network/opfs-worker.mjs +++ b/chrome/player/network/opfs-worker.mjs @@ -6,7 +6,8 @@ import {OpQueue} from './OpQueue.mjs'; // filesystem operation, including the heartbeat write, goes through a single // OpQueue: opening two FileSystemSyncAccessHandles on the same file throws, // and every identifier here is always a one-shot whole-blob get/set, so -// serializing is simpler and cheaper than per-identifier locking. +// serializing is simpler and cheaper than per-identifier locking. The +// identifiers it gets are file names OPFSManager.fileName() chose. const STALE_MS = 10000; // matches IndexedDBManager's own staleness window const HEARTBEAT_MS = 1000; @@ -97,13 +98,29 @@ async function prune(ownName) { })); } +/** + * Writes all of `data` at `at`. A sync access handle's write() answers with the bytes it + * wrote, and Firefox reports a write that failed part-way (the disk or the quota full) only + * through that count, never with an error: a fragment was stored cut short, or a save went + * on with a hole in it, and nothing said so. + * @param {FileSystemSyncAccessHandle} accessHandle + * @param {ArrayBuffer|ArrayBufferView} data + * @param {number} at + */ +function writeAll(accessHandle, data, at) { + const written = accessHandle.write(data, {at}); + if (written !== data.byteLength) { + throw new Error(`OPFS wrote ${written} of ${data.byteLength} bytes`); + } +} + /** Overwrites this session's heartbeat marker with the current time. */ async function writeHeartbeat() { const handle = await sessionDir.getFileHandle(META_FILE, {create: true}); const accessHandle = await handle.createSyncAccessHandle(); try { const bytes = new TextEncoder().encode(JSON.stringify({updated_time: Date.now()})); - accessHandle.write(bytes, {at: 0}); + writeAll(accessHandle, bytes, 0); accessHandle.truncate(bytes.byteLength); accessHandle.flush(); } finally { @@ -143,12 +160,16 @@ async function setFile(identifier, data) { const fileHandle = await sessionDir.getFileHandle(identifier, {create: true}); const accessHandle = await fileHandle.createSyncAccessHandle(); try { - accessHandle.write(data, {at: 0}); + writeAll(accessHandle, data, 0); accessHandle.truncate(data.byteLength); accessHandle.flush(); - } finally { + } catch (e) { accessHandle.close(); + // What was written of it only takes up the space that ran out. + await sessionDir.removeEntry(identifier).catch(() => {}); + throw e; } + accessHandle.close(); } async function getFile(identifier) { @@ -217,7 +238,7 @@ async function saveAppend(identifier, data) { if (!stream) { throw new Error('No open save stream: ' + identifier); } - stream.handle.write(data, {at: stream.offset}); + writeAll(stream.handle, data, stream.offset); stream.offset += data.byteLength; } diff --git a/tests/e2e/specs/storage.e2e.mjs b/tests/e2e/specs/storage.e2e.mjs index f4b72d451..277987d36 100644 --- a/tests/e2e/specs/storage.e2e.mjs +++ b/tests/e2e/specs/storage.e2e.mjs @@ -56,46 +56,55 @@ describe('FSBlob storage backends', function() { const blobStore = new FSBlob(); const payload = new Uint8Array([10, 20, 30, 40, 50]); - const identifier = await blobStore.saveBlobAsync(new Blob([payload])); + // A name FSBlob makes up, and the identifier DownloadManager gives a downloaded + // fragment: its URL and range. OPFS refuses a name with a '/', and the second kind + // was never stored at all - every fragment stayed in RAM. + const identifiers = [ + await blobStore.saveBlobAsync(new Blob([payload])), + await blobStore.saveBlobAsync(new Blob([payload]), 'https://cdn.example/v/seg-1.ts::0-100::arraybuffer'), + ]; // FSBlob does not choose OPFS wherever the API merely exists: a // private window claims it and then refuses. Check what it actually // picked rather than OPFSManager.isSupported(), or this looks for a // fsblob/ OPFS directory that was never created. const usedOPFS = !!blobStore.opfsManager; - - // Verify independently of FSBlob's own bookkeeping: look directly at - // OPFS for a fsblob// file with the right bytes. - let sawOnDisk = false; - // In this store's own session directory: another session's file of the same name - // (an earlier test's, or another tab's) proves nothing about this one. - if (usedOPFS) { - try { - const root = await navigator.storage.getDirectory(); - const fsblobRoot = await root.getDirectoryHandle('fsblob'); - const sessionDir = await fsblobRoot.getDirectoryHandle(blobStore.opfsManager.sessionName); - const fileHandle = await sessionDir.getFileHandle(identifier); - const file = await fileHandle.getFile(); - const bytes = new Uint8Array(await file.arrayBuffer()); - sawOnDisk = bytes.length === payload.length && bytes.every((b, i) => b === payload[i]); - } catch (e) { - // Not there. + const matches = (bytes) => bytes.length === payload.length && bytes.every((b, i) => b === payload[i]); + + const results = []; + for (const identifier of identifiers) { + // Verify independently of FSBlob's own bookkeeping: look directly at + // OPFS for a fsblob// file with the right bytes. + let sawOnDisk = false; + // In this store's own session directory: another session's file of the same name + // (an earlier test's, or another tab's) proves nothing about this one. + const name = usedOPFS ? blobStore.opfsManager.fileName(identifier) : null; + if (name) { + try { + const root = await navigator.storage.getDirectory(); + const fsblobRoot = await root.getDirectoryHandle('fsblob'); + const sessionDir = await fsblobRoot.getDirectoryHandle(blobStore.opfsManager.sessionName); + const fileHandle = await sessionDir.getFileHandle(name); + const file = await fileHandle.getFile(); + sawOnDisk = matches(new Uint8Array(await file.arrayBuffer())); + } catch (e) { + // Not there. + } } - } - // What the store hands back is the file on disk (a File named after its - // identifier), not a copy in RAM: reading each fragment back through the worker - // into a new Blob kept every "offloaded" fragment in memory too. - const stored = blobStore.getBlob(identifier); - const readBack = new Uint8Array(await stored.arrayBuffer()); + // What the store hands back is the file on disk (a File with that name), not a + // copy in RAM: reading each fragment back through the worker into a new Blob kept + // every "offloaded" fragment in memory too. + const stored = blobStore.getBlob(identifier); + results.push({ + sawOnDisk, + diskBacked: stored instanceof File && !!name && stored.name === name, + readBackMatches: matches(new Uint8Array(await stored.arrayBuffer())), + }); + } blobStore.close(); - return { - usedOPFS, - sawOnDisk, - diskBacked: stored instanceof File && stored.name === identifier, - readBackMatches: readBack.length === payload.length && readBack.every((b, i) => b === payload[i]), - }; + return {usedOPFS, results}; }); console.log(' opfs backend:', JSON.stringify(result)); @@ -103,9 +112,11 @@ describe('FSBlob storage backends', function() { // a browser where FSBlob chose Cache/IndexedDB instead, it correctly // stays false rather than being checked against a directory that was // never supposed to exist. - expect(result.sawOnDisk).toBe(result.usedOPFS); - expect(result.diskBacked).toBe(result.usedOPFS); - expect(result.readBackMatches).toBe(true); + for (const stored of result.results) { + expect(stored.sawOnDisk).toBe(result.usedOPFS); + expect(stored.diskBacked).toBe(result.usedOPFS); + expect(stored.readBackMatches).toBe(true); + } }); it('gives two players starting together on a fresh profile the same salts', async function() { diff --git a/tests/unit/opfsStorage.test.mjs b/tests/unit/opfsStorage.test.mjs index bd4b2ec2f..18477cdb7 100644 --- a/tests/unit/opfsStorage.test.mjs +++ b/tests/unit/opfsStorage.test.mjs @@ -11,6 +11,10 @@ import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest'; // - Two players starting together: one's prune deleted the other's brand-new session // directory (made before its first heartbeat). // - clear() while an offload ran: the blob came back after the clear, and stayed. +// - A fragment's identifier is its URL, and OPFS refuses a name with a '/': no fragment +// was ever stored, every one stayed in RAM. +// - A backlog of writes longer than the call timeout counted as a crashed worker. +// - A write the disk took only part of was taken for a whole one. vi.mock('../../chrome/player/network/IndexedDBManager.mjs', () => ({ IndexedDBManager: class { @@ -25,20 +29,49 @@ vi.mock('../../chrome/player/modules/Localize.mjs', () => ({Localize: {getMessag const {OPFSManager} = await import('../../chrome/player/network/OPFSManager.mjs'); const {FSBlob} = await import('../../chrome/player/modules/FSBlob.mjs'); +/** How many more bytes the stand-in disk takes; a write past it is cut short. */ +let diskRoom = Infinity; + +/** + * Firefox's rule for a file or directory name (IsValidName in + * dom/fs/shared/FileSystemHelpers.cpp, as built for Windows): anything else is a TypeError. + * @param {string} name + */ +function checkName(name) { + if (typeof name !== 'string' || name === '' || name === '.' || name === '..' || + name.includes('/') || name.includes('\\')) { + throw new TypeError('Invalid name: ' + name); + } +} + /** A file in the stand-in OPFS. */ class FakeFile { - constructor(text = '') { + constructor(text = '', name = '') { this.bytes = new TextEncoder().encode(text); this.unreadable = false; + // The heartbeat is left out of diskRoom: it is written every second, whenever. + this.isHeartbeat = name === '_meta.json'; } async createSyncAccessHandle() { return { - write: (data) => { - this.bytes = new Uint8Array(data.buffer ? data.buffer.slice(0) : data); - return this.bytes.byteLength; + // As Firefox's: what the disk did not take is left out of the count, not thrown. + write: (data, {at = 0} = {}) => { + const source = data instanceof ArrayBuffer ? new Uint8Array(data) : + new Uint8Array(data.buffer, data.byteOffset, data.byteLength); + const count = this.isHeartbeat ? source.byteLength : Math.min(source.byteLength, diskRoom); + if (!this.isHeartbeat) diskRoom -= count; + if (at + count > this.bytes.byteLength) { + const grown = new Uint8Array(at + count); + grown.set(this.bytes); + this.bytes = grown; + } + this.bytes.set(source.subarray(0, count), at); + return count; }, truncate: (n) => { - this.bytes = this.bytes.subarray(0, n); + const cut = new Uint8Array(n); + cut.set(this.bytes.subarray(0, n)); + this.bytes = cut; }, flush: () => {}, close: () => {}, @@ -56,6 +89,7 @@ class FakeDir { this.children = new Map(); } async getDirectoryHandle(name, {create = false} = {}) { + checkName(name); if (!this.children.has(name)) { if (!create) throw new Error('NotFoundError: ' + name); this.children.set(name, new FakeDir()); @@ -65,13 +99,15 @@ class FakeDir { return child; } async getFileHandle(name, {create = false} = {}) { + checkName(name); if (!this.children.has(name)) { if (!create) throw new Error('NotFoundError: ' + name); - this.children.set(name, new FakeFile()); + this.children.set(name, new FakeFile('', name)); } return this.children.get(name); } async removeEntry(name) { + checkName(name); if (!this.children.delete(name)) throw new Error('NotFoundError: ' + name); } async* keys() { @@ -79,7 +115,57 @@ class FakeDir { } } +/** + * A fresh copy of the real opfs-worker.mjs on the stand-in OPFS, run in this thread: what + * is posted to the returned worker reaches it, and its answers go to `answer`. + * @param {FakeDir} root + * @return {Promise} the worker, as OPFSManager uses one + */ +async function startWorker(root) { + let onMessage; + const worker = { + answer: () => {}, + postMessage: (message) => queueMicrotask(() => onMessage({data: message})), + terminate: vi.fn(), + }; + vi.stubGlobal('self', { + addEventListener: (type, listener) => { + onMessage = listener; + }, + postMessage: (message) => worker.answer(message), + }); + vi.stubGlobal('navigator', {storage: {getDirectory: async () => root}}); + vi.resetModules(); + await import('../../chrome/player/network/opfs-worker.mjs'); + return worker; +} + +/** + * An OPFSManager whose worker is the real one on the stand-in OPFS, set up as setup() does. + * @param {FakeDir} root + * @return {Promise} + */ +async function startManager(root) { + const worker = await startWorker(root); + const manager = new OPFSManager(); + manager.worker = worker; + worker.answer = (message) => manager.handleMessage(message); + manager.sessionName = (await manager.call('init')).sessionName; + return manager; +} + +/** + * The session directory a manager writes to. + * @param {FakeDir} root + * @param {OPFSManager} manager + * @return {FakeDir} + */ +function sessionOf(root, manager) { + return root.children.get('fsblob').children.get(manager.sessionName); +} + afterEach(() => { + diskRoom = Infinity; vi.useRealTimers(); vi.unstubAllGlobals(); vi.restoreAllMocks(); @@ -89,15 +175,16 @@ describe('OPFSManager', () => { it('gives a stored fragment back as the file on disk, not a copy in RAM', async () => { const root = new FakeDir(); const session = await (await root.getDirectoryHandle('fsblob', {create: true})).getDirectoryHandle('fsblob-1-1', {create: true}); - session.children.set('blob0', new FakeFile('fragment bytes')); + session.children.set('f0', new FakeFile('fragment bytes')); vi.stubGlobal('navigator', {storage: {getDirectory: async () => root}}); const manager = new OPFSManager(); manager.sessionName = 'fsblob-1-1'; manager.worker = {postMessage: vi.fn()}; + manager.fileNames.set('blob0', 'f0'); const file = await manager.getFile('blob0'); - expect(file.disk).toBe(session.children.get('blob0')); + expect(file.disk).toBe(session.children.get('f0')); expect(manager.worker.postMessage).not.toHaveBeenCalled(); }); @@ -127,6 +214,45 @@ describe('OPFSManager', () => { expect(await manager.call('set', {identifier: 'blob0'})).toBe('done'); await vi.advanceTimersByTimeAsync(OPFSManager.CallTimeoutMs * 2); expect(manager.worker).not.toBeNull(); + // And no timer is left behind for it. + expect(vi.getTimerCount()).toBe(0); + }); + + it('keeps a worker that is working through a backlog longer than the timeout', async () => { + // It answers one call at a time, in order, each well within the limit; the last one + // waits three times the limit in all, behind the others. + vi.useFakeTimers(); + const manager = new OPFSManager(); + const posted = []; + manager.worker = {postMessage: (message) => posted.push(message), terminate: vi.fn()}; + const step = OPFSManager.CallTimeoutMs * 0.75; + const calls = Array.from({length: 4}, (_, i) => manager.call('set', {identifier: 'f' + i})); + + for (const message of posted) { + await vi.advanceTimersByTimeAsync(step); + manager.handleMessage({id: message.id, ok: true}); + } + + await expect(Promise.all(calls)).resolves.toHaveLength(4); + expect(manager.worker.terminate).not.toHaveBeenCalled(); + expect(manager.worker).not.toBeNull(); + expect(vi.getTimerCount()).toBe(0); + }); + + it('still gives up when the worker stops answering in the middle of a backlog', async () => { + vi.useFakeTimers(); + const manager = new OPFSManager(); + const posted = []; + const worker = {postMessage: (message) => posted.push(message), terminate: vi.fn()}; + manager.worker = worker; + const calls = [0, 1, 2].map((i) => manager.call('set', {identifier: 'f' + i}).then(() => 'answered', (e) => e.message)); + + await vi.advanceTimersByTimeAsync(1000); + manager.handleMessage({id: posted[0].id, ok: true}); + await vi.advanceTimersByTimeAsync(OPFSManager.CallTimeoutMs); + + expect(await Promise.all(calls)).toEqual(['answered', expect.stringMatching(/did not answer set/), expect.stringMatching(/did not answer set/)]); + expect(worker.terminate).toHaveBeenCalled(); }); }); @@ -155,29 +281,62 @@ describe('opfs-worker: cleaning up other tabs\' sessions', () => { // A live one. session(`fsblob-${now - 60000}-5`, heartbeat(now - 2000)); - let onMessage; - const replies = []; - vi.stubGlobal('self', { - addEventListener: (type, listener) => { - onMessage = listener; - }, - postMessage: (message) => replies.push(message), - }); - vi.stubGlobal('navigator', {storage: {getDirectory: async () => root}}); - await import('../../chrome/player/network/opfs-worker.mjs'); - - await onMessage({data: {id: 1, op: 'init'}}); + const manager = await startManager(root); try { - expect(replies[0]).toMatchObject({id: 1, ok: true}); const left = [...fsblob.children.keys()].sort(); expect(left).toEqual([ `fsblob-${now - 50}-1`, `fsblob-${now - 60000}-4`, `fsblob-${now - 60000}-5`, - replies[0].result.sessionName, + manager.sessionName, ].sort()); } finally { - await onMessage({data: {id: 2, op: 'destroy'}}); + await manager.close(); + } + }); +}); + +describe('opfs-worker: writes', () => { + it('fails a fragment the disk took only part of, and leaves nothing of it behind', async () => { + const root = new FakeDir(); + const manager = await startManager(root); + try { + diskRoom = 3; + const blob = new Blob([new Uint8Array([1, 2, 3, 4, 5])]); + await expect(manager.setFile('blob0', blob)).rejects.toThrow(/wrote 3 of 5 bytes/); + expect([...sessionOf(root, manager).children.keys()]).toEqual(['_meta.json']); + } finally { + diskRoom = Infinity; + await manager.close(); + } + }); + + it('fails a save whose chunk the disk took only part of', async () => { + const root = new FakeDir(); + const manager = await startManager(root); + try { + await manager.saveBegin('save-1'); + await manager.saveAppend('save-1', new Uint8Array([1, 2, 3])); + diskRoom = 1; + await expect(manager.saveAppend('save-1', new Uint8Array([4, 5, 6]))).rejects.toThrow(/wrote 1 of 3 bytes/); + } finally { + diskRoom = Infinity; + await manager.close(); + } + }); + + it('writes a whole save, chunk after chunk, into one file', async () => { + const root = new FakeDir(); + const manager = await startManager(root); + try { + await manager.saveBegin('save-1'); + await manager.saveAppend('save-1', new Uint8Array([1, 2, 3])); + await manager.saveAppend('save-1', new Uint8Array([4, 5])); + await manager.saveEnd('save-1'); + const file = await manager.getSavedFile('save-1'); + expect([...file.disk.bytes]).toEqual([1, 2, 3, 4, 5]); + } finally { + await manager.close(); } }); }); @@ -200,8 +359,11 @@ describe('FSBlob', () => { delete: async (url) => cached.delete(url), }; vi.stubGlobal('window', {caches: {open: async () => cache, delete: async () => true}}); + const storage = globalThis.navigator?.storage; vi.stubGlobal('navigator', {}); const fsblob = new FSBlob(); + // The OPFS manager reads the directory from here. + vi.stubGlobal('navigator', storage ? {storage} : {}); fsblob.remainingBackends = ['cache']; fsblob.opfsManager = opfsManager; fsblob.cache = null; @@ -209,6 +371,43 @@ describe('FSBlob', () => { return {fsblob, cached}; } + it('stores a downloaded fragment, named by its URL, on disk and deletes it again', async () => { + const root = new FakeDir(); + const manager = await startManager(root); + const {fsblob} = withOpfs(manager); + try { + const identifier = 'https://cdn.example/video/720p/seg-1.ts::0-1000000::arraybuffer'; + await fsblob.saveBlobAsync(new Blob([new Uint8Array([7, 8, 9])]), identifier); + + // The file on disk, not the Blob in RAM it was given. + const stored = fsblob.getBlob(identifier); + expect(stored.disk).toBeDefined(); + expect([...stored.disk.bytes]).toEqual([7, 8, 9]); + expect(console.warn).not.toHaveBeenCalled(); + + await fsblob.deleteBlob(identifier); + expect([...sessionOf(root, manager).children.keys()]).toEqual(['_meta.json']); + } finally { + await manager.close(); + } + }); + + it('keeps a fragment in RAM that the disk took only part of', async () => { + const root = new FakeDir(); + const manager = await startManager(root); + const {fsblob} = withOpfs(manager); + try { + diskRoom = 2; + const blob = new Blob([new Uint8Array([7, 8, 9])]); + await fsblob.saveBlobAsync(blob, 'blob0'); + expect(fsblob.getBlob('blob0')).toBe(blob); + expect(fsblob.opfsManager).toBe(manager); + } finally { + diskRoom = Infinity; + await manager.close(); + } + }); + it('moves on to the next backend once the OPFS worker is gone', async () => { const dead = { worker: null, From 0e16175fcb404615c6725fe2d3d616f31c961da5 Mon Sep 17 00:00:00 2001 From: Nawid Salehie <102778966+Nawid3333@users.noreply.github.com> Date: Sat, 3 Oct 2026 15:48:10 +0200 Subject: [PATCH 02/15] fix(save): keep a streamed save's file until its download is over A streamed save (accelerated MP4, a direct video, an archive) writes its file into its own OPFS session and hands Firefox a URL to it. The session was closed two minutes after the save ended, whatever the download was doing, and a closed session is deleted by the next player or save that starts (10 s after its last heartbeat). So a download longer than that - a big video to a slow disk - could lose its file midway, with no message. Utils.revokeWhenDownloaded now says (a promise) when the download is over, and StreamSaver closes the store then, never sooner than the old two minutes, which is still all a download without an id to follow gets. Both sinks (OPFS and memory) do the same. tests/unit/StreamSaver.test.mjs: with the old timer the store closes at two minutes of a ten-minute download (fails); without the floor it closes right after a quick download (fails). revokeWhenDownloaded.test.mjs checks the promise. Part of #134: merged saves (DASH, HLS) still close their store on the timer. Co-Authored-By: Claude Opus 5.5 --- chrome/player/modules/StreamSaver.mjs | 33 ++++--- chrome/player/utils/Utils.mjs | 12 ++- tests/unit/StreamSaver.test.mjs | 106 +++++++++++++++++++++++ tests/unit/revokeWhenDownloaded.test.mjs | 11 +++ 4 files changed, 148 insertions(+), 14 deletions(-) create mode 100644 tests/unit/StreamSaver.test.mjs diff --git a/chrome/player/modules/StreamSaver.mjs b/chrome/player/modules/StreamSaver.mjs index 60d04a561..d0cb1895b 100644 --- a/chrome/player/modules/StreamSaver.mjs +++ b/chrome/player/modules/StreamSaver.mjs @@ -54,6 +54,24 @@ function createWriteStreamBlob(filename, opts, size) { }, opts.writableStrategy); } +/** + * Revokes a save's URL and closes its blob store once its download is over. Firefox reads + * the file from the store's OPFS session as it downloads it, and a closed session is + * deleted by the next prune (any player or save starting, 10 s after its last heartbeat): + * closed on a fixed two-minute timer, a longer download (a big video to a slow disk) lost + * its file midway, with no message. Never sooner than those two minutes, which is all a + * download without an id to follow (a link click) has. + * @param {FSBlob} blobManager + * @param {string} url - The blob: URL being downloaded. + * @param {*} download - What Utils.downloadURL resolved with. + */ +function closeWhenDownloaded(blobManager, url, download) { + Promise.all([ + Utils.revokeWhenDownloaded(url, download), + Utils.asyncTimeout(120000), + ]).then(() => blobManager.close()); +} + /** * Progressive, disk-backed sink: every chunk is appended to one OPFS file and * the finished file goes to the download as a File, so RAM stays flat no @@ -90,13 +108,8 @@ function createOPFSSink(filename, blobManager) { throw e; } // chrome.downloads resolves before Firefox has read the blob URL: kept until the - // download is over (revokeWhenDownloaded). Defer closing the worker the same way - // mp4merger.mjs's destroy() does, so a slow transfer still has time to finish - // reading from the OPFS-backed file before its session is torn down. - Utils.revokeWhenDownloaded(url, download); - setTimeout(() => { - blobManager.close(); - }, 120000); + // download is over (revokeWhenDownloaded), and the OPFS file behind it too. + closeWhenDownloaded(blobManager, url, download); }, async abort() { await opfsWriterReady.catch(() => {}); @@ -124,11 +137,7 @@ function createMemorySink(filename, blobManager) { const chunks = await Promise.all(blobs.map((blob) => blobManager.getBlob(blob))); const blob = new Blob(chunks, {type: 'application/octet-stream'}); const url = URL.createObjectURL(blob); - Utils.revokeWhenDownloaded(url, await Utils.downloadURL(url, filename)); - - setTimeout(() => { - blobManager.close(); - }, 120000); + closeWhenDownloaded(blobManager, url, await Utils.downloadURL(url, filename)); }, async abort() { blobs.length = 0; diff --git a/chrome/player/utils/Utils.mjs b/chrome/player/utils/Utils.mjs index b458f6aa7..aa7efce06 100644 --- a/chrome/player/utils/Utils.mjs +++ b/chrome/player/utils/Utils.mjs @@ -260,13 +260,20 @@ export class Utils { * stays a minute. * @param {string} url - The blob: URL. * @param {*} download - What downloadURL resolved with: the download's id, or not. + * @return {Promise} Settles once the URL is revoked: whatever else the download + * reads from (a save's OPFS session) can be let go then too. */ static revokeWhenDownloaded(url, download) { - const revoke = () => URL.revokeObjectURL(url); + let revoked; + const over = new Promise((resolve) => (revoked = resolve)); + const revoke = () => { + URL.revokeObjectURL(url); + revoked(); + }; const downloads = globalThis.chrome?.downloads; if (typeof download !== 'number' || !downloads?.onChanged || !downloads.search) { setTimeout(revoke, 60000); - return; + return over; } let done = false; const finish = () => { @@ -288,6 +295,7 @@ export class Utils { downloads.search({id: download}).then(([item]) => { if (!item || item.state !== 'in_progress') finish(); }).catch(finish); + return over; } /** diff --git a/tests/unit/StreamSaver.test.mjs b/tests/unit/StreamSaver.test.mjs new file mode 100644 index 000000000..29024897b --- /dev/null +++ b/tests/unit/StreamSaver.test.mjs @@ -0,0 +1,106 @@ +import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest'; + +// A streamed save writes its file into its own blob store (an OPFS session), and the +// download reads it from there. The store used to be closed two minutes after the save +// ended, whatever the download was doing; a closed session is deleted by the next player +// or save that starts, so a download longer than that lost its file midway (#134). + +const state = vi.hoisted(() => ({opfs: true, stores: []})); + +vi.mock('../../chrome/player/modules/FSBlob.mjs', () => ({ + FSBlob: class { + constructor() { + this.close = vi.fn(); + this.blobs = new Map(); + this.opfsManager = state.opfs ? { + saveBegin: async () => {}, + saveAppend: async () => {}, + saveEnd: async () => {}, + getSavedFile: async () => new Blob(['video']), + saveAbort: async () => {}, + } : null; + state.stores.push(this); + } + async ready() { + return state.opfs; + } + createBlob(data) { + const id = 'blob' + this.blobs.size; + this.blobs.set(id, new Blob([data])); + return id; + } + getBlob(id) { + return this.blobs.get(id); + } + }, +})); + +const {streamSaver} = await import('../../chrome/player/modules/StreamSaver.mjs'); +const {Utils} = await import('../../chrome/player/utils/Utils.mjs'); + +let listeners; +let items; + +beforeEach(() => { + vi.useFakeTimers(); + state.stores.length = 0; + vi.spyOn(URL, 'revokeObjectURL').mockImplementation(() => {}); + vi.spyOn(Utils, 'downloadURL').mockResolvedValue(7); + listeners = new Set(); + items = new Map([[7, {id: 7, state: 'in_progress'}]]); + globalThis.chrome = { + downloads: { + onChanged: { + addListener: (fn) => listeners.add(fn), + removeListener: (fn) => listeners.delete(fn), + }, + search: async ({id}) => items.has(id) ? [items.get(id)] : [], + }, + }; +}); + +afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + delete globalThis.chrome; +}); + +/** Saves one chunk through a stream, and gives back the save's blob store. */ +async function save() { + const writer = streamSaver.createWriteStream('video.mp4').getWriter(); + await writer.write(new Uint8Array([1, 2, 3])); + await writer.close(); + expect(Utils.downloadURL).toHaveBeenCalledTimes(1); + return state.stores[0]; +} + +const finishDownload = () => [...listeners].forEach((fn) => fn({id: 7, state: {current: 'complete'}})); + +describe.each([['OPFS', true], ['memory', false]])('a streamed save to the %s sink', (sink, opfs) => { + beforeEach(() => { + state.opfs = opfs; + }); + + it('keeps its blob store while the download runs, and closes it once it is over', async () => { + const store = await save(); + // Ten minutes: a big video to a slow disk. + await vi.advanceTimersByTimeAsync(10 * 60 * 1000); + expect(store.close).not.toHaveBeenCalled(); + expect(URL.revokeObjectURL).not.toHaveBeenCalled(); + + finishDownload(); + await vi.advanceTimersByTimeAsync(0); + expect(URL.revokeObjectURL).toHaveBeenCalledTimes(1); + expect(store.close).toHaveBeenCalledTimes(1); + }); + + it('never closes it sooner than two minutes, even after a quick download', async () => { + const store = await save(); + await vi.advanceTimersByTimeAsync(1000); + finishDownload(); + await vi.advanceTimersByTimeAsync(118 * 1000); + expect(store.close).not.toHaveBeenCalled(); + await vi.advanceTimersByTimeAsync(1000); + expect(store.close).toHaveBeenCalledTimes(1); + }); +}); diff --git a/tests/unit/revokeWhenDownloaded.test.mjs b/tests/unit/revokeWhenDownloaded.test.mjs index 4baf6ab58..5c1a8f01d 100644 --- a/tests/unit/revokeWhenDownloaded.test.mjs +++ b/tests/unit/revokeWhenDownloaded.test.mjs @@ -46,6 +46,17 @@ describe('Utils.revokeWhenDownloaded', () => { expect(listeners.size).toBe(0); }); + it('says when it has revoked it, so a save can let go of its file then too', async () => { + items.set(7, {id: 7, state: 'in_progress'}); + let over = false; + Utils.revokeWhenDownloaded('blob:a', 7).then(() => (over = true)); + await vi.advanceTimersByTimeAsync(5000); + expect(over).toBe(false); + change({id: 7, state: {current: 'complete'}}); + await vi.advanceTimersByTimeAsync(0); + expect(over).toBe(true); + }); + it('revokes it when the download was interrupted', async () => { items.set(7, {id: 7, state: 'in_progress'}); Utils.revokeWhenDownloaded('blob:a', 7); From c03554e2e36721486d4cc732f7dab2926ab93670 Mon Sep 17 00:00:00 2001 From: Nawid Salehie <102778966+Nawid3333@users.noreply.github.com> Date: Sat, 3 Oct 2026 15:50:00 +0200 Subject: [PATCH 03/15] fix(archive): check a length from an archive before allocating it, read sizes unsigned An .fsa archive's header and entry sizes come from the file. LargeBuffer.read allocated the buffer before checking the length against what was left, so a damaged or crafted archive could ask for up to 4 GB before "out of range". The 32-bit size was assembled with a signed `<< 24`, so a size from 2 GB up came out negative and failed with an unexplained RangeError, and getParts took a negative length as "read nothing", which put every later read in the wrong place. The length is now checked first (and must be a whole, non-negative number), and uint32() is unsigned. tests/unit/LargeBuffer.test.mjs and FastStreamArchiveUtils.test.mjs ("says a header size past the end of the file is out of range"): all four new cases fail on the old LargeBuffer. Fixes #141. Co-Authored-By: Claude Opus 5.5 --- chrome/player/modules/LargeBuffer.mjs | 12 +++++++++-- tests/unit/LargeBuffer.test.mjs | 31 +++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/chrome/player/modules/LargeBuffer.mjs b/chrome/player/modules/LargeBuffer.mjs index f266df34d..0a3f89dd3 100644 --- a/chrome/player/modules/LargeBuffer.mjs +++ b/chrome/player/modules/LargeBuffer.mjs @@ -26,6 +26,11 @@ export class LargeBuffer { } async getParts(length) { + // Lengths come from the file itself (an archive's headers): a negative one, or one + // that is no number, read nothing, and the next read started in the wrong place. + if (!Number.isSafeInteger(length) || length < 0) { + throw new Error('Invalid length ' + length); + } const parts = []; this.offset += length; if (this.offset > this.byteLength) { @@ -51,8 +56,10 @@ export class LargeBuffer { } async read(length) { - const uint8 = new Uint8Array(length); + // Checked first: a length from a damaged or crafted archive asked for up to 4 GB of + // memory before it was found out of range. const parts = await this.getParts(length); + const uint8 = new Uint8Array(length); let offset = 0; for (let i = 0; i < parts.length; i++) { @@ -74,6 +81,7 @@ export class LargeBuffer { async uint32() { const arr = await this.read(4); - return (arr[0] << 24) | (arr[1] << 16) | (arr[2] << 8) | arr[3]; + // >>> 0: `<< 24` is signed, and a size from 2 GB up came out negative. + return ((arr[0] << 24) | (arr[1] << 16) | (arr[2] << 8) | arr[3]) >>> 0; } } diff --git a/tests/unit/LargeBuffer.test.mjs b/tests/unit/LargeBuffer.test.mjs index 930378f59..70e3143da 100644 --- a/tests/unit/LargeBuffer.test.mjs +++ b/tests/unit/LargeBuffer.test.mjs @@ -82,6 +82,37 @@ describe('integer readers', () => { // 0x04..0x07 begins in chunk 1 - proves the stitch feeds the shift math expect(await buf.uint32()).toBe(0x04050607); }); + + it('reads a uint32 from 2^31 up as unsigned', async () => { + // `<< 24` is signed: 0x80000000 came out as -2147483648. + const buf = await bufferOf([0x80, 0, 0, 0, 0xff, 0xff, 0xff, 0xf0]); + expect(await buf.uint32()).toBe(0x80000000); + expect(await buf.uint32()).toBe(0xfffffff0); + }); +}); + +/** A LargeBuffer over the given bytes, in one chunk. */ +async function bufferOf(bytes) { + const buf = new LargeBuffer(bytes.length, 1); + await buf.initialize(async () => new Uint8Array(bytes)); + return buf; +} + +describe('lengths read from a file', () => { + // An archive's sizes come from the file, so a damaged or crafted one can ask for anything. + it('checks a length against what is left before making room for it', async () => { + const buf = await makeBuffer(2); + // Larger than any typed array: it threw a RangeError of its own before the check. + await expect(buf.read(Number.MAX_SAFE_INTEGER)).rejects.toThrow(/out of range/); + }); + + it('refuses a negative length instead of reading nothing', async () => { + const buf = await makeBuffer(2); + await expect(buf.getParts(-3)).rejects.toThrow(/Invalid length/); + await expect(buf.read(Number.NaN)).rejects.toThrow(/Invalid length/); + // Nothing was read: the next read is where it was. + expect([...await buf.read(2)]).toEqual([0, 1]); + }); }); describe('prefetching', () => { From 1eb6ca3000da2c034b4a8b9ba7ae80ee8dac1008 Mon Sep 17 00:00:00 2001 From: Nawid Salehie <102778966+Nawid3333@users.noreply.github.com> Date: Sat, 3 Oct 2026 15:50:00 +0200 Subject: [PATCH 04/15] fix(archive): finish or abort the save stream when writing an archive writeFSAToStream did not await writer.close(), so "Saved" showed before the save's stream had handed its file to the download, and a failure there (quota, download refused) was an unhandled rejection with no message. An entry that was not complete, or a failed read, threw with the stream left open: the save's OPFS file handle, worker and session stayed until the tab closed. It now awaits close() and, on any failure, aborts the stream (which ends the save's session) before passing the error on. tests/unit/FastStreamArchiveUtils.test.mjs: the three writer cases fail on the old code (and the failing close is an unhandled rejection there). Fixes #136. Co-Authored-By: Claude Opus 5.5 --- .../player/utils/FastStreamArchiveUtils.mjs | 29 +++++- tests/unit/FastStreamArchiveUtils.test.mjs | 97 +++++++++++++++++++ 2 files changed, 122 insertions(+), 4 deletions(-) create mode 100644 tests/unit/FastStreamArchiveUtils.test.mjs diff --git a/chrome/player/utils/FastStreamArchiveUtils.mjs b/chrome/player/utils/FastStreamArchiveUtils.mjs index 984972f49..80ac3f501 100644 --- a/chrome/player/utils/FastStreamArchiveUtils.mjs +++ b/chrome/player/utils/FastStreamArchiveUtils.mjs @@ -17,11 +17,34 @@ export class FastStreamArchiveUtils { * @param {Object} player - The player instance. * @param {Array} entries - The download entries. * @param {Function} [progressCallback] - Optional progress callback. - * @return {Promise} Resolves when writing is complete. - * @throws {Error} If an entry is not complete. + * @return {Promise} Resolves once the stream has finished the file. + * @throws {Error} If an entry is not complete, or the stream fails. */ static async writeFSAToStream(filestream, player, entries, progressCallback) { const writer = filestream.getWriter(); + try { + await this.writeFSA(writer, player, entries, progressCallback); + // Awaited: the save is over only once the stream has finished it (a streamed save + // hands its file to the download here), and a failure there is the save's. It was + // reported as saved before that, and such a failure was an unhandled rejection. + await writer.close(); + } catch (e) { + // Ends the stream's save (its open file and blob store) rather than leaving it + // open until the tab closes. + await writer.abort(e).catch(() => {}); + throw e; + } + } + + /** + * Writes the archive: the header, then each entry's header and data. + * @param {WritableStreamDefaultWriter} writer + * @param {Object} player + * @param {Array} entries + * @param {Function} [progressCallback] + * @return {Promise} + */ + static async writeFSA(writer, player, entries, progressCallback) { const sourceObj = {}; if (player) { const source = player.getSource(); @@ -96,8 +119,6 @@ export class FastStreamArchiveUtils { progressCallback(i / entries.length); } } - - writer.close(); } /** diff --git a/tests/unit/FastStreamArchiveUtils.test.mjs b/tests/unit/FastStreamArchiveUtils.test.mjs new file mode 100644 index 000000000..8f2ec1f85 --- /dev/null +++ b/tests/unit/FastStreamArchiveUtils.test.mjs @@ -0,0 +1,97 @@ +import {describe, expect, it} from 'vitest'; +import {DownloadStatus} from '../../chrome/player/enums/DownloadStatus.mjs'; +import {LargeBuffer} from '../../chrome/player/modules/LargeBuffer.mjs'; +import {FastStreamArchiveUtils} from '../../chrome/player/utils/FastStreamArchiveUtils.mjs'; + +// The .fsa archive ("save the buffer"): writeFSAToStream writes one into a save's stream, +// parseFSA reads one back, from a file dropped on the player. + +/** A complete download entry holding `bytes`. */ +function entry(bytes, status = DownloadStatus.DOWNLOAD_COMPLETE) { + return { + status, + storeRaw: true, + url: 'https://cdn.example/seg.ts', + responseType: 'arraybuffer', + responseHeaders: {}, + getData: async () => new Uint8Array(bytes).buffer, + }; +} + +/** + * A stream like a save's, recording what happens to it. + * @param {Object} [sink] - overrides for the underlying sink + * @return {{stream: WritableStream, log: Array}} + */ +function recordingStream(sink = {}) { + const log = []; + const stream = new WritableStream({ + write: () => { + log.push('write'); + }, + close: async () => { + await sink.close?.(); + log.push('closed'); + }, + abort: () => { + log.push('aborted'); + }, + }); + return {stream, log}; +} + +describe('writeFSAToStream', () => { + it('is done only once the stream has finished the file', async () => { + // A streamed save hands its file to the download when the stream closes; the player + // said "saved" before that had happened. + let finish; + const {stream, log} = recordingStream({close: () => new Promise((resolve) => (finish = resolve))}); + let done = false; + const writing = FastStreamArchiveUtils.writeFSAToStream(stream, null, [entry([1, 2, 3])]).then(() => (done = true)); + await new Promise((resolve) => setTimeout(resolve, 10)); + expect(done).toBe(false); + finish(); + await writing; + expect(log.at(-1)).toBe('closed'); + }); + + it('fails when the stream cannot finish the file', async () => { + const {stream} = recordingStream({close: async () => { + throw new Error('the download was refused'); + }}); + await expect(FastStreamArchiveUtils.writeFSAToStream(stream, null, [entry([1])])).rejects.toThrow('the download was refused'); + }); + + it('ends the stream\'s save when an entry is not complete', async () => { + const {stream, log} = recordingStream(); + const entries = [entry([1]), entry([2], DownloadStatus.DOWNLOAD_FAILED)]; + await expect(FastStreamArchiveUtils.writeFSAToStream(stream, null, entries)).rejects.toThrow('Entry is not complete!'); + expect(log).toContain('aborted'); + expect(log).not.toContain('closed'); + }); +}); + +describe('parseFSA', () => { + /** A LargeBuffer over the given bytes. */ + async function bufferOf(bytes) { + const buf = new LargeBuffer(bytes.length, 1); + await buf.initialize(async () => new Uint8Array(bytes)); + return buf; + } + + it('says a header size past the end of the file is out of range', async () => { + // 0xFFFFFFF0 read as a signed number was -16, and a RangeError said nothing useful. + const archive = await bufferOf([0xff, 0xff, 0xff, 0xf0, 0x7b, 0x7d]); + await expect(FastStreamArchiveUtils.parseFSA(archive)).rejects.toThrow(/out of range/); + }); + + it('reads back what writeFSAToStream wrote', async () => { + const chunks = []; + const stream = new WritableStream({write: (chunk) => { + chunks.push(...chunk); + }}); + await FastStreamArchiveUtils.writeFSAToStream(stream, null, [entry([1, 2, 3]), entry([4, 5])]); + const {entries} = await FastStreamArchiveUtils.parseFSA(await bufferOf(chunks)); + expect(entries.map((e) => [...e.data])).toEqual([[1, 2, 3], [4, 5]]); + }); +}); From f6505cb0e64b4d4243e57932815115d2f421f0db Mon Sep 17 00:00:00 2001 From: Nawid Salehie <102778966+Nawid3333@users.noreply.github.com> Date: Sat, 3 Oct 2026 15:51:16 +0200 Subject: [PATCH 05/15] fix(storage): IndexedDB prune lets go of live databases; transact always answers FSBlob's last backend (used when OPFS and the Cache API both refuse): - prune() opened every other player's temporary database to read its heartbeat and closed only the stale ones. A live player's connection stayed open, so when that player closed, its deleteDatabase was blocked (5 s timeout, then the database stayed on disk) until the pruning tab closed too. The connection is now closed either way (#137). - transact() had no abort handler, so a commit that failed (quota exceeded aborts without an error event) never answered. And a callback that threw before making its request (a value that cannot be stored) left a transaction with nothing in it, which completed: the call answered null as if it had worked, and FSBlob then kept `undefined` as the fragment. Both now reject; the executor is no longer async (its throw was an unhandled rejection). keepAlive catches a failed beat, which is retried a second later (#149). tests/unit/IndexedDBManager.test.mjs: on the old code the first transact case answers "resolved: null", the second stays pending, and the prune case leaves the live connection open. Fixes #137, fixes #149. Co-Authored-By: Claude Opus 5.5 --- chrome/player/network/IndexedDBManager.mjs | 36 +++++-- tests/unit/IndexedDBManager.test.mjs | 112 +++++++++++++++++++++ 2 files changed, 141 insertions(+), 7 deletions(-) create mode 100644 tests/unit/IndexedDBManager.test.mjs diff --git a/chrome/player/network/IndexedDBManager.mjs b/chrome/player/network/IndexedDBManager.mjs index f6a0142b2..63e26fa1d 100644 --- a/chrome/player/network/IndexedDBManager.mjs +++ b/chrome/player/network/IndexedDBManager.mjs @@ -102,13 +102,18 @@ export class IndexedDBManager { try { const db = await IndexedDBManager.requestDB(database.name, false); // check if stale + let stale = false; try { const updatedTime = await IndexedDBManager.getValue(db, 'metadata', 'updated_time'); - if (!updatedTime || Date.now() - updatedTime > 10000) { - throw new Error('Stale'); - } + stale = !updatedTime || Date.now() - updatedTime > 10000; } catch (e) { + stale = true; + } finally { + // A live tab's database too: kept open, it blocked that tab's own + // deleteDatabase when it closed, until this tab closed as well. db.close(); + } + if (stale) { await IndexedDBManager.deleteDB(database.name); console.log('Pruned', database.name); } @@ -229,12 +234,13 @@ export class IndexedDBManager { keepAlive() { if (this.db && !this.isPersistent()) { - IndexedDBManager.setValue(this.db, 'metadata', 'updated_time', Date.now()); + // A beat that fails is tried again in a second. + IndexedDBManager.setValue(this.db, 'metadata', 'updated_time', Date.now()).catch(() => {}); } } static transact(db, storeName, mode, callback) { - return new Promise(async (resolve, reject)=>{ + return new Promise((resolve, reject)=>{ let transaction; try { transaction = db.transaction(storeName, mode); @@ -248,13 +254,29 @@ export class IndexedDBManager { console.error(event); reject(event); }; + // A commit that fails (quota) aborts with no error event: the call never settled. + transaction.onabort = () => { + reject(transaction.error || new Error('the transaction was aborted')); + }; transaction.oncomplete = async (event) => { resolve(await result); }; - result = callback(transaction); - transaction.commit(); + try { + result = callback(transaction); + transaction.commit(); + } catch (e) { + // A request the callback could not make (put() of a value that cannot be stored): + // the transaction, with nothing in it, completed and the call answered null, as if + // it had worked. + try { + transaction.abort(); + } catch (abortError) { + // Already finished. + } + reject(e); + } }); } diff --git a/tests/unit/IndexedDBManager.test.mjs b/tests/unit/IndexedDBManager.test.mjs new file mode 100644 index 000000000..ec06ae2ce --- /dev/null +++ b/tests/unit/IndexedDBManager.test.mjs @@ -0,0 +1,112 @@ +import {afterEach, beforeAll, describe, expect, it, vi} from 'vitest'; + +// FSBlob's last storage backend (after OPFS and the Cache API): a temporary IndexedDB +// database per player, which the other players' prune() deletes once its heartbeat stops. +// - prune() kept the connection to a live player's database open, and that player's own +// deleteDatabase on closing was blocked until the pruning tab closed too (#137). +// - transact() never answered for a commit that failed (an abort with no error event), and +// answered null, as if it had worked, when the callback could not make its request (#149). + +let IndexedDBManager; + +beforeAll(async () => { + // The module listens for the page closing. + vi.stubGlobal('window', {addEventListener: () => {}}); + ({IndexedDBManager} = await import('../../chrome/player/network/IndexedDBManager.mjs')); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +/** + * A stand-in database whose transactions behave as IndexedDB's: one with no request left + * commits by itself once the task is over, and one whose commit fails aborts without an + * error event. + * @param {{commitFails?: boolean, put?: Function}} [options] + * @return {Object} + */ +function fakeDb({commitFails = false, put = () => ({})} = {}) { + return { + transaction() { + const transaction = { + error: null, + finished: false, + objectStore: () => ({put}), + commit() { + transaction.finished = true; + if (commitFails) { + transaction.error = new DOMException('The quota is exceeded', 'QuotaExceededError'); + setTimeout(() => transaction.onabort?.()); + } else { + setTimeout(() => transaction.oncomplete?.()); + } + }, + abort() { + if (transaction.finished) throw new DOMException('Finished', 'InvalidStateError'); + transaction.finished = true; + setTimeout(() => transaction.onabort?.()); + }, + }; + setTimeout(() => { + if (!transaction.finished) { + transaction.finished = true; + transaction.oncomplete?.(); + } + }); + return transaction; + }, + }; +} + +/** How a promise ends within a short while: 'resolved: x', 'rejected: name', or 'pending'. */ +function outcome(promise) { + return Promise.race([ + promise.then((value) => 'resolved: ' + value, (e) => 'rejected: ' + (e?.name || e)), + new Promise((resolve) => setTimeout(() => resolve('pending'), 100)), + ]); +} + +describe('IndexedDBManager.transact', () => { + it('fails when the callback cannot make its request, rather than answering null', async () => { + const db = fakeDb({put: () => { + throw new DOMException('The object could not be cloned', 'DataCloneError'); + }}); + const done = IndexedDBManager.transact(db, 'files', 'readwrite', (transaction) => transaction.objectStore('files').put({}, 'key')); + expect(await outcome(done)).toBe('rejected: DataCloneError'); + }); + + it('fails when the commit fails, rather than never answering', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + const done = IndexedDBManager.transact(fakeDb({commitFails: true}), 'files', 'readwrite', () => Promise.resolve('stored')); + expect(await outcome(done)).toBe('rejected: QuotaExceededError'); + }); + + it('answers with what the callback gave once the transaction is complete', async () => { + const done = IndexedDBManager.transact(fakeDb(), 'files', 'readwrite', () => Promise.resolve('stored')); + expect(await outcome(done)).toBe('resolved: stored'); + }); +}); + +describe('IndexedDBManager.prune', () => { + it('closes its connection to a live database as well as to a stale one', async () => { + vi.spyOn(console, 'log').mockImplementation(() => {}); + window.indexedDB = {databases: async () => [{name: 'faststream-temp-live'}, {name: 'faststream-temp-stale'}, {name: 'other'}]}; + const connections = new Map(); + vi.spyOn(IndexedDBManager, 'requestDB').mockImplementation(async (name) => { + const db = {name, close: vi.fn()}; + connections.set(name, db); + return db; + }); + vi.spyOn(IndexedDBManager, 'getValue').mockImplementation(async (db) => + db.name === 'faststream-temp-live' ? Date.now() - 1000 : Date.now() - 60000); + const deleteDB = vi.spyOn(IndexedDBManager, 'deleteDB').mockResolvedValue(); + + await new IndexedDBManager().prune(); + + expect([...connections.keys()].sort()).toEqual(['faststream-temp-live', 'faststream-temp-stale']); + expect(connections.get('faststream-temp-live').close).toHaveBeenCalledTimes(1); + expect(connections.get('faststream-temp-stale').close).toHaveBeenCalledTimes(1); + expect(deleteDB.mock.calls).toEqual([['faststream-temp-stale']]); + }); +}); From 576cb50b1c6c2f06d5f0c2d82ded016544e4516e Mon Sep 17 00:00:00 2001 From: Nawid Salehie <102778966+Nawid3333@users.noreply.github.com> Date: Sat, 3 Oct 2026 15:51:52 +0200 Subject: [PATCH 06/15] fix(network): a request that fails after its download was aborted fails nothing else StandardDownloader.run makes the request (a DASH segment's preprocessor) before loading it. When the entry was aborted meanwhile (a seek) and the request then failed, the catch used `this.entry`: null after the abort (a TypeError, an unhandled rejection), or the next entry the downloader had started by then, which it failed and cleaned up. It now returns when its own run was aborted. tests/unit/StandardDownloader.test.mjs: both new cases fail on the old code (the next download is failed; an unhandled rejection with nothing else running). Fixes #148. Co-Authored-By: Claude Opus 5.5 --- chrome/player/network/StandardDownloader.mjs | 3 ++ tests/unit/StandardDownloader.test.mjs | 45 ++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/chrome/player/network/StandardDownloader.mjs b/chrome/player/network/StandardDownloader.mjs index 049153f69..9961954da 100644 --- a/chrome/player/network/StandardDownloader.mjs +++ b/chrome/player/network/StandardDownloader.mjs @@ -53,6 +53,9 @@ export class StandardDownloader { this.loader.addCallbacks(this); this.loader.load(request, defaultConfig); }).catch((err) => { + // Aborted while its request was being made: that entry is over (cleanup ran), and + // this downloader may be running the next one, which failing `this.entry` failed. + if (!shouldContinue) return; console.error('Failed to get request for entry:', err); this.entry.onFail(this.loader.stats, this.entry, null); this.cleanup(); diff --git a/tests/unit/StandardDownloader.test.mjs b/tests/unit/StandardDownloader.test.mjs index 4c9f5a899..b5765ec1a 100644 --- a/tests/unit/StandardDownloader.test.mjs +++ b/tests/unit/StandardDownloader.test.mjs @@ -24,4 +24,49 @@ describe('StandardDownloader', () => { expect(downloader.canHandle({})).toBe(true); expect(manager.onDownloaderFinished).toHaveBeenCalledTimes(1); }); + + // An entry whose request is still being made (a DASH segment's preprocessor) can be + // aborted, by a seek; the request failing after that failed whatever entry the + // downloader had by then, or threw on none (#148). + describe('a request that fails after its download was aborted', () => { + /** An entry whose request fails when the test says. */ + function pendingEntry() { + const entry = {onAbort: vi.fn(), onFail: vi.fn()}; + entry.request = new Promise((resolve, reject) => { + entry.failRequest = reject; + }); + entry.getRequest = () => entry.request; + return entry; + } + + it('leaves the download the downloader has moved on to alone', async () => { + const downloader = new StandardDownloader({onDownloaderFinished: vi.fn()}); + const aborted = pendingEntry(); + downloader.run(aborted); + downloader.abort(); + const next = pendingEntry(); + downloader.run(next); + + aborted.failRequest(new Error('the preprocessor failed')); + await new Promise((resolve) => setTimeout(resolve)); + + expect(next.onFail).not.toHaveBeenCalled(); + expect(downloader.entry).toBe(next); + }); + + it('is not an error when the downloader has nothing else', async () => { + const error = vi.spyOn(console, 'error').mockImplementation(() => {}); + const downloader = new StandardDownloader({onDownloaderFinished: vi.fn()}); + const aborted = pendingEntry(); + downloader.run(aborted); + downloader.abort(); + + aborted.failRequest(new Error('the preprocessor failed')); + await new Promise((resolve) => setTimeout(resolve)); + + expect(error).not.toHaveBeenCalled(); + expect(aborted.onFail).not.toHaveBeenCalled(); + error.mockRestore(); + }); + }); }); From edc2a6c55f540835151bc74444815f90c1fcd629 Mon Sep 17 00:00:00 2001 From: Nawid Salehie <102778966+Nawid3333@users.noreply.github.com> Date: Sat, 3 Oct 2026 15:53:16 +0200 Subject: [PATCH 07/15] fix(network): downloaders dropped after failures come back; every free one gets work - A failed download (after XHRLoader's own retries) takes one downloader away, to ease off a struggling server, and nothing gave it back: three failures in a long video left three downloaders of six until the next video. Now, once no download has failed for 10 s, each successful download brings one back, up to as many as were taken. A downloader the user adds or removes resets that. - queueNext started at most one download per call. After a failure it is called once, when the 1 s wait is over, so with five free downloaders and forty queued fragments one ran at a time until other calls came in. It now starts a download on every free downloader. tests/unit/DownloadManager.test.mjs: each case fails with its fix taken out (one download started of three possible; the pool stays at one). Fixes #133. Co-Authored-By: Claude Opus 5.5 --- chrome/player/network/DownloadManager.mjs | 59 +++++++++++++++-------- tests/unit/DownloadManager.test.mjs | 57 ++++++++++++++++++++++ 2 files changed, 96 insertions(+), 20 deletions(-) diff --git a/chrome/player/network/DownloadManager.mjs b/chrome/player/network/DownloadManager.mjs index 6a8349fb2..abe08cae1 100644 --- a/chrome/player/network/DownloadManager.mjs +++ b/chrome/player/network/DownloadManager.mjs @@ -4,6 +4,9 @@ import {DownloadEntry} from './DownloadEntry.mjs'; import {StandardDownloader} from './StandardDownloader.mjs'; export class DownloadManager { + /** How long no download may fail before a downloader dropped for a failure comes back. */ + static FailureRecoveryMs = 10000; + constructor(client) { this.client = client; this.queue = []; @@ -18,6 +21,8 @@ export class DownloadManager { this.testing = true; this.lastSpeed = 0; this.lastFailed = 0; + // Downloaders taken away after failed downloads, and not back yet. + this.droppedDownloaders = 0; this.failed = 0; @@ -208,6 +213,7 @@ export class DownloadManager { addDownloader() { this.testing = false; + this.droppedDownloaders = 0; this.downloaders.push(new StandardDownloader(this)); this.client.predownloadFragments(); this.queueNext(); @@ -215,6 +221,7 @@ export class DownloadManager { removeDownloader() { this.testing = false; + this.droppedDownloaders = 0; const downloader = this.downloaders.pop(); downloader.abort(); @@ -239,6 +246,7 @@ export class DownloadManager { removeAllDownloaders() { this.testing = false; + this.droppedDownloaders = 0; this.downloaders.forEach((downloader) => { downloader.abort(); }); @@ -255,10 +263,18 @@ export class DownloadManager { const ind = this.downloaders.indexOf(downloader); if (ind !== -1) { this.downloaders.splice(ind, 1); + this.droppedDownloaders++; this.client.resetFailed(); console.log('Downloader failed, removing downloader and trying again'); } } + } else if (entry.status === DownloadStatus.DOWNLOAD_COMPLETE && this.droppedDownloaders > 0 && + Date.now() - this.lastFailed > DownloadManager.FailureRecoveryMs) { + // A failure takes a downloader away, easing off a server that is struggling, and it + // never came back: three failures in a long video left three downloaders of six for + // the rest of it. Once downloads succeed again, each success brings one back. + this.droppedDownloaders--; + this.downloaders.push(new StandardDownloader(this)); } if (this.testing) { @@ -312,30 +328,32 @@ export class DownloadManager { this.queueNext(); } + /** + * Starts as many queued downloads as there are free downloaders. It started one per call, + * so after a failure's cooldown, which calls it once, one download ran at a time until + * something else called it. + */ queueNext() { - if (this.paused) return; - if (this.queue.length === 0) return; - - if (this.queue[0].status !== DownloadStatus.ENQUEUED) { - this.queue.shift(); - this.queueNext(); - return; - } + while (!this.paused && this.queue.length > 0) { + if (this.queue[0].status !== DownloadStatus.ENQUEUED) { + this.queue.shift(); + continue; + } - const failCooldown = 1000; - if (this.lastFailed + failCooldown > Date.now()) { - if (this.failCooldown) clearTimeout(this.failCooldown); - this.failCooldown = setTimeout(() => { - this.queueNext(); - }, failCooldown + 100); - return; - } + const failCooldown = 1000; + if (this.lastFailed + failCooldown > Date.now()) { + if (this.failCooldown) clearTimeout(this.failCooldown); + this.failCooldown = setTimeout(() => { + this.queueNext(); + }, failCooldown + 100); + return; + } - const downloader = this.downloaders.find((downloader) => { - return downloader.canHandle(this.queue[0].details); - }); + const downloader = this.downloaders.find((downloader) => { + return downloader.canHandle(this.queue[0].details); + }); + if (!downloader) return; - if (downloader) { const entry = this.queue.shift(); downloader.run(entry); } @@ -355,6 +373,7 @@ export class DownloadManager { this.testing = true; this.downloaders = []; + this.droppedDownloaders = 0; this.speedTestBuffer = []; this.speedTestSeen = []; this.speedTestCount = 0; diff --git a/tests/unit/DownloadManager.test.mjs b/tests/unit/DownloadManager.test.mjs index 38056935d..3742d66ad 100644 --- a/tests/unit/DownloadManager.test.mjs +++ b/tests/unit/DownloadManager.test.mjs @@ -33,6 +33,63 @@ describe('DownloadManager', () => { expect(() => vi.advanceTimersByTime(2000)).not.toThrow(); }); + /** A downloader that takes one download and keeps it. */ + function idleDownloader() { + const downloader = { + entry: null, + canHandle: () => !downloader.entry, + run: (entry) => { + downloader.entry = entry; + }, + }; + return downloader; + } + + it('starts a queued download on every free downloader once a failure\'s wait is over', () => { + // The wait after a failure ends in one queueNext, which started one download: with + // three free downloaders and five queued downloads, one ran at a time (#133). + const manager = new DownloadManager(null); + manager.downloaders = [idleDownloader(), idleDownloader(), idleDownloader()]; + for (let i = 0; i < 5; i++) manager.queue.push({status: DownloadStatus.ENQUEUED, details: {}}); + manager.lastFailed = Date.now(); + manager.queueNext(); + expect(manager.downloaders.filter((downloader) => downloader.entry)).toHaveLength(0); + + vi.advanceTimersByTime(1100); + expect(manager.downloaders.filter((downloader) => downloader.entry)).toHaveLength(3); + expect(manager.queue).toHaveLength(2); + }); + + it('gives a downloader dropped for a failed download back once downloads succeed again', () => { + // A failure takes one away; it never came back, so a few failures in a long video + // left half the downloaders for the rest of it (#133). + vi.stubGlobal('navigator', {onLine: true}); + vi.spyOn(console, 'log').mockImplementation(() => {}); + const client = {resetFailed: vi.fn(), predownloadFragments: vi.fn()}; + const manager = new DownloadManager(client); + manager.testing = false; + const [failed, other] = [idleDownloader(), idleDownloader()]; + manager.downloaders = [failed, other]; + const succeed = () => manager.onDownloaderFinished(other, {status: DownloadStatus.DOWNLOAD_COMPLETE}); + + manager.onDownloaderFinished(failed, {status: DownloadStatus.DOWNLOAD_FAILED}); + expect(manager.downloaders).toEqual([other]); + + // Not while failures are recent. + vi.advanceTimersByTime(DownloadManager.FailureRecoveryMs / 2); + succeed(); + expect(manager.downloaders).toHaveLength(1); + + vi.advanceTimersByTime(DownloadManager.FailureRecoveryMs); + succeed(); + expect(manager.downloaders).toHaveLength(2); + // And no more than were taken away. + succeed(); + expect(manager.downloaders).toHaveLength(2); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + it('reads the downloader limit the same way for the speed test and the key', () => { // 0 meant "never add one" to the speed test, and "no limit" to the add-downloader key. const limit = (maximumDownloaders) => new DownloadManager({options: {maximumDownloaders}}).downloaderLimit(); From a2c3bf4b9a2d24fae088ff23915efd4f7209e55a Mon Sep 17 00:00:00 2001 From: Nawid Salehie <102778966+Nawid3333@users.noreply.github.com> Date: Sat, 3 Oct 2026 15:54:09 +0200 Subject: [PATCH 08/15] fix(network): speed over a zero-length window is 0, and prune's test can fail - SpeedTracker.getSpeed divided by the time since its oldest entry started. A response that arrived within one performance.now() tick of being asked for (a cache hit; Firefox's clock moves in 1 ms steps) gave Infinity. The speed shown smooths each value into the last, so it then said Infinity for the rest of the session, and the download speed limit stopped predownloading while it lasted. A zero window now counts as 0, like no data (#140). - The prune test added its stale entries through update(), which already prunes, so it passed with prune() doing nothing (`while (false)`: 6 of 6). The prune cases now fill the buffer directly and check exactly what is left: four stale entries leave the last two, two stale entries are both kept, and only stale entries go (#251). SpeedTracker.mjs is left out of Stryker's list here; #253 adds it with the other tested modules. Mutation-checked: `while (false)` fails three cases, a floor of 1 instead of 2 fails three, a wider cutoff fails one, and without the zero check the new getSpeed case fails. Fixes #140, fixes #251. Co-Authored-By: Claude Opus 5.5 --- chrome/player/network/SpeedTracker.mjs | 4 ++ tests/unit/SpeedTracker.test.mjs | 51 ++++++++++++++++++++++---- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/chrome/player/network/SpeedTracker.mjs b/chrome/player/network/SpeedTracker.mjs index 81728911c..0f80e748f 100644 --- a/chrome/player/network/SpeedTracker.mjs +++ b/chrome/player/network/SpeedTracker.mjs @@ -35,6 +35,10 @@ export class SpeedTracker { const now = performance.now(); const dt = (now - this.buffer[0].start) / 1000; + // A response that came within the clock's tick (a cache hit) gave Infinity: the shown + // speed, which smooths over what it is given, said Infinity for the rest of the + // session, and the speed limit held predownloading back meanwhile. + if (dt <= 0) return 0; return totalData / dt; } } diff --git a/tests/unit/SpeedTracker.test.mjs b/tests/unit/SpeedTracker.test.mjs index 27f7c983a..97315cee4 100644 --- a/tests/unit/SpeedTracker.test.mjs +++ b/tests/unit/SpeedTracker.test.mjs @@ -1,4 +1,4 @@ -import {describe, expect, it} from 'vitest'; +import {describe, expect, it, vi} from 'vitest'; import {SpeedTracker} from '../../chrome/player/network/SpeedTracker.mjs'; // Drives the displayed download speed and any speed-based ABR decisions. @@ -43,17 +43,54 @@ describe('SpeedTracker', () => { expect(t.buffer).toHaveLength(2); }); - it('prunes entries older than cutoffSize but always keeps at least 2', () => { + // The prune cases fill the buffer directly: update() prunes as it goes, so entries added + // through it are already pruned, and a prune() that did nothing passed (#251). + /** A tracker holding entries that ended at the given times. */ + function trackerWith(...ends) { + const t = new SpeedTracker(); + t.buffer = ends.map((end) => ({dataSize: 100, start: end - 1000, end})); + return t; + } + + it('prunes entries older than cutoffSize down to the last 2', () => { + const now = performance.now(); + // Four entries, all ending well before the 10s cutoff. + const t = trackerWith(now - 21000, now - 19000, now - 17000, now - 15000); + t.prune(); + expect(t.buffer.map((entry) => entry.end)).toEqual([now - 17000, now - 15000]); + }); + + it('keeps 2 entries even when both are older than cutoffSize', () => { + const now = performance.now(); + const t = trackerWith(now - 17000, now - 15000); + t.prune(); + expect(t.buffer).toHaveLength(2); + }); + + it('prunes only the entries older than cutoffSize', () => { + const now = performance.now(); + const t = trackerWith(now - 30000, now - 20000, now - 5000, now - 3000, now - 1000); + t.prune(); + expect(t.buffer.map((entry) => entry.end)).toEqual([now - 5000, now - 3000, now - 1000]); + }); + + it('prunes as it is updated', () => { const now = performance.now(); const t = new SpeedTracker(); - // Three entries, all ending well before the 10s cutoff. t.update(100, now - 20000, now - 19000); t.update(100, now - 18000, now - 17000); t.update(100, now - 16000, now - 15000); - t.prune(); - // The oldest is prunable, but the "keep at least 2" floor stops there. - expect(t.buffer.length).toBeGreaterThanOrEqual(2); - expect(t.buffer[t.buffer.length - 1].end).toBe(now - 15000); + expect(t.buffer.map((entry) => entry.end)).toEqual([now - 17000, now - 15000]); + }); + + it('reports 0, not Infinity, for data that came within one tick of the clock', () => { + // A response from the cache can come within the same performance.now() tick + // it was asked in (1 ms in Firefox). + vi.spyOn(performance, 'now').mockReturnValue(5000); + const t = new SpeedTracker(); + t.update(4096, 5000, 5000); + expect(t.getSpeed()).toBe(0); + vi.restoreAllMocks(); }); it('does not prune recent entries', () => { From 3c26d2c6454923d295c97885a81465dc13339275 Mon Sep 17 00:00:00 2001 From: Nawid Salehie <102778966+Nawid3333@users.noreply.github.com> Date: Sat, 3 Oct 2026 15:56:08 +0200 Subject: [PATCH 09/15] fix(network): ignored Range answers are cut to the range; unsendable headers fail at once - A server that ignores the Range header answers 200 with the whole file. That was taken for the range asked for: the file's first bytes, handed to the player as a range further in (a byte-range HLS segment, a DASH index or segment, an MP4 range). XHRLoader now cuts the asked-for range out of such an answer and stops reading once it is in; a range that starts past the end of the file fails as 416 (#139). - A header fetch() cannot send (a custom header whose name has a space) made fetch() reject with a TypeError, which was retried like a network error, six times over about a minute. The headers are now checked with `new Headers()` first, and such a load fails at once (#150). tests/unit/XHRLoader.test.mjs: the header case and three of the four ignored- Range cases fail on the old loader; without the reader.cancel() the "stops reading" case fails. Fixes #139, fixes #150. Co-Authored-By: Claude Opus 5.5 --- chrome/player/network/XHRLoader.mjs | 32 ++++++++++-- tests/unit/XHRLoader.test.mjs | 79 +++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 4 deletions(-) diff --git a/chrome/player/network/XHRLoader.mjs b/chrome/player/network/XHRLoader.mjs index d64cd0185..fb92fec71 100644 --- a/chrome/player/network/XHRLoader.mjs +++ b/chrome/player/network/XHRLoader.mjs @@ -113,6 +113,9 @@ export class XHRLoader { if (!Object.hasOwn(regularHeaders, header)) continue; fetchHeaders[header] = regularHeaders[header]; } + // A header fetch() cannot send (a name with a space, a value with a line break) + // fails here, for good: from fetch() it was a network error, retried six times. + new Headers(fetchHeaders); if (customHeaderCommands.length) { if (EnvUtils.isExtension()) { @@ -192,10 +195,14 @@ export class XHRLoader { this.response = response; const isArrayBuffer = request.responseType === 'arraybuffer'; + // A server that ignores Range answers 200 with the whole file, from its first byte. That + // was taken for the range asked for (the file's start, labelled as a range further in); + // the range is cut out of it now (readBody). + const wholeFile = status === 200 && !!request.rangeEnd; let data; try { - data = await this.readBody(response, isArrayBuffer, isStale); + data = await this.readBody(response, isArrayBuffer, isStale, wholeFile ? request : null); } catch (e) { if (isStale()) { return; @@ -208,6 +215,12 @@ export class XHRLoader { return; } + if (wholeFile && !(isArrayBuffer ? data.byteLength : data.length)) { + // The file ends before the range starts. + this.handleLoadFailure(416, 'Range Not Satisfiable'); + return; + } + self.clearTimeout(this.requestTimeout); stats.loading.end = Math.max(self.performance.now(), stats.loading.first); stats.loaded = stats.total = isArrayBuffer ? data.byteLength : data.length; @@ -253,9 +266,12 @@ export class XHRLoader { * @param {boolean} isArrayBuffer - true to return an ArrayBuffer, false for text. * @param {function(): boolean} [isStale] - true once this attempt has been torn down; * a chunk still delivered then must not re-arm the next attempt's stall timer. + * @param {?{rangeStart: number, rangeEnd: number}} [range] - for a body that is the whole + * file although a range was asked for: only that range is returned, and reading stops + * once it is in. * @return {Promise} the concatenated response body. */ - async readBody(response, isArrayBuffer, isStale = () => false) { + async readBody(response, isArrayBuffer, isStale = () => false, range = null) { const stats = this.stats; const contentLength = response.headers.get('content-length'); if (contentLength) { @@ -269,8 +285,9 @@ export class XHRLoader { const reader = response.body.getReader(); const chunks = []; let receivedLength = 0; + const limit = range ? range.rangeEnd : Infinity; - while (true) { + while (receivedLength < limit) { const {done, value} = await reader.read(); if (done) break; if (isStale()) { @@ -283,13 +300,20 @@ export class XHRLoader { this.rearmTimeout(); } + if (receivedLength >= limit) { + // The rest of the file is not needed. + reader.cancel().catch(() => {}); + } - const merged = new Uint8Array(receivedLength); + let merged = new Uint8Array(receivedLength); let offset = 0; for (const chunk of chunks) { merged.set(chunk, offset); offset += chunk.byteLength; } + if (range) { + merged = merged.slice(range.rangeStart || 0, range.rangeEnd); + } return isArrayBuffer ? merged.buffer : new TextDecoder('utf-8').decode(merged); } diff --git a/tests/unit/XHRLoader.test.mjs b/tests/unit/XHRLoader.test.mjs index f6dcd8ae4..a259718f5 100644 --- a/tests/unit/XHRLoader.test.mjs +++ b/tests/unit/XHRLoader.test.mjs @@ -416,4 +416,83 @@ describe('XHRLoader', () => { const init = fetchMock.mock.calls[0][1]; expect(init.headers['Range']).toBe('bytes=100-199'); }); + + it('fails at once, without retrying, for a header fetch() cannot send', async () => { + // A name with a space (a custom header) made a real fetch() reject with a TypeError, + // which was retried like a network error, six times over a minute (#150). + const fetchMock = vi.fn(async (url, init) => { + new Headers(init.headers); + return new Response(new ArrayBuffer(0), {status: 200}); + }); + vi.stubGlobal('fetch', fetchMock); + + const loader = new XHRLoader(); + const recorder = makeCallbackRecorder(); + loader.addCallbacks(recorder); + loader.load(makeRequest({headers: {'x-bad name': 'value'}}), makeConfig()); + await vi.runAllTimersAsync(); + + expect(fetchMock).not.toHaveBeenCalled(); + expect(loader.stats.retry).toBe(0); + expect(recorder.calls.map((c) => c.type)).toEqual(['onError']); + }); + + describe('a server that ignores Range and sends the whole file (200)', () => { + // It was taken for the range asked for: the file's first bytes, labelled as a range + // further in (#139). + const file = Uint8Array.from({length: 100}, (_, i) => i); + + /** The data a range of `file` came back as. */ + async function loadRange(rangeStart, rangeEnd, body = file) { + vi.stubGlobal('fetch', vi.fn(async () => new Response(body, {status: 200}))); + const loader = new XHRLoader(); + const recorder = makeCallbackRecorder(); + loader.addCallbacks(recorder); + loader.load(makeRequest({rangeStart, rangeEnd}), makeConfig()); + await vi.runAllTimersAsync(); + return {recorder, loader}; + } + + it('gives the range asked for, cut out of the file', async () => { + const {recorder, loader} = await loadRange(10, 20); + const [response] = recorder.calls.find((c) => c.type === 'onSuccess').args; + expect([...new Uint8Array(response.data)]).toEqual([10, 11, 12, 13, 14, 15, 16, 17, 18, 19]); + expect(loader.stats.loaded).toBe(10); + }); + + it('stops reading the file once the range is in', async () => { + // A body that would go on for ever after its first 30 bytes. + let cancelled = false; + let sent = 0; + const endless = new ReadableStream({ + pull(controller) { + if (sent < 30) { + controller.enqueue(file.slice(sent, sent + 10)); + sent += 10; + return; + } + return new Promise(() => {}); + }, + cancel() { + cancelled = true; + }, + }); + const {recorder} = await loadRange(5, 15, endless); + const [response] = recorder.calls.find((c) => c.type === 'onSuccess').args; + expect([...new Uint8Array(response.data)]).toEqual([5, 6, 7, 8, 9, 10, 11, 12, 13, 14]); + expect(cancelled).toBe(true); + }); + + it('fails a range that starts past the end of the file', async () => { + const {recorder, loader} = await loadRange(200, 300); + expect(recorder.calls.map((c) => c.type)).toEqual(['onError']); + expect(loader.stats.error.code).toBe(416); + }); + + it('leaves a whole-file request alone', async () => { + const {recorder} = await loadRange(undefined, undefined); + const [response] = recorder.calls.find((c) => c.type === 'onSuccess').args; + expect(new Uint8Array(response.data)).toEqual(file); + }); + }); }); From 3afbbd8040f8313442aa39fb3325a0776f3a9e49 Mon Sep 17 00:00:00 2001 From: Nawid Salehie <102778966+Nawid3333@users.noreply.github.com> Date: Sat, 3 Oct 2026 15:56:58 +0200 Subject: [PATCH 10/15] fix(player): no false "not enough storage" on high-bitrate MP4s updateHasDownloadSpace estimates the video's bitrate from the downloaded fragments once five are in: their bytes over their duration. MP4Player starts its 1 MB ranges on whole seconds, so on a video of about 40 Mbit/s or more the first five lie in the first second and last 0 s. The bitrate came out as Infinity: the player said storage was short, set the buffer back, and marked every downloaded fragment to be kept for the rest of the session. The estimate is now Utils.measuredBitrate, which answers null (use the level's own bitrate, as before five fragments) while the fragments span no time. tests/unit/measuredBitrate.test.mjs; without the zero-time check its third case gets Infinity. Fixes #210. Co-Authored-By: Claude Opus 5.5 --- chrome/player/FastStreamClient.mjs | 15 +------------- chrome/player/utils/Utils.mjs | 24 ++++++++++++++++++++++ tests/unit/measuredBitrate.test.mjs | 31 +++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 14 deletions(-) create mode 100644 tests/unit/measuredBitrate.test.mjs diff --git a/chrome/player/FastStreamClient.mjs b/chrome/player/FastStreamClient.mjs index 9e9a5aa7f..72aa3c667 100644 --- a/chrome/player/FastStreamClient.mjs +++ b/chrome/player/FastStreamClient.mjs @@ -644,20 +644,7 @@ export class FastStreamClient extends EventEmitter { let bitrate = level.bitrate; const fragments = this.fragments; if (fragments) { - let count = 0; - let size = 0; - let totalDuration = 0; - fragments.forEach((fragment) => { - if (fragment && fragment.dataSize !== null) { - count++; - size += fragment.dataSize; - totalDuration += fragment.duration; - } - }); - - if (count > 4) { - bitrate = size / totalDuration * 8; - } + bitrate = Utils.measuredBitrate(fragments) ?? bitrate; } if (bitrate && this.duration) { let storageAvailable = (this.storageAvailable * 8) * 0.6; diff --git a/chrome/player/utils/Utils.mjs b/chrome/player/utils/Utils.mjs index aa7efce06..b73a6bc08 100644 --- a/chrome/player/utils/Utils.mjs +++ b/chrome/player/utils/Utils.mjs @@ -127,6 +127,30 @@ export class Utils { return zippedFragments; } + /** + * The bitrate the downloaded fragments show, or null while it cannot be told: fewer than + * five downloaded, or no time between them. An MP4's ranges start on whole seconds, so + * on a high-bitrate one the first few share a second and last 0 s; that gave Infinity, a + * false "not enough storage" warning, and every downloaded fragment kept for the session. + * @param {Array} fragments - Fragments with dataSize (null until downloaded) + * and duration. + * @return {?number} Bits per second. + */ + static measuredBitrate(fragments) { + let count = 0; + let size = 0; + let totalDuration = 0; + fragments.forEach((fragment) => { + if (fragment && fragment.dataSize !== null) { + count++; + size += fragment.dataSize; + totalDuration += fragment.duration; + } + }); + if (count <= 4 || !(totalDuration > 0)) return null; + return size / totalDuration * 8; + } + /** * Loads and parses options from storage, merging with defaults. * @param {string} key - Storage key. diff --git a/tests/unit/measuredBitrate.test.mjs b/tests/unit/measuredBitrate.test.mjs new file mode 100644 index 000000000..d461ea788 --- /dev/null +++ b/tests/unit/measuredBitrate.test.mjs @@ -0,0 +1,31 @@ +import {describe, expect, it} from 'vitest'; +import {Utils} from '../../chrome/player/utils/Utils.mjs'; + +// FastStreamClient.updateHasDownloadSpace works out from this whether the whole video fits +// in the storage left. An MP4's ranges start on whole seconds, so on a high-bitrate one the +// first downloaded ranges all lasted 0 s: the bitrate came out as Infinity, the player +// warned that storage was short, and kept every downloaded fragment for the session (#210). + +/** A fragment, downloaded when it has a size. */ +const fragment = (dataSize, duration) => ({dataSize, duration}); +const MB = 1000000; + +describe('Utils.measuredBitrate', () => { + it('is the downloaded bytes over their time, in bits per second', () => { + const fragments = Array.from({length: 5}, () => fragment(MB, 1)); + expect(Utils.measuredBitrate(fragments)).toBe(8 * MB); + }); + + it('counts only downloaded fragments, and needs five of them', () => { + const fragments = [...Array.from({length: 4}, () => fragment(MB, 1)), fragment(null, 1), undefined]; + expect(Utils.measuredBitrate(fragments)).toBeNull(); + fragments.push(fragment(MB, 1)); + expect(Utils.measuredBitrate(fragments)).toBe(8 * MB); + }); + + it('is unknown, not Infinity, while the downloaded fragments last no time', () => { + // Five 1 MB ranges of a 40 Mbit/s video, all inside its first second. + const fragments = Array.from({length: 5}, () => fragment(MB, 0)); + expect(Utils.measuredBitrate(fragments)).toBeNull(); + }); +}); From fa17116943b7463918469dfd6ee72e87f5a6a649 Mon Sep 17 00:00:00 2001 From: Nawid Salehie <102778966+Nawid3333@users.noreply.github.com> Date: Sat, 3 Oct 2026 15:58:07 +0200 Subject: [PATCH 11/15] perf(progress): derive the two remembered-position hashes side by side Opening a video looks up its remembered position under two PBKDF2 hashes of its address (600,000 rounds each), and the video's start - autoplay, the seek to the time in its URL - waits for that lookup. The two were derived one after the other; they are now derived at once. The hashes, and so the stored records, are unchanged. In Node the pair takes about 100 ms instead of 180 ms; Firefox runs WebCrypto work off the main thread as well, so the wait should drop the same way (not measured there). tests/unit/SecureMemory.test.mjs: the new case fails on the old code (the second hash starts only after the first ends). Part of #146 (the proposal to replace PBKDF2 is left for a decision). Co-Authored-By: Claude Opus 5.5 --- chrome/player/modules/SecureMemory.mjs | 8 ++++++-- tests/unit/SecureMemory.test.mjs | 23 +++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/chrome/player/modules/SecureMemory.mjs b/chrome/player/modules/SecureMemory.mjs index 010bb4452..b1f1d338a 100644 --- a/chrome/player/modules/SecureMemory.mjs +++ b/chrome/player/modules/SecureMemory.mjs @@ -124,8 +124,12 @@ export class SecureMemory { } async getHashes(identifier) { - const identifierHash = await SecureMemory.hash(identifier, this.identifierSalt); - const keyHash= await SecureMemory.hash(identifier, this.keySalt); + // Side by side: each is 600,000 rounds of PBKDF2, and the video's start (autoplay, the + // seek to the time in its URL) waits for both. + const [identifierHash, keyHash] = await Promise.all([ + SecureMemory.hash(identifier, this.identifierSalt), + SecureMemory.hash(identifier, this.keySalt), + ]); return {identifierHash, keyHash}; } diff --git a/tests/unit/SecureMemory.test.mjs b/tests/unit/SecureMemory.test.mjs index ad110a834..361b62380 100644 --- a/tests/unit/SecureMemory.test.mjs +++ b/tests/unit/SecureMemory.test.mjs @@ -184,3 +184,26 @@ describe('SecureMemory.getFile', () => { expect(await memory.getFile(await hashesFor('video'))).toBeNull(); }); }); + +describe('SecureMemory.getHashes', () => { + it('derives the two hashes side by side, not one after the other', async () => { + // Each is 600,000 rounds of PBKDF2, and the video's start waits for both. + const started = []; + const finish = []; + const hash = vi.spyOn(SecureMemory, 'hash').mockImplementation((message, salt) => { + started.push(salt); + return new Promise((resolve) => finish.push(resolve)); + }); + const memory = new SecureMemory('test'); + memory.identifierSalt = 'identifier salt'; + memory.keySalt = 'key salt'; + + const hashes = memory.getHashes('video'); + await Promise.resolve(); + expect(started).toEqual(['identifier salt', 'key salt']); + + finish.forEach((resolve, i) => resolve('hash ' + i)); + expect(await hashes).toEqual({identifierHash: 'hash 0', keyHash: 'hash 1'}); + hash.mockRestore(); + }); +}); From 8af51b0a1bb3831d95d6ef9c660f8636873108a6 Mon Sep 17 00:00:00 2001 From: Nawid Salehie <102778966+Nawid3333@users.noreply.github.com> Date: Sat, 3 Oct 2026 15:58:26 +0200 Subject: [PATCH 12/15] style(save): await the download in closeWhenDownloaded rather than chain it eslint.modules.config.js's promise/catch-or-return flagged the bare .then() chain added in the previous StreamSaver commit. Same behaviour; the StreamSaver tests still pass. Co-Authored-By: Claude Opus 5.5 --- chrome/player/modules/StreamSaver.mjs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/chrome/player/modules/StreamSaver.mjs b/chrome/player/modules/StreamSaver.mjs index d0cb1895b..5e001c59f 100644 --- a/chrome/player/modules/StreamSaver.mjs +++ b/chrome/player/modules/StreamSaver.mjs @@ -64,12 +64,14 @@ function createWriteStreamBlob(filename, opts, size) { * @param {FSBlob} blobManager * @param {string} url - The blob: URL being downloaded. * @param {*} download - What Utils.downloadURL resolved with. + * @return {Promise} Not awaited: the save is done before its download is. */ -function closeWhenDownloaded(blobManager, url, download) { - Promise.all([ +async function closeWhenDownloaded(blobManager, url, download) { + await Promise.all([ Utils.revokeWhenDownloaded(url, download), Utils.asyncTimeout(120000), - ]).then(() => blobManager.close()); + ]); + blobManager.close(); } /** From 85551bfed59e17345a3df8e157df0aa4ceec2bc3 Mon Sep 17 00:00:00 2001 From: Nawid Salehie <102778966+Nawid3333@users.noreply.github.com> Date: Sat, 3 Oct 2026 15:58:55 +0200 Subject: [PATCH 13/15] docs: OPFS file names and the worker watchdog in CLAUDE.md What a later reader of the OPFS code needs: fragments were never stored there until #132's fix, files are found by OPFSManager.fileName(), write() reports a full disk only by its count, and the 30 s limit is a watchdog over the worker's answers, not a per-call timer. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 63931934c..747235757 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -384,9 +384,16 @@ than trusting `FSBlob`'s own self-report. `getFile()`). It read each fragment back through the worker into an `ArrayBuffer` wrapped in a Blob, so every "offloaded" fragment was in RAM as well. A stored fragment is now deleted from disk on eviction, as the Cache backend's always were. -- A worker call unanswered for 30 s counts as a crash (`CallTimeoutMs`), and a dead - worker moves FSBlob on to its next backend instead of keeping every later fragment in - RAM. +- A worker that answers nothing for 30 s while calls wait counts as a crash + (`CallTimeoutMs`, one watchdog re-armed by every answer: it runs calls one at a time, so + timing each call from its post counted a write backlog as a crash), and a dead worker + moves FSBlob on to its next backend instead of keeping every later fragment in RAM. +- **Until 2026-10-03 no playback fragment was ever stored in OPFS** (#132): their + identifiers are URLs, and Firefox refuses a name with a `/` (and `\` on Windows). The + worker now only sees names `OPFSManager.fileName()` hands out (`f0`, `f1`, ...), so a + file on disk is found by `opfsManager.fileName(identifier)`, not by its identifier. A + sync access handle's `write()` reports a disk-full write only by a short count (Gecko + never throws), which the worker checks (`writeAll`). - `prune()` in the worker leaves a session directory younger than `STALE_MS` alone (it exists before its first heartbeat: two players starting together deleted each other's), and a heartbeat that exists but cannot be read (its owner is writing it) means From c6e759b747c8866f2ed5a28c4d30439ff7788308 Mon Sep 17 00:00:00 2001 From: Nawid Salehie <102778966+Nawid3333@users.noreply.github.com> Date: Sat, 3 Oct 2026 16:00:33 +0200 Subject: [PATCH 14/15] fix(network): a 200 that says it is the range is still taken as the range Narrows the previous commit's ignored-Range handling to the clear case. A 200 with a Content-Range header, or exactly as long as the range asked for (from a start past 0, which no whole file can be), is a server sending the range with the wrong status; that worked before and is left alone. Only a 200 without either is the whole file and has the range cut out of it. tests/unit/XHRLoader.test.mjs: "takes a 200 that says it is the range" fails with either check taken out. Co-Authored-By: Claude Opus 5.5 --- chrome/player/network/XHRLoader.mjs | 7 +++++-- tests/unit/XHRLoader.test.mjs | 13 +++++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/chrome/player/network/XHRLoader.mjs b/chrome/player/network/XHRLoader.mjs index fb92fec71..06a138e19 100644 --- a/chrome/player/network/XHRLoader.mjs +++ b/chrome/player/network/XHRLoader.mjs @@ -197,8 +197,11 @@ export class XHRLoader { const isArrayBuffer = request.responseType === 'arraybuffer'; // A server that ignores Range answers 200 with the whole file, from its first byte. That // was taken for the range asked for (the file's start, labelled as a range further in); - // the range is cut out of it now (readBody). - const wholeFile = status === 200 && !!request.rangeEnd; + // the range is cut out of it now (readBody). A 200 that says it is the range (a + // Content-Range, or exactly the range's length) is taken as one, as before. + const length = parseInt(response.headers.get('content-length'), 10); + const wholeFile = status === 200 && !!request.rangeEnd && !response.headers.has('content-range') && + !(request.rangeStart > 0 && length === request.rangeEnd - request.rangeStart); let data; try { diff --git a/tests/unit/XHRLoader.test.mjs b/tests/unit/XHRLoader.test.mjs index a259718f5..71c98e490 100644 --- a/tests/unit/XHRLoader.test.mjs +++ b/tests/unit/XHRLoader.test.mjs @@ -443,8 +443,8 @@ describe('XHRLoader', () => { const file = Uint8Array.from({length: 100}, (_, i) => i); /** The data a range of `file` came back as. */ - async function loadRange(rangeStart, rangeEnd, body = file) { - vi.stubGlobal('fetch', vi.fn(async () => new Response(body, {status: 200}))); + async function loadRange(rangeStart, rangeEnd, body = file, headers = {}) { + vi.stubGlobal('fetch', vi.fn(async () => new Response(body, {status: 200, headers}))); const loader = new XHRLoader(); const recorder = makeCallbackRecorder(); loader.addCallbacks(recorder); @@ -489,6 +489,15 @@ describe('XHRLoader', () => { expect(loader.stats.error.code).toBe(416); }); + it('takes a 200 that says it is the range as the range, as before', async () => { + const range = file.slice(10, 20); + for (const headers of [{'content-range': 'bytes 10-19/100'}, {'content-length': '10'}]) { + const {recorder} = await loadRange(10, 20, range, headers); + const [response] = recorder.calls.find((c) => c.type === 'onSuccess').args; + expect(new Uint8Array(response.data)).toEqual(range); + } + }); + it('leaves a whole-file request alone', async () => { const {recorder} = await loadRange(undefined, undefined); const [response] = recorder.calls.find((c) => c.type === 'onSuccess').args; From 9b6bddc2fbe7696c433d3dcfed9a9343b6403dbb Mon Sep 17 00:00:00 2001 From: Nawid Salehie <102778966+Nawid3333@users.noreply.github.com> Date: Sat, 3 Oct 2026 16:55:28 +0200 Subject: [PATCH 15/15] test(save): the StreamSaver test's stand-in store has whenStored The save PR's memory sink waits on FSBlob.whenStored. With both PRs merged, this test's stand-in lacked it and two memory-sink cases failed; one no-op method fixes that, so the two PRs merge in either order. Co-Authored-By: Claude Opus 5.5 --- tests/unit/StreamSaver.test.mjs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/unit/StreamSaver.test.mjs b/tests/unit/StreamSaver.test.mjs index 29024897b..ca2e79f5b 100644 --- a/tests/unit/StreamSaver.test.mjs +++ b/tests/unit/StreamSaver.test.mjs @@ -32,6 +32,9 @@ vi.mock('../../chrome/player/modules/FSBlob.mjs', () => ({ getBlob(id) { return this.blobs.get(id); } + // The memory sink waits on this once the save PR (#222/#297) is in; nothing to wait + // for in a store that keeps everything in memory. + async whenStored() {} }, }));