diff --git a/package.json b/package.json index 07f66ea..3d3e99f 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,7 @@ "lint": "eslint .", "build": "npm run build:webpack && npm run build:targets", "build:firefox": "npm run build", - "test:e2e": "node --experimental-detect-module --test scripts/e2e-session-persistence.test.mjs scripts/e2e-session-restore-choice.test.mjs scripts/e2e-multifile-build.test.mjs scripts/e2e-workspace-file-tracking.test.mjs scripts/e2e-workspace-scan-progress.test.mjs scripts/e2e-terminal-mkdir.test.mjs scripts/e2e-terminal-stop.test.mjs scripts/e2e-terminal-git-removal.test.mjs scripts/e2e-terminal-stop-icon.test.mjs scripts/e2e-browser-compatibility.test.mjs scripts/e2e-firefox-compatibility.test.mjs scripts/e2e-firefox-jspi-stdin.test.mjs scripts/e2e-wasi-shim.test.mjs scripts/e2e-run-request.test.mjs scripts/e2e-release-packaging.test.mjs", + "test:e2e": "node --experimental-detect-module --test scripts/e2e-page-lifecycle.test.mjs scripts/e2e-session-persistence.test.mjs scripts/e2e-session-restore-choice.test.mjs scripts/e2e-multifile-build.test.mjs scripts/e2e-workspace-file-tracking.test.mjs scripts/e2e-workspace-scan-progress.test.mjs scripts/e2e-terminal-mkdir.test.mjs scripts/e2e-terminal-stop.test.mjs scripts/e2e-terminal-git-removal.test.mjs scripts/e2e-terminal-stop-icon.test.mjs scripts/e2e-browser-compatibility.test.mjs scripts/e2e-firefox-compatibility.test.mjs scripts/e2e-firefox-jspi-stdin.test.mjs scripts/e2e-wasi-shim.test.mjs scripts/e2e-run-request.test.mjs scripts/e2e-release-packaging.test.mjs", "test:e2e:compiler": "npm run test:preflight-clang && node --experimental-detect-module --test scripts/e2e-compiler-link.test.mjs", "test:preflight-clang": "node scripts/preflight-clang-artifacts.js", "test:browser:chrome": "npm run test:e2e:compiler && node scripts/smoke-browser.mjs chrome", diff --git a/scripts/e2e-page-lifecycle.test.mjs b/scripts/e2e-page-lifecycle.test.mjs new file mode 100644 index 0000000..2310048 --- /dev/null +++ b/scripts/e2e-page-lifecycle.test.mjs @@ -0,0 +1,24 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; + +test('e2e: page unload only terminates the worker synchronously', async () => { + const { registerPageUnload } = await import('../src/ui/page-lifecycle.mjs'); + const listeners = new Map(); + const events = []; + const target = { + addEventListener(type, listener) { + listeners.set(type, listener); + }, + }; + const worker = { + terminate() { + events.push('terminate'); + }, + }; + + registerPageUnload(target, worker); + const result = listeners.get('beforeunload')(); + + assert.equal(result, undefined); + assert.deepEqual(events, ['terminate']); +}); diff --git a/scripts/e2e-session-persistence.test.mjs b/scripts/e2e-session-persistence.test.mjs index ed6ebcf..c467fd3 100644 --- a/scripts/e2e-session-persistence.test.mjs +++ b/scripts/e2e-session-persistence.test.mjs @@ -13,6 +13,7 @@ import { restoreWorkspace as restoreToolbarWorkspace, getOpenTabPaths as getToolbarOpenTabPaths, getActiveTabPath as getToolbarActiveTabPath, + getOpenTabsSnapshot as getToolbarOpenTabsSnapshot, } from '../src/ui/toolbar.js'; class FakeElement { @@ -197,6 +198,191 @@ function createFailingHandleStore() { }; } +test('e2e: state-only persistence never writes or clears the directory handle', async () => { + const storage = createStorageArea(); + const directoryHandle = { name: 'project' }; + const handleCalls = []; + const persistence = createSessionPersistence({ + fsAPI: { + getDirectoryHandle: () => directoryHandle, + getWorkspaceSnapshot: () => ({ + name: 'project', + entries: [{ path: 'main.cpp', kind: 'file' }], + }), + openFolderFromHandle: async () => null, + }, + getOpenTabPaths: () => ['main.cpp'], + getActiveTabPath: () => 'main.cpp', + getOpenTabsSnapshot: () => ({ 'main.cpp': 'int main() {}\n' }), + restoreWorkspace: async () => {}, + storage, + handleStore: { + async save(handle) { + handleCalls.push(['save', handle]); + }, + async load() { + return null; + }, + async clear() { + handleCalls.push(['clear']); + }, + }, + }); + + await persistence.persistSessionState(); + + assert.deepEqual(handleCalls, []); + assert.deepEqual( + (await storage.get('browser_cpp_session')).browser_cpp_session.openTabPaths, + ['main.cpp'] + ); +}); + +test('e2e: workspace persistence stores the directory handle before serializable state', async () => { + const events = []; + const directoryHandle = { name: 'project' }; + const persistence = createSessionPersistence({ + fsAPI: { + getDirectoryHandle: () => directoryHandle, + getWorkspaceSnapshot: () => ({ name: 'project', entries: [] }), + openFolderFromHandle: async () => null, + }, + getOpenTabPaths: () => [], + getActiveTabPath: () => null, + getOpenTabsSnapshot: () => ({}), + restoreWorkspace: async () => {}, + storage: { + async get() { + return {}; + }, + async set() { + events.push('state'); + }, + }, + handleStore: { + async save(handle) { + assert.equal(handle, directoryHandle); + events.push('handle'); + }, + async load() { + return null; + }, + async clear() { + events.push('clear'); + }, + }, + }); + + await persistence.persistWorkspaceSession(); + + assert.deepEqual(events, ['handle', 'state']); +}); + +test('e2e: active tab snapshots include edits made without switching tabs', async () => { + const originalDocument = global.document; + global.document = createFakeDocument(); + try { + let editorValue = ''; + initToolbar( + { onmessage: null, postMessage() {} }, + { + getValue: () => editorValue, + setValue: (value) => { editorValue = value; }, + clearDiagnostics: () => {}, + setLanguage: () => {}, + }, + { setWorkspace: () => {}, clearTerminal: () => {} }, + { readWorkspaceFile: async () => 'int main() { return 0; }\n' }, + () => {} + ); + await restoreToolbarWorkspace( + { name: 'project', entries: [{ path: 'main.cpp', kind: 'file' }] }, + ['main.cpp'], + 'main.cpp' + ); + + editorValue = 'int main() { return 42; }\n'; + + assert.deepEqual(getToolbarOpenTabsSnapshot(), { + 'main.cpp': 'int main() { return 42; }\n', + }); + } finally { + global.document = originalDocument; + } +}); + +test('e2e: persistence gate preserves workspace intent while restore is pending', async () => { + const events = []; + const gate = createPersistenceGate({ + persistSessionState: async () => events.push('state'), + persistWorkspaceSession: async () => events.push('workspace'), + }); + + await gate.persistState(); + await gate.persistWorkspace(); + assert.deepEqual(events, []); + + await gate.enable(); + + assert.deepEqual(events, ['workspace']); +}); + +test('e2e: scheduled state persistence coalesces rapid changes', async () => { + let stateSaves = 0; + const gate = createPersistenceGate({ + persistSessionState: async () => { stateSaves += 1; }, + persistWorkspaceSession: async () => {}, + }, { debounceMs: 5 }); + await gate.enable(); + + gate.scheduleState(); + gate.scheduleState(); + gate.scheduleState(); + await new Promise((resolve) => setTimeout(resolve, 15)); + + assert.equal(stateSaves, 1); +}); + +test('e2e: opening a folder uses explicit workspace persistence', async () => { + const originalDocument = global.document; + global.document = createFakeDocument(); + try { + const persistenceEvents = []; + initToolbar( + { onmessage: null, postMessage() {} }, + { + getValue: () => '', + setValue: () => {}, + clearDiagnostics: () => {}, + setLanguage: () => {}, + }, + { + setWorkspace: () => {}, + resetTerminalSession: () => {}, + clearTerminal: () => {}, + }, + { + openFolder: async () => ({ name: 'empty', entries: [] }), + }, + { + persistState: async () => persistenceEvents.push('state'), + persistWorkspace: async () => persistenceEvents.push('workspace'), + scheduleState: () => persistenceEvents.push('scheduled'), + } + ); + + global.document.getElementById('btn-open').click(); + await waitFor( + () => persistenceEvents.length > 0, + 'workspace persistence after folder open' + ); + + assert.deepEqual(persistenceEvents, ['workspace']); + } finally { + global.document = originalDocument; + } +}); + test('e2e: does not fall back to read-only permission when readwrite is denied', async () => { const storage = createStorageArea(); const handleStore = createHandleStore(); @@ -236,7 +422,7 @@ test('e2e: does not fall back to read-only permission when readwrite is denied', handleStore, }); - await firstSession.persistSession(); + await firstSession.persistWorkspaceSession(); const secondSession = createSessionPersistence({ fsAPI: { @@ -298,7 +484,7 @@ test('e2e: prompts to reload and re-requests readwrite, restoring live workspace storage, handleStore, }); - await firstSession.persistSession(); + await firstSession.persistWorkspaceSession(); const secondSession = createSessionPersistence({ fsAPI: { @@ -371,7 +557,7 @@ test('e2e: choosing start-new abandons previous state and clears persisted sessi storage, handleStore, }); - await firstSession.persistSession(); + await firstSession.persistWorkspaceSession(); const secondSession = createSessionPersistence({ fsAPI: { @@ -445,7 +631,7 @@ test('e2e: reload chosen but browser denies permission still restores snapshot', storage, handleStore, }); - await firstSession.persistSession(); + await firstSession.persistWorkspaceSession(); const secondSession = createSessionPersistence({ fsAPI: { @@ -499,7 +685,7 @@ test('e2e: ignores legacy source-only snapshots when no workspace handle is avai handleStore, }); - await firstSession.persistSession(); + await firstSession.persistSessionState(); const secondSession = createSessionPersistence({ fsAPI: { @@ -607,7 +793,7 @@ test('e2e: startup gate prevents pre-restore persistence from wiping workspace s storage, handleStore, }); - await firstSession.persistSession(); + await firstSession.persistWorkspaceSession(); const secondSession = createSessionPersistence({ fsAPI: { @@ -628,8 +814,11 @@ test('e2e: startup gate prevents pre-restore persistence from wiping workspace s handleStore, }); - const gate = createPersistenceGate(secondSession.persistSession); - await gate.persist(); // startup timer fires before restore; must be ignored + const gate = createPersistenceGate({ + persistSessionState: secondSession.persistSessionState, + persistWorkspaceSession: secondSession.persistWorkspaceSession, + }); + await gate.persistState(); // startup timer fires before restore; must be ignored await secondSession.restoreSession(); await gate.enable(); @@ -671,7 +860,7 @@ test('e2e: restores workspace tabs across reopen with callback-style storage', a storage, handleStore, }); - await firstSession.persistSession(); + await firstSession.persistWorkspaceSession(); const secondSession = createSessionPersistence({ fsAPI: { @@ -733,7 +922,7 @@ test('e2e: relaunch requests readwrite permission before restoring workspace', a storage, handleStore, }); - await firstSession.persistSession(); + await firstSession.persistWorkspaceSession(); const secondSession = createSessionPersistence({ fsAPI: { @@ -809,7 +998,10 @@ test('e2e: launch/open-files/close/relaunch restores explorer folder and tabs', storage, handleStore, }); - const launchOneGate = createPersistenceGate(launchOne.persistSession); + const launchOneGate = createPersistenceGate({ + persistSessionState: launchOne.persistSessionState, + persistWorkspaceSession: launchOne.persistWorkspaceSession, + }); const launchOneRestore = launchOne.restoreSession(); // Simulate user flow before startup restore completes: @@ -817,13 +1009,13 @@ test('e2e: launch/open-files/close/relaunch restores explorer folder and tabs', launchOneState.directoryHandle = directoryHandle; launchOneState.openTabPaths = ['README.md']; launchOneState.activeTabPath = 'README.md'; - await launchOneGate.persist(); // folder open + initial tab + await launchOneGate.persistWorkspace(); // folder open + initial tab launchOneState.openTabPaths = ['README.md', 'bitmap.h', 'bitmap.cpp', 'test_runner.sh']; launchOneState.activeTabPath = 'test_runner.sh'; - await launchOneGate.persist(); // multiple file tabs open + await launchOneGate.persistState(); // multiple file tabs open launchOneState.openTabPaths = ['bitmap.h', 'bitmap.cpp', 'test_runner.sh']; launchOneState.activeTabPath = 'test_runner.sh'; - await launchOneGate.persist(); // README closed + await launchOneGate.persistState(); // README closed // Closing and relaunching the extension tab: resolveGet(); @@ -904,7 +1096,7 @@ test('e2e: restores explorer folder and tabs when handle reload is unavailable', warnings.push(args); }; try { - await firstSession.persistSession(); + await firstSession.persistWorkspaceSession(); } finally { console.warn = originalWarn; } diff --git a/scripts/e2e-session-restore-choice.test.mjs b/scripts/e2e-session-restore-choice.test.mjs index d6b3145..c272e21 100644 --- a/scripts/e2e-session-restore-choice.test.mjs +++ b/scripts/e2e-session-restore-choice.test.mjs @@ -60,7 +60,7 @@ async function seedWorkspaceSession({ storage, handleStore, handle, snapshot, op storage, handleStore, }); - await first.persistSession(); + await first.persistWorkspaceSession(); } test('e2e: reload choice re-requests readwrite and restores the live workspace', async () => { @@ -232,7 +232,7 @@ test('e2e: after start-new, the next persist leaves the untitled state unpersist }); await persistence.restoreSession(); - await persistence.persistSession(); + await persistence.persistSessionState(); const saved = (await storage.get('browser_cpp_session')).browser_cpp_session; assert.equal(saved, null, 'the untitled buffer is not persisted'); @@ -306,7 +306,7 @@ test('e2e: untitled source is neither persisted nor restored', async () => { storage, handleStore, }); - await first.persistSession(); + await first.persistSessionState(); assert.equal((await storage.get('browser_cpp_session')).browser_cpp_session, null); let restoredSource = null; diff --git a/specs/issue-92-indexeddb-unload-persistence-20260915.md b/specs/issue-92-indexeddb-unload-persistence-20260915.md new file mode 100644 index 0000000..cf86ef7 --- /dev/null +++ b/specs/issue-92-indexeddb-unload-persistence-20260915.md @@ -0,0 +1,316 @@ +# Implementation Plan: Issue #92 — move directory-handle persistence out of tab unload + +## Overview + +Fix the Chrome crash reported in issue #92 by preventing browser.cpp from +opening IndexedDB and storing a live `FileSystemDirectoryHandle` while the +extension page is unloading. Persist the handle when a folder is acquired or +reconnected, persist serializable workspace and tab state proactively during +normal interaction, and perform no asynchronous persistence from +`beforeunload`. + +This plan supersedes the earlier output-backpressure and unload-worker plans. +No production implementation has started. + +## Decision record + +> @kbuffardi: "discard the previous plan and write a new plan that adopts this newly diagnosed fix" + +Codex (GPT-5) replaced the prior plans with the IndexedDB/unload-persistence +plan below, based on the folder/no-folder and DevTools diagnostic results. + +## Evidence and diagnosis + +### Observed + +- Closing the extension without opening a folder does not crash Chrome. +- Opening the original project and closing the tab crashes without rerunning + its infinite-loop program. +- Opening a different empty folder and closing the tab also crashes. +- After opening an empty folder, replacing `IDBFactory.prototype.open` with a + function that throws prevents the close-tab crash. +- The current `beforeunload` handler calls `worker.terminate()` and then + `persistenceGate.persist()`. +- `persistSession()` obtains the live directory handle and calls + `handleStore.save(dirHandle)`, which opens IndexedDB and stores that handle. + +### Conclusion and confidence + +The DevTools experiment is a strong discriminator: it left the live directory +handle, worker termination, page teardown, and subsequent serializable-storage +path in place, but prevented the unload-time IndexedDB open/write. The +application-controlled trigger is therefore, with high confidence, the +IndexedDB operation that stores the directory handle during page teardown. + +The native `ThreadPoolForegroundWorker` crash is ultimately a Chrome defect; +the application fix is to avoid initiating this unsafe browser path. The +experiment does not identify the faulty Chrome native component or prove that +all IndexedDB directory-handle writes are unsafe outside unload. + +## Architecture decisions + +- Split directory-handle persistence from serializable session-state + persistence. A normal tab/editor/workspace state save must never write or + clear IndexedDB. +- Save the directory handle only at stable workspace lifecycle boundaries: + successful folder open, folder reconnect, and save-to-folder acquisition. +- Clear the stored handle only during an explicit abandon/new-project flow. +- Save JSON-safe workspace, tab, and editor state proactively and with a short + debounce during normal interaction so correctness does not depend on unload. +- `beforeunload` must start no IndexedDB or `chrome.storage` operation. Keep the + existing synchronous `worker.terminate()` behavior unchanged: the successful + diagnostic close still exercised it, so it is not the trigger isolated here. +- Preserve the existing IndexedDB database, object store, and key so previously + saved sessions remain compatible. + +## Scope + +### In scope + +- Refactor the session-persistence API into explicit handle, state, and clear + operations. +- Update folder acquisition/reconnection paths to save the handle immediately. +- Persist current active-editor content and other serializable session state + before unload through event-driven/debounced saves. +- Remove asynchronous persistence from `beforeunload`. +- Preserve live-handle restore, snapshot-only fallback, startup gating, and + explicit session abandonment. +- Add focused persistence tests and real-Chrome/manual close verification. + +### Out of scope + +- Terminal output backpressure, run generations, xterm queues, and compiler + worker protocol changes. +- Changes to STOP or its terminate-and-replace behavior. +- Changes to Chrome, macOS, or the Clang/WASM toolchain. + +## Task 1: Classify and lock down persistence responsibilities + +**Description:** Audit every current persistence caller and classify it as a +handle save, state-only save, full clear, or no persistence. Establish a clear +API contract in `createSessionPersistence()`; recommended operations are +`persistWorkspaceSession()`, `persistSessionState()`, and +`clearPersistedSession()`. + +**Acceptance criteria:** + +- [ ] `persistSessionState()` writes only JSON-safe data to + `chrome.storage.local` and never calls `handleStore.save()` or + `handleStore.clear()`. +- [ ] `persistWorkspaceSession()` stores the current directory handle and then + persists the serializable state, even if handle storage fails. +- [ ] `clearPersistedSession()` clears both stores only for an explicit abandon + flow. +- [ ] Restore continues to use `browser-cpp-handles` / `handles` / + `workspace-dir` without a migration. + +**Verification:** + +- [ ] Focused tests distinguish handle-store calls from storage-area calls. +- [ ] Existing live-handle and snapshot-fallback restore tests pass. + +**Dependencies:** None. + +**Files likely touched:** + +- `src/ui/session-persistence.mjs` +- `scripts/e2e-session-persistence.test.mjs` +- `scripts/e2e-session-restore-choice.test.mjs` + +**Estimated scope:** Medium (3 files). + +## Task 2: Make startup gating support explicit persistence intents + +**Description:** Adapt `createPersistenceGate()` so calls made while startup +restore is pending retain their intent. A queued workspace save must not be +downgraded to a state-only save; when enabled, the gate should perform the +strongest pending operation once. Add a debounced state-save entry point for +frequent editor changes, while keeping workspace acquisition saves immediate. + +**Acceptance criteria:** + +- [ ] A folder opened before restore completes is saved to IndexedDB after the + gate enables. +- [ ] Multiple queued state changes coalesce without losing the latest state. +- [ ] An immediate workspace save is not delayed behind the editor debounce. +- [ ] Enabling the gate with no pending work performs no storage operation. + +**Verification:** + +- [ ] Extend persistence-gate tests for queued state, queued workspace, and + coalescing behavior. + +**Dependencies:** Task 1. + +**Files likely touched:** + +- `src/ui/session-persistence.mjs` +- `scripts/e2e-session-persistence.test.mjs` + +**Estimated scope:** Small (2 files). + +## Task 3: Persist handles at workspace acquisition boundaries + +**Description:** Wire the explicit workspace-save operation into the toolbar +paths that obtain a real directory handle. This includes `openFolderWorkspace`, +the folder acquired by `saveUntitledDocument`, and the folder reconnect path +used after snapshot-only restore. All ordinary tab, file, and workspace +mutations must use state-only persistence. + +**Acceptance criteria:** + +- [ ] A successfully opened or reconnected folder saves its handle immediately + and records the corresponding serializable session state. +- [ ] Cancelling the picker or failing to open a folder writes neither store. +- [ ] Tab switches, tab closes, saves, file creation, filesystem refreshes, and + other ordinary mutations do not touch IndexedDB. +- [ ] Explicitly abandoning a saved session clears both the handle and state. + +**Verification:** + +- [ ] Toolbar/session integration tests assert handle-store call counts and + timing for open, reconnect, cancel, mutation, and abandon paths. + +**Dependencies:** Tasks 1–2. + +**Files likely touched:** + +- `src/ui/app.js` +- `src/ui/toolbar.js` +- `scripts/e2e-session-persistence.test.mjs` +- `scripts/e2e-session-restore-choice.test.mjs` + +**Estimated scope:** Medium (4 files). + +## Checkpoint: persistence boundaries + +- [ ] Handle writes occur only at explicit workspace acquisition boundaries. +- [ ] Existing restore and startup-race tests pass. +- [ ] Snapshot-only sessions remain restorable when IndexedDB is unavailable. + +## Task 4: Preserve active editor state without unload persistence + +**Description:** Make `getOpenTabsSnapshot()` include the current editor value +for the active tab instead of relying on a later tab switch. Schedule a +debounced state-only save from editor content changes after dirty state is +updated. Retain immediate state saves for lower-frequency structural changes. + +**Acceptance criteria:** + +- [ ] Editing the active tab and waiting for the debounce persists its latest + content without a tab switch or unload event. +- [ ] Rapid edits coalesce instead of writing storage once per keystroke. +- [ ] Multiple tabs restore with the correct active path and latest captured + contents. +- [ ] Editor-driven saves never open IndexedDB. + +**Verification:** + +- [ ] Add coverage for an edited active tab, multiple tabs, debounce + coalescing, and state-only handle-store call counts. + +**Dependencies:** Tasks 1–3. + +**Files likely touched:** + +- `src/ui/app.js` +- `src/ui/toolbar.js` +- `scripts/e2e-session-persistence.test.mjs` + +**Estimated scope:** Medium (3 files). + +## Task 5: Remove asynchronous persistence from unload + +**Description:** Change the `beforeunload` handler so it performs only the +existing synchronous worker termination. Do not flush, schedule, or initiate +session persistence from unload. Update the adjacent comment to document that +all session persistence is proactive. + +**Acceptance criteria:** + +- [ ] Closing the extension page does not call the persistence gate. +- [ ] Page unload cannot initiate `indexedDB.open()`, `handleStore.save()`, + `handleStore.clear()`, or `chrome.storage.local.set()` through session + persistence. +- [ ] STOP and its worker replacement remain unchanged. + +**Verification:** + +- [ ] Add a small testable lifecycle helper only if needed to assert the unload + contract without importing the full UI bootstrap. +- [ ] Otherwise, cover the contract through focused persistence tests plus the + Chrome close-path verification in Task 6. + +**Dependencies:** Task 4. + +**Files likely touched:** + +- `src/ui/app.js` +- Optional: `src/ui/lifecycle.mjs` +- Existing E2E test file preferred; add/register a new file only if clearer. + +**Estimated scope:** Small (1–2 files). + +## Task 6: Verify restore and the native close path + +**Description:** Extend browser smoke coverage where it can accurately observe +the extension target closing and the Chrome process remaining alive. Do not +allow hosted-page fallback to count as issue #92 verification. Because the +automation harness may not be able to obtain a genuine +`FileSystemDirectoryHandle`, retain a required manual test on the affected +Chrome/macOS environment. + +**Acceptance criteria:** + +- [ ] Automated close coverage closes the extension tab, waits a bounded + interval, and proves the controlled Chrome process/connection is still alive + before normal cleanup. +- [ ] The issue-92 smoke path fails or reports unsupported if it cannot load the + extension; it does not pass through hosted fallback. +- [ ] Manual testing with a real empty folder no longer crashes Chrome. +- [ ] Relaunch restores the workspace and tabs from proactively persisted + state. + +**Verification:** + +- [ ] Fully quit and relaunch Chrome before the manual run. +- [ ] Test open/close with no folder. +- [ ] Open an empty folder, wait for persistence, close, relaunch, restore, and + close again. +- [ ] Open the original project without running it and close. +- [ ] Run and STOP the original infinite-loop program, then close. +- [ ] Confirm Chrome remains alive and no new crash report is uploaded. + +**Dependencies:** Task 5. + +**Files likely touched:** + +- `scripts/smoke-browser.mjs` + +**Estimated scope:** Small (1 file). + +## Final checkpoint + +- [ ] `npm run test:e2e` +- [ ] `npm run lint` +- [ ] `npm run build` +- [ ] `npm run test:browser:chrome` +- [ ] Manual native-handle test matrix passes on Chrome/macOS. +- [ ] The PR documents any automation limitation and includes `Closes #92`. +- [ ] A human reviews and merges the PR; no agent merges it. + +## Risks and mitigations + +| Risk | Impact | Mitigation | +| --- | --- | --- | +| Removing unload persistence loses a very recent edit | High | Capture the live active-editor value and debounce state-only persistence during editing; test close after the debounce settles. | +| Folder open races startup restore | High | Preserve persistence intent in the startup gate and give workspace saves priority. | +| API split misses a handle-acquisition path | High | Classify every existing caller first and test open, reconnect, and save-to-folder flows. | +| IndexedDB failure prevents all session restore | Medium | Continue serializable state persistence after handle-save failure and retain snapshot fallback. | +| Automated smoke uses a fake handle and misses the native bug | High | Require manual verification with a real `showDirectoryPicker()` handle on the affected environment. | +| Chrome still crashes after removing unload storage | Medium | Re-run the same `IDBFactory.prototype.open` discriminator, capture a new Crash Report ID, and investigate any remaining unload-time IndexedDB caller before revisiting unrelated output/worker hypotheses. | + +## Plan status + +This replacement plan is ready for human review. It is not approval to begin +implementation. diff --git a/src/ui/app.js b/src/ui/app.js index d911f6a..1f34b79 100644 --- a/src/ui/app.js +++ b/src/ui/app.js @@ -24,12 +24,14 @@ import { getOpenTabPaths, getActiveTabPath, getOpenTabsSnapshot, + getWorkspaceSnapshot, restoreWorkspace, resetToNewProject, assembleCompilePayload, applyWorkspaceSnapshot, } from './toolbar.js'; import { createSessionPersistence, createPersistenceGate } from './session-persistence.mjs'; +import { registerPageUnload } from './page-lifecycle.mjs'; import { getExtensionVersionLabel } from '../extension-api.mjs'; // ── Boot ────────────────────────────────────────────────────────────────────── @@ -85,7 +87,7 @@ window.addEventListener('DOMContentLoaded', async () => { const result = await fsAPI.createWorkspaceDirectory(path, { parents }); if (result?.ok) { applyWorkspaceSnapshot(result.snapshot, [result.path]); - await persistenceGate.persist(); + await persistenceGate.persistState(); } return result; }, @@ -93,7 +95,7 @@ window.addEventListener('DOMContentLoaded', async () => { const result = await fsAPI.touchWorkspaceFile(path); if (result?.ok) { applyWorkspaceSnapshot(result.snapshot, [result.path]); - await persistenceGate.persist(); + await persistenceGate.persistState(); } return result; }, @@ -102,25 +104,40 @@ window.addEventListener('DOMContentLoaded', async () => { }); // 4. Toolbar (wires buttons + worker messages + keyboard shortcuts) - const { restoreSession, persistSession } = createSessionPersistence({ + const { + restoreSession, + persistSessionState, + persistWorkspaceSession, + } = createSessionPersistence({ fsAPI, editorAPI, markDirty, getOpenTabPaths, getActiveTabPath, getOpenTabsSnapshot, + getWorkspaceSnapshot, restoreWorkspace, confirmReload: promptReloadPreviousProject, startNewProject: resetToNewProject, setExplorerLoading: (loading) => toolbarController?.setExplorerLoading(loading), setExplorerScanProgress: (update) => toolbarController?.setExplorerScanProgress(update), }); - const persistenceGate = createPersistenceGate(persistSession); - toolbarController = initToolbar(worker, editorAPI, terminalAPI, fsAPI, () => persistenceGate.persist()); + const persistenceGate = createPersistenceGate({ + persistSessionState, + persistWorkspaceSession, + }); + toolbarController = initToolbar(worker, editorAPI, terminalAPI, fsAPI, { + persistState: () => persistenceGate.persistState(), + persistWorkspace: () => persistenceGate.persistWorkspace(), + scheduleState: () => persistenceGate.scheduleState(), + }); resetToNewProject(); // 5. Track unsaved changes - editorAPI.onDidChangeContent(() => markDirty(true)); + editorAPI.onDidChangeContent(() => { + markDirty(true); + persistenceGate.scheduleState(); + }); // 6. Cursor position → status bar editorAPI.onDidChangeCursorPosition((e) => { @@ -139,12 +156,9 @@ window.addEventListener('DOMContentLoaded', async () => { if (terminalPanel) resizeObserver.observe(terminalPanel); initPanelResizers(); - // 9. Persist session on unload. Worker teardown is synchronous: browser - // unload handlers cannot safely wait for terminal or worker cleanup. - window.addEventListener('beforeunload', () => { - worker.terminate(); - persistenceGate.persist(); - }); + // 9. Session state is persisted proactively. Do not start asynchronous + // storage work while the page is unloading. + registerPageUnload(window, worker); editorAPI.focus(); }); diff --git a/src/ui/page-lifecycle.mjs b/src/ui/page-lifecycle.mjs new file mode 100644 index 0000000..285767b --- /dev/null +++ b/src/ui/page-lifecycle.mjs @@ -0,0 +1,8 @@ +'use strict'; + +/** Register synchronous page teardown without starting asynchronous storage work. */ +export function registerPageUnload(target, worker) { + target.addEventListener('beforeunload', () => { + worker.terminate(); + }); +} diff --git a/src/ui/session-persistence.mjs b/src/ui/session-persistence.mjs index 8bda67a..8b369e5 100644 --- a/src/ui/session-persistence.mjs +++ b/src/ui/session-persistence.mjs @@ -144,6 +144,11 @@ export function createSessionPersistence({ getOpenTabPaths, getActiveTabPath, getOpenTabsSnapshot = () => null, + getWorkspaceSnapshot = () => ( + typeof fsAPI.getWorkspaceSnapshot === 'function' + ? fsAPI.getWorkspaceSnapshot() + : null + ), restoreWorkspace, storage = getStorageArea(), handleStore = createIndexedDBHandleStore(), @@ -274,34 +279,23 @@ export function createSessionPersistence({ } } - async function persistSession() { + async function persistSessionState() { try { if (!storage) return; - const dirHandle = fsAPI.getDirectoryHandle(); - if (dirHandle) { - try { - await handleStore.save(dirHandle); - } catch (err) { - // Keep persisting serializable workspace/tab state even if handle storage fails. - console.warn( - 'Failed to persist workspace directory handle (workspace state will still be saved):', - err - ); - } + const workspace = getWorkspaceSnapshot(); + const hasWorkspace = Boolean(workspace || fsAPI.getDirectoryHandle?.()); + if (hasWorkspace) { await storageSet(storage, { [STORAGE_KEY]: { openTabPaths: getOpenTabPaths(), activeTabPath: getActiveTabPath(), openTabContentsByPath: getOpenTabsSnapshot(), - workspace: typeof fsAPI.getWorkspaceSnapshot === 'function' - ? fsAPI.getWorkspaceSnapshot() - : null, + workspace, savedAt: Date.now(), }, }); } else { - await handleStore.clear(); await storageSet(storage, { [STORAGE_KEY]: null }); } } catch (err) { @@ -309,25 +303,86 @@ export function createSessionPersistence({ } } - return { restoreSession, persistSession }; + async function persistWorkspaceSession() { + const dirHandle = fsAPI.getDirectoryHandle(); + if (dirHandle) { + try { + await handleStore.save(dirHandle); + } catch (err) { + // Keep persisting serializable workspace/tab state even if handle storage fails. + console.warn( + 'Failed to persist workspace directory handle (workspace state will still be saved):', + err + ); + } + } + await persistSessionState(); + } + + return { + restoreSession, + persistSessionState, + persistWorkspaceSession, + clearPersistedSession, + }; } -export function createPersistenceGate(persistSession) { +export function createPersistenceGate(persistence, { debounceMs = 300 } = {}) { + const { persistSessionState, persistWorkspaceSession } = persistence; let enabled = false; - let pending = false; + let pendingIntent = null; + let stateTimer = null; + + function queueIntent(intent) { + if (intent === 'workspace' || pendingIntent === null) pendingIntent = intent; + } + + function clearStateTimer() { + if (stateTimer === null) return; + clearTimeout(stateTimer); + stateTimer = null; + } + + function persistState() { + if (!enabled) { + queueIntent('state'); + return; + } + clearStateTimer(); + return persistSessionState(); + } + + function persistWorkspace() { + if (!enabled) { + queueIntent('workspace'); + return; + } + clearStateTimer(); + return persistWorkspaceSession(); + } + + function scheduleState() { + if (!enabled) { + queueIntent('state'); + return; + } + clearStateTimer(); + stateTimer = setTimeout(() => { + stateTimer = null; + void persistSessionState(); + }, debounceMs); + } + return { - persist() { - if (!enabled) { - pending = true; - return; - } - return persistSession(); - }, + persistState, + persistWorkspace, + scheduleState, enable() { enabled = true; - if (!pending) return; - pending = false; - return persistSession(); + const intent = pendingIntent; + pendingIntent = null; + if (intent === 'workspace') return persistWorkspaceSession(); + if (intent === 'state') return persistSessionState(); }, }; } diff --git a/src/ui/toolbar.js b/src/ui/toolbar.js index 1d9c312..8388232 100644 --- a/src/ui/toolbar.js +++ b/src/ui/toolbar.js @@ -56,6 +56,8 @@ let _loadingFile = false; // ── Session persistence callback ────────────────────────────────────────────── /** Optional callback supplied by app.js to persist the session after state changes. */ let _persistSession = null; +let _persistWorkspaceSession = null; +let _schedulePersistSession = null; let _persistTimer = null; function describeRuntimeWritebackIssue(reason) { @@ -75,6 +77,10 @@ function describeRuntimeWritebackIssue(reason) { /** Schedule a debounced session persist (e.g. after active-tab switches). */ function schedulePersist() { + if (_schedulePersistSession) { + _schedulePersistSession(); + return; + } if (!_persistSession) return; clearTimeout(_persistTimer); _persistTimer = setTimeout(() => _persistSession(), 300); @@ -89,14 +95,24 @@ function schedulePersist() { * @param {object} editorAPI – module exports from editor.js * @param {object} terminalAPI – module exports from terminal.js * @param {object} fsAPI – module exports from filesystem.js - * @param {Function} [persistSession] – optional callback to persist session state + * @param {Function|object} [persistence] – session persistence callbacks */ -export function initToolbar(worker, editorAPI, terminalAPI, fsAPI, persistSession) { +export function initToolbar(worker, editorAPI, terminalAPI, fsAPI, persistence) { _worker = null; _editorAPI = editorAPI; _terminalAPI = terminalAPI; _fsAPI = fsAPI; - _persistSession = persistSession ?? null; + clearTimeout(_persistTimer); + _persistTimer = null; + if (typeof persistence === 'function') { + _persistSession = persistence; + _persistWorkspaceSession = persistence; + _schedulePersistSession = null; + } else { + _persistSession = persistence?.persistState ?? null; + _persistWorkspaceSession = persistence?.persistWorkspace ?? _persistSession; + _schedulePersistSession = persistence?.scheduleState ?? null; + } bindButtons(); bindKeyboardShortcuts(); @@ -676,7 +692,7 @@ async function saveUntitledDocument() { applyWorkspaceSnapshot(result.snapshot ?? workspace); openTabForFile(result.path, _editorAPI.getValue()); markDirty(false); - _persistSession?.(); + await _persistWorkspaceSession?.(); } async function actionSaveAs() { @@ -1130,7 +1146,7 @@ async function openWorkspaceFile(path) { if (!reconnectedWorkspace) return; setWorkspaceMode(reconnectedWorkspace); renderWorkspaceSidebar(reconnectedWorkspace); - _persistSession?.(); + await _persistWorkspaceSession?.(); try { content = await _fsAPI.readWorkspaceFile(path); } catch { @@ -1256,7 +1272,7 @@ async function openFolderWorkspace() { const statusFile = document.getElementById('status-file'); if (statusFile) statusFile.textContent = ''; renderWorkspaceSidebar(workspace); - _persistSession?.(); // persist immediately so the new workspace survives unload + await _persistWorkspaceSession?.(); return true; } finally { setExplorerLoading(false); @@ -1316,9 +1332,17 @@ export function getOpenTabsSnapshot() { for (const [path, tab] of _openTabs.entries()) { snapshot[path] = tab.content; } + if (_activeTabPath && _openTabs.has(_activeTabPath) && _editorAPI?.getValue) { + snapshot[_activeTabPath] = _editorAPI.getValue(); + } return snapshot; } +/** Return the current serializable workspace snapshot, including fallback restores. */ +export function getWorkspaceSnapshot() { + return _workspace; +} + /** * Restore a previously persisted workspace and its open tabs. * Called from app.js after the directory handle has been re-authenticated.