From ce5ab9bcf6280f7ab21115babf9c056cba794ea4 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 13 May 2026 23:15:12 +0000 Subject: [PATCH 01/14] webview: navigate/reload/goBack/goForward accept {waitUntil, timeout} MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit navigate() (and reload/goBack/goForward) previously settled only on Page.loadEventFired. A page that holds a subresource connection open (SSE, long-poll, a hung image) never fires `load`, so `await view.navigate()` hung forever. Now accepts Playwright-shaped options: await view.navigate(url, { waitUntil: "load" | "domcontentloaded", // default "load" timeout: 30000, // ms; 0 disables }); Chrome backend: Page.setLifecycleEventsEnabled is sent in the attach chain; Page.frameNavigated stashes the main frame id + loaderId; the Page.lifecycleEvent handler settles the Navigate slot when frameId/loaderId match and name == DOMContentLoaded (or load). Subframe events and the about:blank replay never match because m_frameId/m_loaderId are only set after the user URL commits. Timeout is a parent-side RunLoop::dispatchAfter timer. It captures a per-view navigation generation counter; settleSlot bumps the counter, so a late fire no-ops instead of rejecting the next navigation. No explicit cancel, no RefPtr lifetime dance. WebKit backend: parses the same options. `timeout` applies (same parent-side timer). WKNavigationDelegate has no DOMContentLoaded hook, so `domcontentloaded` degrades to `load` there — noted in the type docs. Tests: new webview-navigate-options.test.ts drives a mock CDP WebSocket server so the full lifecycleEvent path is exercised without a Chrome binary; webview-chrome.test.ts gets real-Chrome coverage (todo-gated where Chrome is absent). --- packages/bun-types/bun.d.ts | 42 ++- src/runtime/webview/ChromeBackend.cpp | 87 ++++- src/runtime/webview/JSWebView.cpp | 99 +++++- src/runtime/webview/JSWebView.h | 41 ++- src/runtime/webview/JSWebViewConstructor.cpp | 4 +- src/runtime/webview/JSWebViewPrototype.cpp | 74 ++++- test/js/bun/webview/webview-chrome.test.ts | 184 ++++++++++ .../webview/webview-navigate-options.test.ts | 314 ++++++++++++++++++ 8 files changed, 811 insertions(+), 34 deletions(-) create mode 100644 test/js/bun/webview/webview-navigate-options.test.ts diff --git a/packages/bun-types/bun.d.ts b/packages/bun-types/bun.d.ts index 37d9bc45fcf..1c7f4d26de6 100644 --- a/packages/bun-types/bun.d.ts +++ b/packages/bun-types/bun.d.ts @@ -8478,6 +8478,30 @@ declare module "bun" { modifiers?: Modifier[]; } + interface NavigateOptions { + /** + * When to consider the navigation finished: + * + * - `"load"` — wait for the window `load` event (all subresources + * finished). Matches Playwright's default. + * - `"domcontentloaded"` — wait for `DOMContentLoaded`. Use this for + * pages that hold a connection open (SSE, long-polling, a hung + * subresource) and so never fire `load`. + * + * With the Chrome backend this subscribes to CDP + * `Page.lifecycleEvent`. The WebKit backend has no separate + * DOMContentLoaded delegate hook, so `"domcontentloaded"` behaves + * like `"load"` there — use `timeout` to bound the wait instead. + * @default "load" + */ + waitUntil?: "load" | "domcontentloaded"; + /** + * Maximum time to wait in milliseconds. `0` disables the timeout. + * @default 30000 + */ + timeout?: number; + } + /** * Browser backend selection. * @@ -8704,16 +8728,22 @@ declare module "bun" { onNavigationFailed: ((error: Error) => void) | null; /** - * Navigate to a URL. Resolves when the main frame's load completes - * (WKNavigationDelegate `didFinishNavigation`). + * Navigate to a URL. Resolves when the navigation reaches the + * `options.waitUntil` milestone (default: the window `load` event), + * or rejects after `options.timeout` ms. * * @example * ```ts * await view.navigate("https://example.com"); * await view.navigate("data:text/html,

hello

"); + * + * // Page holds an SSE stream open — `load` never fires. + * await view.navigate("https://example.com/stream", { + * waitUntil: "domcontentloaded", + * }); * ``` */ - navigate(url: string): Promise; + navigate(url: string, options?: WebView.NavigateOptions): Promise; /** * Run a JavaScript expression in the page's main frame and return the @@ -8931,11 +8961,11 @@ declare module "bun" { resize(width: number, height: number): Promise; /** Navigate back in session history. */ - back(): Promise; + back(options?: WebView.NavigateOptions): Promise; /** Navigate forward in session history. */ - forward(): Promise; + forward(options?: WebView.NavigateOptions): Promise; /** Reload the current page. */ - reload(): Promise; + reload(options?: WebView.NavigateOptions): Promise; /** * Close the view and release its WebContent process. After close, diff --git a/src/runtime/webview/ChromeBackend.cpp b/src/runtime/webview/ChromeBackend.cpp index 59292fd6df1..f85af4e7b81 100644 --- a/src/runtime/webview/ChromeBackend.cpp +++ b/src/runtime/webview/ChromeBackend.cpp @@ -765,6 +765,18 @@ void Transport::handleResponse(uint32_t id, std::span result, std::s uint32_t rid = nextId(); send(0, Command(rid, "Runtime.enable"_s, sidSpan)); + // Page.setLifecycleEventsEnabled — fire-and-forget. Chrome then + // emits Page.lifecycleEvent {frameId, loaderId, name} for commit/ + // DOMContentLoaded/load/networkIdle. navigate({waitUntil: + // 'domcontentloaded'}) settles on that instead of loadEventFired, + // so pages that never fire `load` (SSE, long-poll, a hung + // subresource) don't hang the await. Enabling replays the current + // document's events, but m_frameId/m_loaderId are unset until the + // USER url's frameNavigated, so the about:blank replay never + // matches. + uint32_t lid = nextId(); + send(0, Command(lid, "Page.setLifecycleEventsEnabled"_s, sidSpan).boolean("enabled"_s, true)); + // Page.navigate with the url stashed by the first navigate() call. // The response confirms the navigation STARTED; Page.loadEventFired // confirms completion. We keep the pending entry alive for the @@ -1091,14 +1103,35 @@ void Transport::handleEvent(std::span method, std::span return; } + // Chained from lifecycleEvent/loadEventFired: Runtime.evaluate( + // "document.title") so view.title is populated when navigate() + // resolves — matches WKWebView's NavDone which packs url+title in + // one reply. One extra roundtrip (~1ms), but the user-visible + // guarantee (`await view.navigate(); view.title` works) is worth + // it. PageTitle's response handler is the settle point. + auto chainTitle = [&]() { + uint32_t tid = nextId(); + m_pending.add(tid, Pending { Method::PageTitle, PendingSlot::Navigate, view->m_viewId }); + send(tid, Command(tid, "Runtime.evaluate"_s, sidSpan(view->m_sessionId)).str("expression"_s, "document.title"_s).boolean("returnByValue"_s, true)); + }; + // Page.frameNavigated — commit. Update m_url and fire onNavigated. // Same timing as WKWebView's NavDone (didFinishNavigation): the URL is // now the new document, resources may still be loading. if (method.size() == 19 && memcmp(method.data(), "Page.frameNavigated", 19) == 0) { auto frame = jsonField(params, { "frame", 5 }); + // Subframe commits have frame.parentId set; only the main frame + // updates m_url and the lifecycle loaderId. (frameNavigated for + // subframes is rare without Page.setFrameTree, but an

main

`), + { waitUntil: "domcontentloaded" }, + ); + // If the subframe's DCL had settled us, #m wouldn't exist yet (the + // iframe is before it in the stream). Settling on the MAIN frame's + // DCL means the whole document is parsed. + expect(await view.evaluate("document.getElementById('m')?.textContent")).toBe("main"); +}); + +it("chrome: navigate() option validation", () => { + // Option parsing throws before any I/O. Needs a valid `this` so we + // construct a view (Chrome is already up from earlier tests); the + // throws happen in parseNavOptions before the slot check. + const v = new Bun.WebView({ backend: chrome, width: 100, height: 100 }); + try { + expect(() => v.navigate("about:blank", { waitUntil: "networkidle" as any })).toThrow( + /must be "load" or "domcontentloaded"/, + ); + expect(() => v.navigate("about:blank", { waitUntil: 42 as any })).toThrow(/waitUntil.*string/); + expect(() => v.navigate("about:blank", { timeout: -1 })).toThrow(/non-negative finite/); + expect(() => v.navigate("about:blank", { timeout: Infinity })).toThrow(/non-negative finite/); + // reload/goBack/goForward share parseNavOptions. + expect(() => v.reload({ waitUntil: "x" as any })).toThrow(/must be "load" or "domcontentloaded"/); + } finally { + v.close(); + } +}); + it("chrome: sequential navigates work", async () => { await using view = new Bun.WebView({ backend: chrome, width: 200, height: 200 }); diff --git a/test/js/bun/webview/webview-navigate-options.test.ts b/test/js/bun/webview/webview-navigate-options.test.ts new file mode 100644 index 00000000000..2b375ef80bd --- /dev/null +++ b/test/js/bun/webview/webview-navigate-options.test.ts @@ -0,0 +1,314 @@ +// navigate({ waitUntil, timeout }) against a MOCK CDP endpoint. +// +// The real-Chrome coverage lives in webview-chrome.test.ts (gated on a +// local Chrome binary, todo'd otherwise). This file exercises the same +// code paths WITHOUT a browser: a Bun.serve WebSocket handler speaks +// just enough CDP to drive the attach chain and emit Page.lifecycleEvent +// / Page.loadEventFired on demand. That makes the waitUntil + timeout +// logic testable on any CI lane, and makes the assertions exact (no +// Chrome timing variance). +// +// Separate file because CDP::Transport is a process singleton — the +// first `new Bun.WebView()` locks the backend mode (pipe vs. WebSocket) +// and the endpoint. Mixing a mock WS here with the spawned-Chrome tests +// in the same process would poison the other file. +// +// Each test runs in a SUBPROCESS for the same reason: one mock server +// per test, one fresh Transport singleton per test. + +import { expect, test } from "bun:test"; +import { bunEnv, bunExe } from "harness"; + +// --- Mock CDP server -------------------------------------------------------- +// Inlined into the -e script so no fixture file is needed. The mock +// understands exactly the attach-chain methods (Target.createTarget → +// Target.attachToTarget → Page.enable → Page.setLifecycleEventsEnabled → +// Runtime.enable → Page.navigate) plus the post-navigate Runtime.evaluate +// for document.title. Everything else gets an empty {} result. +// +// `behavior` controls what lifecycle events the mock emits after +// Page.navigate — this is what distinguishes the test cases: +// - "dcl-only": frameNavigated + lifecycleEvent(DOMContentLoaded). +// NEVER sends loadEventFired. navigate({waitUntil:'domcontentloaded'}) +// should resolve; default navigate() should time out. +// - "load": frameNavigated + loadEventFired. Default navigate resolves. +// - "silent": nothing. navigate() hangs until timeout. +// +// frameId "F" / loaderId "L" are the main frame; the mock ALSO emits a +// subframe lifecycleEvent (frameId "SUB", loaderId "SL") to prove the +// frameId/loaderId gate filters it out. +const mockCDP = ` +function startMockCDP(behavior) { + const sid = "SESS"; + const send = (ws, obj) => ws.send(JSON.stringify(obj)); + const ev = (ws, method, params) => send(ws, { method, params, sessionId: sid }); + + return Bun.serve({ + port: 0, + fetch(req, server) { + if (server.upgrade(req)) return; + return new Response("not ws", { status: 400 }); + }, + websocket: { + open() {}, + message(ws, raw) { + const msg = JSON.parse(String(raw)); + const reply = (result) => + send(ws, msg.sessionId + ? { id: msg.id, result, sessionId: msg.sessionId } + : { id: msg.id, result }); + + switch (msg.method) { + case "Target.createTarget": + return reply({ targetId: "T" }); + case "Target.attachToTarget": + return reply({ sessionId: sid }); + case "Page.enable": + case "Page.setLifecycleEventsEnabled": + case "Runtime.enable": + case "Target.closeTarget": + return reply({}); + case "Page.reload": + case "Page.navigate": { + reply({ frameId: "F", loaderId: "L" }); + // Subframe DCL FIRST — must be ignored by the frameId gate. + // If the handler matched on name alone, this would settle + // the navigate before the main document committed. + ev(ws, "Page.lifecycleEvent", { + frameId: "SUB", loaderId: "SL", name: "DOMContentLoaded", timestamp: 1, + }); + // Main-frame commit: sets m_frameId/m_loaderId. + ev(ws, "Page.frameNavigated", { + frame: { id: "F", loaderId: "L", url: msg.params?.url ?? "about:blank", + domainAndRegistry: "", securityOrigin: "null", mimeType: "text/html", + adFrameStatus: { adFrameType: "none" }, secureContextType: "Secure", + crossOriginIsolatedContextType: "NotIsolated", gatedAPIFeatures: [] }, + type: "Navigation", + }); + if (behavior === "dcl-only") { + ev(ws, "Page.lifecycleEvent", { + frameId: "F", loaderId: "L", name: "DOMContentLoaded", timestamp: 2, + }); + // No loadEventFired — the page "never finishes loading". + } else if (behavior === "load") { + ev(ws, "Page.lifecycleEvent", { + frameId: "F", loaderId: "L", name: "DOMContentLoaded", timestamp: 2, + }); + ev(ws, "Page.lifecycleEvent", { + frameId: "F", loaderId: "L", name: "load", timestamp: 3, + }); + ev(ws, "Page.loadEventFired", { timestamp: 3 }); + } + // "silent": nothing — navigate() has only the timeout to save it. + return; + } + case "Runtime.evaluate": + // document.title → PageTitle chain. The handler reads + // result.result.value. + return reply({ result: { type: "string", value: "mock-title" } }); + default: + return reply({}); + } + }, + }, + }); +} +`; + +async function run(behavior: "dcl-only" | "load" | "silent", body: string) { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + mockCDP + + ` +await using server = startMockCDP(${JSON.stringify(behavior)}); +const view = new Bun.WebView({ + backend: { type: "chrome", url: \`ws://127.0.0.1:\${server.port}/devtools/browser/mock\` }, + width: 100, height: 100, +}); +try { +${body} +} finally { + view.close(); +} +`, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([ + proc.stdout.text(), + proc.stderr.text(), + proc.exited, + ]); + return { stdout, stderr, exitCode }; +} + +// --- waitUntil: 'domcontentloaded' ----------------------------------------- + +test("navigate({waitUntil:'domcontentloaded'}) settles on Page.lifecycleEvent when load never fires", async () => { + // The mock emits frameNavigated + lifecycleEvent(DOMContentLoaded) for + // the main frame, and NEVER loadEventFired. Without waitUntil: + // 'domcontentloaded', navigate() would hang until the 30s timeout. + // With it, the lifecycleEvent handler matches frameId=="F" && + // loaderId=="L" && name=="DOMContentLoaded", chains a document.title + // fetch, and resolves. + // + // The mock ALSO sends a subframe DCL (frameId "SUB") BEFORE the main + // frame commits — the frameId gate must drop it. A naive name-only + // match would settle on the subframe event. + const { stdout, stderr, exitCode } = await run( + "dcl-only", + ` + await view.navigate("http://example/dcl", { waitUntil: "domcontentloaded", timeout: 10_000 }); + // PageTitle chain ran — Runtime.evaluate("document.title") → "mock-title". + console.log("title=" + view.title); + // m_loading tracks the REAL load state; loadEventFired never came. + console.log("loading=" + view.loading); + console.log("url=" + view.url); + `, + ); + expect(stderr).toBe(""); + expect(stdout.trim().split("\n")).toEqual([ + "title=mock-title", + "loading=true", + "url=http://example/dcl", + ]); + expect(exitCode).toBe(0); +}); + +test("navigate() default waitUntil:'load' settles on Page.loadEventFired", async () => { + // Backward compat: no options → waitUntil:'load' → loadEventFired + // settles. The lifecycleEvent(DOMContentLoaded) arrives first but is + // ignored because m_navWaitUntil == Load. + const { stdout, stderr, exitCode } = await run( + "load", + ` + await view.navigate("http://example/load"); + console.log("title=" + view.title + " loading=" + view.loading); + `, + ); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("title=mock-title loading=false"); + expect(exitCode).toBe(0); +}); + +test("reload({waitUntil:'domcontentloaded'}) settles on lifecycleEvent", async () => { + // reload() shares the Navigate slot and the same lifecycle path. + // First navigate with 'load' (mock emits loadEventFired), then + // reload with 'domcontentloaded' against a dcl-only behavior — we + // switch the mock mid-script by emitting the reload as Page.reload + // which the mock handles identically to Page.navigate. + const { stdout, stderr, exitCode } = await run( + "dcl-only", + ` + await view.navigate("http://example/a", { waitUntil: "domcontentloaded" }); + await view.reload({ waitUntil: "domcontentloaded", timeout: 10_000 }); + console.log("ok title=" + view.title); + `, + ); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("ok title=mock-title"); + expect(exitCode).toBe(0); +}); + +// --- timeout --------------------------------------------------------------- + +test("navigate({timeout}) rejects when no lifecycle event arrives", async () => { + // "silent" mock: Page.navigate reply + frameNavigated, then nothing. + // Actually the "silent" arm skips frameNavigated too — navigate() + // has only the parent-side dispatchAfter timer to save it. + const { stdout, stderr, exitCode } = await run( + "silent", + ` + const t0 = performance.now(); + try { + await view.navigate("http://example/hang", { timeout: 300 }); + console.log("FAIL: resolved"); + } catch (e) { + const elapsed = performance.now() - t0; + // Fired after ~300ms (WTFTimer via Bun's event loop). Loose + // lower bound; upper bound generous for slow CI. + console.log("rejected=" + /Navigation timeout of 300ms exceeded/.test(e.message) + + " elapsed>=250=" + (elapsed >= 250)); + } + // Slot is clear — a second navigate with timeout:0 would hang + // forever on this mock, so give it a short one and assert it + // rejects with ITS OWN message (generation-counter isolated the + // previous timer). + try { + await view.navigate("http://example/hang2", { timeout: 200 }); + console.log("FAIL: second resolved"); + } catch (e) { + console.log("second=" + /Navigation timeout of 200ms exceeded/.test(e.message)); + } + `, + ); + expect(stderr).toBe(""); + expect(stdout.trim().split("\n")).toEqual([ + "rejected=true elapsed>=250=true", + "second=true", + ]); + expect(exitCode).toBe(0); +}); + +test("navigate({timeout}): stale timer does not reject a later navigation", async () => { + // First navigate settles on DCL at ~0ms with a 400ms timeout armed. + // Second navigate (silent mock would hang) starts immediately with + // timeout:0 (disabled). The first navigate's 400ms timer FIRES while + // the second navigate is pending — the generation-counter check must + // make it no-op instead of rejecting the second navigate's promise. + const { stdout, stderr, exitCode } = await run( + "dcl-only", + ` + await view.navigate("http://example/a", { waitUntil: "domcontentloaded", timeout: 400 }); + // Second navigate: never settles on this mock (no loadEventFired), + // no timeout. Race it against a 700ms sleep — if the stale 400ms + // timer from the first navigate wrongly rejected it, the promise + // would settle before 700ms. + const nav2 = view.navigate("http://example/b", { waitUntil: "load", timeout: 0 }); + let settled = "pending"; + nav2.then(() => settled = "resolved", e => settled = "rejected:" + e.message); + await Bun.sleep(700); + console.log("after-stale=" + settled); + `, + ); + expect(stderr).toBe(""); + // Still pending after the first navigate's stale 400ms timer fired. + expect(stdout.trim()).toBe("after-stale=pending"); + expect(exitCode).toBe(0); +}); + +// --- validation ------------------------------------------------------------ + +test("navigate() option validation throws before I/O", async () => { + // No CDP traffic needed — the throws happen in parseNavOptions + // before the slot check. Use the silent mock just to get a view. + const { stdout, stderr, exitCode } = await run( + "silent", + ` + const cases = [ + ["waitUntil networkidle", () => view.navigate("about:blank", { waitUntil: "networkidle" })], + ["waitUntil number", () => view.navigate("about:blank", { waitUntil: 42 })], + ["timeout negative", () => view.navigate("about:blank", { timeout: -1 })], + ["timeout Infinity", () => view.navigate("about:blank", { timeout: Infinity })], + ["reload waitUntil", () => view.reload({ waitUntil: "nope" })], + ]; + for (const [name, fn] of cases) { + try { fn(); console.log("FAIL", name); } + catch (e) { console.log(name + ": " + e.code); } + } + `, + ); + expect(stderr).toBe(""); + expect(stdout.trim().split("\n")).toEqual([ + "waitUntil networkidle: ERR_INVALID_ARG_VALUE", + "waitUntil number: ERR_INVALID_ARG_TYPE", + "timeout negative: ERR_INVALID_ARG_VALUE", + "timeout Infinity: ERR_INVALID_ARG_VALUE", + "reload waitUntil: ERR_INVALID_ARG_VALUE", + ]); + expect(exitCode).toBe(0); +}); From 17fdf0905f17cc896d37f5a55e18deface830cea Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Wed, 13 May 2026 23:17:50 +0000 Subject: [PATCH 02/14] [autofix.ci] apply automated fixes --- test/js/bun/webview/webview-chrome.test.ts | 27 +++++++++---------- .../webview/webview-navigate-options.test.ts | 17 +++--------- 2 files changed, 15 insertions(+), 29 deletions(-) diff --git a/test/js/bun/webview/webview-chrome.test.ts b/test/js/bun/webview/webview-chrome.test.ts index 531cca98986..36f92ab7308 100644 --- a/test/js/bun/webview/webview-chrome.test.ts +++ b/test/js/bun/webview/webview-chrome.test.ts @@ -809,10 +809,9 @@ it("chrome: navigate({waitUntil:'domcontentloaded'}) settles when `load` never f { headers: { "content-type": "image/png" } }, ); } - return new Response( - `dclready`, - { headers: { "content-type": "text/html" } }, - ); + return new Response(`dclready`, { + headers: { "content-type": "text/html" }, + }); }, }); @@ -870,9 +869,9 @@ it("chrome: navigate({timeout}) rejects when `load` never fires", async () => { // parent-side RunLoop::dispatchAfter timer rejects. No cancel: the // timer captures m_navGeneration and no-ops if a later navigate // (or this settle) bumped it. - await expect( - view.navigate(`http://127.0.0.1:${server.port}/`, { timeout: 500 }), - ).rejects.toThrow(/Navigation timeout of 500ms exceeded/); + await expect(view.navigate(`http://127.0.0.1:${server.port}/`, { timeout: 500 })).rejects.toThrow( + /Navigation timeout of 500ms exceeded/, + ); // The slot is clear after the timeout rejection — a fresh // navigate works and its own 30s default timer is independent // (generation bumped). @@ -907,10 +906,9 @@ it("chrome: reload({waitUntil:'domcontentloaded'}) settles on DCL", async () => ); } hits++; - return new Response( - ``, - { headers: { "content-type": "text/html" } }, - ); + return new Response(``, { + headers: { "content-type": "text/html" }, + }); }, }); @@ -932,10 +930,9 @@ it("chrome: navigate({waitUntil:'domcontentloaded'}) ignores subframe lifecycle // frameId gate (main frame only, matched against m_frameId from // frameNavigated) must filter it out. await using view = new Bun.WebView({ backend: chrome, width: 200, height: 200 }); - await view.navigate( - html(`

main

`), - { waitUntil: "domcontentloaded" }, - ); + await view.navigate(html(`

main

`), { + waitUntil: "domcontentloaded", + }); // If the subframe's DCL had settled us, #m wouldn't exist yet (the // iframe is before it in the stream). Settling on the MAIN frame's // DCL means the whole document is parsed. diff --git a/test/js/bun/webview/webview-navigate-options.test.ts b/test/js/bun/webview/webview-navigate-options.test.ts index 2b375ef80bd..89f26113fc7 100644 --- a/test/js/bun/webview/webview-navigate-options.test.ts +++ b/test/js/bun/webview/webview-navigate-options.test.ts @@ -138,11 +138,7 @@ ${body} stdout: "pipe", stderr: "pipe", }); - const [stdout, stderr, exitCode] = await Promise.all([ - proc.stdout.text(), - proc.stderr.text(), - proc.exited, - ]); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); return { stdout, stderr, exitCode }; } @@ -171,11 +167,7 @@ test("navigate({waitUntil:'domcontentloaded'}) settles on Page.lifecycleEvent wh `, ); expect(stderr).toBe(""); - expect(stdout.trim().split("\n")).toEqual([ - "title=mock-title", - "loading=true", - "url=http://example/dcl", - ]); + expect(stdout.trim().split("\n")).toEqual(["title=mock-title", "loading=true", "url=http://example/dcl"]); expect(exitCode).toBe(0); }); @@ -247,10 +239,7 @@ test("navigate({timeout}) rejects when no lifecycle event arrives", async () => `, ); expect(stderr).toBe(""); - expect(stdout.trim().split("\n")).toEqual([ - "rejected=true elapsed>=250=true", - "second=true", - ]); + expect(stdout.trim().split("\n")).toEqual(["rejected=true elapsed>=250=true", "second=true"]); expect(exitCode).toBe(0); }); From a72ad8c92f5a19917efe85b77555e43c1e868c56 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 13 May 2026 23:24:37 +0000 Subject: [PATCH 03/14] webview: clarify goBack/goForward/reload timeout comment for WebKit path The WebKit branch uses the Misc slot and WK_DISPATCH returns directly; armNavTimeout is intentionally not called there (it would no-op on an empty Navigate slot). Comment now says so instead of implying symmetry. --- src/runtime/webview/JSWebView.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/runtime/webview/JSWebView.cpp b/src/runtime/webview/JSWebView.cpp index 97f9c68d43c..89424026c30 100644 --- a/src/runtime/webview/JSWebView.cpp +++ b/src/runtime/webview/JSWebView.cpp @@ -318,10 +318,11 @@ JSPromise* JSWebView::resize(JSGlobalObject* g, uint32_t width, uint32_t height) } // Chrome's goBack/goForward/reload all settle via Page.lifecycleEvent / -// loadEventFired → same waitUntil + timeout handling as navigate(). -// WebKit's Ack is sync (Misc slot) for these, so waitUntil is moot there; -// the timeout still applies but will never fire in practice (Ack is -// immediate). Arming it anyway keeps the two backends symmetric. +// loadEventFired on the Navigate slot → same waitUntil + timeout handling +// as navigate(). WebKit's Op::GoBack/GoForward/Reload Ack immediately on +// the MISC slot (see navSlot in JSWebViewPrototype.cpp), so neither +// waitUntil nor timeout applies — WK_DISPATCH returns without arming; +// armNavTimeout's m_pendingNavigate guard would short-circuit anyway. JSPromise* JSWebView::goBack(JSGlobalObject* g, NavWaitUntil waitUntil, uint32_t timeoutMs) { m_navWaitUntil = waitUntil; From 53f12de09a58b70c236edb93c92011beb21ee251 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 13 May 2026 23:58:45 +0000 Subject: [PATCH 04/14] webview: gate stale load events, fix back/forward type names, scope generation bump Three review findings: 1) A navigate({waitUntil:'domcontentloaded'}) can settle before the page's `load` fires. If a second navigate() starts in that window, the first page's trailing Page.lifecycleEvent(load)/loadEventFired would pass the gate (the old loaderId was still cached) and chainTitle() would settle the SECOND navigate's promise with the FIRST page's title. Fix: clear m_loaderId at the start of every Chrome navigation (beginChromeNavigation helper); lifecycleEvent's loaderId check and loadEventFired's !m_loaderId.isEmpty() guard then drop the stale events. loadEventFired also gates on m_pendingNavigate so an uninitiated load can't enqueue a PageTitle that races a later navigate. New mock-CDP regression test. 2) bun.d.ts declared back()/forward() but the runtime prototype registers goBack()/goForward(). Pre-existing, but this PR touched those lines. Renamed to match runtime. 3) On WebKit, goBack/goForward/reload use the Misc slot and can run concurrently with a navigate() on the Navigate slot. The unconditional ++m_navGeneration before WK_DISPATCH was silently disarming that concurrent navigate's timeout. Moved the waitUntil/ generation/loaderId setup into the Chrome branch only (via beginChromeNavigation); WebKit leaves Navigate-slot state alone. --- packages/bun-types/bun.d.ts | 4 +- src/runtime/webview/ChromeBackend.cpp | 22 ++-- src/runtime/webview/JSWebView.cpp | 38 +++++-- .../webview/webview-navigate-options.test.ts | 102 +++++++++++++++--- 4 files changed, 130 insertions(+), 36 deletions(-) diff --git a/packages/bun-types/bun.d.ts b/packages/bun-types/bun.d.ts index 1c7f4d26de6..19cc4df29db 100644 --- a/packages/bun-types/bun.d.ts +++ b/packages/bun-types/bun.d.ts @@ -8961,9 +8961,9 @@ declare module "bun" { resize(width: number, height: number): Promise; /** Navigate back in session history. */ - back(options?: WebView.NavigateOptions): Promise; + goBack(options?: WebView.NavigateOptions): Promise; /** Navigate forward in session history. */ - forward(options?: WebView.NavigateOptions): Promise; + goForward(options?: WebView.NavigateOptions): Promise; /** Reload the current page. */ reload(options?: WebView.NavigateOptions): Promise; diff --git a/src/runtime/webview/ChromeBackend.cpp b/src/runtime/webview/ChromeBackend.cpp index f85af4e7b81..65f4d777381 100644 --- a/src/runtime/webview/ChromeBackend.cpp +++ b/src/runtime/webview/ChromeBackend.cpp @@ -1163,7 +1163,10 @@ void Transport::handleEvent(std::span method, std::span if (!(name.size() == 16 && memcmp(name.data(), "DOMContentLoaded", 16) == 0) && !(name.size() == 4 && memcmp(name.data(), "load", 4) == 0)) return; - if (view->m_frameId.isEmpty()) return; // frameNavigated hasn't committed yet + // beginChromeNavigation() cleared m_loaderId; frameNavigated for + // THIS navigation repopulates it. Empty → the event is for the + // previous document (or the setLifecycleEventsEnabled replay). + if (view->m_loaderId.isEmpty()) return; auto frameId = jsonString(jsonField(params, { "frameId", 7 })); auto loaderId = jsonString(jsonField(params, { "loaderId", 8 })); auto fUtf = view->m_frameId.utf8(); @@ -1179,15 +1182,18 @@ void Transport::handleEvent(std::span method, std::span // always flips here regardless of waitUntil — it tracks the REAL // load state, not the user's chosen milestone. // - // chainTitle() runs unconditionally so m_title tracks uninitiated - // navigations (window.location, meta refresh) too. If the Navigate - // slot already settled (domcontentloaded earlier), PageTitle's - // settleSlot sees an empty slot and no-ops — one extra ~1ms - // roundtrip for 'domcontentloaded' navigates, but m_title then - // reflects any mutation between DCL and load. + // chainTitle() is gated on m_loaderId: beginChromeNavigation() + // clears it and frameNavigated repopulates it. A stale + // loadEventFired for the PREVIOUS document (possible after a + // 'domcontentloaded' navigate settled and a new one started) + // arrives with m_loaderId empty → skip; otherwise PageTitle would + // settle the NEW navigate's promise with the OLD page's title + // before it committed. m_pendingNavigate must also be set so an + // idle loadEventFired (uninitiated window.location) doesn't enqueue + // a PageTitle that could race a later navigate() the same way. if (method.size() == 19 && memcmp(method.data(), "Page.loadEventFired", 19) == 0) { view->m_loading = false; - chainTitle(); + if (view->m_pendingNavigate && !view->m_loaderId.isEmpty()) chainTitle(); return; } diff --git a/src/runtime/webview/JSWebView.cpp b/src/runtime/webview/JSWebView.cpp index 89424026c30..2eadbbd5ffe 100644 --- a/src/runtime/webview/JSWebView.cpp +++ b/src/runtime/webview/JSWebView.cpp @@ -229,17 +229,34 @@ void JSWebView::armNavTimeout(JSGlobalObject* g, uint32_t timeoutMs) }); } +// Per-navigation Chrome setup. Clearing m_loaderId is the stale-event +// gate: a previous navigation's trailing lifecycleEvent(load) / +// loadEventFired can arrive AFTER this one starts (waitUntil: +// 'domcontentloaded' settles before `load`). With m_loaderId cleared, +// the lifecycleEvent loaderId check fails and loadEventFired's +// m_loaderId.isEmpty() guard skips chainTitle() until frameNavigated +// for THIS navigation repopulates it. +static inline void beginChromeNavigation(JSWebView* v, NavWaitUntil waitUntil) +{ + v->m_navWaitUntil = waitUntil; + ++v->m_navGeneration; + v->m_loaderId = WTF::String(); +} + JSPromise* JSWebView::navigate(JSGlobalObject* g, const WTF::String& url, NavWaitUntil waitUntil, uint32_t timeoutMs) { - m_navWaitUntil = waitUntil; - ++m_navGeneration; if (m_backend == WebViewBackend::Chrome) { + beginChromeNavigation(this, waitUntil); auto* p = CDP::Ops::navigate(g, this, url); if (m_pendingNavigate) m_loading = true; armNavTimeout(g, timeoutMs); return p; } #if OS(DARWIN) + // WebKit navigate() uses the Navigate slot too — waitUntil is moot + // (didFinishNavigation only), but the generation + timeout apply. + m_navWaitUntil = waitUntil; + ++m_navGeneration; auto* p = WK::Ops::navigate(g, this, url); armNavTimeout(g, timeoutMs); return p; @@ -320,14 +337,15 @@ JSPromise* JSWebView::resize(JSGlobalObject* g, uint32_t width, uint32_t height) // Chrome's goBack/goForward/reload all settle via Page.lifecycleEvent / // loadEventFired on the Navigate slot → same waitUntil + timeout handling // as navigate(). WebKit's Op::GoBack/GoForward/Reload Ack immediately on -// the MISC slot (see navSlot in JSWebViewPrototype.cpp), so neither -// waitUntil nor timeout applies — WK_DISPATCH returns without arming; -// armNavTimeout's m_pendingNavigate guard would short-circuit anyway. +// the MISC slot (see navSlot in JSWebViewPrototype.cpp), so they can run +// CONCURRENTLY with a pending navigate() on the Navigate slot — touching +// m_navWaitUntil / m_navGeneration / m_loaderId here would clobber that +// navigate's timeout and lifecycle gate. WK_DISPATCH returns the bare +// promise; the WebKit branch leaves Navigate-slot state alone. JSPromise* JSWebView::goBack(JSGlobalObject* g, NavWaitUntil waitUntil, uint32_t timeoutMs) { - m_navWaitUntil = waitUntil; - ++m_navGeneration; if (m_backend == WebViewBackend::Chrome) { + beginChromeNavigation(this, waitUntil); auto* p = CDP::Ops::goBack(g, this); armNavTimeout(g, timeoutMs); return p; @@ -337,9 +355,8 @@ JSPromise* JSWebView::goBack(JSGlobalObject* g, NavWaitUntil waitUntil, uint32_t JSPromise* JSWebView::goForward(JSGlobalObject* g, NavWaitUntil waitUntil, uint32_t timeoutMs) { - m_navWaitUntil = waitUntil; - ++m_navGeneration; if (m_backend == WebViewBackend::Chrome) { + beginChromeNavigation(this, waitUntil); auto* p = CDP::Ops::goForward(g, this); armNavTimeout(g, timeoutMs); return p; @@ -349,9 +366,8 @@ JSPromise* JSWebView::goForward(JSGlobalObject* g, NavWaitUntil waitUntil, uint3 JSPromise* JSWebView::reload(JSGlobalObject* g, NavWaitUntil waitUntil, uint32_t timeoutMs) { - m_navWaitUntil = waitUntil; - ++m_navGeneration; if (m_backend == WebViewBackend::Chrome) { + beginChromeNavigation(this, waitUntil); auto* p = CDP::Ops::reload(g, this); armNavTimeout(g, timeoutMs); return p; diff --git a/test/js/bun/webview/webview-navigate-options.test.ts b/test/js/bun/webview/webview-navigate-options.test.ts index 89f26113fc7..b9e61b54828 100644 --- a/test/js/bun/webview/webview-navigate-options.test.ts +++ b/test/js/bun/webview/webview-navigate-options.test.ts @@ -33,15 +33,30 @@ import { bunEnv, bunExe } from "harness"; // should resolve; default navigate() should time out. // - "load": frameNavigated + loadEventFired. Default navigate resolves. // - "silent": nothing. navigate() hangs until timeout. +// - "stale-load": first navigate → DCL only (settles). Second +// navigate → emit the FIRST nav's trailing lifecycleEvent(load) + +// loadEventFired BEFORE the second nav commits, then never emit +// anything for the second nav. Proves beginChromeNavigation()'s +// m_loaderId clear stops stale events from settling a later +// navigate. // -// frameId "F" / loaderId "L" are the main frame; the mock ALSO emits a -// subframe lifecycleEvent (frameId "SUB", loaderId "SL") to prove the -// frameId/loaderId gate filters it out. +// frameId "F" / loaderId "L<n>" are the main frame; the mock ALSO emits +// a subframe lifecycleEvent (frameId "SUB") to prove the frameId/ +// loaderId gate filters it out. const mockCDP = ` function startMockCDP(behavior) { const sid = "SESS"; + let navN = 0; const send = (ws, obj) => ws.send(JSON.stringify(obj)); const ev = (ws, method, params) => send(ws, { method, params, sessionId: sid }); + const frameNavigated = (ws, loaderId, url) => + ev(ws, "Page.frameNavigated", { + frame: { id: "F", loaderId, url, + domainAndRegistry: "", securityOrigin: "null", mimeType: "text/html", + adFrameStatus: { adFrameType: "none" }, secureContextType: "Secure", + crossOriginIsolatedContextType: "NotIsolated", gatedAPIFeatures: [] }, + type: "Navigation", + }); return Bun.serve({ port: 0, @@ -70,32 +85,53 @@ function startMockCDP(behavior) { return reply({}); case "Page.reload": case "Page.navigate": { - reply({ frameId: "F", loaderId: "L" }); + const n = ++navN; + const L = "L" + n; + const url = msg.params?.url ?? "about:blank"; + reply({ frameId: "F", loaderId: L }); // Subframe DCL FIRST — must be ignored by the frameId gate. // If the handler matched on name alone, this would settle // the navigate before the main document committed. ev(ws, "Page.lifecycleEvent", { frameId: "SUB", loaderId: "SL", name: "DOMContentLoaded", timestamp: 1, }); + + if (behavior === "stale-load") { + if (n === 1) { + // First nav: DCL-only so the user settles and can + // start a second navigate. + frameNavigated(ws, L, url); + ev(ws, "Page.lifecycleEvent", { + frameId: "F", loaderId: L, name: "DOMContentLoaded", timestamp: 2, + }); + } else { + // Second nav: emit the FIRST nav's trailing load + // events (loaderId L1) BEFORE this nav commits. With + // the stale-gate, m_loaderId is empty here so both + // the lifecycleEvent loaderId check and + // loadEventFired's isEmpty() guard drop them. + ev(ws, "Page.lifecycleEvent", { + frameId: "F", loaderId: "L1", name: "load", timestamp: 3, + }); + ev(ws, "Page.loadEventFired", { timestamp: 3 }); + // Never commit url2 — the test asserts it stays pending. + } + return; + } + // Main-frame commit: sets m_frameId/m_loaderId. - ev(ws, "Page.frameNavigated", { - frame: { id: "F", loaderId: "L", url: msg.params?.url ?? "about:blank", - domainAndRegistry: "", securityOrigin: "null", mimeType: "text/html", - adFrameStatus: { adFrameType: "none" }, secureContextType: "Secure", - crossOriginIsolatedContextType: "NotIsolated", gatedAPIFeatures: [] }, - type: "Navigation", - }); + frameNavigated(ws, L, url); if (behavior === "dcl-only") { ev(ws, "Page.lifecycleEvent", { - frameId: "F", loaderId: "L", name: "DOMContentLoaded", timestamp: 2, + frameId: "F", loaderId: L, name: "DOMContentLoaded", timestamp: 2, }); // No loadEventFired — the page "never finishes loading". } else if (behavior === "load") { ev(ws, "Page.lifecycleEvent", { - frameId: "F", loaderId: "L", name: "DOMContentLoaded", timestamp: 2, + frameId: "F", loaderId: L, name: "DOMContentLoaded", timestamp: 2, }); ev(ws, "Page.lifecycleEvent", { - frameId: "F", loaderId: "L", name: "load", timestamp: 3, + frameId: "F", loaderId: L, name: "load", timestamp: 3, }); ev(ws, "Page.loadEventFired", { timestamp: 3 }); } @@ -115,7 +151,7 @@ function startMockCDP(behavior) { } `; -async function run(behavior: "dcl-only" | "load" | "silent", body: string) { +async function run(behavior: "dcl-only" | "load" | "silent" | "stale-load", body: string) { await using proc = Bun.spawn({ cmd: [ bunExe(), @@ -206,6 +242,42 @@ test("reload({waitUntil:'domcontentloaded'}) settles on lifecycleEvent", async ( expect(exitCode).toBe(0); }); +test("stale loadEventFired from a prior 'domcontentloaded' navigate does not settle the next one", async () => { + // Regression: navigate(url1, {waitUntil:'domcontentloaded'}) settles + // before url1's window `load` fires. A second navigate() can then + // start, and url1's trailing lifecycleEvent(load)+loadEventFired + // arrive while nav2 is pending. Without the m_loaderId clear in + // beginChromeNavigation(), those stale events pass the gate (the + // old loaderId is still cached) and chainTitle() settles nav2's + // promise before its own document committed. + // + // The mock's "stale-load" arm emits exactly that: nav1 → DCL only; + // nav2 → stale lifecycleEvent(load,L1) + loadEventFired, then + // nothing for nav2. With the fix, nav2 stays pending. + const { stdout, stderr, exitCode } = await run( + "stale-load", + ` + await view.navigate("http://example/one", { waitUntil: "domcontentloaded" }); + // nav1 settled on DCL; its load hasn't fired. nav2 starts and + // clears m_loaderId. The mock then sends nav1's trailing load + // events — they must NOT settle nav2. + const nav2 = view.navigate("http://example/two", { waitUntil: "load", timeout: 0 }); + let settled = "pending"; + nav2.then(() => settled = "resolved", e => settled = "rejected:" + e.message); + await Bun.sleep(300); + // url should still be nav1's — nav2 never committed in the mock. + console.log("nav2=" + settled + " url=" + view.url); + // And with waitUntil:'domcontentloaded' on nav2 the stale + // lifecycleEvent(load, L1) must ALSO be rejected by the loaderId + // check (m_loaderId empty). Close the view to reject nav2 so the + // process exits; the test only cares it was still pending. + `, + ); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("nav2=pending url=http://example/one"); + expect(exitCode).toBe(0); +}); + // --- timeout --------------------------------------------------------------- test("navigate({timeout}) rejects when no lifecycle event arrives", async () => { From f94f9df5a2004d24e833421d0dd197b9a97d67be Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 14 May 2026 00:19:15 +0000 Subject: [PATCH 05/14] webview: address review nits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - uint32 ms is ~49.7 days, not ~24.8 (that's INT32_MAX ms). Comment-only. - Constructor's initial navigate (for {url}) drops the returned promise, so a 30s timeout would surface as unhandledrejection on slow pages. Pass timeoutMs=0 there instead — the user's first explicit await serializes behind it anyway. - goBack/goForward/reload: re-check m_closed after parseNavOptions(), same as navigate() — option getters can call user code that closes the view. --- src/runtime/webview/JSWebViewConstructor.cpp | 8 ++++++-- src/runtime/webview/JSWebViewPrototype.cpp | 8 +++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/runtime/webview/JSWebViewConstructor.cpp b/src/runtime/webview/JSWebViewConstructor.cpp index acd8bb6ff0f..57f4682f40c 100644 --- a/src/runtime/webview/JSWebViewConstructor.cpp +++ b/src/runtime/webview/JSWebViewConstructor.cpp @@ -353,7 +353,11 @@ JSC_DEFINE_HOST_FUNCTION(constructWebView, (JSGlobalObject * globalObject, CallF } view->m_consoleIsGlobal = consoleIsGlobal; if (consoleCallback) view->m_onConsole.set(vm, view, consoleCallback); - if (!initialUrl.isEmpty()) view->navigate(globalObject, initialUrl, NavWaitUntil::Load, 30000); + // timeoutMs=0: the returned promise is dropped (constructor + // can't hand it out), so a timeout rejection would surface as + // unhandledrejection on any page that loads slowly. The user's + // first explicit await serializes behind this navigate anyway. + if (!initialUrl.isEmpty()) view->navigate(globalObject, initialUrl, NavWaitUntil::Load, 0); return JSValue::encode(view); } @@ -377,7 +381,7 @@ JSC_DEFINE_HOST_FUNCTION(constructWebView, (JSGlobalObject * globalObject, CallF // first navigate via view.onNavigated or a second navigate that picks // up the pending state. Same semantics as `view.navigate(url)` right // after construction — just one line shorter. - if (!initialUrl.isEmpty()) view->navigate(globalObject, initialUrl, NavWaitUntil::Load, 30000); + if (!initialUrl.isEmpty()) view->navigate(globalObject, initialUrl, NavWaitUntil::Load, 0); return JSValue::encode(view); #endif } diff --git a/src/runtime/webview/JSWebViewPrototype.cpp b/src/runtime/webview/JSWebViewPrototype.cpp index 98c72a7b7ce..edd848ac958 100644 --- a/src/runtime/webview/JSWebViewPrototype.cpp +++ b/src/runtime/webview/JSWebViewPrototype.cpp @@ -250,7 +250,7 @@ static bool parseNavOptions(JSGlobalObject* g, ThrowScope& scope, JSValue opts, "must be a non-negative finite number"_s); return false; } - // Clamp to ~24.8 days — uint32 ms range. Anything larger is + // Clamp to uint32 ms range (~49.7 days). Anything larger is // effectively "no timeout" anyway. timeout = static_cast<uint32_t>(std::min(tn, static_cast<double>(std::numeric_limits<uint32_t>::max()))); } @@ -686,6 +686,8 @@ JSC_DEFINE_HOST_FUNCTION(jsWebViewProtoFuncBack, (JSGlobalObject * globalObject, NavWaitUntil waitUntil; uint32_t timeout; if (!parseNavOptions(globalObject, scope, callFrame->argument(0), waitUntil, timeout)) return {}; + if (thisObject->m_closed) + return Bun::throwError(globalObject, scope, ErrorCode::ERR_INVALID_STATE, "WebView is closed"_s); if (!checkSlot(globalObject, scope, navSlot(thisObject), "a navigation"_s)) return {}; return JSValue::encode(thisObject->goBack(globalObject, waitUntil, timeout)); } @@ -699,6 +701,8 @@ JSC_DEFINE_HOST_FUNCTION(jsWebViewProtoFuncForward, (JSGlobalObject * globalObje NavWaitUntil waitUntil; uint32_t timeout; if (!parseNavOptions(globalObject, scope, callFrame->argument(0), waitUntil, timeout)) return {}; + if (thisObject->m_closed) + return Bun::throwError(globalObject, scope, ErrorCode::ERR_INVALID_STATE, "WebView is closed"_s); if (!checkSlot(globalObject, scope, navSlot(thisObject), "a navigation"_s)) return {}; return JSValue::encode(thisObject->goForward(globalObject, waitUntil, timeout)); } @@ -712,6 +716,8 @@ JSC_DEFINE_HOST_FUNCTION(jsWebViewProtoFuncReload, (JSGlobalObject * globalObjec NavWaitUntil waitUntil; uint32_t timeout; if (!parseNavOptions(globalObject, scope, callFrame->argument(0), waitUntil, timeout)) return {}; + if (thisObject->m_closed) + return Bun::throwError(globalObject, scope, ErrorCode::ERR_INVALID_STATE, "WebView is closed"_s); if (!checkSlot(globalObject, scope, navSlot(thisObject), "a navigation"_s)) return {}; return JSValue::encode(thisObject->reload(globalObject, waitUntil, timeout)); } From 9031bc330564e1dc3781dc3fbb1f9a7b0dc272c3 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 14 May 2026 00:49:09 +0000 Subject: [PATCH 06/14] ci: retrigger From 5b3948770bfca77d153b9660e3e24b0992f438e7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 14 May 2026 01:02:35 +0000 Subject: [PATCH 07/14] webview: dedupe chainTitle per navigation, keep m_loading honest on timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit chainTitle() now clears m_loaderId after enqueuing the PageTitle fetch. On a fast page with waitUntil:'domcontentloaded', lifecycleEvent(DCL), lifecycleEvent(load) and loadEventFired can all arrive before the first PageTitle response — each was passing the gate and enqueuing a duplicate PageTitle, and a duplicate response could settle the NEXT navigate()'s promise with the previous page's title. With the clear, only the first trigger chains; subsequent ones for the same document hit the empty- m_loaderId gate. New mock-CDP regression test. Also: the timeout closure no longer clears m_loading — it tracks the REAL load state (flipped by loadEventFired), and a timeout is the user giving up on the wait, not the browser finishing. And the WebKit constructor comment no longer claims equivalence with view.navigate(url) (it passes timeout:0, navigate() defaults 30s). --- src/runtime/webview/ChromeBackend.cpp | 10 ++++++ src/runtime/webview/JSWebView.cpp | 5 ++- src/runtime/webview/JSWebViewConstructor.cpp | 5 +-- .../webview/webview-navigate-options.test.ts | 34 +++++++++++++++++++ 4 files changed, 51 insertions(+), 3 deletions(-) diff --git a/src/runtime/webview/ChromeBackend.cpp b/src/runtime/webview/ChromeBackend.cpp index 65f4d777381..17fafe68ae7 100644 --- a/src/runtime/webview/ChromeBackend.cpp +++ b/src/runtime/webview/ChromeBackend.cpp @@ -1109,7 +1109,17 @@ void Transport::handleEvent(std::span<const char> method, std::span<const char> // one reply. One extra roundtrip (~1ms), but the user-visible // guarantee (`await view.navigate(); view.title` works) is worth // it. PageTitle's response handler is the settle point. + // + // Clears m_loaderId: on a fast page, lifecycleEvent(DCL), + // lifecycleEvent(load) and loadEventFired can all arrive before + // the first PageTitle response — each would otherwise pass the + // gate and enqueue a duplicate PageTitle whose LATER response + // could settle a subsequent navigate()'s promise. Once the chain + // is started for THIS navigation, further triggers for the same + // document hit the empty-m_loaderId gate and drop. The next + // navigation's frameNavigated repopulates it. auto chainTitle = [&]() { + view->m_loaderId = WTF::String(); uint32_t tid = nextId(); m_pending.add(tid, Pending { Method::PageTitle, PendingSlot::Navigate, view->m_viewId }); send(tid, Command(tid, "Runtime.evaluate"_s, sidSpan(view->m_sessionId)).str("expression"_s, "document.title"_s).boolean("returnByValue"_s, true)); diff --git a/src/runtime/webview/JSWebView.cpp b/src/runtime/webview/JSWebView.cpp index 2eadbbd5ffe..62b11804d2e 100644 --- a/src/runtime/webview/JSWebView.cpp +++ b/src/runtime/webview/JSWebView.cpp @@ -223,7 +223,10 @@ void JSWebView::armNavTimeout(JSGlobalObject* g, uint32_t timeoutMs) [global, vid, gen, backend, timeoutMs]() { JSWebView* v = viewByIdForBackend(backend, vid); if (!v || v->m_navGeneration != gen || !v->m_pendingNavigate) return; - v->m_loading = false; + // m_loading left untouched — it tracks the REAL load state + // (flipped by Page.loadEventFired / NavDone). A timeout is + // the user giving up on the wait, not the browser + // finishing the load. settleSlot(global, v, v->m_pendingNavigate, false, createError(global, makeString("Navigation timeout of "_s, timeoutMs, "ms exceeded"_s))); }); diff --git a/src/runtime/webview/JSWebViewConstructor.cpp b/src/runtime/webview/JSWebViewConstructor.cpp index 57f4682f40c..884a4431b6d 100644 --- a/src/runtime/webview/JSWebViewConstructor.cpp +++ b/src/runtime/webview/JSWebViewConstructor.cpp @@ -379,8 +379,9 @@ JSC_DEFINE_HOST_FUNCTION(constructWebView, (JSGlobalObject * globalObject, CallF // (bad URL), the next op's checkSlot sees the slot cleared and proceeds; // the rejection is unobserved unless the user explicitly awaits the // first navigate via view.onNavigated or a second navigate that picks - // up the pending state. Same semantics as `view.navigate(url)` right - // after construction — just one line shorter. + // up the pending state. timeoutMs=0: the promise is dropped, so a + // timeout rejection would surface as unhandledrejection on a slow + // page — same rationale as the Chrome path above. if (!initialUrl.isEmpty()) view->navigate(globalObject, initialUrl, NavWaitUntil::Load, 0); return JSValue::encode(view); #endif diff --git a/test/js/bun/webview/webview-navigate-options.test.ts b/test/js/bun/webview/webview-navigate-options.test.ts index b9e61b54828..af8256ecf98 100644 --- a/test/js/bun/webview/webview-navigate-options.test.ts +++ b/test/js/bun/webview/webview-navigate-options.test.ts @@ -242,6 +242,40 @@ test("reload({waitUntil:'domcontentloaded'}) settles on lifecycleEvent", async ( expect(exitCode).toBe(0); }); +test("navigate({waitUntil:'domcontentloaded'}) on a fast page doesn't enqueue duplicate title fetches", async () => { + // "load" mock emits DCL + load + loadEventFired all before the + // first PageTitle response arrives. Without the m_loaderId clear + // in chainTitle(), each of the three passes the gate and enqueues + // its own PageTitle — and a duplicate response can settle the + // NEXT navigate's promise early. With the clear, only the first + // trigger chains; the rest hit m_loaderId.isEmpty() and drop. + // + // Two back-to-back DCL navigates: if duplicate PageTitle from + // nav1 leaked and settled nav2, nav2 would resolve with + // view.url == nav1's url (nav2's frameNavigated hadn't arrived + // yet at the time of the stale settle). + const { stdout, stderr, exitCode } = await run( + "load", + ` + await view.navigate("http://example/one", { waitUntil: "domcontentloaded" }); + await view.navigate("http://example/two", { waitUntil: "domcontentloaded" }); + // Each navigate committed — url reflects the LAST one. A leaked + // duplicate PageTitle from /one would have settled /two with + // url still /one. + console.log("url=" + view.url + " title=" + view.title); + // loadEventFired fired for /two (mock emits it in "load" mode) + // so m_loading flipped even though we settled on DCL. + console.log("loading=" + view.loading); + `, + ); + expect(stderr).toBe(""); + expect(stdout.trim().split("\n")).toEqual([ + "url=http://example/two title=mock-title", + "loading=false", + ]); + expect(exitCode).toBe(0); +}); + test("stale loadEventFired from a prior 'domcontentloaded' navigate does not settle the next one", async () => { // Regression: navigate(url1, {waitUntil:'domcontentloaded'}) settles // before url1's window `load` fires. A second navigate() can then From 34e1e74016eb0e56f5577d0a152fffd7776f7c37 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 14 May 2026 01:04:32 +0000 Subject: [PATCH 08/14] [autofix.ci] apply automated fixes --- test/js/bun/webview/webview-navigate-options.test.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/js/bun/webview/webview-navigate-options.test.ts b/test/js/bun/webview/webview-navigate-options.test.ts index af8256ecf98..326f9077ac0 100644 --- a/test/js/bun/webview/webview-navigate-options.test.ts +++ b/test/js/bun/webview/webview-navigate-options.test.ts @@ -269,10 +269,7 @@ test("navigate({waitUntil:'domcontentloaded'}) on a fast page doesn't enqueue du `, ); expect(stderr).toBe(""); - expect(stdout.trim().split("\n")).toEqual([ - "url=http://example/two title=mock-title", - "loading=false", - ]); + expect(stdout.trim().split("\n")).toEqual(["url=http://example/two title=mock-title", "loading=false"]); expect(exitCode).toBe(0); }); From ce32bd342184a1264aa032cf6c7d69a563a23693 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 14 May 2026 01:20:09 +0000 Subject: [PATCH 09/14] webview: m_navTitleChained flag, reload() sets m_loading, tidy test comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split the chainTitle() dedupe from the stale-event gate: m_navTitleChained (set by chainTitle, cleared by beginChromeNavigation) prevents duplicate PageTitle enqueues; m_loaderId stays populated until the NEXT navigation clears it, so Page.loadEventFired can distinguish "this document just finished loading" (m_loaderId non-empty → clear m_loading) from "a prior document's trailing load arrived after a new nav started" (m_loaderId empty → drop). The stale-load test now also asserts view.loading stays true. reload() now sets m_loading = true on Chrome (pre-existing omission in code this PR was already editing). goBack/goForward left as-is since the at-boundary case resolves without navigating. Two stale test comments corrected. --- src/runtime/webview/ChromeBackend.cpp | 47 ++++++++++--------- src/runtime/webview/JSWebView.cpp | 7 +++ src/runtime/webview/JSWebView.h | 9 ++++ .../webview/webview-navigate-options.test.ts | 23 +++++---- 4 files changed, 54 insertions(+), 32 deletions(-) diff --git a/src/runtime/webview/ChromeBackend.cpp b/src/runtime/webview/ChromeBackend.cpp index 17fafe68ae7..7ac5a97f253 100644 --- a/src/runtime/webview/ChromeBackend.cpp +++ b/src/runtime/webview/ChromeBackend.cpp @@ -1110,16 +1110,18 @@ void Transport::handleEvent(std::span<const char> method, std::span<const char> // guarantee (`await view.navigate(); view.title` works) is worth // it. PageTitle's response handler is the settle point. // - // Clears m_loaderId: on a fast page, lifecycleEvent(DCL), + // Sets m_navTitleChained: on a fast page, lifecycleEvent(DCL), // lifecycleEvent(load) and loadEventFired can all arrive before - // the first PageTitle response — each would otherwise pass the - // gate and enqueue a duplicate PageTitle whose LATER response - // could settle a subsequent navigate()'s promise. Once the chain - // is started for THIS navigation, further triggers for the same - // document hit the empty-m_loaderId gate and drop. The next - // navigation's frameNavigated repopulates it. + // the first PageTitle response — each would otherwise enqueue a + // duplicate PageTitle whose LATER response could settle a + // subsequent navigate()'s promise. After the first call, further + // triggers for the same document see the flag and drop. Cleared + // by beginChromeNavigation() for the next navigation. m_loaderId + // is left populated so loadEventFired can still tell THIS + // document's event from a stale one (m_loaderId empty = a new + // navigation started and hasn't committed yet). auto chainTitle = [&]() { - view->m_loaderId = WTF::String(); + view->m_navTitleChained = true; uint32_t tid = nextId(); m_pending.add(tid, Pending { Method::PageTitle, PendingSlot::Navigate, view->m_viewId }); send(tid, Command(tid, "Runtime.evaluate"_s, sidSpan(view->m_sessionId)).str("expression"_s, "document.title"_s).boolean("returnByValue"_s, true)); @@ -1183,27 +1185,28 @@ void Transport::handleEvent(std::span<const char> method, std::span<const char> auto lUtf = view->m_loaderId.utf8(); if (frameId.size() != fUtf.length() || memcmp(frameId.data(), fUtf.data(), frameId.size()) != 0) return; if (loaderId.size() != lUtf.length() || memcmp(loaderId.data(), lUtf.data(), loaderId.size()) != 0) return; - chainTitle(); + if (!view->m_navTitleChained) chainTitle(); return; } // Page.loadEventFired — window `load` fired on the main frame. This - // is the settle path for waitUntil:'load' (default). m_loading - // always flips here regardless of waitUntil — it tracks the REAL - // load state, not the user's chosen milestone. + // is the settle path for waitUntil:'load' (default). // - // chainTitle() is gated on m_loaderId: beginChromeNavigation() - // clears it and frameNavigated repopulates it. A stale - // loadEventFired for the PREVIOUS document (possible after a - // 'domcontentloaded' navigate settled and a new one started) - // arrives with m_loaderId empty → skip; otherwise PageTitle would - // settle the NEW navigate's promise with the OLD page's title - // before it committed. m_pendingNavigate must also be set so an - // idle loadEventFired (uninitiated window.location) doesn't enqueue - // a PageTitle that could race a later navigate() the same way. + // Stale detection: beginChromeNavigation() clears m_loaderId; this + // document's frameNavigated repopulates it. m_loaderId empty = + // a NEW navigation started and hasn't committed yet, so this + // loadEventFired is for the PREVIOUS document — don't clear + // m_loading (the new nav set it true) and don't chainTitle(). + // + // m_navTitleChained dedupes: a fast page's lifecycleEvent(DCL) + // already chained the title fetch; a second PageTitle could + // settle a LATER navigate. m_pendingNavigate must also be set so + // an idle loadEventFired (uninitiated window.location) doesn't + // enqueue a PageTitle that races a later navigate(). if (method.size() == 19 && memcmp(method.data(), "Page.loadEventFired", 19) == 0) { + if (view->m_loaderId.isEmpty()) return; // stale — prior document view->m_loading = false; - if (view->m_pendingNavigate && !view->m_loaderId.isEmpty()) chainTitle(); + if (view->m_pendingNavigate && !view->m_navTitleChained) chainTitle(); return; } diff --git a/src/runtime/webview/JSWebView.cpp b/src/runtime/webview/JSWebView.cpp index 62b11804d2e..5b2089d0696 100644 --- a/src/runtime/webview/JSWebView.cpp +++ b/src/runtime/webview/JSWebView.cpp @@ -244,6 +244,7 @@ static inline void beginChromeNavigation(JSWebView* v, NavWaitUntil waitUntil) v->m_navWaitUntil = waitUntil; ++v->m_navGeneration; v->m_loaderId = WTF::String(); + v->m_navTitleChained = false; } JSPromise* JSWebView::navigate(JSGlobalObject* g, const WTF::String& url, NavWaitUntil waitUntil, uint32_t timeoutMs) @@ -372,6 +373,12 @@ JSPromise* JSWebView::reload(JSGlobalObject* g, NavWaitUntil waitUntil, uint32_t if (m_backend == WebViewBackend::Chrome) { beginChromeNavigation(this, waitUntil); auto* p = CDP::Ops::reload(g, this); + // reload() always commits a navigation (no boundary no-op + // like goBack/goForward), so m_loading should reflect that. + // goBack/goForward don't set it here because the + // PageGetNavigationHistory response might resolve undefined + // (at boundary) without ever navigating. + if (m_pendingNavigate) m_loading = true; armNavTimeout(g, timeoutMs); return p; } diff --git a/src/runtime/webview/JSWebView.h b/src/runtime/webview/JSWebView.h index 389407bb444..84e8db6ee49 100644 --- a/src/runtime/webview/JSWebView.h +++ b/src/runtime/webview/JSWebView.h @@ -114,8 +114,17 @@ class JSWebView final : public WebCore::JSEventTarget { // updates both (parentId absent = main frame); Page.lifecycleEvent // only settles when its frameId/loaderId match — subframe events and // the about:blank replay from setLifecycleEventsEnabled don't match. + // m_loaderId is cleared by beginChromeNavigation() (so a PRIOR + // document's trailing lifecycle/loadEventFired can be identified as + // stale) and repopulated by the next frameNavigated. WTF::String m_frameId; WTF::String m_loaderId; + // Set once chainTitle() has enqueued the PageTitle fetch for THIS + // navigation. Further lifecycle/loadEventFired triggers for the + // same document drop instead of enqueuing duplicate PageTitle + // requests (whose responses could settle a LATER navigate). Cleared + // by beginChromeNavigation(). + bool m_navTitleChained = false; // clickSelector stash — the actionability eval chains into a // dispatchMouseEvent that needs these. WebViewHost has the same fields // on its side (m_selButton etc.) for the same chain. diff --git a/test/js/bun/webview/webview-navigate-options.test.ts b/test/js/bun/webview/webview-navigate-options.test.ts index 326f9077ac0..7261af8801b 100644 --- a/test/js/bun/webview/webview-navigate-options.test.ts +++ b/test/js/bun/webview/webview-navigate-options.test.ts @@ -224,11 +224,12 @@ test("navigate() default waitUntil:'load' settles on Page.loadEventFired", async }); test("reload({waitUntil:'domcontentloaded'}) settles on lifecycleEvent", async () => { - // reload() shares the Navigate slot and the same lifecycle path. - // First navigate with 'load' (mock emits loadEventFired), then - // reload with 'domcontentloaded' against a dcl-only behavior — we - // switch the mock mid-script by emitting the reload as Page.reload - // which the mock handles identically to Page.navigate. + // reload() shares the Navigate slot and the same lifecycle path as + // navigate(). "dcl-only" never emits loadEventFired, so both the + // initial navigate and the reload must settle via + // Page.lifecycleEvent(DOMContentLoaded). The mock handles + // Page.reload identically to Page.navigate (same event sequence, + // fresh loaderId). const { stdout, stderr, exitCode } = await run( "dcl-only", ` @@ -297,7 +298,9 @@ test("stale loadEventFired from a prior 'domcontentloaded' navigate does not set nav2.then(() => settled = "resolved", e => settled = "rejected:" + e.message); await Bun.sleep(300); // url should still be nav1's — nav2 never committed in the mock. - console.log("nav2=" + settled + " url=" + view.url); + // loading should still be TRUE — nav2 set it and nav1's stale + // loadEventFired (m_loaderId empty) must not clear it. + console.log("nav2=" + settled + " url=" + view.url + " loading=" + view.loading); // And with waitUntil:'domcontentloaded' on nav2 the stale // lifecycleEvent(load, L1) must ALSO be rejected by the loaderId // check (m_loaderId empty). Close the view to reject nav2 so the @@ -305,16 +308,16 @@ test("stale loadEventFired from a prior 'domcontentloaded' navigate does not set `, ); expect(stderr).toBe(""); - expect(stdout.trim()).toBe("nav2=pending url=http://example/one"); + expect(stdout.trim()).toBe("nav2=pending url=http://example/one loading=true"); expect(exitCode).toBe(0); }); // --- timeout --------------------------------------------------------------- test("navigate({timeout}) rejects when no lifecycle event arrives", async () => { - // "silent" mock: Page.navigate reply + frameNavigated, then nothing. - // Actually the "silent" arm skips frameNavigated too — navigate() - // has only the parent-side dispatchAfter timer to save it. + // "silent" mock: Page.navigate reply + frameNavigated, but no + // DCL/load/loadEventFired — navigate() has only the parent-side + // dispatchAfter timer to save it. const { stdout, stderr, exitCode } = await run( "silent", ` From 6b0287848731a067724582394ab85610bd94cb5f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 14 May 2026 02:15:09 +0000 Subject: [PATCH 10/14] webview: drop stale PageTitle responses, ceil sub-ms timeouts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A PageTitle request enqueued by chainTitle() can outlive its navigation: if the navigate's timeout fires (or the user retries from .catch()) before the Runtime.evaluate response arrives, the PageTitle handler's settle(Navigate) would hit the NEW navigate's promise. m_navTitleChained already distinguishes the two — chainTitle() set it, the new beginChromeNavigation() cleared it — so gate the response handler (both success and the generic CDP error path) on it. A stale PageTitle now drops instead of mis-settling. Also: ceil options.timeout before the uint32 cast so values in (0,1) arm a 1ms timer instead of truncating to the 0="disable" sentinel. And one more stale test comment from the 742d651 → b6ea944 refactor. --- src/runtime/webview/ChromeBackend.cpp | 8 ++++++++ src/runtime/webview/JSWebViewPrototype.cpp | 7 +++++-- test/js/bun/webview/webview-navigate-options.test.ts | 10 +++++----- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/runtime/webview/ChromeBackend.cpp b/src/runtime/webview/ChromeBackend.cpp index 7ac5a97f253..648828fd67a 100644 --- a/src/runtime/webview/ChromeBackend.cpp +++ b/src/runtime/webview/ChromeBackend.cpp @@ -710,6 +710,14 @@ void Transport::handleResponse(uint32_t id, std::span<const char> result, std::s JSWebView* view = viewFor(entry.viewId); if (!view) return; // user dropped both view and the awaited promise + // PageTitle is generation-gated via m_navTitleChained: chainTitle() + // set it; a subsequent beginChromeNavigation() (new navigate, or + // navigate retried from a timeout .catch()) cleared it. If a + // PageTitle response — success OR error ("Execution context was + // destroyed") — arrives after that, settling Navigate would hit + // the NEW navigation's promise. Drop instead. + if (entry.method == Method::PageTitle && !view->m_navTitleChained) return; + if (!error.empty()) { // {"code":-32000,"message":"..."} auto msgSlice = jsonString(jsonField(error, { "message", 7 })); diff --git a/src/runtime/webview/JSWebViewPrototype.cpp b/src/runtime/webview/JSWebViewPrototype.cpp index edd848ac958..7eece20f2cc 100644 --- a/src/runtime/webview/JSWebViewPrototype.cpp +++ b/src/runtime/webview/JSWebViewPrototype.cpp @@ -251,8 +251,11 @@ static bool parseNavOptions(JSGlobalObject* g, ThrowScope& scope, JSValue opts, return false; } // Clamp to uint32 ms range (~49.7 days). Anything larger is - // effectively "no timeout" anyway. - timeout = static_cast<uint32_t>(std::min(tn, static_cast<double>(std::numeric_limits<uint32_t>::max()))); + // effectively "no timeout" anyway. ceil so a positive sub-ms + // value (e.g. a computed `deadline - now` that lands at 0.3) + // still arms a 1ms timer instead of truncating to the 0 = + // "disable" sentinel. + timeout = static_cast<uint32_t>(std::min(std::ceil(tn), static_cast<double>(std::numeric_limits<uint32_t>::max()))); } return true; } diff --git a/test/js/bun/webview/webview-navigate-options.test.ts b/test/js/bun/webview/webview-navigate-options.test.ts index 7261af8801b..72839ce34b8 100644 --- a/test/js/bun/webview/webview-navigate-options.test.ts +++ b/test/js/bun/webview/webview-navigate-options.test.ts @@ -245,11 +245,11 @@ test("reload({waitUntil:'domcontentloaded'}) settles on lifecycleEvent", async ( test("navigate({waitUntil:'domcontentloaded'}) on a fast page doesn't enqueue duplicate title fetches", async () => { // "load" mock emits DCL + load + loadEventFired all before the - // first PageTitle response arrives. Without the m_loaderId clear - // in chainTitle(), each of the three passes the gate and enqueues - // its own PageTitle — and a duplicate response can settle the - // NEXT navigate's promise early. With the clear, only the first - // trigger chains; the rest hit m_loaderId.isEmpty() and drop. + // first PageTitle response arrives. Without the m_navTitleChained + // flag set by chainTitle(), each of the three would enqueue its + // own PageTitle — and a duplicate response can settle the NEXT + // navigate's promise early. With the flag, only the first trigger + // chains; the rest see m_navTitleChained and drop. // // Two back-to-back DCL navigates: if duplicate PageTitle from // nav1 leaked and settled nav2, nav2 would resolve with From 85dbb21dbaae731ffe935e9db0e2fcc658350d6f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 14 May 2026 03:09:18 +0000 Subject: [PATCH 11/14] webview: generation-gate all Navigate-slot CDP responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pending now carries navGen (the view's m_navGeneration at enqueue time, 0 = ungated). handleResponse drops a Navigate-slot response whose navGen doesn't match the current generation — covers every in-flight request armNavTimeout can orphan (PageNavigate errorText, PageGetNavigationHistory boundary, PageTitle, and the whole TargetCreateTarget → AttachToTarget → PageEnable chain) in one place, instead of the PageTitle-only m_navTitleChained shortcut. Chained responses carry entry.navGen forward. For a stale TargetCreateTarget response (first-navigate attach chain timed out and user retried), Chrome already created the tab; send Target.closeTarget(targetId) so it doesn't leak. --- src/runtime/webview/ChromeBackend.cpp | 44 +++++++++++++++++++-------- src/runtime/webview/ChromeBackend.h | 9 ++++++ 2 files changed, 40 insertions(+), 13 deletions(-) diff --git a/src/runtime/webview/ChromeBackend.cpp b/src/runtime/webview/ChromeBackend.cpp index 648828fd67a..f4015a4a6a7 100644 --- a/src/runtime/webview/ChromeBackend.cpp +++ b/src/runtime/webview/ChromeBackend.cpp @@ -710,13 +710,27 @@ void Transport::handleResponse(uint32_t id, std::span<const char> result, std::s JSWebView* view = viewFor(entry.viewId); if (!view) return; // user dropped both view and the awaited promise - // PageTitle is generation-gated via m_navTitleChained: chainTitle() - // set it; a subsequent beginChromeNavigation() (new navigate, or - // navigate retried from a timeout .catch()) cleared it. If a - // PageTitle response — success OR error ("Execution context was - // destroyed") — arrives after that, settling Navigate would hit - // the NEW navigation's promise. Drop instead. - if (entry.method == Method::PageTitle && !view->m_navTitleChained) return; + // Navigate-slot entries carry the view's m_navGeneration at + // enqueue time. armNavTimeout rejects the navigate (bumping gen) + // without pruning m_pending, so a response for the abandoned + // navigation can arrive after a .catch() retry refilled + // m_pendingNavigate. Mismatch → this response is stale; settling + // would resolve/reject the RETRY's promise (or, for the attach + // chain, create a second tab whose events route to this view). + // Covers PageTitle, PageNavigate errorText, PageGetNavigationHistory + // boundary, and TargetCreateTarget→…→PageEnable in one place. + if (entry.navGen && entry.navGen != view->m_navGeneration) { + // TargetCreateTarget already ran in Chrome by the time we + // see the response — close the orphaned tab so it doesn't + // leak for process lifetime. + if (entry.method == Method::TargetCreateTarget && error.empty()) { + auto tid = jsonString(jsonField(result, { "targetId", 8 })); + if (!tid.empty()) + send(0, Command(nextId(), "Target.closeTarget"_s) + .str("targetId"_s, WTF::String::fromUTF8(tid))); + } + return; + } if (!error.empty()) { // {"code":-32000,"message":"..."} @@ -739,7 +753,7 @@ void Transport::handleResponse(uint32_t id, std::span<const char> result, std::s auto tid = jsonString(jsonField(result, { "targetId", 8 })); view->m_targetId = WTF::String::fromUTF8(tid); uint32_t cid = nextId(); - m_pending.add(cid, Pending { Method::TargetAttachToTarget, entry.slot, entry.viewId }); + m_pending.add(cid, Pending { Method::TargetAttachToTarget, entry.slot, entry.viewId, entry.navGen }); send(cid, Command(cid, "Target.attachToTarget"_s).str("targetId"_s, view->m_targetId).boolean("flatten"_s, true)); return; } @@ -758,7 +772,7 @@ void Transport::handleResponse(uint32_t id, std::span<const char> result, std::s auto ss = view->m_sessionId.utf8(); std::span<const char> sidSpan(ss.data(), ss.length()); uint32_t cid = nextId(); - m_pending.add(cid, Pending { Method::PageEnable, entry.slot, entry.viewId }); + m_pending.add(cid, Pending { Method::PageEnable, entry.slot, entry.viewId, entry.navGen }); send(cid, Command(cid, "Page.enable"_s, sidSpan)); return; } @@ -790,7 +804,7 @@ void Transport::handleResponse(uint32_t id, std::span<const char> result, std::s // confirms completion. We keep the pending entry alive for the // response so errorText rejects the right slot. uint32_t cid = nextId(); - m_pending.add(cid, Pending { Method::PageNavigate, entry.slot, entry.viewId }); + m_pending.add(cid, Pending { Method::PageNavigate, entry.slot, entry.viewId, entry.navGen }); send(cid, Command(cid, "Page.navigate"_s, sidSpan).str("url"_s, view->m_pendingChromeNavigateUrl)); view->m_pendingChromeNavigateUrl = WTF::String(); return; @@ -857,7 +871,7 @@ void Transport::handleResponse(uint32_t id, std::span<const char> result, std::s int32_t entryId = elem ? elem->getInteger("id"_s).value_or(0) : 0; // Chain into navigateToHistoryEntry. Page.loadEventFired settles. uint32_t cid = nextId(); - m_pending.add(cid, Pending { Method::PageNavigateToHistoryEntry, entry.slot, entry.viewId }); + m_pending.add(cid, Pending { Method::PageNavigateToHistoryEntry, entry.slot, entry.viewId, entry.navGen }); send(cid, Command(cid, "Page.navigateToHistoryEntry"_s, sidSpan(view->m_sessionId)).num("entryId"_s, entryId)); return; } @@ -1131,7 +1145,7 @@ void Transport::handleEvent(std::span<const char> method, std::span<const char> auto chainTitle = [&]() { view->m_navTitleChained = true; uint32_t tid = nextId(); - m_pending.add(tid, Pending { Method::PageTitle, PendingSlot::Navigate, view->m_viewId }); + m_pending.add(tid, Pending { Method::PageTitle, PendingSlot::Navigate, view->m_viewId, view->m_navGeneration }); send(tid, Command(tid, "Runtime.evaluate"_s, sidSpan(view->m_sessionId)).str("expression"_s, "document.title"_s).boolean("returnByValue"_s, true)); }; @@ -1461,7 +1475,11 @@ static JSPromise* sendChromeOp(JSGlobalObject* g, JSWebView* v, } v->m_pendingActivityCount.fetch_add(1, std::memory_order_release); slot.set(vm, v, promise); - t.m_pending.add(id, Pending { m, ps, v->m_viewId }); + // Navigate-slot entries carry m_navGeneration so handleResponse + // can drop a response that arrives after this navigation was + // abandoned (armNavTimeout rejected it) and replaced by a retry. + uint32_t gen = ps == PendingSlot::Navigate ? v->m_navGeneration : 0; + t.m_pending.add(id, Pending { m, ps, v->m_viewId, gen }); t.send(id, WTF::move(cmd)); t.updateKeepAlive(); return promise; diff --git a/src/runtime/webview/ChromeBackend.h b/src/runtime/webview/ChromeBackend.h index 9f13970fb93..81fc84062e9 100644 --- a/src/runtime/webview/ChromeBackend.h +++ b/src/runtime/webview/ChromeBackend.h @@ -346,10 +346,19 @@ enum class PendingSlot : uint8_t { // (one Weak per view) instead of holding its own Weak — a burst of // operations creates N ids but only one weak slot allocation. Response // handlers do m_views.find(entry.viewId)->value.get() to reach the view. +// +// navGen is the view's m_navGeneration at enqueue time for Navigate- +// slot entries (0 = ungated, all other slots). armNavTimeout rejects +// the navigate without pruning m_pending, so a stale response for the +// timed-out navigation can arrive after a .catch() retry refilled +// m_pendingNavigate — handleResponse drops it on gen mismatch instead +// of settling the retry's promise (or, for the attach chain, creating +// a second tab). Chained responses carry entry.navGen forward. struct Pending { Method method; PendingSlot slot; uint32_t viewId; + uint32_t navGen = 0; }; // Transport mode. Pipe = we spawned Chrome with --remote-debugging-pipe, From 2ee8f6b97ee6c8f19cd16b6898eb0c9713add265 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 14 May 2026 03:11:09 +0000 Subject: [PATCH 12/14] [autofix.ci] apply automated fixes --- src/runtime/webview/ChromeBackend.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/runtime/webview/ChromeBackend.cpp b/src/runtime/webview/ChromeBackend.cpp index f4015a4a6a7..56e5c368ab2 100644 --- a/src/runtime/webview/ChromeBackend.cpp +++ b/src/runtime/webview/ChromeBackend.cpp @@ -726,8 +726,7 @@ void Transport::handleResponse(uint32_t id, std::span<const char> result, std::s if (entry.method == Method::TargetCreateTarget && error.empty()) { auto tid = jsonString(jsonField(result, { "targetId", 8 })); if (!tid.empty()) - send(0, Command(nextId(), "Target.closeTarget"_s) - .str("targetId"_s, WTF::String::fromUTF8(tid))); + send(0, Command(nextId(), "Target.closeTarget"_s).str("targetId"_s, WTF::String::fromUTF8(tid))); } return; } From b7cd7f6652d3b0e9c74299d3bc0643bf6eba20de Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 14 May 2026 04:31:56 +0000 Subject: [PATCH 13/14] test(webview): run mock-CDP navigate-options tests concurrently --- .../bun/webview/webview-navigate-options.test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/js/bun/webview/webview-navigate-options.test.ts b/test/js/bun/webview/webview-navigate-options.test.ts index 72839ce34b8..cb0ee9ff925 100644 --- a/test/js/bun/webview/webview-navigate-options.test.ts +++ b/test/js/bun/webview/webview-navigate-options.test.ts @@ -180,7 +180,7 @@ ${body} // --- waitUntil: 'domcontentloaded' ----------------------------------------- -test("navigate({waitUntil:'domcontentloaded'}) settles on Page.lifecycleEvent when load never fires", async () => { +test.concurrent("navigate({waitUntil:'domcontentloaded'}) settles on Page.lifecycleEvent when load never fires", async () => { // The mock emits frameNavigated + lifecycleEvent(DOMContentLoaded) for // the main frame, and NEVER loadEventFired. Without waitUntil: // 'domcontentloaded', navigate() would hang until the 30s timeout. @@ -207,7 +207,7 @@ test("navigate({waitUntil:'domcontentloaded'}) settles on Page.lifecycleEvent wh expect(exitCode).toBe(0); }); -test("navigate() default waitUntil:'load' settles on Page.loadEventFired", async () => { +test.concurrent("navigate() default waitUntil:'load' settles on Page.loadEventFired", async () => { // Backward compat: no options → waitUntil:'load' → loadEventFired // settles. The lifecycleEvent(DOMContentLoaded) arrives first but is // ignored because m_navWaitUntil == Load. @@ -223,7 +223,7 @@ test("navigate() default waitUntil:'load' settles on Page.loadEventFired", async expect(exitCode).toBe(0); }); -test("reload({waitUntil:'domcontentloaded'}) settles on lifecycleEvent", async () => { +test.concurrent("reload({waitUntil:'domcontentloaded'}) settles on lifecycleEvent", async () => { // reload() shares the Navigate slot and the same lifecycle path as // navigate(). "dcl-only" never emits loadEventFired, so both the // initial navigate and the reload must settle via @@ -243,7 +243,7 @@ test("reload({waitUntil:'domcontentloaded'}) settles on lifecycleEvent", async ( expect(exitCode).toBe(0); }); -test("navigate({waitUntil:'domcontentloaded'}) on a fast page doesn't enqueue duplicate title fetches", async () => { +test.concurrent("navigate({waitUntil:'domcontentloaded'}) on a fast page doesn't enqueue duplicate title fetches", async () => { // "load" mock emits DCL + load + loadEventFired all before the // first PageTitle response arrives. Without the m_navTitleChained // flag set by chainTitle(), each of the three would enqueue its @@ -274,7 +274,7 @@ test("navigate({waitUntil:'domcontentloaded'}) on a fast page doesn't enqueue du expect(exitCode).toBe(0); }); -test("stale loadEventFired from a prior 'domcontentloaded' navigate does not settle the next one", async () => { +test.concurrent("stale loadEventFired from a prior 'domcontentloaded' navigate does not settle the next one", async () => { // Regression: navigate(url1, {waitUntil:'domcontentloaded'}) settles // before url1's window `load` fires. A second navigate() can then // start, and url1's trailing lifecycleEvent(load)+loadEventFired @@ -314,7 +314,7 @@ test("stale loadEventFired from a prior 'domcontentloaded' navigate does not set // --- timeout --------------------------------------------------------------- -test("navigate({timeout}) rejects when no lifecycle event arrives", async () => { +test.concurrent("navigate({timeout}) rejects when no lifecycle event arrives", async () => { // "silent" mock: Page.navigate reply + frameNavigated, but no // DCL/load/loadEventFired — navigate() has only the parent-side // dispatchAfter timer to save it. @@ -349,7 +349,7 @@ test("navigate({timeout}) rejects when no lifecycle event arrives", async () => expect(exitCode).toBe(0); }); -test("navigate({timeout}): stale timer does not reject a later navigation", async () => { +test.concurrent("navigate({timeout}): stale timer does not reject a later navigation", async () => { // First navigate settles on DCL at ~0ms with a 400ms timeout armed. // Second navigate (silent mock would hang) starts immediately with // timeout:0 (disabled). The first navigate's 400ms timer FIRES while @@ -378,7 +378,7 @@ test("navigate({timeout}): stale timer does not reject a later navigation", asyn // --- validation ------------------------------------------------------------ -test("navigate() option validation throws before I/O", async () => { +test.concurrent("navigate() option validation throws before I/O", async () => { // No CDP traffic needed — the throws happen in parseNavOptions // before the slot check. Use the silent mock just to get a view. const { stdout, stderr, exitCode } = await run( From 3534b57110db3c35eac53720eeeb5890c3d52da4 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 14 May 2026 04:33:40 +0000 Subject: [PATCH 14/14] [autofix.ci] apply automated fixes --- .../webview/webview-navigate-options.test.ts | 127 ++++++++++-------- 1 file changed, 68 insertions(+), 59 deletions(-) diff --git a/test/js/bun/webview/webview-navigate-options.test.ts b/test/js/bun/webview/webview-navigate-options.test.ts index cb0ee9ff925..2f39d99497e 100644 --- a/test/js/bun/webview/webview-navigate-options.test.ts +++ b/test/js/bun/webview/webview-navigate-options.test.ts @@ -180,20 +180,22 @@ ${body} // --- waitUntil: 'domcontentloaded' ----------------------------------------- -test.concurrent("navigate({waitUntil:'domcontentloaded'}) settles on Page.lifecycleEvent when load never fires", async () => { - // The mock emits frameNavigated + lifecycleEvent(DOMContentLoaded) for - // the main frame, and NEVER loadEventFired. Without waitUntil: - // 'domcontentloaded', navigate() would hang until the 30s timeout. - // With it, the lifecycleEvent handler matches frameId=="F" && - // loaderId=="L" && name=="DOMContentLoaded", chains a document.title - // fetch, and resolves. - // - // The mock ALSO sends a subframe DCL (frameId "SUB") BEFORE the main - // frame commits — the frameId gate must drop it. A naive name-only - // match would settle on the subframe event. - const { stdout, stderr, exitCode } = await run( - "dcl-only", - ` +test.concurrent( + "navigate({waitUntil:'domcontentloaded'}) settles on Page.lifecycleEvent when load never fires", + async () => { + // The mock emits frameNavigated + lifecycleEvent(DOMContentLoaded) for + // the main frame, and NEVER loadEventFired. Without waitUntil: + // 'domcontentloaded', navigate() would hang until the 30s timeout. + // With it, the lifecycleEvent handler matches frameId=="F" && + // loaderId=="L" && name=="DOMContentLoaded", chains a document.title + // fetch, and resolves. + // + // The mock ALSO sends a subframe DCL (frameId "SUB") BEFORE the main + // frame commits — the frameId gate must drop it. A naive name-only + // match would settle on the subframe event. + const { stdout, stderr, exitCode } = await run( + "dcl-only", + ` await view.navigate("http://example/dcl", { waitUntil: "domcontentloaded", timeout: 10_000 }); // PageTitle chain ran — Runtime.evaluate("document.title") → "mock-title". console.log("title=" + view.title); @@ -201,11 +203,12 @@ test.concurrent("navigate({waitUntil:'domcontentloaded'}) settles on Page.lifecy console.log("loading=" + view.loading); console.log("url=" + view.url); `, - ); - expect(stderr).toBe(""); - expect(stdout.trim().split("\n")).toEqual(["title=mock-title", "loading=true", "url=http://example/dcl"]); - expect(exitCode).toBe(0); -}); + ); + expect(stderr).toBe(""); + expect(stdout.trim().split("\n")).toEqual(["title=mock-title", "loading=true", "url=http://example/dcl"]); + expect(exitCode).toBe(0); + }, +); test.concurrent("navigate() default waitUntil:'load' settles on Page.loadEventFired", async () => { // Backward compat: no options → waitUntil:'load' → loadEventFired @@ -243,21 +246,23 @@ test.concurrent("reload({waitUntil:'domcontentloaded'}) settles on lifecycleEven expect(exitCode).toBe(0); }); -test.concurrent("navigate({waitUntil:'domcontentloaded'}) on a fast page doesn't enqueue duplicate title fetches", async () => { - // "load" mock emits DCL + load + loadEventFired all before the - // first PageTitle response arrives. Without the m_navTitleChained - // flag set by chainTitle(), each of the three would enqueue its - // own PageTitle — and a duplicate response can settle the NEXT - // navigate's promise early. With the flag, only the first trigger - // chains; the rest see m_navTitleChained and drop. - // - // Two back-to-back DCL navigates: if duplicate PageTitle from - // nav1 leaked and settled nav2, nav2 would resolve with - // view.url == nav1's url (nav2's frameNavigated hadn't arrived - // yet at the time of the stale settle). - const { stdout, stderr, exitCode } = await run( - "load", - ` +test.concurrent( + "navigate({waitUntil:'domcontentloaded'}) on a fast page doesn't enqueue duplicate title fetches", + async () => { + // "load" mock emits DCL + load + loadEventFired all before the + // first PageTitle response arrives. Without the m_navTitleChained + // flag set by chainTitle(), each of the three would enqueue its + // own PageTitle — and a duplicate response can settle the NEXT + // navigate's promise early. With the flag, only the first trigger + // chains; the rest see m_navTitleChained and drop. + // + // Two back-to-back DCL navigates: if duplicate PageTitle from + // nav1 leaked and settled nav2, nav2 would resolve with + // view.url == nav1's url (nav2's frameNavigated hadn't arrived + // yet at the time of the stale settle). + const { stdout, stderr, exitCode } = await run( + "load", + ` await view.navigate("http://example/one", { waitUntil: "domcontentloaded" }); await view.navigate("http://example/two", { waitUntil: "domcontentloaded" }); // Each navigate committed — url reflects the LAST one. A leaked @@ -268,27 +273,30 @@ test.concurrent("navigate({waitUntil:'domcontentloaded'}) on a fast page doesn't // so m_loading flipped even though we settled on DCL. console.log("loading=" + view.loading); `, - ); - expect(stderr).toBe(""); - expect(stdout.trim().split("\n")).toEqual(["url=http://example/two title=mock-title", "loading=false"]); - expect(exitCode).toBe(0); -}); + ); + expect(stderr).toBe(""); + expect(stdout.trim().split("\n")).toEqual(["url=http://example/two title=mock-title", "loading=false"]); + expect(exitCode).toBe(0); + }, +); -test.concurrent("stale loadEventFired from a prior 'domcontentloaded' navigate does not settle the next one", async () => { - // Regression: navigate(url1, {waitUntil:'domcontentloaded'}) settles - // before url1's window `load` fires. A second navigate() can then - // start, and url1's trailing lifecycleEvent(load)+loadEventFired - // arrive while nav2 is pending. Without the m_loaderId clear in - // beginChromeNavigation(), those stale events pass the gate (the - // old loaderId is still cached) and chainTitle() settles nav2's - // promise before its own document committed. - // - // The mock's "stale-load" arm emits exactly that: nav1 → DCL only; - // nav2 → stale lifecycleEvent(load,L1) + loadEventFired, then - // nothing for nav2. With the fix, nav2 stays pending. - const { stdout, stderr, exitCode } = await run( - "stale-load", - ` +test.concurrent( + "stale loadEventFired from a prior 'domcontentloaded' navigate does not settle the next one", + async () => { + // Regression: navigate(url1, {waitUntil:'domcontentloaded'}) settles + // before url1's window `load` fires. A second navigate() can then + // start, and url1's trailing lifecycleEvent(load)+loadEventFired + // arrive while nav2 is pending. Without the m_loaderId clear in + // beginChromeNavigation(), those stale events pass the gate (the + // old loaderId is still cached) and chainTitle() settles nav2's + // promise before its own document committed. + // + // The mock's "stale-load" arm emits exactly that: nav1 → DCL only; + // nav2 → stale lifecycleEvent(load,L1) + loadEventFired, then + // nothing for nav2. With the fix, nav2 stays pending. + const { stdout, stderr, exitCode } = await run( + "stale-load", + ` await view.navigate("http://example/one", { waitUntil: "domcontentloaded" }); // nav1 settled on DCL; its load hasn't fired. nav2 starts and // clears m_loaderId. The mock then sends nav1's trailing load @@ -306,11 +314,12 @@ test.concurrent("stale loadEventFired from a prior 'domcontentloaded' navigate d // check (m_loaderId empty). Close the view to reject nav2 so the // process exits; the test only cares it was still pending. `, - ); - expect(stderr).toBe(""); - expect(stdout.trim()).toBe("nav2=pending url=http://example/one loading=true"); - expect(exitCode).toBe(0); -}); + ); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("nav2=pending url=http://example/one loading=true"); + expect(exitCode).toBe(0); + }, +); // --- timeout ---------------------------------------------------------------