Skip to content

Commit e2004c9

Browse files
committed
Fix OOPIF invalidation fallback and add frame collection docs
1 parent 9d41e64 commit e2004c9

3 files changed

Lines changed: 57 additions & 3 deletions

File tree

packages/agent/src/translator/browser-frame-collection.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,21 @@
11
import { CdpProtocolError } from "./cdp";
22

3+
/**
4+
* Wraps unexpected iframe collection failures with context about which frame
5+
* and collection stage failed.
6+
*/
37
export class FrameCollectionError extends Error {
48
constructor(message: string, cause: unknown) {
59
super(message, { cause });
610
this.name = "FrameCollectionError";
711
}
812
}
913

14+
/**
15+
* True when a CDP error is one of the known transient "frame disappeared"
16+
* variants that should mark the frame as incomplete instead of failing the
17+
* whole observation.
18+
*/
1019
export function isExpectedFrameCollectionError(
1120
error: unknown,
1221
method: "DOM.describeNode" | "Accessibility.getFullAXTree",
@@ -21,6 +30,10 @@ export function isExpectedFrameCollectionError(
2130
);
2231
}
2332

33+
/**
34+
* Build a contextual {@link FrameCollectionError} for an unexpected iframe
35+
* collection failure.
36+
*/
2437
export function frameCollectionError(
2538
backendNodeId: number,
2639
frameId: string | undefined,

packages/agent/src/translator/browser.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ export class BrowserExecutor {
142142
if (this.frameTargets.has(sessionTargetId)) {
143143
// The OOPIF tree and any same-process descendants fetched through
144144
// its session share the OOPIF generation key.
145-
const owner = this.frameOwners.get(sessionTargetId);
145+
const owner = this.ownerForFrameTarget(sessionTargetId);
146146
if (owner) {
147147
this.lifecycle.invalidateFrame(owner, sessionTargetId);
148148
if (frame.loaderId) this.lifecycle.recordDocument(sessionTargetId, owner, frame.loaderId);
@@ -168,7 +168,7 @@ export class BrowserExecutor {
168168
const sessionTargetId = this.targetsBySession.get(event.sessionId);
169169
if (!frameId || !sessionTargetId) return;
170170
if (this.frameTargets.has(sessionTargetId)) {
171-
const owner = this.frameOwners.get(sessionTargetId);
171+
const owner = this.ownerForFrameTarget(sessionTargetId);
172172
if (owner) this.lifecycle.invalidateFrame(owner, sessionTargetId);
173173
} else {
174174
this.lifecycle.removeFrame(sessionTargetId, frameId);
@@ -218,7 +218,7 @@ export class BrowserExecutor {
218218
this.targetsBySession.delete(sessionId);
219219
if (!targetId) return;
220220
if (this.frameTargets.has(targetId)) {
221-
const owner = this.frameOwners.get(targetId);
221+
const owner = this.ownerForFrameTarget(targetId);
222222
if (owner) this.lifecycle.removeFrame(owner, targetId);
223223
this.frameSessions.delete(targetId);
224224
this.frameOwners.delete(targetId);
@@ -967,6 +967,17 @@ export class BrowserExecutor {
967967
return entry;
968968
}
969969

970+
private ownerForFrameTarget(frameTargetId: string): string | undefined {
971+
const mapped = this.frameOwners.get(frameTargetId);
972+
if (mapped) return mapped;
973+
for (const entry of this.refs.values()) {
974+
if (entry.frameId !== frameTargetId) continue;
975+
this.frameOwners.set(frameTargetId, entry.targetId);
976+
return entry.targetId;
977+
}
978+
return undefined;
979+
}
980+
970981
private dropTarget(targetId: string): void {
971982
this.lifecycle.dropTarget(targetId);
972983
this.selfNavigations.delete(targetId);

packages/agent/test/translator-browser.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -783,6 +783,26 @@ describe("BrowserExecutor iframe stitching", () => {
783783
fake.setSessionTree("session-oop", OOPIF_CHILD);
784784
return fake;
785785
};
786+
const importOopifRefWithoutOwner = async () => {
787+
const source = setupOopif();
788+
const mint = new BrowserExecutor(source.cdp);
789+
await snapshotText(mint);
790+
const state = mint.exportRefState();
791+
792+
const fake = createFakeCdp(OOPIF_PAGE);
793+
fake.setIframeFrame(50, "FRAME-OOP");
794+
fake.setSessionTree("session-oop", OOPIF_CHILD);
795+
const executor = new BrowserExecutor(fake.cdp);
796+
executor.importRefState(state);
797+
// Simulate a frame session that attached without a parent session id, so
798+
// the target->owner mapping is absent until a later observation rebuilds it.
799+
fake.emit({
800+
method: "Target.attachedToTarget",
801+
params: { sessionId: "session-oop", targetInfo: { targetId: "FRAME-OOP", type: "iframe" } },
802+
});
803+
expect([...refsOf(executor).keys()].sort()).toEqual(["e1", "e2", "e3"]);
804+
return { executor, fake };
805+
};
786806

787807
it("resolves an OOPIF ref's node through the child session but dispatches input on the page session", async () => {
788808
const { cdp, sent } = setupOopif();
@@ -900,6 +920,16 @@ describe("BrowserExecutor iframe stitching", () => {
900920
expect(text).toContain('button "Pay" [e');
901921
});
902922

923+
it.each([
924+
{ label: "Page.frameNavigated", event: { method: "Page.frameNavigated", params: { frame: { id: "FRAME-OOP" } }, sessionId: "session-oop" } },
925+
{ label: "Page.frameDetached", event: { method: "Page.frameDetached", params: { frameId: "FRAME-OOP", reason: "swap" }, sessionId: "session-oop" } },
926+
{ label: "Target.detachedFromTarget", event: { method: "Target.detachedFromTarget", params: { sessionId: "session-oop" } } },
927+
] as const)("drops imported OOPIF refs when $label fires before owner mapping is known", async ({ event }) => {
928+
const { fake, executor } = await importOopifRefWithoutOwner();
929+
fake.emit(event);
930+
expect([...refsOf(executor).keys()].sort()).toEqual(["e1", "e2"]);
931+
});
932+
903933
it("invalidates and releases a same-process frame when it detaches and rotates", async () => {
904934
const root = [
905935
ax({ nodeId: "1", role: "RootWebArea", name: "Page", childIds: ["2"] }),

0 commit comments

Comments
 (0)