fix(vom): surface incomplete iframe geometry in observations

This commit is contained in:
Ljy-0827
2026-09-09 13:20:21 +08:00
committed by GitHub
parent c39874591f
commit a28b9d55e7
9 changed files with 240 additions and 36 deletions
@@ -60,15 +60,16 @@ describe("measurement geometry context", () => {
width: 1000,
height: 1000,
});
if (projection.status !== "available") throw new Error("expected available projection");
const input = snapshotViewportRect([17, 23, 120, 40], source, { x: 0, y: 0 });
expect(projectSnapshotRect(input!, projection!)).toEqual({
expect(projectSnapshotRect(input!, projection.projection)).toEqual({
x: 73.75,
y: 641.25,
width: 150,
height: 50,
});
const scrolled = snapshotViewportRect([17, 23, 120, 40], source, { x: 0, y: 10 });
expect(projectSnapshotRect(scrolled!, projection!)?.y).toBe(628.75);
expect(projectSnapshotRect(scrolled!, projection.projection)?.y).toBe(628.75);
await context.snapshotProjection(source, 10, [], { width: 1000, height: 1000 });
expect(cdp.calls.mock.calls.filter(([, method]) => method === "DOM.resolveNode")).toHaveLength(
1,
@@ -118,7 +119,11 @@ describe("measurement geometry context", () => {
width: 100,
height: 100,
}),
).toBeNull();
).toEqual({
status: "unavailable",
source: { target, frameId: "child" },
ownerBackendNodeId: 10,
});
});
it("keeps ancestor clips in target coordinates instead of applying nested offsets twice", async () => {
@@ -129,12 +134,13 @@ describe("measurement geometry context", () => {
[rectPolygon({ x: 60, y: 620, w: 100, h: 100 })],
{ width: 1000, height: 1000 },
);
if (projection.status !== "available") throw new Error("expected available projection");
const input = snapshotViewportRect(
[0, 0, 300, 200],
{ target, frameId: "nested" },
{ x: 0, y: 0 },
);
expect(projectSnapshotRect(input!, projection!)).toEqual({
expect(projectSnapshotRect(input!, projection.projection)).toEqual({
x: 60,
y: 620,
width: 100,
@@ -3147,7 +3147,9 @@ describe("handleSnapshot", () => {
cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 0, pageY: 0 },
};
function makeFrameAwareDeps() {
function makeFrameAwareDeps(
ownerGeometry: "unavailable" | "available" | "clipped" = "unavailable",
) {
const strings = ["html", "body", "iframe", "button", "title", "Remote", "static", "auto"];
const i = (value: string) => strings.indexOf(value);
const styles = [i("static"), i("auto"), i("auto")];
@@ -3227,6 +3229,16 @@ describe("handleSnapshot", () => {
];
const send = vi.fn(async (_tabId: number, method: string) => {
if (method === "Page.getLayoutMetrics") return VP_METRICS;
if (ownerGeometry !== "unavailable") {
if (method === "DOM.getBoxModel") {
const x = ownerGeometry === "clipped" ? 10000 : 100;
return { model: { content: [x, 100, x + 400, 100, x + 400, 400, x, 400] } };
}
if (method === "DOM.resolveNode") return { object: { objectId: "owner" } };
if (method === "Runtime.callFunctionOn")
return { result: { value: { width: 400, height: 300 } } };
if (method === "Runtime.releaseObject") return {};
}
if (method === "DOMSnapshot.enable" || method === "Accessibility.enable") return {};
if (method === "DOMSnapshot.captureSnapshot") return snapshot;
if (method === "Accessibility.getFullAXTree") return { nodes: mainAx };
@@ -3467,6 +3479,43 @@ describe("handleSnapshot", () => {
expect(result.text).toContain("•••");
});
it.each([
undefined,
100,
0,
])("reports unavailable iframe geometry within the observation budget %s", async (maxTokens) => {
const result = await captureVomObservation(makeFrameAwareDeps().cdp, 4, "https://example.com", {
maxTokens,
});
expect(result.text.match(/@warning/g)).toHaveLength(1);
expect(result.text).toContain("iframe geometry incomplete");
expect(result.text.startsWith("@vom 1\n")).toBe(true);
if (maxTokens === undefined) {
expect(result.truncated).toBe(false);
expect(result.text).toContain("Frame action");
expect(result.matchNodes.find((node) => node.backendNodeId === 22)).toMatchObject({
rect: null,
localRect: { x: 20, y: 30, w: 120, h: 40 },
});
}
if (maxTokens === 100) expect(Math.ceil(result.text.length / 4)).toBeLessThanOrEqual(maxTokens);
});
it.each([
"available",
"clipped",
] as const)("does not report successful %s geometry as an observation failure", async (ownerGeometry) => {
const result = await captureVomObservation(
makeFrameAwareDeps(ownerGeometry).cdp,
4,
"https://example.com",
);
expect(result.text).not.toContain("@warning");
const node = result.matchNodes.find((item) => item.backendNodeId === 22);
if (ownerGeometry === "clipped") expect(node?.rect).toBeNull();
else expect(node?.rect).toEqual({ x: 120, y: 130, w: 120, h: 40 });
});
it("keeps frames and rendered refs on the same frame identity", async () => {
const deps = makeFrameAwareDeps();
@@ -18,6 +18,24 @@ export interface SnapshotProjection {
geometry: GeometryProjection;
}
/** A failed owner read is different from a successfully projected, clipped-out rect. */
export interface UnavailableFrameProjection {
status: "unavailable";
source: CoordinateOwner;
ownerBackendNodeId: number;
}
export type SnapshotProjectionResult =
| { status: "available"; projection: SnapshotProjection }
| UnavailableFrameProjection;
/** Descendants inherit the failed boundary without attempting another owner read. */
export type FrameProjectionIssue =
| UnavailableFrameProjection
| { status: "blocked"; source: CoordinateOwner; cause: UnavailableFrameProjection };
export type FrameProjectionState = SnapshotProjectionResult | FrameProjectionIssue;
/** DOMSnapshot bounds are document-relative CSS pixels, independent of raster DPR. */
export function snapshotViewportRect(
bounds: number[],
@@ -13,7 +13,7 @@ import {
type Size,
} from "../geometry";
import { type CdpRunner, sendToCdpTarget } from "../shared";
import type { CoordinateOwner, CssViewport, SnapshotProjection } from "./coordinate-types";
import type { CoordinateOwner, CssViewport, SnapshotProjectionResult } from "./coordinate-types";
export interface LayoutMetrics {
cssVisualViewport?: { zoom?: number };
@@ -149,30 +149,41 @@ export class GeometryContext {
ownerBackendNodeId: number,
ancestorClips: Polygon[],
viewport: Size,
): Promise<SnapshotProjection | null> {
): Promise<SnapshotProjectionResult> {
const key = `${cdpTargetKey(source.target)}:${ownerBackendNodeId}`;
let promise = this.snapshotOwners.get(key);
if (!promise) {
promise = this.snapshotOwner(source.target, ownerBackendNodeId);
this.snapshotOwners.set(key, promise);
}
const owner = await promise;
let owner: { quad: Quad; size: Size } | null;
try {
owner = await promise;
} catch (error) {
if (error && typeof error === "object" && "name" in error && error.name === "AbortError")
throw error;
console.debug("[bsk geometry] frame owner unavailable", error);
owner = null;
}
return owner
? {
source,
geometry: {
sourceClips: [],
edges: [
{
sourceViewport: owner.size,
destinationQuad: owner.quad,
destinationClips: ancestorClips,
},
],
topViewport: viewport,
status: "available",
projection: {
source,
geometry: {
sourceClips: [],
edges: [
{
sourceViewport: owner.size,
destinationQuad: owner.quad,
destinationClips: ancestorClips,
},
],
topViewport: viewport,
},
},
}
: null;
: { status: "unavailable", source, ownerBackendNodeId };
}
private async snapshotOwner(
+10 -1
View File
@@ -1133,12 +1133,21 @@ export async function captureVomObservation(
captured,
hoverProbes.surfaceProbes,
);
// Reserve space using the renderer's character-based token estimate. Like VOM
// headers, this integrity notice remains visible even under a tiny token budget.
const geometryNotice = captured.frameGeometryIssues?.length
? "@warning iframe geometry incomplete: some frame content has no top-level coordinates.\n"
: "";
const rendered = renderVom(decoratedScene, {
maxDepth: options.maxDepth,
maxTokens: options.maxTokens,
maxTokens:
!geometryNotice || options.maxTokens === undefined
? options.maxTokens
: Math.max(0, options.maxTokens - Math.ceil(geometryNotice.length / 4)),
redactValues: options.redactValues,
activeRegionPolicy: options.activeRegionPolicy,
});
if (geometryNotice) rendered.text += `\n${geometryNotice.trimEnd()}`;
throwIfAborted(options.signal, "observation");
return projectRecordSafeObservation({
rootFrameId: captured.rootFrameId ?? normalizedDocuments[0]?.frameId ?? "root",
@@ -1340,6 +1340,7 @@ describe("sibling frame measurement scheduling", () => {
expect(fixture.measured()).not.toContain(200);
for (const id of [105, 102, 101, 100]) pending.get(id)!();
const captured = await capture;
expect(captured.frameGeometryIssues).toEqual([]);
expect(fixture.peak()).toBe(4);
expect(fixture.active()).toBe(0);
expect(fixture.measured()).toEqual([100, 101, 102, 103, 104, 105, 200]);
@@ -1360,6 +1361,18 @@ describe("sibling frame measurement scheduling", () => {
throw new Error("owner replaced");
});
const captured = await captureViewModel(fixture.cdp, 4);
expect(captured.frameGeometryIssues).toEqual([
{
status: "unavailable",
source: { target: { tabId: 4 }, frameId: "frame-1" },
ownerBackendNodeId: 100,
},
{
status: "blocked",
source: { target: { tabId: 4 }, frameId: "frame-7" },
cause: captured.frameGeometryIssues?.[0],
},
]);
expect(fixture.measured()).not.toContain(200);
expect(captured.iframeNodes.get(200)?.[0]).toMatchObject({
tag: "body",
@@ -50,6 +50,68 @@ function childSnapshot(frameId: string, backendNodeId: number) {
}
describe("captureFrameData", () => {
it("retains nested owner failures when merging a captured OOPIF", async () => {
const owner = ownerNode(10, 50);
const captured: CapturedViewModel = {
nodes: [owner],
viewport: { width: 1000, height: 800 },
iframeNodes: new Map(),
frameNodes: new Map([["main", [owner]]]),
rootFrameId: "main",
excludedBackendNodeIds: new Set(),
};
const child = childSnapshot("child", 101);
const nested = childSnapshot("nested", 201);
const document = child.documents[0];
const snapshot = {
strings: child.strings,
documents: [
{
...document,
nodes: { ...document.nodes, contentDocumentIndex: { index: [1], value: [1] } },
},
nested.documents[0],
],
};
const reply = async (_target: unknown, method: string) => {
if (method === "Page.getLayoutMetrics")
return { cssLayoutViewport: { clientWidth: 300, clientHeight: 200 } };
if (method === "DOMSnapshot.captureSnapshot") return snapshot;
if (method === "DOM.getBoxModel")
return { model: { content: [50, 100, 350, 100, 350, 300, 50, 300] } };
if (method === "DOM.resolveNode") throw new Error("nested owner replaced");
return {};
};
const cdp: CdpRunner = {
send: vi.fn(reply) as CdpRunner["send"],
sendToTarget: vi.fn(reply) as NonNullable<CdpRunner["sendToTarget"]>,
getFrameGraph: vi.fn(async () => ({
rootFrameId: "main",
frames: [
{ frameId: "main", target: { tabId: 4 } },
{
frameId: "child",
parentFrameId: "main",
ownerBackendNodeId: 10,
target: { tabId: 4, sessionId: "child-session" },
},
],
})),
};
await captureFrameData(cdp, 4, captured);
expect(captured.frameGeometryIssues).toEqual([
{
status: "unavailable",
source: { target: { tabId: 4, sessionId: "child-session" }, frameId: "nested" },
ownerBackendNodeId: 101,
},
]);
expect(captured.frameNodes?.get("nested")?.[1]).toMatchObject({
rect: null,
localRect: { x: 10, y: 20, w: 100, h: 40 },
});
});
it("captures and positions multiple OOPIF documents missing from the root snapshot", async () => {
const leftOwner = ownerNode(10, 50);
const rightOwner = ownerNode(20, 500);
+48 -15
View File
@@ -9,8 +9,9 @@ import type { CdpTarget } from "@/browser-driver/frame-graph";
import { evaluateHoverTrigger } from "@/lib/hover-trigger-policy";
import { isOverlayHostNode, OVERLAY_HOST_SELECTOR } from "../../lib/overlay-bridge";
import {
type FrameProjectionIssue,
type FrameProjectionState,
projectSnapshotRect,
type SnapshotProjection,
snapshotViewportRect,
} from "../geometry/coordinate-types";
import { cssViewport, GeometryContext, type LayoutMetrics } from "../geometry/frame-context";
@@ -77,6 +78,8 @@ export interface CapturedViewModel {
/** Explicit DOMSnapshot frame ancestry; never inferred from backend node ids. */
frameParentIds?: Map<string, string>;
rootFrameId?: string;
/** Failed boundaries and descendants blocked by them; clipping is not a failure. */
frameGeometryIssues?: FrameProjectionIssue[];
/** Backend node ids belonging to the agent overlay host + its shadow subtree. */
excludedBackendNodeIds: Set<number>;
}
@@ -351,7 +354,7 @@ interface ParseDocumentResult {
interface FrameContext {
frameId?: string;
ownerFrameBackendNodeId: number | null;
projection: SnapshotProjection | null;
projection: FrameProjectionState;
target: CdpTarget;
scrollX: number;
scrollY: number;
@@ -896,7 +899,9 @@ function parseDocumentNodes(
localRect = { x, y, w, h };
}
const bounds =
input && context.projection ? projectSnapshotRect(input, context.projection) : null;
input && context.projection.status === "available"
? projectSnapshotRect(input, context.projection.projection)
: null;
rect = bounds ? { x: bounds.x, y: bounds.y, w: bounds.width, h: bounds.height } : null;
}
paintOrder = dl?.paintOrders?.[li] ?? 0;
@@ -964,6 +969,7 @@ function parseDocumentNodes(
}
interface ParseFrameDocumentsResult {
frameGeometryIssues: FrameProjectionIssue[];
iframeNodes: CapturedIframeNodes;
frameOwnerBackendNodeIds: Map<string, number>;
frameParentIds: Map<string, string>;
@@ -979,6 +985,7 @@ async function parseChildFrameDocuments(
signal?: AbortSignal,
visited = new Set<number>(),
): Promise<ParseFrameDocumentsResult> {
const frameGeometryIssues: FrameProjectionIssue[] = [];
const iframeNodes: CapturedIframeNodes = new Map();
const frameOwnerBackendNodeIds = new Map<string, number>();
const frameParentIds = new Map<string, string>();
@@ -986,7 +993,13 @@ async function parseChildFrameDocuments(
const parentDoc = documents[parentDocIndex];
const cdi = sparseIndexMap(parentDoc?.nodes?.contentDocumentIndex);
if (cdi.size === 0) {
return { iframeNodes, frameOwnerBackendNodeIds, frameParentIds, excludedBackendNodeIds };
return {
iframeNodes,
frameOwnerBackendNodeIds,
frameParentIds,
excludedBackendNodeIds,
frameGeometryIssues,
};
}
const parentBackendIds = parentDoc?.nodes?.backendNodeId ?? [];
@@ -995,17 +1008,26 @@ async function parseChildFrameDocuments(
const childDoc = documents[childDocIndex];
const iframeBackendId = parentBackendIds[nodeArrayIdx];
if (visited.has(childDocIndex) || !childDoc || iframeBackendId === undefined) return [];
const source = { target: parentContext.target, frameId: snapshotFrameId(childDoc, strings) };
const parent = parentContext.projection;
const projection: FrameProjectionState =
parent.status === "available"
? { status: "unavailable", source, ownerBackendNodeId: iframeBackendId }
: { status: "blocked", source, cause: parent.status === "blocked" ? parent.cause : parent };
return [
{
childDocIndex,
childDoc,
iframeBackendId,
source: { target: parentContext.target, frameId: snapshotFrameId(childDoc, strings) },
projection: null as SnapshotProjection | null,
source,
projection: projection as FrameProjectionState,
},
];
});
const parentProjection = parentContext.projection?.geometry;
const parentProjection =
parentContext.projection.status === "available"
? parentContext.projection.projection.geometry
: null;
if (parentProjection) {
// Measure only direct siblings. Recursion starts after these workers finish,
// so descendants never hold or recursively acquire a measurement slot.
@@ -1014,10 +1036,10 @@ async function parseChildFrameDocuments(
? [edge.destinationQuad, ...(edge.destinationClips ?? [])]
: parentProjection.sourceClips;
let cursor = 0;
let aborted: unknown;
let failure: unknown;
await Promise.all(
Array.from({ length: Math.min(4, children.length) }, async () => {
while (cursor < children.length && !signal?.aborted && !aborted) {
while (cursor < children.length && !signal?.aborted && !failure) {
const child = children[cursor++];
try {
// Owner quads are target-relative; do not apply the parent transform twice.
@@ -1028,20 +1050,20 @@ async function parseChildFrameDocuments(
parentProjection.topViewport,
);
} catch (error) {
if (isAbortError(error)) aborted = error;
else console.debug("[bsk capture] frame owner geometry unavailable", error);
failure ??= error;
}
}
}),
);
// Join all active reads, including object cleanup, before propagating abort.
if (aborted) throw aborted;
if (failure) throw failure;
}
throwIfAborted(signal);
// Completion order must not change document traversal or result insertion order.
for (const { childDocIndex, childDoc, iframeBackendId, source, projection } of children) {
throwIfAborted(signal);
if (projection.status !== "available") frameGeometryIssues.push(projection);
const childContext: FrameContext = {
frameId: source.frameId,
target: source.target,
@@ -1069,6 +1091,7 @@ async function parseChildFrameDocuments(
signal,
nextVisited,
);
frameGeometryIssues.push(...nested.frameGeometryIssues);
for (const [nestedFrameId, nestedNodes] of nested.iframeNodes) {
iframeNodes.set(nestedFrameId, nestedNodes);
}
@@ -1081,7 +1104,13 @@ async function parseChildFrameDocuments(
for (const id of nested.excludedBackendNodeIds) excludedBackendNodeIds.add(id);
}
return { iframeNodes, frameOwnerBackendNodeIds, frameParentIds, excludedBackendNodeIds };
return {
iframeNodes,
frameOwnerBackendNodeIds,
frameParentIds,
excludedBackendNodeIds,
frameGeometryIssues,
};
}
export async function captureViewModel(
@@ -1131,8 +1160,11 @@ export async function captureViewModel(
ownerFrameBackendNodeId: null,
target,
projection: {
source: { target, frameId: snapshotFrameId(doc0, strings) },
geometry: { sourceClips: [], edges: [], topViewport: viewport },
status: "available",
projection: {
source: { target, frameId: snapshotFrameId(doc0, strings) },
geometry: { sourceClips: [], edges: [], topViewport: viewport },
},
},
scrollX,
scrollY,
@@ -1174,6 +1206,7 @@ export async function captureViewModel(
viewport,
iframeNodes,
frameNodes,
frameGeometryIssues: frameParsed.frameGeometryIssues,
frameOwnerBackendNodeIds: frameParsed.frameOwnerBackendNodeIds,
frameParentIds: frameParsed.frameParentIds,
...(topContext.frameId ? { rootFrameId: topContext.frameId } : {}),
@@ -56,6 +56,9 @@ async function captureMissingFrameDocuments(
geometry,
target: frame.target,
});
if (child.frameGeometryIssues?.length) {
(captured.frameGeometryIssues ??= []).push(...child.frameGeometryIssues);
}
let projection: GeometryProjection | null = null;
try {
projection = await geometry.targetProjection(frame.frameId);