History

fix(desktop): stop hidden previews draining battery (#8567)

Bilal Bakr committed 2 months ago

Commit ff176101 parent a40aef49 files · +113 −31Browse files

Unified · Split

9 files
  1. apps/web/src/browser/ElectronBrowserHost.tsx +4 −1
  2. apps/web/src/browser/HostedBrowserWebview.tsx +11 −1
  3. apps/web/src/browser/browserRecording.test.ts +4 −4
  4. apps/web/src/browser/browserRecording.ts +8 −6
  5. apps/web/src/browser/browserSurfaceStore.test.ts +13 −1
  6. apps/web/src/browser/browserSurfaceStore.ts +26 −0
  7. apps/web/src/browser/hostedBrowserWebviewStyle.test.ts +23 −1
  8. apps/web/src/browser/hostedBrowserWebviewStyle.ts +4 −6
  9. apps/web/src/components/preview/PreviewAutomationHosts.tsx +20 −11
apps/web/src/browser/ElectronBrowserHost.tsx+4−1
@@ -29,6 +29,8 @@ export function ElectronBrowserHost() { · 28 unmodified lines
2929 previewState.serverEpoch,
3030 snapshot.tabId,
3131 ),
32+ pictureInPicture:
33+ previewState.desktopByTabId[snapshot.tabId]?.pictureInPicture ?? false,
3234 zoomFactor: previewState.desktopByTabId[snapshot.tabId]?.zoomFactor ?? 1,
3335 }))
3436 : [];
@@ -80,7 +82,7 @@ export function ElectronBrowserHost() { · 45 unmodified lines
8082 if (!isElectron) return null;
8183 return (
8284 <div className="contents" data-electron-browser-host>
83− {sessions.map(({ threadRef, snapshot, runtimeTabId, zoomFactor }) => {
85+ {sessions.map(({ threadRef, snapshot, runtimeTabId, pictureInPicture, zoomFactor }) => {
8486 const url = snapshot.navStatus._tag === "Idle" ? null : snapshot.navStatus.url;
8587 return (
8688 <HostedBrowserWebview
@@ -90,6 +92,7 @@ export function ElectronBrowserHost() { · 3 unmodified lines
9092 runtimeTabId={runtimeTabId}
9193 initialUrl={url}
9294 viewport={snapshot.viewport ?? FILL_PREVIEW_VIEWPORT}
95+ pictureInPicture={pictureInPicture}
9396 zoomFactor={zoomFactor}
9497 />
9598 );
apps/web/src/browser/HostedBrowserWebview.tsx+11−1
@@ -9,6 +9,7 @@ import { usePreviewBridge } from "~/components/preview/usePreviewBridge"; · 8 unmodified lines
99 import { cn } from "~/lib/utils";
1010
1111 import { resolveBrowserSurfacePanelRect, useBrowserSurfaceStore } from "./browserSurfaceStore";
12+import { useActiveBrowserRecordingTabIds } from "./browserRecording";
1213 import {
1314 browserViewportSettingKey,
1415 resolveBrowserViewportLayout,
@@ -47,9 +48,11 @@ export function HostedBrowserWebview(props: { · 32 unmodified lines
4748 readonly runtimeTabId: string;
4849 readonly initialUrl: string | null;
4950 readonly viewport: PreviewViewportSetting;
51+ readonly pictureInPicture: boolean;
5052 readonly zoomFactor: number;
5153 }) {
52− const { threadRef, tabId, runtimeTabId, initialUrl, viewport, zoomFactor } = props;
54+ const { threadRef, tabId, runtimeTabId, initialUrl, viewport, pictureInPicture, zoomFactor } =
55+ props;
5356 const config = usePreviewWebviewConfig(threadRef.environmentId);
5457 const [initialSrc] = useState(() => initialUrl ?? "about:blank");
5558 const tabLeaseRef = useRef<AcquiredDesktopTab | null>(null);
@@ -70,6 +73,10 @@ export function HostedBrowserWebview(props: { · 14 unmodified lines
7073 };
7174 }),
7275 );
76+ const backgroundActivity = useBrowserSurfaceStore(
77+ (state) => (state.activityByTabId[runtimeTabId] ?? 0) > 0,
78+ );
79+ const recordingActive = useActiveBrowserRecordingTabIds().has(runtimeTabId);
7380 usePreviewBridge({ threadRef, tabId, runtimeTabId });
7481
7582 useEffect(() => {
@@ -231,8 +238,10 @@ export function HostedBrowserWebview(props: { · 155 unmodified lines
231238
232239 if (!config) return null;
233240
241+ const renderingActive = active || backgroundActivity || pictureInPicture || recordingActive;
234242 const wrapperStyle = resolveHostedBrowserWebviewWrapperStyle({
235243 active,
244+ renderingActive,
236245 cornerRadius: presentation.cornerRadius,
237246 rect: lastRect,
238247 hiddenSize,
@@ -244,6 +253,7 @@ export function HostedBrowserWebview(props: { · 5 unmodified lines
244253 className="fixed overflow-hidden bg-muted/35"
245254 style={{ ...wrapperStyle, overscrollBehavior: "contain" }}
246255 onScroll={syncContentPresentation}
256+ data-preview-rendering={renderingActive ? "active" : "suspended"}
247257 data-preview-viewport={runtimeTabId}
248258 >
249259 <div className="relative" style={{ width: layout.canvasWidth, height: layout.canvasHeight }}>
apps/web/src/browser/browserRecording.test.ts+4−4
@@ -179,10 +179,10 @@ describe("browser recording", () => { · 178 unmodified lines
179179
180180 it("starts recording for a visible tab", async () => {
181181 await startBrowserRecording("recording-tab");
182−
183− expect(events).toEqual(["start-screencast", "publish:recording-tab"]);
182+ const startupEvents = [...events];
184183
185184 await stopBrowserRecording("recording-tab");
185+ expect(startupEvents).toEqual(["publish:recording-tab", "start-screencast"]);
186186 });
187187
188188 it("records a hidden tab without requiring it to become visible", async () => {
@@ -195,11 +195,11 @@ describe("browser recording", () => { · 6 unmodified lines
195195 };
196196
197197 await startBrowserRecording("recording-tab");
198+ const startupEvents = [...events];
198199
199200 expect(startScreencast).toHaveBeenCalledWith("recording-tab");
200− expect(events).toEqual(["start-screencast", "publish:recording-tab"]);
201−
202201 await stopBrowserRecording("recording-tab");
202+ expect(startupEvents).toEqual(["publish:recording-tab", "start-screencast"]);
203203 });
204204
205205 it("fails startup instead of locking a fallback size when no frame arrives", async () => {
apps/web/src/browser/browserRecording.ts+8−6
@@ -122,6 +122,12 @@ export function useActiveBrowserRecordingTabIds(): ReadonlySet<string> { · 121 unmodified lines
122122 const activeRecordings = new Map<string, ActiveRecording>();
123123 let unsubscribeFrames: (() => void) | null = null;
124124
125+const publishActiveRecordingTabIds = (): void => {
126+ appAtomRegistry.set(activeBrowserRecordingTabIdsAtom, {
127+ tabIds: new Set(activeRecordings.keys()),
128+ });
129+};
130+
125131 export const BROWSER_RECORDING_STARTUP_SETTLE_TIMEOUT_MS = 5_000;
126132 export const BROWSER_RECORDING_FIRST_FRAME_SIZE_TIMEOUT_MS = 5_000;
127133
@@ -228,9 +234,7 @@ const clearActiveRecording = (recording: ActiveRecording): void => { · 100 unmodified lines
228234 unsubscribeFrames?.();
229235 unsubscribeFrames = null;
230236 }
231− appAtomRegistry.set(activeBrowserRecordingTabIdsAtom, {
232− tabIds: new Set(activeRecordings.keys()),
233− });
237+ publishActiveRecordingTabIds();
234238 };
235239
236240 const cleanupFailedRecordingStart = async (
@@ -377,6 +381,7 @@ export async function startBrowserRecording( · 140 unmodified lines
377381 lifecycle: { phase: "starting" },
378382 };
379383 activeRecordings.set(tabId, recording);
384+ publishActiveRecordingTabIds();
380385 try {
381386 try {
382387 unsubscribeFrames ??= bridge.recording.onFrame(drawFrame);
@@ -487,9 +492,6 @@ export async function startBrowserRecording( · 104 unmodified lines
487492 if (recording.lifecycle.phase === "starting") {
488493 recording.lifecycle = { phase: "recording" };
489494 }
490− appAtomRegistry.set(activeBrowserRecordingTabIdsAtom, {
491− tabIds: new Set(activeRecordings.keys()),
492− });
493495 return startedAt;
494496 } finally {
495497 settleStartup?.();
apps/web/src/browser/browserSurfaceStore.test.ts+13−1
@@ -2,13 +2,25 @@ import { beforeEach, describe, expect, it } from "vite-plus/test"; · 1 unmodified line
22
33 import {
44 acquireBrowserSurface,
5+ acquireBrowserSurfaceActivity,
56 resolveBrowserSurfacePanelRect,
67 useBrowserSurfaceStore,
78 } from "./browserSurfaceStore";
89
910 describe("browserSurfaceStore", () => {
1011 beforeEach(() => {
11− useBrowserSurfaceStore.setState({ byTabId: {} });
12+ useBrowserSurfaceStore.setState({ activityByTabId: {}, byTabId: {} });
13+ });
14+
15+ it("keeps concurrent background work active until every lease is released", () => {
16+ const first = acquireBrowserSurfaceActivity("background-browser");
17+ const second = acquireBrowserSurfaceActivity("background-browser");
18+
19+ first();
20+ expect(useBrowserSurfaceStore.getState().activityByTabId["background-browser"]).toBe(1);
21+
22+ second();
23+ expect(useBrowserSurfaceStore.getState().activityByTabId["background-browser"]).toBeUndefined();
1224 });
1325
1426 it("freezes the source content dimensions for a fitted presentation", () => {
apps/web/src/browser/browserSurfaceStore.ts+26−0
@@ -29,7 +29,9 @@ export interface BrowserSurfaceContentPresentation { · 28 unmodified lines
2929 }
3030
3131 interface BrowserSurfaceStoreState {
32+ readonly activityByTabId: Record<string, number>;
3233 readonly byTabId: Record<string, BrowserSurfacePresentation>;
34+ readonly acquireActivity: (tabId: string) => () => void;
3335 readonly claim: (tabId: string, owner: symbol, fitSourceContent: boolean) => void;
3436 readonly present: (
3537 tabId: string,
@@ -63,7 +65,28 @@ const rectEquals = (left: BrowserSurfaceRect | null, right: BrowserSurfaceRect): · 27 unmodified lines
6365 left.height === right.height;
6466
6567 export const useBrowserSurfaceStore = create<BrowserSurfaceStoreState>()((set) => ({
68+ activityByTabId: {},
6669 byTabId: {},
70+ acquireActivity: (tabId) => {
71+ let released = false;
72+ set((state) => ({
73+ activityByTabId: {
74+ ...state.activityByTabId,
75+ [tabId]: (state.activityByTabId[tabId] ?? 0) + 1,
76+ },
77+ }));
78+ return () => {
79+ if (released) return;
80+ released = true;
81+ set((state) => {
82+ const count = state.activityByTabId[tabId] ?? 0;
83+ const activityByTabId = { ...state.activityByTabId };
84+ if (count <= 1) delete activityByTabId[tabId];
85+ else activityByTabId[tabId] = count - 1;
86+ return { activityByTabId };
87+ });
88+ };
89+ },
6790 claim: (tabId, owner, fitSourceContent) =>
6891 set((state) => {
6992 const current = state.byTabId[tabId];
@@ -171,6 +194,9 @@ export const useBrowserSurfaceStore = create<BrowserSurfaceStoreState>()((set) = · 101 unmodified lines
171194 }),
172195 }));
173196
197+export const acquireBrowserSurfaceActivity = (tabId: string): (() => void) =>
198+ useBrowserSurfaceStore.getState().acquireActivity(tabId);
199+
174200 export function acquireBrowserSurface(
175201 tabId: string,
176202 fitSourceContent = false,
apps/web/src/browser/hostedBrowserWebviewStyle.test.ts+23−1
@@ -10,6 +10,7 @@ describe("resolveHostedBrowserWebviewWrapperStyle", () => { · 9 unmodified lines
1010 expect(
1111 resolveHostedBrowserWebviewWrapperStyle({
1212 active: true,
13+ renderingActive: true,
1314 rect: { x: 12, y: 34, width: 800, height: 600 },
1415 hiddenSize: { width: 1280, height: 800 },
1516 }),
@@ -27,6 +28,7 @@ describe("resolveHostedBrowserWebviewWrapperStyle", () => { · 11 unmodified lines
2728 expect(
2829 resolveHostedBrowserWebviewWrapperStyle({
2930 active: true,
31+ renderingActive: true,
3032 cornerRadius: 12,
3133 rect: { x: 12, y: 34, width: 360, height: 203 },
3234 hiddenSize: { width: 1280, height: 800 },
@@ -40,9 +42,10 @@ describe("resolveHostedBrowserWebviewWrapperStyle", () => { · 7 unmodified lines
4042 });
4143 });
4244
43− it("keeps an inactive webview paintable while moving it offscreen", () => {
45+ it("suspends painting for an inactive webview", () => {
4446 const style = resolveHostedBrowserWebviewWrapperStyle({
4547 active: false,
48+ renderingActive: false,
4649 rect: { x: 12, y: 34, width: 800, height: 600 },
4750 hiddenSize: { width: 393, height: 852 },
4851 });
@@ -54,6 +57,25 @@ describe("resolveHostedBrowserWebviewWrapperStyle", () => { · 5 unmodified lines
5457 height: 852,
5558 zIndex: -1,
5659 pointerEvents: "none",
60+ visibility: "hidden",
61+ });
62+ });
63+
64+ it("keeps an active background task paintable offscreen", () => {
65+ const style = resolveHostedBrowserWebviewWrapperStyle({
66+ active: false,
67+ renderingActive: true,
68+ rect: null,
69+ hiddenSize: { width: 1280, height: 800 },
70+ });
71+
72+ expect(style).toEqual({
73+ left: HIDDEN_BROWSER_WEBVIEW_OFFSET,
74+ top: HIDDEN_BROWSER_WEBVIEW_OFFSET,
75+ width: 1280,
76+ height: 800,
77+ zIndex: -1,
78+ pointerEvents: "none",
5779 visibility: "visible",
5880 });
5981 });
apps/web/src/browser/hostedBrowserWebviewStyle.ts+4−6
@@ -13,18 +13,19 @@ export interface HostedBrowserWebviewWrapperStyle { · 12 unmodified lines
1313 readonly zIndex: number;
1414 readonly pointerEvents: "auto" | "none";
1515 readonly borderRadius?: number;
16− readonly visibility?: "visible";
16+ readonly visibility?: "hidden" | "visible";
1717 }
1818
1919 export const HIDDEN_BROWSER_WEBVIEW_OFFSET = -100_000;
2020
2121 export function resolveHostedBrowserWebviewWrapperStyle(input: {
2222 readonly active: boolean;
23+ readonly renderingActive: boolean;
2324 readonly cornerRadius?: number;
2425 readonly rect: BrowserSurfaceRect | null;
2526 readonly hiddenSize: HostedBrowserWebviewSize;
2627 }): HostedBrowserWebviewWrapperStyle {
27− const { active, cornerRadius = 0, hiddenSize, rect } = input;
28+ const { active, cornerRadius = 0, hiddenSize, rect, renderingActive } = input;
2829 if (active && rect) {
2930 return {
3031 left: rect.x,
@@ -44,9 +45,6 @@ export function resolveHostedBrowserWebviewWrapperStyle(input: { · 13 unmodified lines
4445 height: hiddenSize.height,
4546 zIndex: -1,
4647 pointerEvents: "none",
47− // Keep the guest CSS-visible even while physically offscreen. Electron
48− // webviews can keep metadata/status alive under `visibility:hidden` while
49− // CDP Runtime/Input commands stall, which breaks offscreen automation.
50− visibility: "visible",
48+ visibility: renderingActive ? "visible" : "hidden",
5149 };
5250 }
apps/web/src/components/preview/PreviewAutomationHosts.tsx+20−11
@@ -37,7 +37,10 @@ import { · 36 unmodified lines
3737 stopBrowserRecording,
3838 } from "~/browser/browserRecording";
3939 import { resolveBrowserRecordingStopTarget } from "~/browser/browserRecordingScope";
40−import { useBrowserSurfaceStore } from "~/browser/browserSurfaceStore";
40+import {
41+ acquireBrowserSurfaceActivity,
42+ useBrowserSurfaceStore,
43+} from "~/browser/browserSurfaceStore";
4144 import { browserDefaultOpenViewport, resolveBrowserDefaults } from "~/browser/browserDefaults";
4245 import { runBrowserViewportMutation } from "~/browser/browserViewportActions";
4346 import { previewRuntimeTabId } from "~/browser/previewRuntimeTabId";
@@ -98,7 +101,7 @@ const waitForDesktopOverlay = async ( · 54 unmodified lines
98101 operation,
99102 requestId,
100103 });
101− if (state.desktopByTabId[tabId] && previewBridge) {
104+ if (state.desktopByTabId[tabId] && previewBridge && isPreviewWebviewRendering(runtimeTabId)) {
102105 const status = await previewBridge.automation.status(runtimeTabId);
103106 if (status.available) return;
104107 }
@@ -121,6 +124,11 @@ const findPreviewWebview = (tabId: string): ExecutablePreviewWebview | null => · 16 unmodified lines
121124 (candidate) => candidate.getAttribute("data-preview-tab") === tabId,
122125 ) ?? null;
123126
127+const isPreviewWebviewRendering = (runtimeTabId: string): boolean => {
128+ const wrapper = findPreviewWebview(runtimeTabId)?.closest<HTMLElement>("[data-preview-viewport]");
129+ return wrapper?.getAttribute("data-preview-rendering") === "active";
130+};
131+
124132 const readWebviewViewport = async (
125133 webview: ExecutablePreviewWebview,
126134 ): Promise<PreviewRenderedViewportSize | null> => {
@@ -212,8 +220,12 @@ const currentStatus = async ( · 85 unmodified lines
212220 const visible = runtimeTabId
213221 ? (useBrowserSurfaceStore.getState().byTabId[runtimeTabId]?.visible ?? false)
214222 : false;
223+ const renderingActive = runtimeTabId ? isPreviewWebviewRendering(runtimeTabId) : false;
215224 const viewportSetting = snapshot ? (snapshot.viewport ?? FILL_PREVIEW_VIEWPORT) : undefined;
216− const viewport = runtimeTabId ? await readRenderedViewport(runtimeTabId).catch(() => null) : null;
225+ const viewport =
226+ runtimeTabId && renderingActive
227+ ? await readRenderedViewport(runtimeTabId).catch(() => null)
228+ : null;
217229 const viewportStatus = {
218230 ...(viewportSetting === undefined ? {} : { viewportSetting }),
219231 ...(viewport === null ? {} : { viewport }),
@@ -307,6 +319,7 @@ function PreviewAutomationHost(props: { readonly environmentId: EnvironmentId }) · 87 unmodified lines
307319 threadId: request.threadId,
308320 };
309321 let tabId = request.tabId ?? null;
322+ const browserActivity = { release: null as (() => void) | null };
310323 try {
311324 let state = readThreadPreviewState(threadRef);
312325 const needsSessionSync = needsPreviewAutomationSessionSync(state, request.tabId);
@@ -340,6 +353,7 @@ function PreviewAutomationHost(props: { readonly environmentId: EnvironmentId }) · 27 unmodified lines
340353 }
341354 const readyState = readThreadPreviewState(threadRef);
342355 const runtimeTabId = previewRuntimeTabId(threadRef, readyState.serverEpoch, readyTabId);
356+ browserActivity.release ??= acquireBrowserSurfaceActivity(runtimeTabId);
343357 await waitForDesktopOverlay(
344358 threadRef,
345359 request.requestId,
@@ -440,14 +454,7 @@ function PreviewAutomationHost(props: { readonly environmentId: EnvironmentId }) · 94 unmodified lines
440454 usePreviewMiniPlayerStore.getState().open(threadRef, activeTabId);
441455 }
442456 if (activeSnapshot && previewAutomationOpenNeedsOverlay(input, activeSnapshot)) {
443− await waitForDesktopOverlay(
444− threadRef,
445− request.requestId,
446− activeTabId,
447− activeRuntimeTabId,
448− request.operation,
449− request.timeoutMs,
450− );
457+ await requireReadyTab();
451458 }
452459 if (shouldPresentPreview) {
453460 // React commits the thread-bound surface asynchronously. Settle
@@ -678,6 +685,8 @@ function PreviewAutomationHost(props: { readonly environmentId: EnvironmentId }) · 224 unmodified lines
678685 tabId,
679686 cause,
680687 });
688+ } finally {
689+ browserActivity.release?.();
681690 }
682691 },
683692 [environmentId, listPreviews, open, registry, resize],