-
Notifications
You must be signed in to change notification settings - Fork 1.7k
refactor(extension): extract target-neutral recorder host and platform layer #1998
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,59 @@ | ||||||
| // The recorder document (recorder.html) hosts capture, upload, device | ||||||
| // enumeration, the mic probe and the camera-preview relay. On Chrome it runs | ||||||
| // as an offscreen document; this module owns its lifecycle so the rest of the | ||||||
| // service worker never touches chrome.offscreen directly. | ||||||
| export const RECORDER_URL = "recorder.html"; | ||||||
|
|
||||||
| let recorderDocumentCreation: Promise<void> | null = null; | ||||||
|
|
||||||
| const getRecorderContexts = async () => { | ||||||
| const recorderUrl = chrome.runtime.getURL(RECORDER_URL); | ||||||
| return new Promise<Array<{ documentUrl?: string }>>((resolve) => { | ||||||
| chrome.runtime.getContexts( | ||||||
| { | ||||||
| contextTypes: [chrome.runtime.ContextType.OFFSCREEN_DOCUMENT], | ||||||
| documentUrls: [recorderUrl], | ||||||
| }, | ||||||
| (contexts) => resolve(contexts), | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| ); | ||||||
| }); | ||||||
| }; | ||||||
|
|
||||||
| export const hasRecorderHost = async () => | ||||||
| (await getRecorderContexts()).length > 0; | ||||||
|
|
||||||
| const createOffscreenDocument = () => | ||||||
| new Promise<void>((resolve, reject) => { | ||||||
| chrome.offscreen.createDocument( | ||||||
| { | ||||||
| url: RECORDER_URL, | ||||||
| reasons: ["USER_MEDIA", "DISPLAY_MEDIA", "BLOBS", "AUDIO_PLAYBACK"], | ||||||
| justification: "Record and upload Cap videos from an extension page.", | ||||||
| }, | ||||||
| () => { | ||||||
| const error = chrome.runtime.lastError; | ||||||
| if (!error) { | ||||||
| resolve(); | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| const message = error.message ?? "Failed to create offscreen document"; | ||||||
| if (message.toLowerCase().includes("single offscreen document")) { | ||||||
| resolve(); | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| reject(new Error(message)); | ||||||
| }, | ||||||
| ); | ||||||
| }); | ||||||
|
|
||||||
| export const ensureRecorderHost = async () => { | ||||||
| const contexts = await getRecorderContexts(); | ||||||
| if (contexts.length > 0) return; | ||||||
|
|
||||||
| recorderDocumentCreation ??= createOffscreenDocument().finally(() => { | ||||||
| recorderDocumentCreation = null; | ||||||
| }); | ||||||
| await recorderDocumentCreation; | ||||||
| }; | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| import { TARGET } from "./target"; | ||
|
|
||
| // Per-target feature availability. Firefox has no chrome.offscreen or | ||
| // chrome.tabCapture, its getDisplayMedia exposes no system audio or per-tab | ||
| // surface, MV3 host permissions are user-grantable rather than granted at | ||
| // install, and getDisplayMedia requires transient user activation so capture | ||
| // cannot start without a click inside the recorder document. | ||
| export const capabilities = { | ||
| supportsTabCapture: TARGET === "chrome", | ||
| supportsOffscreen: TARGET === "chrome", | ||
| supportsSystemAudioCapture: TARGET === "chrome", | ||
| hostPermissionsGrantedAtInstall: TARGET === "chrome", | ||
| recorderNeedsUserGesture: TARGET === "firefox", | ||
| } as const; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| // "chrome-extension:" on Chromium, "moz-extension:" on Firefox. Sender checks | ||
| // must use this instead of a hardcoded literal or Firefox extension pages get | ||
| // misclassified as web pages. | ||
| export const EXTENSION_PROTOCOL = new URL(chrome.runtime.getURL("")).protocol; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| // Injected by vite `define` per build target; undefined under vitest, which | ||
| // runs without a define and must behave like the Chrome build. | ||
| declare const __TARGET__: "chrome" | "firefox" | undefined; | ||
|
|
||
| export type ExtensionTarget = "chrome" | "firefox"; | ||
|
|
||
| export const TARGET: ExtensionTarget = | ||
| typeof __TARGET__ === "undefined" ? "chrome" : __TARGET__; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/chrome-extension/src/platform/target.ts
Line: 8
Comment:
**Missing Target Becomes Chrome**
`TARGET` defaults to `"chrome"` whenever `__TARGET__` is absent, but the changed Vite config does not define `__TARGET__`. A Firefox build that uses this config without an injected define will enable Chrome-only capabilities like offscreen and tab capture, sending the Firefox worker into unsupported APIs.
How can I resolve this? If you propose a fix, please make it concise. |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
recorder-host.tsis imported by the service worker at module load time, but this line readschrome.runtime.ContextType.OFFSCREEN_DOCUMENTbefore any capability guard can run. In a Firefox build where that Chrome-only API is missing, the background worker can throw during startup instead of reaching thesupportsOffscreenchecks.Prompt To Fix With AI