Bound extension job and window-alias state
Two service-worker leaks. Running jobs were only bounded by their watchdog, so a command flood could hold many timers and results for up to JOB_TIMEOUT_MS; cap concurrent jobs at 32 and reject beyond that, since a clear error beats an unresponsive worker. Window aliases were never removed, so storage kept an entry for every window the user had ever renamed. Prune aliases against the live window set on list, rename, and close rather than only on close, because windows also disappear without going through windows.close. Also extract the repeated job summary in perf.status into a helper.
This commit is contained in:
@@ -15,6 +15,11 @@ import type { Job, Serializable, ErrorLike, DispatchArgs } from '../types';
|
|||||||
// jobs only need to survive long enough for the CLI to poll their result.
|
// jobs only need to survive long enough for the CLI to poll their result.
|
||||||
export const MAX_FINISHED_JOBS = 20;
|
export const MAX_FINISHED_JOBS = 20;
|
||||||
|
|
||||||
|
// Cap simultaneously running background jobs. A hung job has a watchdog, but a
|
||||||
|
// command flood could still pin many timers/results for up to JOB_TIMEOUT_MS.
|
||||||
|
// Rejecting above this bound keeps service-worker memory predictable.
|
||||||
|
export const MAX_RUNNING_JOBS = 32;
|
||||||
|
|
||||||
// Watchdog: if a runner never resolves/rejects (e.g. executeScript against a
|
// Watchdog: if a runner never resolves/rejects (e.g. executeScript against a
|
||||||
// dead tab), finalize the job as an error so its persist interval stops instead
|
// dead tab), finalize the job as an error so its persist interval stops instead
|
||||||
// of writing to api.storage.local every second forever.
|
// of writing to api.storage.local every second forever.
|
||||||
@@ -77,6 +82,11 @@ export class JobManager {
|
|||||||
}
|
}
|
||||||
|
|
||||||
async start(command: string, args: DispatchArgs, runner: JobRunner) {
|
async start(command: string, args: DispatchArgs, runner: JobRunner) {
|
||||||
|
const runningCount = [...this.jobs.values()].filter(job => job.status === "running").length;
|
||||||
|
if (runningCount >= MAX_RUNNING_JOBS) {
|
||||||
|
throw new Error(`too many background jobs running (${runningCount}); wait for jobs to finish or cancel one`);
|
||||||
|
}
|
||||||
|
|
||||||
const jobId = `${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 10)}`;
|
const jobId = `${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 10)}`;
|
||||||
const job: Job = {
|
const job: Job = {
|
||||||
id: jobId,
|
id: jobId,
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
import { getLargeOperationThrottle, getPerformanceProfile, hasAudibleTabs, setPerformanceProfile } from '../core';
|
import { getLargeOperationThrottle, getPerformanceProfile, hasAudibleTabs, setPerformanceProfile } from '../core';
|
||||||
import { CommandGroup } from '../classes/CommandGroup';
|
import { CommandGroup } from '../classes/CommandGroup';
|
||||||
import type { CommandEntry } from '../classes/CommandGroup';
|
import type { CommandEntry } from '../classes/CommandGroup';
|
||||||
import type { PerfSetProfileArgs, JobIdArgs } from '../types';
|
import type { Job, PerfSetProfileArgs, JobIdArgs } from '../types';
|
||||||
|
|
||||||
// PerfCommands also owns the jobs.* status/cancel queries: they read the same
|
// PerfCommands also owns the jobs.* status/cancel queries: they read the same
|
||||||
// JobManager (ctx.jobs) that perf.status reports, and there is no dedicated
|
// JobManager (ctx.jobs) that perf.status reports, and there is no dedicated
|
||||||
@@ -15,6 +15,19 @@ export class PerfCommands extends CommandGroup {
|
|||||||
"jobs.cancel": (a: JobIdArgs) => this.ctx.jobs.cancel(a),
|
"jobs.cancel": (a: JobIdArgs) => this.ctx.jobs.cancel(a),
|
||||||
};
|
};
|
||||||
|
|
||||||
|
private jobSummary(job: Job) {
|
||||||
|
return {
|
||||||
|
id: job.id,
|
||||||
|
command: job.command,
|
||||||
|
status: job.status,
|
||||||
|
phase: job.phase,
|
||||||
|
current: job.current,
|
||||||
|
total: job.total,
|
||||||
|
percent: job.percent,
|
||||||
|
cancelRequested: job.cancelRequested,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
private async perfStatus() {
|
private async perfStatus() {
|
||||||
const profile = await getPerformanceProfile();
|
const profile = await getPerformanceProfile();
|
||||||
const audible = await hasAudibleTabs();
|
const audible = await hasAudibleTabs();
|
||||||
@@ -23,16 +36,7 @@ export class PerfCommands extends CommandGroup {
|
|||||||
performanceProfile: profile,
|
performanceProfile: profile,
|
||||||
audible,
|
audible,
|
||||||
throttle,
|
throttle,
|
||||||
jobs: this.ctx.jobs.list().map(job => ({
|
jobs: this.ctx.jobs.list().map(job => this.jobSummary(job)),
|
||||||
id: job.id,
|
|
||||||
command: job.command,
|
|
||||||
status: job.status,
|
|
||||||
phase: job.phase,
|
|
||||||
current: job.current,
|
|
||||||
total: job.total,
|
|
||||||
percent: job.percent,
|
|
||||||
cancelRequested: job.cancelRequested,
|
|
||||||
})),
|
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -14,9 +14,31 @@ export class WindowsCommands extends CommandGroup {
|
|||||||
"windows.open": (a: WindowsOpenArgs) => this.windowsOpen(a),
|
"windows.open": (a: WindowsOpenArgs) => this.windowsOpen(a),
|
||||||
};
|
};
|
||||||
|
|
||||||
|
private async activeWindowIds(): Promise<Set<number>> {
|
||||||
|
const windows = await api.windows.getAll({});
|
||||||
|
return new Set(windows.map(w => w.id).filter(id => typeof id === "number"));
|
||||||
|
}
|
||||||
|
|
||||||
|
private async pruneAliases(activeIds?: Set<number>): Promise<Record<string, string>> {
|
||||||
|
const aliases = await getAliases();
|
||||||
|
const liveIds = activeIds || await this.activeWindowIds();
|
||||||
|
const pruned: Record<string, string> = {};
|
||||||
|
let changed = false;
|
||||||
|
for (const [id, alias] of Object.entries(aliases)) {
|
||||||
|
if (liveIds.has(Number(id))) {
|
||||||
|
pruned[id] = alias;
|
||||||
|
} else {
|
||||||
|
changed = true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (changed) await api.storage.local.set({ windowAliases: pruned });
|
||||||
|
return pruned;
|
||||||
|
}
|
||||||
|
|
||||||
private async windowsList() {
|
private async windowsList() {
|
||||||
const windows = await api.windows.getAll({ populate: true });
|
const windows = await api.windows.getAll({ populate: true });
|
||||||
const aliases = await getAliases();
|
const activeIds = new Set(windows.map(w => w.id).filter(id => typeof id === "number"));
|
||||||
|
const aliases = await this.pruneAliases(activeIds);
|
||||||
return windows.map(w => ({
|
return windows.map(w => ({
|
||||||
id: w.id,
|
id: w.id,
|
||||||
alias: aliases[w.id] || null,
|
alias: aliases[w.id] || null,
|
||||||
@@ -27,7 +49,7 @@ export class WindowsCommands extends CommandGroup {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private async windowsRename({ windowId, name }: WindowsRenameArgs) {
|
private async windowsRename({ windowId, name }: WindowsRenameArgs) {
|
||||||
const aliases = await getAliases();
|
const aliases = await this.pruneAliases();
|
||||||
aliases[windowId] = name;
|
aliases[windowId] = name;
|
||||||
await api.storage.local.set({ windowAliases: aliases });
|
await api.storage.local.set({ windowAliases: aliases });
|
||||||
return { windowId, name };
|
return { windowId, name };
|
||||||
@@ -35,6 +57,11 @@ export class WindowsCommands extends CommandGroup {
|
|||||||
|
|
||||||
private async windowsClose({ windowId }: WindowsCloseArgs) {
|
private async windowsClose({ windowId }: WindowsCloseArgs) {
|
||||||
await api.windows.remove(windowId);
|
await api.windows.remove(windowId);
|
||||||
|
const aliases = await this.pruneAliases();
|
||||||
|
if (windowId in aliases) {
|
||||||
|
delete aliases[windowId];
|
||||||
|
await api.storage.local.set({ windowAliases: aliases });
|
||||||
|
}
|
||||||
return { windowId };
|
return { windowId };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
// @ts-nocheck
|
// @ts-nocheck
|
||||||
import { test, mock } from 'node:test';
|
import { test, mock } from 'node:test';
|
||||||
import assert from 'node:assert/strict';
|
import assert from 'node:assert/strict';
|
||||||
import { JobManager, JOB_TIMEOUT_MS, MAX_FINISHED_JOBS, pruneFinishedJobs } from '../src/classes/JobManager';
|
import { JobManager, JOB_TIMEOUT_MS, MAX_FINISHED_JOBS, MAX_RUNNING_JOBS, pruneFinishedJobs } from '../src/classes/JobManager';
|
||||||
import { makeChromeMock } from './chrome-mock';
|
import { makeChromeMock } from './chrome-mock';
|
||||||
|
|
||||||
// Drain pending microtasks (finalize() chains several awaits). setImmediate is
|
// Drain pending microtasks (finalize() chains several awaits). setImmediate is
|
||||||
@@ -129,6 +129,21 @@ test('JobManager: a runner that settles after the watchdog cannot resurrect the
|
|||||||
mock.timers.reset();
|
mock.timers.reset();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('JobManager: rejects new background jobs above the running-job cap', async () => {
|
||||||
|
mock.timers.enable({ apis: ['setInterval', 'setTimeout'] });
|
||||||
|
globalThis.chrome = makeChromeMock();
|
||||||
|
const mgr = new JobManager();
|
||||||
|
for (let i = 0; i < MAX_RUNNING_JOBS; i++) {
|
||||||
|
await mgr.start(`running${i}`, {}, () => new Promise(() => {}));
|
||||||
|
}
|
||||||
|
await assert.rejects(
|
||||||
|
() => mgr.start('overflow', {}, async () => 'nope'),
|
||||||
|
/too many background jobs running/,
|
||||||
|
);
|
||||||
|
assert.equal(mgr.list().filter(job => job.status === 'running').length, MAX_RUNNING_JOBS);
|
||||||
|
mock.timers.reset();
|
||||||
|
});
|
||||||
|
|
||||||
test('JobManager: persisted set keeps running jobs even past the finished cap', async () => {
|
test('JobManager: persisted set keeps running jobs even past the finished cap', async () => {
|
||||||
mock.timers.enable({ apis: ['setInterval', 'setTimeout'] });
|
mock.timers.enable({ apis: ['setInterval', 'setTimeout'] });
|
||||||
globalThis.chrome = makeChromeMock();
|
globalThis.chrome = makeChromeMock();
|
||||||
|
|||||||
@@ -0,0 +1,69 @@
|
|||||||
|
// @ts-nocheck
|
||||||
|
import { test } from 'node:test';
|
||||||
|
import assert from 'node:assert/strict';
|
||||||
|
import { WindowsCommands } from '../src/commands/windows';
|
||||||
|
import { makeChromeMock } from './chrome-mock';
|
||||||
|
|
||||||
|
function makeWindowsChromeMock(windows) {
|
||||||
|
const chrome = makeChromeMock();
|
||||||
|
chrome.windows = {
|
||||||
|
getAll: async () => windows,
|
||||||
|
remove: async () => {},
|
||||||
|
create: async () => ({ id: 99 }),
|
||||||
|
};
|
||||||
|
return chrome;
|
||||||
|
}
|
||||||
|
|
||||||
|
test('windows.list prunes aliases for closed windows', async () => {
|
||||||
|
globalThis.chrome = makeWindowsChromeMock([
|
||||||
|
{ id: 1, focused: true, state: 'normal', tabs: [{ id: 10 }] },
|
||||||
|
{ id: 2, focused: false, state: 'minimized', tabs: [] },
|
||||||
|
]);
|
||||||
|
globalThis.chrome.storage.local._store.windowAliases = {
|
||||||
|
1: 'main',
|
||||||
|
2: 'side',
|
||||||
|
999: 'closed',
|
||||||
|
};
|
||||||
|
|
||||||
|
const commands = new WindowsCommands({ jobs: {} });
|
||||||
|
const result = await commands.commands['windows.list']({});
|
||||||
|
|
||||||
|
assert.deepEqual(result.map(w => [w.id, w.alias]), [[1, 'main'], [2, 'side']]);
|
||||||
|
assert.deepEqual(globalThis.chrome.storage.local._store.windowAliases, { 1: 'main', 2: 'side' });
|
||||||
|
});
|
||||||
|
|
||||||
|
test('windows.rename prunes stale aliases before saving the new name', async () => {
|
||||||
|
globalThis.chrome = makeWindowsChromeMock([
|
||||||
|
{ id: 1, focused: true, state: 'normal', tabs: [] },
|
||||||
|
{ id: 2, focused: false, state: 'normal', tabs: [] },
|
||||||
|
]);
|
||||||
|
globalThis.chrome.storage.local._store.windowAliases = {
|
||||||
|
1: 'main',
|
||||||
|
999: 'closed',
|
||||||
|
};
|
||||||
|
|
||||||
|
const commands = new WindowsCommands({ jobs: {} });
|
||||||
|
await commands.commands['windows.rename']({ windowId: 2, name: 'work' });
|
||||||
|
|
||||||
|
assert.deepEqual(globalThis.chrome.storage.local._store.windowAliases, { 1: 'main', 2: 'work' });
|
||||||
|
});
|
||||||
|
|
||||||
|
test('windows.close removes the closed window alias immediately', async () => {
|
||||||
|
let removed = null;
|
||||||
|
globalThis.chrome = makeWindowsChromeMock([
|
||||||
|
{ id: 1, focused: true, state: 'normal', tabs: [] },
|
||||||
|
{ id: 2, focused: false, state: 'normal', tabs: [] },
|
||||||
|
]);
|
||||||
|
globalThis.chrome.windows.remove = async id => { removed = id; };
|
||||||
|
globalThis.chrome.storage.local._store.windowAliases = {
|
||||||
|
1: 'main',
|
||||||
|
2: 'side',
|
||||||
|
999: 'closed',
|
||||||
|
};
|
||||||
|
|
||||||
|
const commands = new WindowsCommands({ jobs: {} });
|
||||||
|
await commands.commands['windows.close']({ windowId: 2 });
|
||||||
|
|
||||||
|
assert.equal(removed, 2);
|
||||||
|
assert.deepEqual(globalThis.chrome.storage.local._store.windowAliases, { 1: 'main' });
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user