From dee634503b4de02368c2318b00681aaabb831287 Mon Sep 17 00:00:00 2001 From: stone-jin <1520006273@qq.com> Date: Tue, 24 Feb 2026 22:23:53 +0800 Subject: [PATCH] fix(browser): encapsulate targetId resolution logic after navigation Introduced a new function `resolveTargetIdAfterNavigate` to handle the resolution of the correct targetId after a navigation event that may trigger a renderer swap. This refactor improves code clarity and reuses the logic for determining the current targetId, ensuring that the correct tab is identified even when multiple tabs share the same URL. --- src/browser/routes/agent.snapshot.test.ts | 121 ++++++++++++++++++++++ src/browser/routes/agent.snapshot.ts | 66 +++++++----- 2 files changed, 161 insertions(+), 26 deletions(-) create mode 100644 src/browser/routes/agent.snapshot.test.ts diff --git a/src/browser/routes/agent.snapshot.test.ts b/src/browser/routes/agent.snapshot.test.ts new file mode 100644 index 00000000000..2e09e40b06c --- /dev/null +++ b/src/browser/routes/agent.snapshot.test.ts @@ -0,0 +1,121 @@ +import { describe, expect, it, vi } from "vitest"; +import { resolveTargetIdAfterNavigate } from "./agent.snapshot.js"; + +type Tab = { targetId: string; url: string }; + +function staticListTabs(tabs: Tab[]): () => Promise { + return async () => tabs; +} + +describe("resolveTargetIdAfterNavigate", () => { + it("returns original targetId when old target still exists (no swap)", async () => { + const result = await resolveTargetIdAfterNavigate({ + oldTargetId: "old-123", + navigatedUrl: "https://example.com", + listTabs: staticListTabs([ + { targetId: "old-123", url: "https://example.com" }, + { targetId: "other-456", url: "https://other.com" }, + ]), + }); + expect(result).toBe("old-123"); + }); + + it("resolves new targetId when old target is gone (renderer swap)", async () => { + const result = await resolveTargetIdAfterNavigate({ + oldTargetId: "old-123", + navigatedUrl: "https://example.com", + listTabs: staticListTabs([ + { targetId: "new-456", url: "https://example.com" }, + ]), + }); + expect(result).toBe("new-456"); + }); + + it("prefers non-stale targetId when multiple tabs share the URL", async () => { + const result = await resolveTargetIdAfterNavigate({ + oldTargetId: "old-123", + navigatedUrl: "https://example.com", + listTabs: staticListTabs([ + { targetId: "preexisting-000", url: "https://example.com" }, + { targetId: "fresh-777", url: "https://example.com" }, + ]), + }); + // Both differ from old targetId; the first non-stale match wins. + expect(result).toBe("preexisting-000"); + }); + + it("retries and resolves targetId when first listTabs has no URL match", async () => { + vi.useFakeTimers(); + let calls = 0; + + const result$ = resolveTargetIdAfterNavigate({ + oldTargetId: "old-123", + navigatedUrl: "https://delayed.com", + listTabs: async () => { + calls++; + if (calls === 1) { + return [{ targetId: "unrelated-1", url: "https://unrelated.com" }]; + } + return [{ targetId: "delayed-999", url: "https://delayed.com" }]; + }, + }); + + await vi.advanceTimersByTimeAsync(800); + const result = await result$; + + expect(result).toBe("delayed-999"); + expect(calls).toBe(2); + + vi.useRealTimers(); + }); + + it("falls back to original targetId when no match found after retry", async () => { + vi.useFakeTimers(); + + const result$ = resolveTargetIdAfterNavigate({ + oldTargetId: "old-123", + navigatedUrl: "https://no-match.com", + listTabs: staticListTabs([ + { targetId: "unrelated-1", url: "https://unrelated.com" }, + { targetId: "unrelated-2", url: "https://unrelated2.com" }, + ]), + }); + + await vi.advanceTimersByTimeAsync(800); + const result = await result$; + + expect(result).toBe("old-123"); + + vi.useRealTimers(); + }); + + it("falls back to single remaining tab when no URL match after retry", async () => { + vi.useFakeTimers(); + + const result$ = resolveTargetIdAfterNavigate({ + oldTargetId: "old-123", + navigatedUrl: "https://single-tab.com", + listTabs: staticListTabs([ + { targetId: "only-tab", url: "https://some-other.com" }, + ]), + }); + + await vi.advanceTimersByTimeAsync(800); + const result = await result$; + + expect(result).toBe("only-tab"); + + vi.useRealTimers(); + }); + + it("falls back to original targetId when listTabs throws", async () => { + const result = await resolveTargetIdAfterNavigate({ + oldTargetId: "old-123", + navigatedUrl: "https://error.com", + listTabs: async () => { + throw new Error("CDP connection lost"); + }, + }); + expect(result).toBe("old-123"); + }); +}); diff --git a/src/browser/routes/agent.snapshot.ts b/src/browser/routes/agent.snapshot.ts index ada92b32c9b..7739caa051e 100644 --- a/src/browser/routes/agent.snapshot.ts +++ b/src/browser/routes/agent.snapshot.ts @@ -48,6 +48,41 @@ async function saveBrowserMediaResponse(params: { }); } +/** Resolve the correct targetId after a navigation that may trigger a renderer swap. */ +export async function resolveTargetIdAfterNavigate(opts: { + oldTargetId: string; + navigatedUrl: string; + listTabs: () => Promise>; +}): Promise { + let currentTargetId = opts.oldTargetId; + try { + const refreshed = await opts.listTabs(); + if (!refreshed.some((t) => t.targetId === opts.oldTargetId)) { + // Renderer swap: old target gone, resolve the replacement. + // Prefer a URL match whose targetId differs from the old one + // to avoid picking a pre-existing tab when multiple share the URL. + const byUrl = refreshed.filter((t) => t.url === opts.navigatedUrl); + const replaced = byUrl.find((t) => t.targetId !== opts.oldTargetId) ?? byUrl[0]; + if (replaced) { + currentTargetId = replaced.targetId; + } else { + await new Promise((r) => setTimeout(r, 800)); + const retried = await opts.listTabs(); + const match = + retried.find((t) => t.url === opts.navigatedUrl && t.targetId !== opts.oldTargetId) ?? + retried.find((t) => t.url === opts.navigatedUrl) ?? + (retried.length === 1 ? retried[0] : null); + if (match) { + currentTargetId = match.targetId; + } + } + } + } catch { + // Best-effort: fall back to pre-navigation targetId + } + return currentTargetId; +} + export function registerBrowserAgentSnapshotRoutes( app: BrowserRouteRegistrar, ctx: BrowserRouteContext, @@ -72,32 +107,11 @@ export function registerBrowserAgentSnapshotRoutes( url, ...withBrowserNavigationPolicy(ctx.state().resolved.ssrfPolicy), }); - let currentTargetId = tab.targetId; - try { - const refreshed = await profileCtx.listTabs(); - if (!refreshed.some((t) => t.targetId === tab.targetId)) { - // Renderer swap: old target gone, resolve the replacement. - // Prefer a URL match whose targetId differs from the old one - // to avoid picking a pre-existing tab when multiple share the URL. - const byUrl = refreshed.filter((t) => t.url === result.url); - const replaced = byUrl.find((t) => t.targetId !== tab.targetId) ?? byUrl[0]; - if (replaced) { - currentTargetId = replaced.targetId; - } else { - await new Promise((r) => setTimeout(r, 800)); - const retried = await profileCtx.listTabs(); - const match = - retried.find((t) => t.url === result.url && t.targetId !== tab.targetId) ?? - retried.find((t) => t.url === result.url) ?? - (retried.length === 1 ? retried[0] : null); - if (match) { - currentTargetId = match.targetId; - } - } - } - } catch { - // Best-effort: fall back to pre-navigation targetId - } + const currentTargetId = await resolveTargetIdAfterNavigate({ + oldTargetId: tab.targetId, + navigatedUrl: result.url, + listTabs: () => profileCtx.listTabs(), + }); res.json({ ok: true, targetId: currentTargetId, ...result }); }, });