fix(chromium): do not hang connectOverCDP on a page stuck loading#41128
Open
yury-s wants to merge 5 commits into
Open
fix(chromium): do not hang connectOverCDP on a page stuck loading#41128yury-s wants to merge 5 commits into
yury-s wants to merge 5 commits into
Conversation
When connecting over CDP, a pre-existing tab whose first navigation never commits (e.g. stuck behind a hung proxy) left the page on the initial empty document forever, blocking the connection while it waited for a navigation that never arrived. Report such pre-existing pages as-is instead. Fixes: microsoft#41093
Verify connectOverCDP surfaces both pages and that releasing the held response lets the previously-stuck tab finish navigating to its URL.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
dgozman
reviewed
Jun 3, 2026
| }), | ||
| browser._defaultContext.initialize(), | ||
| ]); | ||
| browser._attachingPreExistingTargets = false; |
Collaborator
There was a problem hiding this comment.
Perhaps we should not wait for all pages to be initialized instead?
Member
Author
There was a problem hiding this comment.
I don't think we can, it will lead to unpredictable page list for the context right after connect.
Member
Author
There was a problem hiding this comment.
trying to not wait for the pages which are on the initial page and we are in connectOverCDP mode
This comment has been minimized.
This comment has been minimized.
pavelfeldman
reviewed
Jun 3, 2026
Per review feedback, rather than reporting a stuck pre-existing page early, do not wait for all pages to be initialized when connecting over CDP. A tab stuck loading no longer blocks the connection; pages are reported as they initialize. Existing tests that read pages right after connect now wait for them to appear.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Rather than skipping the page-initialization wait entirely, parametrize it: when connecting over CDP, still wait for pages that have committed a navigation, but not for pages stuck on the initial empty document (their first navigation may never commit). This keeps the page list available right after connect for normal pages, while not hanging on a stuck tab.
Contributor
Test results for "MCP"7230 passed, 1103 skipped Merge workflow run. |
Contributor
Test results for "tests 1"1 failed 2 flaky39546 passed, 771 skipped Merge workflow run. |
dgozman
approved these changes
Jun 5, 2026
| // The stuck tab has not committed a navigation yet, so it is not reported. Releasing the | ||
| // response lets it finish navigating to /hang and show up in the context. | ||
| releaseHang(); | ||
| await expect.poll(() => context.pages().map(page => page.url())).toContain(server.PREFIX + '/hang'); |
Collaborator
There was a problem hiding this comment.
Can just const page = await context.waitForEvent('page').
|
|
||
| // connectOverCDP must not hang waiting for the stuck page to commit a navigation. | ||
| const cdpBrowser = await browserType.connectOverCDP(`http://127.0.0.1:${port}/`); | ||
| const context = cdpBrowser.contexts()[0]; |
Collaborator
There was a problem hiding this comment.
expect there is just a single about:blank page?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
connectOverCDPhung forever when a pre-existing tab's first navigation never committed (e.g. a tab stuck behind a hung proxy, which Edge 148's online new-tab page made easy to hit): the page stayed on the initial empty document and the connection blocked waiting for a navigation that never arrived.Fixes #41093