Skip to content

Let Chrome choose its remote debugging port - #2778

Merged
natebosch merged 5 commits into
masterfrom
chrome-debug-port-from-output
Oct 6, 2026
Merged

natebosch merged 5 commits into
masterfrom
chrome-debug-port-from-output

Conversation

@natebosch

@natebosch natebosch commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

When debugging or collecting coverage, the runner picked a probably-unused
port with getUnusedPort and passed it to Chrome with
--remote-debugging-port. Chrome gives no reliable signal that it bound
the port, so there was a window where another process could take it, and
tryPort had to report the chosen port back through a completer for
_connect to use.

Pass --remote-debugging-port=0 so Chrome binds an unused port itself,
and read the port from the URL in the "DevTools listening on" line, which
_connect already waits for. With no port to choose or pass along,
tryPort, the port completer, and the private constructor go away, and
Chrome has a plain generative constructor. spawn already falls back
to the default Chrome settings.

Update the fake Chrome test to report a fake DevTools server's port in its
DevTools line, and check that connecting reaches that server and that
Chrome is started with --remote-debugging-port=0.

When Chrome runs with a remote debugging port, which happens with
`--debug`, `--pause-after-load`, and `--coverage`, `_connect` listened to
the process's stderr to wait for the "DevTools listening" line. The
`Browser` base class listens to the same single-subscription stream to
collect the browser output, and that second listen threw a `StateError`
which was silently caught. Once `firstWhere` found the line it cancelled
the subscription, which closes the pipe, so all of Chrome's stderr,
including its console logging, was missing from the "Browser output" in
load failures and timeouts.

Rename the `output` list on `Browser` to `accumulatedOutput`, and add an
`output` stream built with `Stream.multi` which replays the accumulated
lines to each new listener and then forwards new lines. It closes when
both stdout and stderr are done, when the browser is closed, or when
starting or running the browser fails.

In `Chrome`, `tryPort` now only reports which debugging port it used, and
`_connect` is an instance method which reads that port and waits for the
DevTools line on `output`. The tab connection and the remote debugger URL
come from a single lazy `_connection` future in place of the two
completers, started by the first read of `remoteDebuggerUrl` or
`gatherCoverage`. `BrowserManager` reads `remoteDebuggerUrl` before it
loads any suites, so coverage collection still starts before tests run,
and the replay means a late start still sees the DevTools line.

Drop the `StateError` catch in `drainOutput`. Nothing else listens to the
process streams now, and a future second listener should fail loudly
instead of losing output.
When debugging or collecting coverage, the runner picked a probably-unused
port with `getUnusedPort` and passed it to Chrome with
`--remote-debugging-port`. Chrome gives no reliable signal that it bound
the port, so there was a window where another process could take it, and
`tryPort` had to report the chosen port back through a completer for
`_connect` to use.

Pass `--remote-debugging-port=0` so Chrome binds an unused port itself,
and read the port from the URL in the "DevTools listening on" line, which
`_connect` already waits for. With no port to choose or pass along,
`tryPort`, the port completer, and the private constructor go away, and
`Chrome` has a plain generative constructor. `spawn` already falls back
to the default Chrome settings.

Update the fake Chrome test to report a fake DevTools server's port in its
DevTools line, and check that connecting reaches that server and that
Chrome is started with `--remote-debugging-port=0`.
@natebosch
natebosch requested a review from a team as a code owner October 6, 2026 05:04
@natebosch
natebosch requested a review from sigurdm October 6, 2026 05:09
Base automatically changed from chrome-debug-stderr to master October 6, 2026 16:35
# Conflicts:
#	pkgs/test/lib/src/runner/browser/chrome.dart
#	pkgs/test/test/runner/browser/chrome_test.dart
@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

PR Health

Changelog Entry ✔️
Package Changed Files

Changes to files need to be accounted for in their respective changelogs.

This check can be disabled by tagging the PR with skip-changelog-check.

`Uri.port` is 0 when the URL has no port, which would turn into a
confusing connection failure on port 0. Throw a `StateError` naming the
line instead, which also covers a URL that does not parse.
@natebosch
natebosch merged commit 4a363d7 into master Oct 6, 2026
71 checks passed
@natebosch
natebosch deleted the chrome-debug-port-from-output branch October 6, 2026 17:46
copybara-service Bot pushed a commit to dart-lang/sdk that referenced this pull request Oct 13, 2026
Revisions updated by `dart tools/rev_sdk_deps.dart`.

dartdoc (https://github.com/dart-lang/dartdoc/compare/5eb0f2c..e1859cf):
  e1859cfc  Fri Oct 9 14:53:01 2026 +0200  Sarah Zakarias  Link a bare prefix only when it maps to exactly one library (dart-lang/dartdoc#4295)
  c48f6fb9  Fri Oct 9 14:25:42 2026 +0200  Sarah Zakarias  Resolve prefixes through the analyzer's PrefixElement.scopeLibraries (dart-lang/dartdoc#4294)
  3a6297e2  Wed Oct 7 12:56:14 2026 +0200  Sarah Zakarias  Require analyzer 14.5.0 and bump to 9.0.10 (dart-lang/dartdoc#4292)

ecosystem (https://github.com/dart-lang/ecosystem/compare/16dca7a..622ef61):
  622ef61  Mon Oct 12 00:23:27 2026 -0700  Kevin Moore  fix(firehose): enforce -wip on published versions and tighten changelog checks (dart-lang/ecosystem#467)
  5576a85  Wed Oct 7 09:29:44 2026 +0200  Moritz  Support use-flutter option in groundskeeper workflow (dart-lang/ecosystem#444)

http (https://github.com/dart-lang/http/compare/585d433..23f8574):
  23f8574  Thu Oct 8 23:27:25 2026 +0700  Cong Bang  feat(http2): configurable receive windows and frame size (dart-lang/http#1999)
  18051a6  Thu Oct 8 10:39:38 2026 +0200  Moritz  Enable Flutter in Groundskeeper workflow for Flutter packages (dart-lang/http#2004)

native (https://github.com/dart-lang/native/compare/a2a6179..10515af):
  10515afca  Mon Oct 12 12:21:52 2026 +0200  Daco Harkes  [native_toolchain_c] Do not pass linker flags when compiling static libraries (dart-lang/native#3731)

native is held back at 10515af, before dart-lang/native#3728, because it
breaks the dartdev `dart install` native assets tests. See
dart-lang/native#3743.

test (https://github.com/dart-lang/test/compare/d749b10..30acb69):
  30acb69e  Mon Oct 12 12:42:54 2026 -0700  Nate Bosch  Time out browser loads just before the suite load timeout (dart-lang/test#2786)
  597a0326  Mon Oct 12 12:22:53 2026 -0700  Kevin Moore  Fix misplaced changelog entries and remove no-op health check (dart-lang/test#2789)
  0b6efee8  Mon Oct 12 12:16:29 2026 -0700  Nate Bosch  [checks] Collapse nested subjects in condition descriptions (dart-lang/test#2784)
  17c98493  Mon Oct 12 12:08:23 2026 -0700  Nate Bosch  Follow symlinks in precompiled browser test directories (dart-lang/test#2787)
  1cc813ec  Fri Oct 9 11:59:56 2026 -0700  Nate Bosch  [checks] Join nested collapsed subjects with "that" (dart-lang/test#2783)
  ada7b950  Thu Oct 8 12:03:42 2026 -0700  Nate Bosch  Prepare to publish (dart-lang/test#2780)
  c4b165b3  Wed Oct 7 11:29:08 2026 -0700  Kevin Moore  [checks] Require package:meta ^1.17.0 (dart-lang/test#2779)
  4a363d70  Tue Oct 6 10:46:31 2026 -0700  Nate Bosch  Let Chrome choose its remote debugging port (dart-lang/test#2778)
  465d66ab  Tue Oct 6 09:35:22 2026 -0700  Nate Bosch  Keep Chrome stderr in the browser output when debugging (dart-lang/test#2777)

tools (https://github.com/dart-lang/tools/compare/a66d517..2b54c71):
  2b54c71af  Mon Oct 12 10:16:45 2026 +0200  dart-ecosystem-bot[bot]  [boring] Update BoringSSL to 3fed4f20 (dart-lang/tools#2655)
  2337f995a  Sat Oct 10 03:11:40 2026 +0300  Hasan Armoush  [json_rpc_2] Add strictProtocolChecks parameter to Client (dart-lang/tools#2555)
  c65c69ed1  Fri Oct 9 08:54:25 2026 +0200  Moritz  [boring] Move package:boring to dart-lang/tools (2/4): FFI helpers from package:webcrypto (dart-lang/tools#2615)
  a04e45491  Thu Oct 8 21:07:18 2026 -0700  Kevin Moore  chore: wire package:api_summary and package:string_scanner PR auto-labeler (dart-lang/tools#2652)
  8bdd4b92c  Thu Oct 8 09:20:48 2026 +0200  Morgan :)  [code_builder] Allow built_collection 6.x. (dart-lang/tools#2645)
  c16040d35  Wed Oct 7 17:30:08 2026 -0700  Kevin Moore  [api_summary] Sort primary barrel first and experimental entry points last (v1.2.0) (dart-lang/tools#2619)
  b2d0780db  Wed Oct 7 11:27:16 2026 -0700  Kevin Moore  [stream_channel] Require package:async ^2.6.0 (dart-lang/tools#2644)
  3ab7bf9d0  Wed Oct 7 11:35:27 2026 +0200  Moritz  [boring] Move package:boring to dart-lang/tools (1/4): vendored BoringSSL, bindings and source build (dart-lang/tools#2614)
  29df6348b  Tue Oct 6 15:21:44 2026 -0700  Kevin Moore  feat(coverage): support web coverage reports in test_with_coverage (dart-lang/tools#2448)

Regenerate pkg/{analyzer,analyzer_plugin,analyzer_testing}/api.txt. The
rolled package:api_summary v1.2.0 (dart-lang/tools#2619) sorts the primary
barrel first and other entry points after it, so entries in these files
are reordered. The APIs themselves are unchanged.

Change-Id: Ie8ff19c5a981ea8ecee8939c75457986a947e0a7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/563220
Reviewed-by: Samuel Rawlins <[email protected]>
Auto-Submit: Nate Bosch <[email protected]>
Reviewed-by: Paul Berry <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants