refactor(extension): class-based command registry + modular src layout
Restructure the MV3 background worker from a monolithic core.ts/index.ts
into a class-based command architecture. Behavior is identical — the 83
registered commands dispatch byte-for-byte the same as before.
Structure
- One class per command group, each extending CommandGroup and exporting a
`commands` map keyed by the full command id ("tabs.close"). Groups:
Navigation, TabsMutation, TabsQuery, Groups, Windows, Dom (dom/extract/
page), BrowserData (storage/cookies), Session (session/clients + autosave
+ lazy-tab activation), Perf (perf + jobs.status/cancel), Extension.
- CommandRegistry merges the group maps (throws on duplicate ids), routes
background specs to JobManager and paginates array results via
makePagedData. JobManager owns the job map + lifecycle. NativeConnection
owns the native-port lifecycle and the inbound message router.
- index.ts is now thin wiring: JobManager -> ctx -> assembleRegistry ->
onActivated -> NativeConnection.start().
- Infra classes live in classes/ (PascalCase, file = class name); command
groups in commands/; shared helpers split out of core.ts into core/
(errors, throttle, scripting, tab-helpers, group-helpers, storage); all
types moved into types/ (json, jobs, session, tabs, messages,
command-args) behind a barrel.
DRY cleanup
- resolveTabUrl(tabId) and assertScriptableUrl(url, action) collapse the
tab/URL-guard boilerplate duplicated across dom.ts and browser-data.ts.
- processInBatches() centralizes the throttled, cancellable batch loop
shared by tabs.close, group.close and tabs.merge_windows.
- captureCurrentSession() dedups the snapshot-and-signature block shared by
session.save and the autosave path.
- DomArgs type alias replaces 21 inline ContentArgs & { tabId? } copies.
- Drop fetchTabHtml's redundant retry loop (executeScript already retries
transient frame/tab errors), a dead tabInfo import, and two stale
comments referencing a removed asArgs helper.
Type safety & tests
- Full noImplicitAny; no `any`/`unknown` annotations remain in src.
- JS unit-test harness using node --test + node:assert (zero new deps),
bundled via the existing esbuild. Covers JobManager retention/lifecycle
and the autosave listener-wiring/debounce with an in-memory chrome mock.
- The structural pytest checks track the new file homes and the centralized
processInBatches helper.
Verification: npm run check:extension green (tsc + esbuild 84.5kb +
node --check + 18 JS tests); uv run pytest -q -> 409 passed, 105 skipped.
No version bump.
This commit is contained in:
@@ -3,16 +3,20 @@ from pathlib import Path
|
||||
ROOT = Path(__file__).resolve().parents[1]
|
||||
|
||||
def test_extension_retries_error_page_script_injection_before_failing():
|
||||
core = (ROOT / "extension" / "src" / "core.ts").read_text()
|
||||
# core.ts was split into a core/ subfolder during the structure refactor:
|
||||
# the URL/error classifiers live in core/errors.ts and the executeScript
|
||||
# retry wrapper (which calls isTransientScriptError) lives in core/scripting.ts.
|
||||
errors = (ROOT / "extension" / "src" / "core" / "errors.ts").read_text()
|
||||
scripting = (ROOT / "extension" / "src" / "core" / "scripting.ts").read_text()
|
||||
|
||||
assert "isBrowserErrorUrl" in core
|
||||
assert "isErrorPageScriptError" in core
|
||||
assert "chrome-error://" in core
|
||||
assert "edge-error://" in core
|
||||
assert "brave-error://" in core
|
||||
assert "about:neterror" in core
|
||||
assert "about:certerror" in core
|
||||
assert "isTransientScriptError(e)" in core
|
||||
assert "isBrowserErrorUrl" in errors
|
||||
assert "isErrorPageScriptError" in errors
|
||||
assert "chrome-error://" in errors
|
||||
assert "edge-error://" in errors
|
||||
assert "brave-error://" in errors
|
||||
assert "about:neterror" in errors
|
||||
assert "about:certerror" in errors
|
||||
assert "isTransientScriptError(e)" in scripting
|
||||
|
||||
def test_read_only_dom_commands_have_error_page_fallbacks():
|
||||
dom = (ROOT / "extension" / "src" / "commands" / "dom.ts").read_text()
|
||||
@@ -26,7 +30,9 @@ def test_read_only_dom_commands_have_error_page_fallbacks():
|
||||
assert "isErrorPageScriptError(e)" in dom
|
||||
|
||||
def test_navigation_and_tabs_report_browser_error_pages():
|
||||
tabs = (ROOT / "extension" / "src" / "commands" / "tabs.ts").read_text()
|
||||
# tabs.watch_url (which reports tab error pages) moved into the read-only
|
||||
# TabsQueryCommands class in tabs-query.ts during the structure refactor.
|
||||
tabs = (ROOT / "extension" / "src" / "commands" / "tabs-query.ts").read_text()
|
||||
navigation = (ROOT / "extension" / "src" / "commands" / "navigation.ts").read_text()
|
||||
|
||||
assert "lastUrl" in tabs
|
||||
@@ -37,7 +43,12 @@ def test_navigation_and_tabs_report_browser_error_pages():
|
||||
assert "showing an error page while waiting for load" in navigation
|
||||
|
||||
def test_large_extension_operations_yield_between_batches():
|
||||
core = (ROOT / "extension" / "src" / "core.ts").read_text()
|
||||
# The large-operation throttling/queue helpers moved from core.ts into
|
||||
# core/throttle.ts when core was split into a subfolder. The slice-based
|
||||
# batch loop (cancel-check -> batch call -> progress -> yield) was then
|
||||
# centralized into the processInBatches() helper in core/throttle.ts, so the
|
||||
# command modules call that instead of inlining the loop.
|
||||
core = (ROOT / "extension" / "src" / "core" / "throttle.ts").read_text()
|
||||
tabs = (ROOT / "extension" / "src" / "commands" / "tabs.ts").read_text()
|
||||
groups = (ROOT / "extension" / "src" / "commands" / "groups.ts").read_text()
|
||||
session = (ROOT / "extension" / "src" / "commands" / "session.ts").read_text()
|
||||
@@ -56,14 +67,19 @@ def test_large_extension_operations_yield_between_batches():
|
||||
assert "itemCount >= 300" in core
|
||||
assert "itemCount >= 100" in core
|
||||
assert "chrome.tabs.query({ audible: true })" in core
|
||||
# The centralized batch loop drives cancellation + progress + throttled yield.
|
||||
assert "processInBatches" in core
|
||||
assert "throwIfJobCancelled(progress.job)" in core
|
||||
assert "yieldForLargeOperation(done" in core
|
||||
# tabs.close / tabs.merge_windows now batch via processInBatches; tabs.sort
|
||||
# still runs its own per-item move loop with yieldForLargeOperation.
|
||||
assert "processInBatches(toClose" in tabs
|
||||
assert "processInBatches(ids" in tabs
|
||||
assert "yieldForLargeOperation" in tabs
|
||||
assert "toClose.slice" in tabs
|
||||
assert "ids.slice" in tabs
|
||||
assert "w.tabs.every" in tabs
|
||||
assert "getLargeOperationThrottle" in tabs
|
||||
assert "runLargeOperation(\"tabs.sort\"" in tabs
|
||||
assert "yieldForLargeOperation" in groups
|
||||
assert "tabIds.slice" in groups
|
||||
assert "processInBatches(tabIds" in groups
|
||||
assert "getLargeOperationThrottle" in groups
|
||||
assert "runLargeOperation(\"group.close\"" in groups
|
||||
assert "yieldForLargeOperation(createdTabs.length" in session
|
||||
@@ -76,35 +92,50 @@ def test_large_extension_operations_yield_between_batches():
|
||||
assert "throwIfJobCancelled" in session
|
||||
assert "updateJobProgress" in session
|
||||
|
||||
index = (ROOT / "extension" / "src" / "index.ts").read_text()
|
||||
assert "BACKGROUND_COMMANDS" in index
|
||||
assert "startBackgroundJob" in index
|
||||
assert "persistJobs" in index
|
||||
assert "recentJobs" in index
|
||||
assert "jobs.status" in index
|
||||
assert "jobs.cancel" in index
|
||||
assert "perf.status" in index
|
||||
assert "perf.set_profile" in index
|
||||
assert "__background" in index
|
||||
# The background-job machinery moved out of index.ts into the JobManager
|
||||
# (classes/JobManager.ts), the message router (classes/NativeConnection.ts)
|
||||
# and the command registry/groups during the class-based refactor. The
|
||||
# behavior is identical; assert the defining patterns still exist in their
|
||||
# new homes.
|
||||
jobs_module = (ROOT / "extension" / "src" / "classes" / "JobManager.ts").read_text()
|
||||
connection = (ROOT / "extension" / "src" / "classes" / "NativeConnection.ts").read_text()
|
||||
perf = (ROOT / "extension" / "src" / "commands" / "perf.ts").read_text()
|
||||
tabs_module = (ROOT / "extension" / "src" / "commands" / "tabs.ts").read_text()
|
||||
|
||||
assert "background: true" in tabs_module # background command flag (was BACKGROUND_COMMANDS)
|
||||
assert "persistJobs" in jobs_module
|
||||
assert "recentJobs" in jobs_module
|
||||
assert "jobs.status" in perf
|
||||
assert "jobs.cancel" in perf
|
||||
assert "perf.status" in perf
|
||||
assert "perf.set_profile" in perf
|
||||
assert "__background" in connection
|
||||
|
||||
|
||||
def test_session_autosave_is_debounced_and_non_overlapping():
|
||||
session = (ROOT / "extension" / "src" / "commands" / "session.ts").read_text()
|
||||
# The autosave lifecycle moved out of session.ts into a dedicated
|
||||
# AutoSaveManager (autosave.ts) during the structure refactor; the shared
|
||||
# snapshot/signature helpers live in session-snapshot.ts. Behavior is
|
||||
# identical — the defining patterns just live in their new homes.
|
||||
autosave = (ROOT / "extension" / "src" / "commands" / "autosave.ts").read_text()
|
||||
snapshot = (ROOT / "extension" / "src" / "commands" / "session-snapshot.ts").read_text()
|
||||
|
||||
assert "autoSaveTimer" in session
|
||||
assert "autoSaveInFlight" in session
|
||||
assert "autoSavePending" in session
|
||||
assert "scheduleAutoSave" in session
|
||||
assert "autoSaveUpdatedHandler" in session
|
||||
assert "saveAutoSessionIfChanged" in session
|
||||
assert "sessionSignature" in session
|
||||
assert "autoSaveSignature" in session
|
||||
assert "chrome.tabs.onUpdated.addListener(autoSaveUpdatedHandler)" in session
|
||||
assert "chrome.tabs.onCreated.addListener(autoSaveHandler)" in session
|
||||
assert "chrome.tabs.onMoved.addListener(autoSaveHandler)" in session
|
||||
assert "if (!(\"url\" in changeInfo)) return;" in session
|
||||
assert "setTimeout(runAutoSave, delayMs)" in session
|
||||
assert "clearTimeout(autoSaveTimer)" in session
|
||||
assert "autoSaveTimer" in autosave
|
||||
assert "autoSaveInFlight" in autosave
|
||||
assert "autoSavePending" in autosave
|
||||
assert "scheduleAutoSave" in autosave
|
||||
assert "autoSaveUpdatedHandler" in autosave
|
||||
assert "saveAutoSessionIfChanged" in autosave
|
||||
assert "sessionSignature" in snapshot
|
||||
assert "autoSaveSignature" in autosave
|
||||
# AutoSaveManager binds the handlers as instance fields (this.*), so the
|
||||
# add/removeListener references stay identity-stable across enable/disable.
|
||||
assert "chrome.tabs.onUpdated.addListener(this.autoSaveUpdatedHandler)" in autosave
|
||||
assert "chrome.tabs.onCreated.addListener(this.autoSaveHandler)" in autosave
|
||||
assert "chrome.tabs.onMoved.addListener(this.autoSaveHandler)" in autosave
|
||||
assert "if (!(\"url\" in changeInfo)) return;" in autosave
|
||||
assert "setTimeout(() => this.runAutoSave(), delayMs)" in autosave
|
||||
assert "clearTimeout(this.autoSaveTimer)" in autosave
|
||||
|
||||
|
||||
def test_cli_and_sdk_expose_gentle_restore_controls():
|
||||
|
||||
Reference in New Issue
Block a user