webview: re-check that the view is open after argument conversion in click/press/scroll/scrollTo/resize

[robobun]

Problem

• Bun.WebView: if user code that runs while click(), press(), scroll(), scrollTo() or resize() convert their arguments (a valueOf(), a modifiers entry's toString(), an options getter) calls view.close(), the returned promise never settles. screenshot() and cdp() in the same position throw WebView is closed.
• The cause is in src/runtime/webview/JSWebViewPrototype.cpp. These five methods checked m_closed only in unwrapThis(), before toNumber()/toUInt32()/JSObject::get() ran. close() removes the view from the transport's routing table (CDP::Ops::close, WK::Ops::close). The command sent after that gets a reply that handleResponse cannot route (viewFor() is null), so nothing settles the slot and m_pendingActivityCount never returns to zero.

Fix

• Rename the shared pre-dispatch guard checkSlot to checkReady and make it check m_closed before the slot. Every method already calls it as the last step before sending, after all argument conversion.
• Delete the two ad-hoc m_closed re-checks in screenshot() and cdp(). The helper now does the same thing with the same error (ERR_INVALID_STATE, WebView is closed), so their behavior is unchanged.
• Correct because the guard now runs after the last call that can enter user code in every method, on both backends. The early check in unwrapThis() stays: it avoids running any conversion on a view that is already closed.
• Verified: test/js/bun/webview/webview-chrome-pipe.test.ts (new test, 11 shapes, runs against the fake-Chrome fixture so it needs no browser; stock bun fails 9 of 11). Also ran webview-chrome.test.ts (58 pass), webview-chrome-disconnect.test.ts and webview-chrome-ws.test.ts against Chrome 143 with the debug build.

Background

• Each WebView method validates its arguments in JSWebViewPrototype.cpp, then calls an instance method that builds the wire command and stores the returned promise in a per-operation WriteBarrier<JSPromise> slot (m_pendingMisc for the input methods).
• Replies are routed by request id to a view id, then through the transport's m_views (Chrome) or viewsById (WebKit) weak map. close() rejects the view's slots and erases it from that map, so a reply for a closed view is dropped by design.
• m_pendingActivityCount is what keeps a view with an in-flight operation alive for the GC. A slot that is set but never settled pins the view.

Notes

• Reproduces on 1.4.2, canary 5f554969b and main with real Chrome and with test/js/bun/webview/fake-chrome-fixture.ts. With the fake, the unfixed build returns a pending promise for all nine input shapes; with the fix each call throws synchronously, like every other argument error in these methods.
• navigate(), evaluate() and type() only call toWTFString() on a value already checked with isString(), and goBack()/goForward()/reload() take no arguments, so they could not hit this today. They go through the same helper now, which also covers webview: navigate/reload/goBack/goForward accept { waitUntil, timeout } #30645 (it adds option parsing to the navigation methods and carried its own copies of the re-check).
• A re-entrant call that starts another operation on the same view during conversion was already handled: the slot check ran after conversion. Only the closed check was missing.
• webview: close the tab created by a Target.createTarget that was in flight at close() #39075 is a different path (close() while Target.createTarget is in flight).

---

no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/webview/webview.test.ts, test/js/bun/webview/webview-chrome-pipe.test.ts

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{On-demand reviews are free for the next 11 days. After that, they cost $0.25 per reviewed file.}

Or wait 11 seconds for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 6fb18127-9a19-4d4f-b6c8-a90f889d399c

📥 Commits

Reviewing files that changed from the base of the PR and between 4ff9193 and 199e9b5.

📒 Files selected for processing (4)

• src/runtime/webview/JSWebViewConstructor.cpp
• src/runtime/webview/JSWebViewPrototype.cpp
• test/js/bun/webview/webview-chrome-pipe.test.ts
• test/js/bun/webview/webview.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

Status: ready for review. CI build #113651: 180 of 181 jobs passed. The one red job is the debian 13 x64-asan shard that runs test/js/bun/http/serve-pending-promise-abort-leak.test.ts ("client abort of a streaming Response releases the body stream it held", alive is 1, expected 0). That test fails the same way on unrelated branches right now (for example builds #113650, #113648, #113647, #113633) and this diff does not touch Bun.serve or streams, so it is not caused by this change. Every WebView test passed on every lane.

Reproduced on 1.4.3-canary 5f554969b and main 4ff91937 without a real browser: with test/js/bun/webview/fake-chrome-fixture.ts standing in for Chrome, click(), press(), scroll(), scrollTo() and resize() return a promise that never settles when an argument's valueOf()/toString()/getter calls view.close() (9 shapes). screenshot() and cdp() throw WebView is closed in the same position. Also confirmed with Chrome 143 (--no-sandbox) using the original repro script.

Test: bun bd test test/js/bun/webview/webview-chrome-pipe.test.ts -t "argument conversion" fails 9 of 11 shapes on stock bun, passes with this branch. The real-Chrome suites (webview-chrome.test.ts, -disconnect, -ws) pass locally with the debug build.

[claude]

LGTM — the shared guard is the right place for the m_closed re-check, and it now runs after the last user-code entry point in every dispatching method.

What was reviewed:

• Traced each of the 15 checkReady call sites and confirmed the guard sits after the final toNumber/toWTFString/get/JSONStringify with RETURN_IF_EXCEPTION in between; no coercion runs afterward.
• screenshot()/cdp() behavior is unchanged: close() clears the pending slots, so checking m_closed before the slot in the merged helper cannot change which error fires.
• Test is hermetic (fake-Chrome fixture via runScenario), covers both click overloads and the two previously-guarded methods, and asserts the exact code + message — no sleeps or timing dependence.

Extended reasoning...

Overview

This PR touches one C++ file (src/runtime/webview/JSWebViewPrototype.cpp) and one test file. It renames the static pre-dispatch helper checkSlot to checkReady, adds an m_closed check to it, and updates all 15 callers. Two now-redundant inline m_closed re-checks in screenshot() and cdp() are removed. A new test.concurrent case in the existing pipe-transport test file exercises 11 method/argument shapes that close the view mid-coercion and asserts each throws ERR_INVALID_STATE: WebView is closed synchronously.

Security risks

None. This is a liveness re-check on an internal WebView object; no auth, crypto, path handling, or untrusted-input parsing is touched. The error goes through the centralized Bun::throwError(..., ErrorCode::ERR_INVALID_STATE, ...) machinery like the code it replaces.

Level of scrutiny

Low-to-moderate. The change is mechanical and directly implements the REVIEW.md rule "re-validate liveness guards after every callback" by moving the guard into the shared helper. I verified every call site places checkReady after all argument conversion (each toNumber/get/toWTFString is followed by RETURN_IF_EXCEPTION before the guard), and grepped for stragglers — no remaining checkSlot call sites exist (only a stale comment reference in JSWebViewConstructor.cpp, which is harmless). The ordering swap in screenshot() (slot-then-closed → closed-then-slot) is not observable because close() rejects and clears the slots.

Other factors

The test follows repo conventions exactly: appended to the existing feature test file, test.concurrent, single .toEqual on a results object, hermetic via the fake-Chrome fixture, no timing dependence. It covers the variant matrix (both click overloads, options getters, valueOf, toString, toJSON) and includes screenshot/cdp as regression coverage for the folded-in checks. The bug hunt exited on dry_streak with no findings and no outstanding reviewer objections exist on the PR.

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[robobun]

^{Updated 8:01 PM PT - Sep 9th, 2026}

❌ @robobun, your commit 199e9b5 has 1 failures in Build #113651 (All Failures):

• test/js/bun/http/serve-pending-promise-abort-leak.test.ts - code 1 on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 42160

That installs a local version of the PR into your bun-42160 executable, so you can run:

bun-42160 --bun