fix(system): keep a restore review's upload form alive until it is answered (#583) (#584)

* fix(system): keep a restore review's upload form alive until it is answered (#583)

WebKit can read a FormData's file part after fetch() returned, and with a
request interceptor armed it does. uploadRestore held its FormData only in a
local that was dead after the call, so a garbage collection in between dropped
the form and the server got an empty file ("Uploaded file is empty"); the
Restore preview never opened. The page now holds each upload's form until the
upload is answered.

* docs(system): say what the #583 regression spec proves in each browser (#583)

tests/browser/workspace-system-backups.spec.mjs

@@ -284,6 +284,23 @@ test('a restore seen finishing elsewhere closes an open restore review and disca
     expect((await serverJson(request, '/__test__/system/backups')).stages).toBe(0);
 });
 
+// #583: in WebKit, while a page.route is armed, a file part of a FormData body is read only when the request goes on;
+// a garbage collection in between dropped the form once nothing referenced it, and the server got an empty file
+// ("Uploaded file is empty"). The page keeps the form until its upload is answered. The route holds the upload
+// through a collection; without the fix WebKit lost the file every time (0/10 in the pinned image). Chromium and
+// Firefox keep the body anyway, so there it only checks that a held upload still opens the review.
+test('a review upload survives a garbage collection while it is in flight', async ({ page, request }) => {
+    const bytes = await backupBytes(request);
+    await page.route(`**${PREVIEW_URL}`, async route => {
+        await page.requestGC();
+        await route.continue();
+    });
+    await openBackups(page);
+    expect((await chooseBackup(page, bytes)).status()).toBe(200);
+    await expect(dialog(page).locator('#modal-title')).toHaveText('Restore preview');
+    expect((await serverJson(request, '/__test__/system/backups')).stages).toBe(1);
+});
+
 test('invalid file shows the validator message; nothing is staged', async ({ page, request }) => {
     const before = await serverJson(request, '/__test__/system/digest');
     await openBackups(page);

tests/js/workspace-system-backups.test.js

@@ -128,6 +128,24 @@ test('invalid file shows the validator message; a refusal names its job; a lost
     assert.deepEqual(await uploadRestore(file, { ApiError, fetchUrl: async () => response(200, PREVIEW) }), PREVIEW);
 });
 
+// #583: WebKit reads a FormData's file part by reference, possibly after a garbage collection; the page holds the form
+// until the upload is answered, then lets it go, whether the server staged it, refused it or the request failed.
+test('a review upload keeps its form referenced until the upload is answered', async () => {
+    const { uploadRestore, uploadsInFlight } = await backups;
+    const file = new File(['bytes'], 'b.db');
+    for (const answer of [async () => response(200, PREVIEW), async () => response(400, { success: false, error: 'x' }),
+        async () => { throw new TypeError('offline'); }]) {
+        let held;
+        const upload = uploadRestore(file, { ApiError, fetchUrl: async (url, init) => {
+            held = uploadsInFlight.has(init.body) && init.body.get('database_file').size === file.size;
+            return answer();
+        } }).catch(() => {});
+        await upload;
+        assert.equal(held, true);
+        assert.equal(uploadsInFlight.size, 0);
+    }
+});
+
 test('Cancel discards the stage through the api client and never throws', async () => {
     const { discardRestore, PREVIEW_URL } = await backups;
     const calls = [];

web/js/workspace/system-backups.mjs

@@ -179,13 +179,27 @@ export function reviewRefusal(error) {
     return REVIEW_FAILED;
 }
 
+/**
+ * #583: the review uploads not answered yet. WebKit can read a FormData's file part after the fetch call returned (it
+ * did whenever a request interceptor, here Playwright's, was armed); a garbage collection in between dropped a form
+ * nothing referenced any more, and the server got an empty file. Each form is held here until its upload is answered.
+ * Exported for the unit tests only.
+ */
+export const uploadsInFlight = new Set();
+
 /** Stage `file` for review; the server's answer, or an ApiError naming its refusal. */
 export async function uploadRestore(file, { fetchUrl = window.ApiClient.fetchUrl, ApiError = window.ApiClient.ApiError,
     signal } = {}) {
     const form = new FormData();
     form.append('database_file', file, file.name);
-    const response = await fetchUrl(PREVIEW_URL, { method: 'POST', body: form, cache: 'no-store', credentials: 'same-origin',
-        signal });
+    uploadsInFlight.add(form);
+    let response;
+    try {
+        response = await fetchUrl(PREVIEW_URL, { method: 'POST', body: form, cache: 'no-store', credentials: 'same-origin',
+            signal });
+    } finally {
+        uploadsInFlight.delete(form);
+    }
     let data = null;
     try {
         data = await response.json();