diff options
| -rw-r--r-- | CHANGELOG.md | 2 | ||||
| -rw-r--r-- | extensions/chrome-profile-bridge/browser-extension/service_worker.js | 86 | ||||
| -rw-r--r-- | test-suite/unit/ownership-persistence.test.mjs | 124 |
3 files changed, 196 insertions, 16 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index accad1b..627193f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ All notable user-facing changes to `pi-chrome`. ## Unreleased +- **Pi keeps track of its tabs across extension reloads.** Ownership of Pi's automation window and session tabs lived only in `chrome.storage.session`, which Chrome wipes when the extension reloads. The extension reloads itself on every pi-chrome update, so afterwards Pi could not close or reuse the windows and tabs it had opened, and they were left behind. Ownership is now also kept in `chrome.storage.local` and restored after a reload. A restored tab is trusted only if it still exists in the window it was recorded in (adopted tabs must also still be in Pi's group). The copy is dropped on browser startup, since tab ids do not carry over between browser runs. + ## 0.15.56 — 2026-09-28 - **Pi's automation tab stays in its own window.** Joining the session tab group called `chrome.tabs.group` without a window. Chrome then creates the group in the current window (yours) and moves the tab there, where it sits as an inactive, hidden tab and trusted input is refused. This happened on a session's first page action and is the main source of the hidden-tab errors. New groups are now created in the tab's own window. diff --git a/extensions/chrome-profile-bridge/browser-extension/service_worker.js b/extensions/chrome-profile-bridge/browser-extension/service_worker.js index 68fe8c9..ac48579 100644 --- a/extensions/chrome-profile-bridge/browser-extension/service_worker.js +++ b/extensions/chrome-profile-bridge/browser-extension/service_worker.js @@ -33,6 +33,11 @@ let polling = false; // // State is mirrored to chrome.storage.session so a service-worker restart (MV3 can suspend the // worker at any time) re-hydrates ownership instead of orphaning the window it already created. +// storage.session is also cleared when the extension reloads, which it does itself on every +// pi-chrome version bump, so ownership is mirrored to chrome.storage.local too (see +// readOwnership). The local copy is restored only when the session copy is missing, every restored +// tab is re-checked, and the local copy is dropped on browser startup because tab ids do not carry +// over between browser runs. // storage.session is cleared on browser restart; any window restored by Chrome's session-restore // is then untracked and simply left alone (we only ever close ids we still recognize as ours). const automationTargets = new Map(); // sessionKey -> { windowId?: number, tabId: number } @@ -50,6 +55,36 @@ function sessionKeyOf(params) { : DEFAULT_SESSION_KEY; } +// Read an ownership record: the storage.session copy if present, else the storage.local copy +// (after an extension reload). Returns { value, restored } where restored marks a local copy. +async function readOwnership(key) { + const fromSession = await chrome.storage?.session?.get?.(key).catch(() => null); + if (fromSession && fromSession[key] && typeof fromSession[key] === "object") return { value: fromSession[key], restored: false }; + const fromLocal = await chrome.storage?.local?.get?.(key).catch(() => null); + if (fromLocal && fromLocal[key] && typeof fromLocal[key] === "object") return { value: fromLocal[key], restored: true }; + return { value: null, restored: false }; +} + +async function writeOwnership(key, value) { + await Promise.all([ + Promise.resolve(chrome.storage?.session?.set?.({ [key]: value })).catch(() => {}), + Promise.resolve(chrome.storage?.local?.set?.({ [key]: value })).catch(() => {}), + ]); +} + +// A restored (local) record is trusted only if its tab still exists in the window it was recorded +// in. Anything else is forgotten, so a reused id or a tab the user moved is never closed by Pi. +async function restoredTabStillOurs(tabId, windowId) { + if (!Number.isInteger(tabId) || tabId < 0 || !Number.isInteger(windowId)) return null; + const tab = await chrome.tabs.get(tabId).catch(() => null); + return tab && tab.windowId === windowId ? tab : null; +} + +async function tabWindowId(tabId) { + const tab = Number.isInteger(tabId) ? await chrome.tabs.get(tabId).catch(() => null) : null; + return Number.isInteger(tab?.windowId) ? tab.windowId : undefined; +} + // Re-hydrate the in-memory ownership map from storage.session once per worker lifetime. Best // effort: storage may be unavailable on old Chrome, and a failure just means we may create a // fresh window (a harmless orphan) rather than reusing one. @@ -57,17 +92,19 @@ async function hydrateAutomationTargets() { if (automationHydrated) return automationHydrated; automationHydrated = (async () => { try { - const stored = await chrome.storage?.session?.get?.(AUTOMATION_STORAGE_KEY); - const saved = stored && stored[AUTOMATION_STORAGE_KEY]; + const { value: saved, restored } = await readOwnership(AUTOMATION_STORAGE_KEY); if (saved && typeof saved === "object") { for (const [key, value] of Object.entries(saved)) { - if (value && typeof value.tabId === "number") { - automationTargets.set(key, { - windowId: typeof value.windowId === "number" ? value.windowId : undefined, - tabId: value.tabId, - }); - } + if (!value || typeof value.tabId !== "number") continue; + const tabWindow = typeof value.tabWindowId === "number" ? value.tabWindowId : undefined; + if (restored && !(await restoredTabStillOurs(value.tabId, tabWindow))) continue; + automationTargets.set(key, { + windowId: typeof value.windowId === "number" ? value.windowId : undefined, + tabId: value.tabId, + tabWindowId: tabWindow, + }); } + if (restored) await persistAutomationTargets(); } } catch { // Ignore: treat as "no persisted state". @@ -79,19 +116,28 @@ async function hydrateAutomationTargets() { async function hydrateSessionTabs() { if (!sessionTabsReady) sessionTabsReady = (async () => { try { - const stored = await chrome.storage?.session?.get?.(SESSION_TABS_STORAGE_KEY); - for (const [key, entries] of Object.entries(stored?.[SESSION_TABS_STORAGE_KEY] || {})) { + const { value: saved, restored } = await readOwnership(SESSION_TABS_STORAGE_KEY); + for (const [key, entries] of Object.entries(saved || {})) { if (!Array.isArray(entries)) continue; const tabs = new Map(); for (const entry of entries) { if (!entry || !Number.isInteger(entry.tabId) || entry.tabId < 0) continue; - if (entry.created === true) tabs.set(entry.tabId, { created: true }); + const windowId = Number.isInteger(entry.windowId) ? entry.windowId : undefined; + let tab = null; + if (restored) { + tab = await restoredTabStillOurs(entry.tabId, windowId); + if (!tab) continue; + } + if (entry.created === true) tabs.set(entry.tabId, { created: true, windowId }); else if (entry.created === false && Number.isInteger(entry.groupId) && entry.groupId >= 0) { - tabs.set(entry.tabId, { created: false, groupId: entry.groupId }); + // An adopted tab must also still be in the group Pi put it in. + if (restored && tab.groupId !== entry.groupId) continue; + tabs.set(entry.tabId, { created: false, groupId: entry.groupId, windowId }); } } if (tabs.size) sessionTabs.set(key, tabs); } + if (restored) await persistSessionTabs(); } catch { // Missing ownership must leave tabs alone, not guess ownership from group names. } @@ -105,7 +151,7 @@ function persistSessionTabs() { const saved = Object.fromEntries([...sessionTabs].map(([key, tabs]) => [ key, [...tabs].map(([tabId, record]) => ({ tabId, ...record })), ])); - await chrome.storage?.session?.set?.({ [SESSION_TABS_STORAGE_KEY]: saved }); + await writeOwnership(SESSION_TABS_STORAGE_KEY, saved); }).catch(() => {}); return sessionTabsWrite; } @@ -119,7 +165,8 @@ async function trackSessionTab(sessionKey, tabId, created, groupId) { } let tabs = sessionTabs.get(sessionKey); if (!tabs) sessionTabs.set(sessionKey, tabs = new Map()); - tabs.set(tabId, created ? { created: true } : { created: false, groupId }); + const windowId = await tabWindowId(tabId); + tabs.set(tabId, created ? { created: true, windowId } : { created: false, groupId, windowId }); await persistSessionTabs(); } @@ -153,9 +200,14 @@ async function persistAutomationTargets() { try { const obj = {}; for (const [key, value] of automationTargets) { - obj[key] = { windowId: typeof value.windowId === "number" ? value.windowId : null, tabId: value.tabId }; + if (typeof value.tabWindowId !== "number") value.tabWindowId = await tabWindowId(value.tabId); + obj[key] = { + windowId: typeof value.windowId === "number" ? value.windowId : null, + tabId: value.tabId, + tabWindowId: typeof value.tabWindowId === "number" ? value.tabWindowId : null, + }; } - await chrome.storage?.session?.set?.({ [AUTOMATION_STORAGE_KEY]: obj }); + await writeOwnership(AUTOMATION_STORAGE_KEY, obj); } catch { // Ignore: persistence is an optimization, not a correctness requirement. } @@ -1537,6 +1589,8 @@ chrome.runtime.onInstalled.addListener(() => { }); chrome.runtime.onStartup.addListener(() => { + // A new browser run: tab ids from the last run mean nothing now. + Promise.resolve(chrome.storage?.local?.remove?.([AUTOMATION_STORAGE_KEY, SESSION_TABS_STORAGE_KEY])).catch(() => {}); armKeepaliveAlarm(); void pollLoop(); }); diff --git a/test-suite/unit/ownership-persistence.test.mjs b/test-suite/unit/ownership-persistence.test.mjs new file mode 100644 index 0000000..988dd78 --- /dev/null +++ b/test-suite/unit/ownership-persistence.test.mjs @@ -0,0 +1,124 @@ +// Tab ownership must survive an extension reload (storage.session is wiped; the extension reloads +// itself on every version bump) without ever claiming a tab that is no longer provably Pi's. +import assert from "node:assert/strict"; +import fs from "node:fs"; +import vm from "node:vm"; +import { test } from "node:test"; + +const workerSource = fs.readFileSync(new URL("../../extensions/chrome-profile-bridge/browser-extension/service_worker.js", import.meta.url), "utf8"); +const clone = (v) => (v === undefined ? undefined : JSON.parse(JSON.stringify(v))); + +// One browser (tabs/windows/groups) shared by successive extension instances. +function makeBrowser() { + let nextId = 100; + const b = { tabs: new Map(), windows: new Set([1]), groups: new Map(), local: {}, removed: [], ungrouped: [] }; + b.addTab = (props) => { const tab = { id: nextId++, windowId: 1, active: false, url: "about:blank", groupId: -1, ...props }; b.tabs.set(tab.id, tab); return tab; }; + b.newWindow = () => { const id = nextId++; b.windows.add(id); return id; }; + return b; +} + +function loadExtension(b, { session = {} } = {}) { + const listeners = { startup: [] }; + const noop = { addListener() {}, removeListener() {} }; + const area = (store) => ({ + get: async (key) => (key in store ? { [key]: clone(store[key]) } : {}), + set: async (obj) => { for (const [k, v] of Object.entries(obj)) store[k] = clone(v); }, + remove: async (keys) => { for (const k of [].concat(keys)) delete store[k]; }, + }); + const chrome = { + runtime: { id: "t", getManifest: () => ({ version: "0.0.0" }), onInstalled: noop, onStartup: { addListener: (fn) => listeners.startup.push(fn) } }, + alarms: { create() {}, onAlarm: noop }, action: { onClicked: noop, setBadgeText() {}, setBadgeBackgroundColor() {} }, + webNavigation: { onCommitted: noop }, debugger: { onDetach: noop, onEvent: noop }, + scripting: { executeScript: async () => [] }, + storage: { session: area(session), local: area(b.local) }, + tabs: { + get: async (id) => { const t = b.tabs.get(id); if (!t) throw new Error(`No tab with id: ${id}`); return clone(t); }, + query: async () => [...b.tabs.values()].map(clone), + create: async (props) => clone(b.addTab({ url: props.url, windowId: props.windowId ?? 1, active: !!props.active })), + remove: async (id) => { b.tabs.delete(id); b.removed.push(id); }, + ungroup: async (id) => { b.tabs.get(id).groupId = -1; b.ungrouped.push(id); }, + group: async ({ groupId, tabIds, createProperties }) => { + const gid = groupId ?? 900 + b.groups.size; + if (!b.groups.has(gid)) b.groups.set(gid, { id: gid, windowId: createProperties?.windowId ?? 1, title: "" }); + for (const id of tabIds) Object.assign(b.tabs.get(id), { groupId: gid, windowId: b.groups.get(gid).windowId }); + return gid; + }, + }, + tabGroups: { + query: async ({ windowId } = {}) => [...b.groups.values()].filter((g) => windowId === undefined || g.windowId === windowId).map(clone), + get: async (id) => clone(b.groups.get(id)), + update: async (id, props) => Object.assign(b.groups.get(id), props), + }, + windows: { + getLastFocused: async () => ({ id: 1, state: "normal" }), + get: async (id) => { if (!b.windows.has(id)) throw new Error("no window"); return { id }; }, + create: async ({ url }) => { const id = b.newWindow(); const tab = b.addTab({ windowId: id, url, active: true }); return { id, tabs: [clone(tab)] }; }, + }, + }; + const w = { chrome, console, setTimeout, clearTimeout, setInterval: () => 0, navigator: { userAgent: "unit-test" }, fetch: async () => { throw new Error("offline"); } }; + w.self = w; + vm.runInNewContext(workerSource, w); + w.formatTab = async (t) => t; + w.waitForTabSettled = async () => true; + return { w, session, startup: () => Promise.all(listeners.startup.map((fn) => fn())) }; +} + +async function setUpOwnership(b) { + const ext = loadExtension(b); + const target = await ext.w.getOrCreateAutomationTarget("s", undefined); // own window + const created = await ext.w.dispatch("tab.new", { url: "https://a.test/", background: true, sessionKey: "s", groupTitle: "Pi Session: s" }); + const userTab = b.addTab({ url: "https://user.test/" }); + await ext.w.dispatch("tab.group", { targetId: String(userTab.id), sessionKey: "s", groupTitle: "Pi Session: s" }); + return { ext, targetTab: target.id, createdTab: created.tab.id, adoptedTab: userTab.id }; +} + +test("after an extension reload, cleanup still closes Pi's tabs and ungroups the adopted one", async () => { + const b = makeBrowser(); + const ids = await setUpOwnership(b); + const reloaded = loadExtension(b); // fresh worker, empty storage.session, same storage.local + const result = await reloaded.w.dispatch("automation.cleanup", { sessionKey: "s" }); + assert.equal(result.closedTabId, ids.targetTab); + assert.equal(result.closedCreatedTabs, 1); + assert.equal(result.ungroupedAdoptedTabs, 1); + assert.deepEqual(b.removed.sort(), [ids.targetTab, ids.createdTab].sort()); + assert.deepEqual(b.ungrouped, [ids.adoptedTab]); + assert.ok(b.tabs.has(ids.adoptedTab), "the user's tab is never closed"); +}); + +test("a reloaded extension reuses its automation tab instead of opening another window", async () => { + const b = makeBrowser(); + const { targetTab } = await setUpOwnership(b); + const windowsBefore = b.windows.size; + const reloaded = loadExtension(b); + const again = await reloaded.w.getOrCreateAutomationTarget("s", undefined); + assert.equal(again.id, targetTab); + assert.equal(b.windows.size, windowsBefore); +}); + +test("restored records are dropped when the tab moved windows, left Pi's group, or no longer exists", async () => { + const b = makeBrowser(); + const ids = await setUpOwnership(b); + b.tabs.get(ids.targetTab).windowId = b.newWindow(); // user dragged it out + b.tabs.get(ids.adoptedTab).groupId = -1; // user removed it from Pi's group + b.tabs.delete(ids.createdTab); // closed; its id could later belong to another tab + const reloaded = loadExtension(b); + const result = await reloaded.w.dispatch("automation.cleanup", { sessionKey: "s" }); + assert.equal(result.closedTabId, null); + assert.equal(result.closedCreatedTabs, 0); + assert.equal(result.ungroupedAdoptedTabs, 0); + assert.deepEqual(b.removed, []); + assert.deepEqual(b.ungrouped, []); +}); + +test("a browser restart drops the local copy; a service-worker restart keeps using storage.session", async () => { + const b = makeBrowser(); + const { ext, targetTab } = await setUpOwnership(b); + // Service-worker restart: session storage survives, and it wins over local. + const swRestart = loadExtension(b, { session: ext.session }); + assert.equal((await swRestart.w.dispatch("automation.status", { sessionKey: "s" })).tabId, targetTab); + // Browser restart: onStartup clears the local copy before anything reads it. + const restarted = loadExtension(b); + await restarted.startup(); + assert.equal(Object.keys(b.local).length, 0); + assert.equal((await restarted.w.dispatch("automation.status", { sessionKey: "s" })).tabId, null); +}); |