Skip to content

fix(chromium): keep closing the target so page.close() cannot hang on a navigation commit#41912

Closed
dcrousso wants to merge 1 commit into
microsoft:mainfrom
dcrousso:fix-41900
Closed

fix(chromium): keep closing the target so page.close() cannot hang on a navigation commit#41912
dcrousso wants to merge 1 commit into
microsoft:mainfrom
dcrousso:fix-41900

Conversation

@dcrousso

@dcrousso dcrousso commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

page.close() has no timeout and resolves only once the browser reports Target.detachedFromTarget

if the close happens at the same time as a navigation commit, the Target.closeTarget responds with success but silently drops the actual close (crbug.com/536385539)

repeatedly dispatch Target.closeTarget until the target actually detaches (note that a close on an already-closing target is a no-op)

this fix is very similar to #41695

fixes #41900

@dcrousso
dcrousso requested review from dgozman and yury-s July 21, 2026 19:28
@dcrousso dcrousso changed the title fix(chromium): keep closing the target so page.close() cannot hang on… fix(chromium): keep closing the target so page.close() cannot hang on a navigation commit Jul 21, 2026
… a commit race

`page.close()` has no timeout and resolves only once the browser reports `Target.detachedFromTarget`

if the close happens at the same time as a navigation commit, the `Target.closeTarget` responds with success but silently drops the actual close (crbug.com/536385539)

repeatedly dispatch `Target.closeTarget` until the target actually detaches (note that a close on an already-closing target is a no-op)
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "MCP"

7786 passed, 1249 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "tests 1"

1 failed
❌ [playwright-test] › playwright.trace.spec.ts:346 › should retain traces for interrupted tests @windows-latest-node22

4 flaky ⚠️ [chromium-library] › library/video.spec.ts:736 › screencast › should work with video+trace `@frozen-time-library-chromium-linux`
⚠️ [chromium-library] › library/video.spec.ts:736 › screencast › should work with video+trace `@chromium-ubuntu-22.04-arm-node20`
⚠️ [firefox-page] › page/page-emulate-media.spec.ts:144 › should keep reduced motion and color emulation after reload `@firefox-ubuntu-22.04-node20`
⚠️ [firefox-page] › page/page-event-request.spec.ts:181 › should return response body when Cross-Origin-Opener-Policy is set `@firefox-ubuntu-22.04-node20`

50016 passed, 1192 skipped


Merge workflow run.

// actually doesnt do anything, leaving the target alive and causing this to hang forever (crbug.com/536385539).
const closed = crPage._page.closedPromise.then(() => true);
while (true) {
await this._session.send('Target.closeTarget', { targetId: crPage._targetId }).catch(() => {});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's fix it upstream. This is clearly a bug in the close command.

@github-actions

Copy link
Copy Markdown
Contributor

🟢 CI is clear — the one failure is a pre-existing flake

Hi, I'm the Playwright bot and I took a first look at the failing CI.

The only real failure — [playwright-test] › playwright.trace.spec.ts:346 › should retain traces for interrupted tests on windows-latest-node22 — also fails on main outside this PR, and this PR doesn't touch the test runner's trace-retention path. So it's not caused by your change.

Details

This PR only changes Chromium page.close() retry logic (crBrowser.ts) plus a Chromium page test (page-autowaiting-no-hang.spec.ts). Nothing in the diff reaches the @playwright/test trace lifecycle for interrupted tests.

Pre-existing flake / infra

  • [playwright-test] › playwright.trace.spec.ts:346 › should retain traces for interrupted tests (@windows-latest-node22) — pre-existing flake, windows-latest-node22 only. In the aggregated CI results this test's final verdict is red 2 of 494 runs and green the other 492. One of the two failures is run 29845392348 on sha 403727349c, a non-PR push where this PR can't be responsible. Same test, same bot, same signature — that's the flake. The diff (Chromium Target.closeTarget retries) doesn't exercise the test runner's trace code path.

Flaky (rescued on retry, nothing to triage)

  • [chromium-library] › library/video.spec.ts:736 › screencast › should work with video+trace
  • [firefox-page] › page/page-emulate-media.spec.ts:144 › should keep reduced motion and color emulation after reload
  • [firefox-page] › page/page-event-request.spec.ts:181 › should return response body when Cross-Origin-Opener-Policy is set

These passed on retry and are unrelated to the Chromium close path.

I'm a first pass, not the final word — but this one looks safe to ignore.

Triaged by the Playwright bot - agent run

@dcrousso dcrousso closed this Jul 21, 2026
@dcrousso
dcrousso deleted the fix-41900 branch July 21, 2026 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: page.close() hangs on Chromium when racing with cross-origin navigation commit

2 participants