From 63a5b49ad45996c8757a7cce752e13a4035e4696 Mon Sep 17 00:00:00 2001 From: Kevin Buffardi Date: Tue, 15 Sep 2026 16:47:56 -0700 Subject: [PATCH 1/2] refactor(session): split handle and state persistence Separate IndexedDB directory-handle storage from serializable session state so callers can avoid handle writes during page teardown. Capture the live active editor value for proactive state saves.\n\nRefs #92 --- scripts/e2e-session-persistence.test.mjs | 114 +++++++ ...2-indexeddb-unload-persistence-20260915.md | 316 ++++++++++++++++++ src/ui/session-persistence.mjs | 51 ++- src/ui/toolbar.js | 3 + 4 files changed, 467 insertions(+), 17 deletions(-) create mode 100644 specs/issue-92-indexeddb-unload-persistence-20260915.md diff --git a/scripts/e2e-session-persistence.test.mjs b/scripts/e2e-session-persistence.test.mjs index ed6ebcf..799b829 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,119 @@ 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: does not fall back to read-only permission when readwrite is denied', async () => { const storage = createStorageArea(); const handleStore = createHandleStore(); 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/session-persistence.mjs b/src/ui/session-persistence.mjs index 8bda67a..0e255f0 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,7 +303,30 @@ 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, + // Keep the original API available while call sites migrate to explicit intent. + persistSession: persistWorkspaceSession, + }; } export function createPersistenceGate(persistSession) { diff --git a/src/ui/toolbar.js b/src/ui/toolbar.js index 1d9c312..71b1a4a 100644 --- a/src/ui/toolbar.js +++ b/src/ui/toolbar.js @@ -1316,6 +1316,9 @@ 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; } From e13365c4a27700b07803304735800f9a762309f5 Mon Sep 17 00:00:00 2001 From: Kevin Buffardi Date: Tue, 15 Sep 2026 16:52:16 -0700 Subject: [PATCH 2/2] fix(session): avoid directory handle writes during unload Persist directory handles at workspace acquisition, debounce serializable editor state during normal interaction, and keep page unload synchronous. This avoids Chrome's native crash path while preserving session restore.\n\nRefs #92 --- package.json | 2 +- scripts/e2e-page-lifecycle.test.mjs | 24 +++++ scripts/e2e-session-persistence.test.mjs | 108 +++++++++++++++++--- scripts/e2e-session-restore-choice.test.mjs | 6 +- src/ui/app.js | 38 ++++--- src/ui/page-lifecycle.mjs | 8 ++ src/ui/session-persistence.mjs | 66 +++++++++--- src/ui/toolbar.js | 33 ++++-- 8 files changed, 234 insertions(+), 51 deletions(-) create mode 100644 scripts/e2e-page-lifecycle.test.mjs create mode 100644 src/ui/page-lifecycle.mjs 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 799b829..c467fd3 100644 --- a/scripts/e2e-session-persistence.test.mjs +++ b/scripts/e2e-session-persistence.test.mjs @@ -311,6 +311,78 @@ test('e2e: active tab snapshots include edits made without switching tabs', asyn } }); +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(); @@ -350,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: { @@ -412,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: { @@ -485,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: { @@ -559,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: { @@ -613,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: { @@ -721,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: { @@ -742,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(); @@ -785,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: { @@ -847,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: { @@ -923,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: @@ -931,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(); @@ -1018,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/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 0e255f0..8b369e5 100644 --- a/src/ui/session-persistence.mjs +++ b/src/ui/session-persistence.mjs @@ -324,27 +324,65 @@ export function createSessionPersistence({ persistSessionState, persistWorkspaceSession, clearPersistedSession, - // Keep the original API available while call sites migrate to explicit intent. - persistSession: persistWorkspaceSession, }; } -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 71b1a4a..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); @@ -1322,6 +1338,11 @@ export function getOpenTabsSnapshot() { 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.