fix(chromium): re-issue Target.closeTarget when the target survives a racing navigation commit

[Jeff Witt (wittjeff)]

Fixes#42068. References #42366 and https://issues.chromium.org/issues/536385539.

The bug

Chromium can acknowledge Target.closeTarget with success:true without actually closing the target: when the close races a navigation commit that swaps the main RenderFrameHost, the pending close request dies with the old frame host and is never re-issued (root-caused with code pointers in the crbug, which has a pending Chromium CL). Target.targetDestroyed never fires, so page.close() awaits closedPromise forever — no resolution, no rejection.

The fix

In CRBrowser._closePage, wait briefly (1 s) for the page to report closed after Target.closeTarget, and re-issue the close if the target is still alive (bounded at 3 attempts). Re-issuing reliably destroys such a target — verified over raw CDP as well as through Playwright. The normal path is unchanged: the close event resolves the wait immediately.

Why a client-side guard when a Chromium CL is pending

• Channel builds.channel: 'chrome' / 'msedge' users are on 151-class stable builds where the hang reproduces on 7–8 of 10 attempts with a common real-world trigger — a page carrying <meta http-equiv="refresh" content="0; URL=..."> closed right after the refresh commits (the W3C ACT-Rules bc659a "passed" fixtures are examples; we hit this scanning real pages, freezing long crawls for 35+ minutes with no error anywhere).
• The bug is still present in the rolled 152.0.7977.54: a Playwright-free raw-CDP repro (closeTarget → success:true, no targetDestroyed, target still in Target.getTargets) reproduces ~2/10 on 152 vs ~1-3/10 on 151. What changed in 152 is timing — through Playwright's call sequence the window has become hard to hit (0/10 in our runs), which is why no new test accompanies this PR: a race test that cannot reliably fail without the fix on the bundled browser would only be flaky. On 151-class builds the hang was near-deterministic through Playwright.

Test changes

• Removed the stale fixme from should close page while a reload is committing: it passes 6/6 repeats on the bundled 152 both with and without this change (the reload trigger no longer loses the close there), and with the guard it is robust on 151-class builds too, where it previously hung.
• All of tests/library/page-close.spec.ts, beforeunload.spec.ts, and browsercontext-pages.spec.ts pass with the change (41/41).

Verification matrix (10 rounds per cell, meta-refresh repro)

	default close()	close({runBeforeUnload:true})	context.close()
Chromium 151 (unpatched client)	8/10 hang	0/10	0/10
Chromium 151 (this change)	0/10	0/10	0/10
Firefox / WebKit	0/10	0/10	0/10

🤖 Generated with Claude Code

[Jeff Witt (wittjeff)]

@microsoft-github-policy-service agree [company="YRA Tech"]

[Jeff Witt (wittjeff)]

@microsoft-github-policy-service agree company="YRA Tech"

[Anton Petnitsky (Mukller)]

Nice find and clean fix. The retry loop handles the tricky parts correctly: registering the Page.Events.Close listener before checking page.isClosed() avoids the registration/check race, and treating a closeTarget error on attempt > 0 as success (the previous attempt landed) is the right interpretation of that error.

One thing worth making explicit before merge: if all 3 attempts time out (~3s), _closePage resolves silently and page.close() proceeds as if the target were gone, even though it may still be alive. That intentionally preserves the old best-effort semantics (one send, zero verification), which I think is fine — but a one-line comment saying so would prevent a future reader from "fixing" this into a throw and reintroducing hangs for users hitting the Chromium bug repeatedly:

// Exhausting retries keeps the historical best-effort behavior of// _closePage; we deliberately do not throw here.

(Optional nit: 3 / 1000 could be named constants since they're latency-relevant.)

[Devin Rousso (dcrousso)]

please see #41912