Skip to content

Commit 1e83cd4

Browse files
Ljy-0827Ljy-0827
committed
fix(vom): add tests and resolve problems
1 parent 1d09ef5 commit 1e83cd4

22 files changed

Lines changed: 609 additions & 154 deletions

‎apps/extension/src/browser-driver/__tests__/chromium-cdp.test.ts‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,63 @@ describe("ChromiumCdp", () => {
101101
);
102102
});
103103

104+
it("waits for recursively attached iframe targets without a depth limit", async () => {
105+
const { api, onEvent } = fakeApi();
106+
const depth = 6;
107+
const scheduled = new Set<string>();
108+
const nestedTree = (index: number): Record<string, unknown> => ({
109+
frame: {
110+
id: index === 0 ? "main" : `frame-${index}`,
111+
...(index > 0 ? { parentId: index === 1 ? "main" : `frame-${index - 1}` } : {}),
112+
},
113+
...(index < depth ? { childFrames: [nestedTree(index + 1)] } : {}),
114+
});
115+
(api.sendCommand as ReturnType<typeof vi.fn>).mockImplementation(
116+
async (target: CdpDebuggee, method: string, params?: { frameId?: string }) => {
117+
if (method === "Target.setAutoAttach") {
118+
const parentIndex = target.sessionId
119+
? Number(target.sessionId.replace("session-", ""))
120+
: 0;
121+
const nextIndex = parentIndex + 1;
122+
const key = `${target.sessionId ?? "root"}:${nextIndex}`;
123+
if (nextIndex <= depth && !scheduled.has(key)) {
124+
scheduled.add(key);
125+
setTimeout(() => {
126+
onEvent.fire(
127+
{ tabId: 4, ...(target.sessionId ? { sessionId: target.sessionId } : {}) },
128+
"Target.attachedToTarget",
129+
{
130+
sessionId: `session-${nextIndex}`,
131+
targetInfo: { type: "iframe" },
132+
},
133+
);
134+
}, 5);
135+
}
136+
return {};
137+
}
138+
if (method === "Page.getFrameTree") {
139+
if (!target.sessionId) return { frameTree: nestedTree(0) };
140+
const index = Number(target.sessionId.replace("session-", ""));
141+
return { frameTree: nestedTree(index) };
142+
}
143+
if (method === "DOM.getFrameOwner") {
144+
return { backendNodeId: Number(params?.frameId?.replace("frame-", "")) + 100 };
145+
}
146+
return {};
147+
},
148+
);
149+
const cdp = new ChromiumCdp(api);
150+
151+
const graph = await cdp.getFrameGraph(4);
152+
153+
expect(graph.frames).toHaveLength(depth + 1);
154+
for (let index = 1; index <= depth; index += 1) {
155+
expect(
156+
graph.frames.find((frame) => frame.frameId === `frame-${index}`)?.target.sessionId,
157+
).toBe(`session-${index}`);
158+
}
159+
});
160+
104161
it("coalesces concurrent attach calls for the same tab", async () => {
105162
const { api } = fakeApi();
106163
let releaseAttach!: () => void;

‎apps/extension/src/browser-driver/__tests__/frame-graph.test.ts‎

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, it } from "vitest";
2-
import { buildFrameGraph } from "../frame-graph";
2+
import { buildFrameGraph, type CdpFrameTreeNode, type CdpFrameTreeSource } from "../frame-graph";
33

44
describe("buildFrameGraph", () => {
55
it("keeps sibling frames distinct and routes nested OOPIFs to their child sessions", () => {
@@ -43,4 +43,94 @@ describe("buildFrameGraph", () => {
4343
target: { tabId: 4, sessionId: "nested-session" },
4444
});
4545
});
46+
47+
it("propagates each target boundary to its same-process descendants", () => {
48+
const sources: CdpFrameTreeSource[] = [
49+
{
50+
target: { tabId: 4 },
51+
tree: {
52+
frame: { id: "main" },
53+
childFrames: [
54+
{
55+
frame: { id: "oopif-a", parentId: "main" },
56+
childFrames: [
57+
{
58+
frame: { id: "same-process-b", parentId: "oopif-a" },
59+
childFrames: [
60+
{
61+
frame: { id: "oopif-c", parentId: "same-process-b" },
62+
childFrames: [{ frame: { id: "same-process-d", parentId: "oopif-c" } }],
63+
},
64+
],
65+
},
66+
],
67+
},
68+
],
69+
},
70+
},
71+
{
72+
target: { tabId: 4, sessionId: "session-a" },
73+
tree: {
74+
frame: { id: "oopif-a" },
75+
childFrames: [
76+
{
77+
frame: { id: "same-process-b", parentId: "oopif-a" },
78+
childFrames: [{ frame: { id: "oopif-c", parentId: "same-process-b" } }],
79+
},
80+
],
81+
},
82+
},
83+
{
84+
target: { tabId: 4, sessionId: "session-c" },
85+
tree: {
86+
frame: { id: "oopif-c" },
87+
childFrames: [{ frame: { id: "same-process-d", parentId: "oopif-c" } }],
88+
},
89+
},
90+
];
91+
92+
const graph = buildFrameGraph(sources);
93+
94+
expect(graph?.frames.map((frame) => [frame.frameId, frame.target.sessionId])).toEqual([
95+
["main", undefined],
96+
["oopif-a", "session-a"],
97+
["same-process-b", "session-a"],
98+
["oopif-c", "session-c"],
99+
["same-process-d", "session-c"],
100+
]);
101+
expect(graph?.frames.find((frame) => frame.frameId === "oopif-a")?.parentFrameId).toBe("main");
102+
expect(graph?.frames.find((frame) => frame.frameId === "oopif-c")?.parentFrameId).toBe(
103+
"same-process-b",
104+
);
105+
106+
const reordered = buildFrameGraph([sources[2], sources[1], sources[0]]);
107+
expect(reordered).toEqual(graph);
108+
});
109+
110+
it("walks deeply nested frame trees without consuming the JavaScript call stack", () => {
111+
const depth = 5_000;
112+
const root: CdpFrameTreeNode = { frame: { id: "frame-0" } };
113+
let parent = root;
114+
for (let index = 1; index <= depth; index += 1) {
115+
const child: CdpFrameTreeNode = {
116+
frame: { id: `frame-${index}`, parentId: `frame-${index - 1}` },
117+
};
118+
parent.childFrames = [child];
119+
parent = child;
120+
}
121+
122+
const graph = buildFrameGraph([{ target: { tabId: 4 }, tree: root }]);
123+
124+
expect(graph?.frames).toHaveLength(depth + 1);
125+
expect(graph?.frames.at(-1)?.frameId).toBe(`frame-${depth}`);
126+
});
127+
128+
it("does not loop when an in-memory frame tree contains an object cycle", () => {
129+
const root: CdpFrameTreeNode = { frame: { id: "main" } };
130+
root.childFrames = [root];
131+
132+
expect(buildFrameGraph([{ target: { tabId: 4 }, tree: root }])?.frames).toEqual([
133+
{ frameId: "main", target: { tabId: 4 } },
134+
]);
135+
});
46136
});

‎apps/extension/src/browser-driver/chromium-cdp.ts‎

Lines changed: 70 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,8 @@ const MAX_CONSOLE_STACK_FRAMES = 20;
113113
const MAX_NETWORK_BUFFER = 200;
114114
const MAX_NETWORK_FIELD_LENGTH = 4096;
115115
const MAX_NETWORK_REQUEST_META = 1024;
116+
const FRAME_DISCOVERY_TIMEOUT_MS = 1000;
117+
const FRAME_DISCOVERY_QUIET_MS = 20;
116118

117119
interface ParsedDialogOpening {
118120
type: JavaScriptDialogType;
@@ -134,6 +136,27 @@ interface NetworkRequestMeta {
134136
truncated: boolean;
135137
}
136138

139+
interface FrameDiscoveryState {
140+
sessions: Set<string>;
141+
pending: Set<Promise<void>>;
142+
generation: number;
143+
}
144+
145+
async function settleBeforeDeadline(promises: Promise<void>[], deadline: number): Promise<boolean> {
146+
if (promises.length === 0) return true;
147+
const remaining = deadline - Date.now();
148+
if (remaining <= 0) return false;
149+
let timer: ReturnType<typeof setTimeout> | undefined;
150+
const timedOut = await Promise.race([
151+
Promise.allSettled(promises).then(() => false),
152+
new Promise<boolean>((resolve) => {
153+
timer = setTimeout(() => resolve(true), remaining);
154+
}),
155+
]);
156+
if (timer) clearTimeout(timer);
157+
return !timedOut;
158+
}
159+
137160
/**
138161
* Wrapper around `chrome.debugger` that owns the "attach once per
139162
* tabId" cache and exposes typed `send<T>()`.
@@ -152,8 +175,7 @@ export class ChromiumCdp {
152175
private readonly networkSequences = new Map<number, number>();
153176
private readonly networkDomainsEnabledTabs = new Set<number>();
154177
private readonly networkRequestMeta = new Map<number, Map<string, NetworkRequestMeta>>();
155-
private readonly frameSessions = new Map<number, Set<string>>();
156-
private readonly frameAttachTasks = new Map<number, Set<Promise<void>>>();
178+
private readonly frameDiscovery = new Map<number, FrameDiscoveryState>();
157179
private detachSubscription: { dispose(): void } | null = null;
158180
private dialogSubscription: { dispose(): void } | null = null;
159181
private consoleSubscription: { dispose(): void } | null = null;
@@ -256,7 +278,7 @@ export class ChromiumCdp {
256278
);
257279
if (root.frameTree) sources.push({ target: { tabId }, tree: root.frameTree });
258280

259-
const sessions = [...(this.frameSessions.get(tabId) ?? [])];
281+
const sessions = [...(this.frameDiscovery.get(tabId)?.sessions ?? [])];
260282
const childTrees = await Promise.all(
261283
sessions.map(async (sessionId): Promise<CdpFrameTreeSource | null> => {
262284
const target = { tabId, sessionId };
@@ -456,8 +478,7 @@ export class ChromiumCdp {
456478
this.networkSequences.clear();
457479
this.networkDomainsEnabledTabs.clear();
458480
this.networkRequestMeta.clear();
459-
this.frameSessions.clear();
460-
this.frameAttachTasks.clear();
481+
this.frameDiscovery.clear();
461482
await Promise.all(
462483
tabs.map(async (tabId) => {
463484
try {
@@ -527,24 +548,23 @@ export class ChromiumCdp {
527548
if (!sessionId) return;
528549

529550
if (method === "Target.detachedFromTarget") {
530-
this.frameSessions.get(tabId)?.delete(sessionId);
551+
const state = this.frameDiscovery.get(tabId);
552+
if (state?.sessions.delete(sessionId)) state.generation += 1;
531553
return;
532554
}
533555
if (method !== "Target.attachedToTarget") return;
534556
const targetInfo = raw.targetInfo as { type?: string } | undefined;
535557
if (targetInfo?.type && targetInfo.type !== "iframe") return;
536558

537-
const sessions = this.frameSessions.get(tabId) ?? new Set<string>();
538-
sessions.add(sessionId);
539-
this.frameSessions.set(tabId, sessions);
559+
const state = this.frameDiscoveryState(tabId);
560+
if (state.sessions.has(sessionId)) return;
561+
state.sessions.add(sessionId);
562+
state.generation += 1;
540563

541564
const task = this.initializeFrameTarget({ tabId, sessionId });
542-
const tasks = this.frameAttachTasks.get(tabId) ?? new Set<Promise<void>>();
543-
tasks.add(task);
544-
this.frameAttachTasks.set(tabId, tasks);
565+
state.pending.add(task);
545566
void task.finally(() => {
546-
tasks.delete(task);
547-
if (tasks.size === 0) this.frameAttachTasks.delete(tabId);
567+
state.pending.delete(task);
548568
});
549569
};
550570
this.api.onEvent.addListener(listener);
@@ -557,23 +577,51 @@ export class ChromiumCdp {
557577
try {
558578
await this.enableFrameDiscovery(target);
559579
} catch (err) {
560-
this.frameSessions.get(target.tabId)?.delete(target.sessionId as string);
580+
const state = this.frameDiscovery.get(target.tabId);
581+
if (state?.sessions.delete(target.sessionId as string)) state.generation += 1;
561582
console.debug("[bsk cdp] child frame target initialization failed", { target, err });
562583
}
563584
}
564585

565586
private async drainFrameAttachTasks(tabId: number): Promise<void> {
566-
for (let round = 0; round < 4; round += 1) {
567-
await Promise.resolve();
568-
const tasks = [...(this.frameAttachTasks.get(tabId) ?? [])];
569-
if (tasks.length === 0) return;
570-
await Promise.allSettled(tasks);
587+
const deadline = Date.now() + FRAME_DISCOVERY_TIMEOUT_MS;
588+
while (Date.now() < deadline) {
589+
const state = this.frameDiscovery.get(tabId);
590+
const generation = state?.generation ?? 0;
591+
if (state?.pending.size && !(await settleBeforeDeadline([...state.pending], deadline))) break;
592+
593+
// Target.setAutoAttach may enqueue the next attachedToTarget event after
594+
// its command promise settles. Wait for a short quiet window, then finish
595+
// only if neither the state object nor its generation changed.
596+
const quietTime = Math.min(FRAME_DISCOVERY_QUIET_MS, deadline - Date.now());
597+
if (quietTime <= 0) break;
598+
await new Promise<void>((resolve) => setTimeout(resolve, quietTime));
599+
const current = this.frameDiscovery.get(tabId);
600+
if (
601+
current === state &&
602+
(current?.generation ?? 0) === generation &&
603+
!current?.pending.size
604+
) {
605+
return;
606+
}
571607
}
608+
console.debug("[bsk cdp] frame discovery did not reach quiescence before timeout", { tabId });
609+
}
610+
611+
private frameDiscoveryState(tabId: number): FrameDiscoveryState {
612+
const existing = this.frameDiscovery.get(tabId);
613+
if (existing) return existing;
614+
const created: FrameDiscoveryState = {
615+
sessions: new Set(),
616+
pending: new Set(),
617+
generation: 0,
618+
};
619+
this.frameDiscovery.set(tabId, created);
620+
return created;
572621
}
573622

574623
private clearFrameState(tabId: number): void {
575-
this.frameSessions.delete(tabId);
576-
this.frameAttachTasks.delete(tabId);
624+
this.frameDiscovery.delete(tabId);
577625
}
578626

579627
private bindDialogHandler(): void {

0 commit comments

Comments
 (0)