From 4aade4b94fd8fb59abfea781aac7c60165acd47f Mon Sep 17 00:00:00 2001 From: Ljy-0827 Date: Fri, 21 Aug 2026 13:16:15 +0800 Subject: [PATCH] fix(recorder): bug fix, oopif load too late --- .../record-frame-coordinator.test.ts | 98 +++++++++++++++++- .../lib/__tests__/recording-runtime.test.ts | 89 ++++++++++++++++- .../src/lib/recording/frame-coordinator.ts | 99 ++++++++++++++++--- .../src/lib/recording/recording-runtime.ts | 43 +++++++- apps/extension/src/tools/record.ts | 11 ++- 5 files changed, 319 insertions(+), 21 deletions(-) diff --git a/apps/extension/src/lib/__tests__/record-frame-coordinator.test.ts b/apps/extension/src/lib/__tests__/record-frame-coordinator.test.ts index e5e7449..abefbaa 100644 --- a/apps/extension/src/lib/__tests__/record-frame-coordinator.test.ts +++ b/apps/extension/src/lib/__tests__/record-frame-coordinator.test.ts @@ -1,5 +1,5 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; -import { RECORD_FRAME_PORT } from "../recording/frame-bridge"; +import { RECORD_FRAME_PORT, RECORD_FRAME_QUERY } from "../recording/frame-bridge"; import { RecordFrameCoordinator } from "../recording/frame-coordinator"; class ListenerSet unknown> { @@ -46,6 +46,98 @@ describe("RecordFrameCoordinator", () => { }); }); + it("answers child-document queries for the initial tab before it is armed", () => { + const coordinator = new RecordFrameCoordinator({ + getAllFrames: async () => [], + sendToDocument: async () => ({ ok: true }), + }); + coordinator.attach(); + coordinator.begin("rec-1", 10, 3); + + const query = (tabId: number) => { + let response: unknown; + for (const listener of onMessage.listeners) { + listener( + { type: RECORD_FRAME_QUERY }, + { + tab: { id: tabId }, + frameId: 7, + documentId: "child-document", + } as chrome.runtime.MessageSender, + (value) => { + response = value; + }, + ); + } + return response; + }; + + expect(query(3)).toEqual({ active: true, requestId: "rec-1", startedAtMs: 10 }); + expect(query(4)).toEqual({ active: false }); + }); + + it("starts a child Document that appears after the initial frame snapshot", async () => { + let notifyFrameNavigation: + | ((frame: { + tabId: number; + frameId: number; + documentId?: string; + lifecycle: "committed" | "completed"; + }) => void) + | undefined; + const sendToDocument = vi.fn(async () => ({ ok: true })); + const coordinator = new RecordFrameCoordinator({ + getAllFrames: async () => [{ frameId: 0, documentId: "top-document" }], + sendToDocument, + subscribeFrameNavigation(listener) { + notifyFrameNavigation = listener; + return () => {}; + }, + }); + coordinator.attach(); + const onDocumentReady = vi.fn(); + coordinator.begin("rec-1", 10, 3, onDocumentReady); + await coordinator.armTab("rec-1", 3); + sendToDocument.mockClear(); + + notifyFrameNavigation?.({ + tabId: 3, + frameId: 7, + documentId: "late-child", + lifecycle: "completed", + }); + await vi.waitFor(() => { + expect(sendToDocument).toHaveBeenCalledWith( + 3, + { type: "bsk-record-frame-start", requestId: "rec-1", startedAtMs: 10 }, + { documentId: "late-child" }, + ); + }); + expect(onDocumentReady).not.toHaveBeenCalled(); + + const child = fakePort({ + tab: { id: 3 }, + frameId: 7, + documentId: "late-child", + } as chrome.runtime.MessageSender); + for (const listener of onConnect.listeners) listener(child.port); + child.receive({ type: "ready", requestId: "rec-1", producerId: "producer-1" }); + notifyFrameNavigation?.({ + tabId: 3, + frameId: 7, + documentId: "late-child", + lifecycle: "completed", + }); + await vi.waitFor(() => { + expect(onDocumentReady).toHaveBeenCalledWith({ + tabId: 3, + documentId: "late-child", + browserFrameId: 7, + producerId: "producer-1", + }); + }); + }); + it("binds a producer to its sender Document and keeps final steps valid while stopping", async () => { const sendToDocument = vi.fn(async () => ({ ok: true })); const coordinator = new RecordFrameCoordinator({ @@ -56,7 +148,7 @@ describe("RecordFrameCoordinator", () => { sendToDocument, }); coordinator.attach(); - coordinator.begin("rec-1", 10); + coordinator.begin("rec-1", 10, 3); await expect(coordinator.armTab("rec-1", 3)).resolves.toBe(true); const sender = { @@ -100,7 +192,7 @@ describe("RecordFrameCoordinator", () => { sendToDocument: async () => ({ ok: true }), }); coordinator.attach(); - coordinator.begin("rec-1", 10); + coordinator.begin("rec-1", 10, 3); await coordinator.armTab("rec-1", 3); const sender = { tab: { id: 3 }, diff --git a/apps/extension/src/lib/__tests__/recording-runtime.test.ts b/apps/extension/src/lib/__tests__/recording-runtime.test.ts index 6dfe524..5889e65 100644 --- a/apps/extension/src/lib/__tests__/recording-runtime.test.ts +++ b/apps/extension/src/lib/__tests__/recording-runtime.test.ts @@ -2,12 +2,18 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import type { CdpRunner, ChromeTabsApi } from "@/tools/shared"; const captureRecordingObservation = vi.hoisted(() => vi.fn()); +const waitForDocumentSettled = vi.hoisted(() => vi.fn(async () => "quiet" as const)); vi.mock("../recording/observation-capture", async (importOriginal) => { const actual = await importOriginal(); return { ...actual, captureRecordingObservation }; }); +vi.mock("../recording/document-settle", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, waitForDocumentSettled }; +}); + import { ObservationNodeIndex } from "../recording/observation-capture"; import { RecordingObservationRuntime } from "../recording/recording-runtime"; import type { RecordingDraftStep } from "../recording/types"; @@ -31,7 +37,10 @@ function runtime(): RecordingObservationRuntime { } describe("RecordingObservationRuntime", () => { - afterEach(() => captureRecordingObservation.mockReset()); + afterEach(() => { + captureRecordingObservation.mockReset(); + waitForDocumentSettled.mockClear(); + }); it("shares one initial capture and includes it in flush", async () => { let release!: (value: ReturnType) => void; @@ -199,4 +208,82 @@ describe("RecordingObservationRuntime", () => { }); expect(draft.matchedTarget?.ref).toBe("e1"); }); + + it("refreshes a late iframe before matching its first action", async () => { + const iframeObservation = { + ...observation(), + index: new ObservationNodeIndex({ + rootFrameId: "root", + frames: [ + { + frameId: "child", + target: { tabId: 7, sessionId: "oopif-session" }, + recordingDocumentId: "producer-1", + }, + ], + matchNodes: [ + { + backendNodeId: 42, + frameId: "child", + tag: "button", + rect: { x: 410, y: 20, w: 100, h: 30 }, + localRect: { x: 10, y: 20, w: 100, h: 30 }, + }, + ], + refs: [ + { + ref: "e1", + backendNodeId: 42, + frameId: "child", + role: "button", + name: "表格视图", + line: 1, + }, + ], + }), + }; + captureRecordingObservation + .mockResolvedValueOnce(observation()) + .mockResolvedValueOnce(iframeObservation) + .mockResolvedValueOnce(iframeObservation); + const recording = runtime(); + + await recording.captureInitial(7); + await recording.refreshDocument(7, "producer-1"); + const drafts: RecordingDraftStep[] = [ + { + op: "click", + captureTarget: { tag: "button", role: "button", name: "表格视图" }, + targetHint: { + geometry: { rect: { x: 10, y: 20, w: 100, h: 30 }, tag: "button" }, + }, + }, + ]; + await recording.processDraft(7, drafts, 0, "producer-1"); + recording.cancel(); + + expect(captureRecordingObservation).toHaveBeenCalledTimes(3); + expect(waitForDocumentSettled).toHaveBeenCalledWith( + expect.anything(), + { frameId: "child", target: { tabId: 7, sessionId: "oopif-session" } }, + { signal: expect.any(AbortSignal) }, + ); + const draft = drafts[0]; + expect(draft?.op).toBe("click"); + if (!draft || draft.op !== "click") throw new Error("expected click draft"); + expect(draft.matchedTarget).toMatchObject({ ref: "e1", name: "表格视图" }); + }); + + it("does not poison final flush when a frame readiness refresh fails", async () => { + captureRecordingObservation + .mockResolvedValueOnce(observation()) + .mockRejectedValueOnce(new Error("child Document replaced")); + const recording = runtime(); + + await recording.captureInitial(7); + await expect(recording.refreshDocument(7, "producer-1")).rejects.toThrow( + "child Document replaced", + ); + await expect(recording.flush()).resolves.toBeUndefined(); + }); }); diff --git a/apps/extension/src/lib/recording/frame-coordinator.ts b/apps/extension/src/lib/recording/frame-coordinator.ts index 8bdf188..52ca0ad 100644 --- a/apps/extension/src/lib/recording/frame-coordinator.ts +++ b/apps/extension/src/lib/recording/frame-coordinator.ts @@ -19,6 +19,7 @@ interface ArmedRecording { startedAtMs: number; tabIds: Set; agents: Map; + onDocumentReady?: (scope: RecordingCaptureScope) => void | Promise; finishing: boolean; } @@ -33,6 +34,11 @@ interface BrowserFrame { documentId?: string; } +interface BrowserFrameNavigation extends BrowserFrame { + tabId: number; + lifecycle: "committed" | "completed"; +} + const RECORD_FRAME_STOP_TIMEOUT_MS = 5_000; export interface RecordFrameCoordinatorDeps { @@ -42,6 +48,7 @@ export interface RecordFrameCoordinatorDeps { message: RecordFrameStartMessage, target: { documentId?: string; frameId?: number }, ): Promise; + subscribeFrameNavigation?(listener: (frame: BrowserFrameNavigation) => void): () => void; } function documentKey(tabId: number, documentId: string): string { @@ -72,6 +79,18 @@ function defaultDeps(): RecordFrameCoordinatorDeps { sendToDocument(tabId, message, target) { return chrome.tabs.sendMessage(tabId, message, target); }, + subscribeFrameNavigation(listener) { + const onCommitted = (details: chrome.webNavigation.WebNavigationTransitionCallbackDetails) => + listener({ ...details, lifecycle: "committed" }); + const onCompleted = (details: chrome.webNavigation.WebNavigationFramedCallbackDetails) => + listener({ ...details, lifecycle: "completed" }); + chrome.webNavigation.onCommitted.addListener(onCommitted); + chrome.webNavigation.onCompleted.addListener(onCompleted); + return () => { + chrome.webNavigation.onCommitted.removeListener(onCommitted); + chrome.webNavigation.onCompleted.removeListener(onCompleted); + }; + }, }; } @@ -79,6 +98,7 @@ export class RecordFrameCoordinator { readonly #deps: RecordFrameCoordinatorDeps; readonly #recordings = new Map(); #attached = false; + #detachFrameNavigation: (() => void) | null = null; constructor(deps: RecordFrameCoordinatorDeps = defaultDeps()) { this.#deps = deps; @@ -89,24 +109,37 @@ export class RecordFrameCoordinator { this.#attached = true; chrome.runtime.onConnect.addListener(this.#onConnect); chrome.runtime.onMessage.addListener(this.#onMessage); + this.#detachFrameNavigation = + this.#deps.subscribeFrameNavigation?.(this.#onFrameNavigation) ?? null; return () => { if (!this.#attached) return; this.#attached = false; chrome.runtime.onConnect.removeListener(this.#onConnect); chrome.runtime.onMessage.removeListener(this.#onMessage); + this.#detachFrameNavigation?.(); + this.#detachFrameNavigation = null; for (const recording of this.#recordings.values()) this.#cancelAgents(recording); this.#recordings.clear(); }; } - begin(requestId: string, startedAtMs: number): void { + begin( + requestId: string, + startedAtMs: number, + initialTabId: number, + onDocumentReady?: (scope: RecordingCaptureScope) => void | Promise, + ): void { const previous = this.#recordings.get(requestId); if (previous) this.#cancelAgents(previous); this.#recordings.set(requestId, { requestId, startedAtMs, - tabIds: new Set(), + // Register the initial tab before navigation begins. A newly-created + // child document can query at document_start before armTab's frame + // snapshot sees it, especially when Chromium promotes it to an OOPIF. + tabIds: new Set([initialTabId]), agents: new Map(), + ...(onDocumentReady ? { onDocumentReady } : {}), finishing: false, }); } @@ -124,21 +157,10 @@ export class RecordFrameCoordinator { } if (!frames.some((frame) => frame.frameId === 0)) frames.unshift({ frameId: 0 }); - const message: RecordFrameStartMessage = { - type: RECORD_FRAME_START, - requestId, - startedAtMs: recording.startedAtMs, - }; const results = await Promise.all( frames.map(async (frame) => { - try { - const response = await this.#deps.sendToDocument(tabId, message, { - ...(frame.documentId ? { documentId: frame.documentId } : { frameId: frame.frameId }), - }); - return { frameId: frame.frameId, started: isStarted(response) }; - } catch { - return { frameId: frame.frameId, started: false }; - } + const started = await this.#startFrame(recording, tabId, frame); + return { frameId: frame.frameId, started }; }), ); return results.some((result) => result.frameId === 0 && result.started); @@ -208,6 +230,33 @@ export class RecordFrameCoordinator { return false; }; + readonly #onFrameNavigation = (frame: BrowserFrameNavigation): void => { + if (frame.frameId === 0) return; + for (const recording of this.#recordings.values()) { + if (recording.finishing || !recording.tabIds.has(frame.tabId)) continue; + // onCommitted can race document_start, while onCompleted runs after the + // content script is available. Starting twice is safe and closes both + // the early-query and late-SPA-frame gaps. + void (async () => { + const started = await this.#startFrame(recording, frame.tabId, frame); + if (!started || frame.lifecycle !== "completed" || recording.finishing) return; + const agent = frame.documentId + ? recording.agents.get(documentKey(frame.tabId, frame.documentId)) + : [...recording.agents.values()].find( + (candidate) => + candidate.tabId === frame.tabId && candidate.browserFrameId === frame.frameId, + ); + if (!agent) return; + await recording.onDocumentReady?.({ + tabId: agent.tabId, + documentId: agent.documentId, + browserFrameId: agent.browserFrameId, + producerId: agent.producerId, + }); + })(); + } + }; + readonly #onConnect = (port: chrome.runtime.Port): void => { if (port.name !== RECORD_FRAME_PORT) return; const address = senderAddress(port.sender ?? {}); @@ -309,6 +358,26 @@ export class RecordFrameCoordinator { }); } + async #startFrame( + recording: ArmedRecording, + tabId: number, + frame: BrowserFrame, + ): Promise { + const message: RecordFrameStartMessage = { + type: RECORD_FRAME_START, + requestId: recording.requestId, + startedAtMs: recording.startedAtMs, + }; + try { + const response = await this.#deps.sendToDocument(tabId, message, { + ...(frame.documentId ? { documentId: frame.documentId } : { frameId: frame.frameId }), + }); + return isStarted(response); + } catch { + return false; + } + } + #cancelAgents(recording: ArmedRecording): void { for (const agent of recording.agents.values()) { try { diff --git a/apps/extension/src/lib/recording/recording-runtime.ts b/apps/extension/src/lib/recording/recording-runtime.ts index d9a396e..5542373 100644 --- a/apps/extension/src/lib/recording/recording-runtime.ts +++ b/apps/extension/src/lib/recording/recording-runtime.ts @@ -1,6 +1,6 @@ import type { CdpRunner, ChromeTabsApi } from "@/tools/shared"; import type { StopReason, TraceV3 } from "@/transport/types"; -import type { DocumentSettleScope } from "./document-settle"; +import { type DocumentSettleScope, waitForDocumentSettled } from "./document-settle"; import type { RegisteredObservation } from "./observation-capture"; import { RecordingObservationSession } from "./observation-session"; import { inferMissingPostStates, SettleController } from "./settle-controller"; @@ -12,6 +12,8 @@ interface TabRecordingContext { session: RecordingObservationSession; settle: SettleController; pendingCapture: PendingCapture | null; + frameRefresh: Promise; + frameRefreshAbort: AbortController | null; } interface PendingCapture { @@ -58,6 +60,8 @@ export class RecordingObservationRuntime { tabId, }), pendingCapture: null, + frameRefresh: Promise.resolve(), + frameRefreshAbort: null, }; this.#contexts.set(tabId, context); return context; @@ -86,6 +90,41 @@ export class RecordingObservationRuntime { await this.#capture(tabId); } + /** Refresh the safe observation after a late child Document becomes usable. */ + async refreshDocument(tabId: number, producerId: string): Promise { + const context = this.#context(tabId); + context.frameRefreshAbort?.abort(); + const abort = new AbortController(); + context.frameRefreshAbort = abort; + const previous = context.frameRefresh; + const refresh = (async () => { + await previous; + if (abort.signal.aborted) return; + if (context.pendingCapture) await Promise.allSettled([context.pendingCapture.promise]); + if (abort.signal.aborted) return; + + // The first capture discovers the safe producer -> CDP Document scope. + await this.#capture(tabId); + if (abort.signal.aborted) return; + const scope = context.session.cursor.lastSettled?.index.documentScope(producerId); + if (scope) { + await waitForDocumentSettled(this.#cdp, scope, { signal: abort.signal }); + } + if (abort.signal.aborted) return; + + // Capture again after the child SPA is quiet so the next action can be + // matched against refs that actually exist in its pre-action state. + await this.#capture(tabId); + })(); + const tracked = refresh.catch(() => {}); + context.frameRefresh = tracked; + try { + await refresh; + } finally { + if (context.frameRefresh === tracked) context.frameRefreshAbort = null; + } + } + async captureTabTransition( fromTabId: number, toTabId: number, @@ -177,6 +216,7 @@ export class RecordingObservationRuntime { } async #flushContext(context: TabRecordingContext): Promise { + await context.frameRefresh; if (context.pendingCapture) await Promise.allSettled([context.pendingCapture.promise]); await context.settle.flushRedirects(); await context.settle.flush(); @@ -206,6 +246,7 @@ export class RecordingObservationRuntime { cancel(): void { for (const context of this.#contexts.values()) { + context.frameRefreshAbort?.abort(); context.pendingCapture?.abort.abort(); context.settle.cancel(); } diff --git a/apps/extension/src/tools/record.ts b/apps/extension/src/tools/record.ts index 3143046..294788d 100644 --- a/apps/extension/src/tools/record.ts +++ b/apps/extension/src/tools/record.ts @@ -888,7 +888,16 @@ export async function handleRecordStart( actionQueue: Promise.resolve(), lastStepSequenceByProducer: new Map(), }); - deps.frameCoordinator?.begin(requestId, startedAtMs); + deps.frameCoordinator?.begin(requestId, startedAtMs, target.tabId, async (scope) => { + const recording = recordings.get(params.session_id); + if (!recording || recording.requestId !== requestId || recording.settled) return; + try { + await recording.observation?.refreshDocument(scope.tabId, scope.producerId); + } catch { + // The normal action-time capture remains available if a child Document + // is replaced while its readiness refresh is in flight. + } + }); // Observe navigations for the whole recording lifetime; attach before // optional navigate so the destination load can rearm capture. ensureBrowserObservationListeners(deps);