Skip to content

[flutter_tools] Report when the browser disconnects during a web test run - #192198

Merged
auto-submit[bot] merged 7 commits into
flutter:masterfrom
gabrimatic:web-test-browser-disconnect
Nov 3, 2026
Merged

auto-submit[bot] merged 7 commits into
flutter:masterfrom
gabrimatic:web-test-browser-disconnect

Conversation

@gabrimatic

@gabrimatic gabrimatic commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #191920

When the browser closes its connection to the test host during flutter test --platform chrome, unfinished suites are reported as did not complete without a diagnostic explaining the lost connection. The issue includes a deterministic renderer-kill reproduction confirmed on master.

The WebSocket can close before the browser's process exit notification arrives. BrowserManager.close() sets _closed, and Chromium.close() sets _didClose, so cleanup can suppress the existing process-exit diagnostics. A renderer or page disconnect can also leave the browser process alive until the tool closes it.

This change does two things when the browser disconnects before the manager initiates shutdown:

  • Reports the disconnect before cleanup starts, so the run explains why its suites did not complete.
  • Records the unexpected disconnect and keeps the manager's exit-code callback active after cleanup starts. Cleanup begins immediately, without a grace-period timeout. The exit-code message describes an exit after disconnection, rather than claiming that a cleanup-induced exit was the original crash.

Intentional shutdown remains silent.

Tests

Six BrowserManager regressions in test/commands.shard/hermetic/flutter_web_platform_test.dart cover unexpected disconnection, immediate cleanup, exit notifications arriving after cleanup starts, the process exiting before the connection closes, intentional shutdown, and normal cleanup without a false crash diagnostic.

All 43 tests across that file and test/web.shard/chrome_test.dart pass. Formatting, analysis of the changed files, and git diff --check pass.

Also verified with real Chrome using the tool from source: normal completion, an ordinary assertion failure, a renderer kill, and a browser-process kill. Neither normal completion nor the assertion failure produces a false disconnect diagnostic. Both kill paths report the disconnect and clean up the test browser; the process-kill path reports exit code -9. Both kill paths reproduce the missing disconnect diagnostic on unpatched master.

The broader Flutter Tools run attempted 340 test files. It found 16 failures that also reproduce on unpatched master, in widget-preview path normalization, native-assets filesystem-guard handling, and the workspace-member tests. A native-assets batch also hung and was stopped. The full suite is not green; the browser-related tests are.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the AI contribution guidelines and understand my responsibilities, or I am not using AI tools.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant in-code documentation (doc comments with ///).
  • If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in flutter/website (or verified none is needed).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • I followed the breaking change policy and added Data Driven Fixes where supported.
  • All existing and new tests are passing. Browser-related tests pass; the broader failures reproduce on unpatched master as noted above.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@gabrimatic
gabrimatic force-pushed the web-test-browser-disconnect branch from be0e426 to b27af58 Compare October 9, 2026 06:44
bkonyi
bkonyi previously approved these changes Oct 14, 2026

@bkonyi bkonyi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for removing the timeout workaround! Tracking _unexpectedDisconnect is much cleaner.

LGTM with a couple of small nits in the test file. We'll just need one more review before this can land.

bkonyi
bkonyi previously approved these changes Oct 15, 2026
mdebbar
mdebbar previously approved these changes Oct 16, 2026

@mdebbar mdebbar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM with some minor suggestions. Thanks for fixing this!

@gabrimatic
gabrimatic dismissed stale reviews from mdebbar and bkonyi via 0249820 October 16, 2026 17:18
@mdebbar mdebbar added the CICD Run CI/CD label Oct 19, 2026
@mdebbar
mdebbar requested a review from bkonyi October 19, 2026 17:25
@flutter-dashboard flutter-dashboard Bot removed the CICD Run CI/CD label Oct 20, 2026
@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 20, 2026
@mdebbar mdebbar added the CICD Run CI/CD label Oct 21, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 21, 2026
@auto-submit

auto-submit Bot commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/flutter/192198, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label.

@flutter-dashboard flutter-dashboard Bot removed the CICD Run CI/CD label Nov 2, 2026
@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 2, 2026
@mdebbar mdebbar added the CICD Run CI/CD label Nov 2, 2026
@auto-submit
auto-submit Bot added this pull request to the merge queue Nov 2, 2026
Merged via the queue into flutter:master with commit 724e813 Nov 3, 2026
29 of 30 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 3, 2026
gmackall pushed a commit to gmackall/flutter that referenced this pull request Nov 3, 2026
… run (flutter#192198)

Fixes flutter#191920

When the browser closes its connection to the test host during `flutter
test --platform chrome`, unfinished suites are reported as `did not
complete` without a diagnostic explaining the lost connection. The issue
includes a deterministic renderer-kill reproduction confirmed on master.

The WebSocket can close before the browser's process exit notification
arrives. `BrowserManager.close()` sets `_closed`, and `Chromium.close()`
sets `_didClose`, so cleanup can suppress the existing process-exit
diagnostics. A renderer or page disconnect can also leave the browser
process alive until the tool closes it.

This change does two things when the browser disconnects before the
manager initiates shutdown:

- Reports the disconnect before cleanup starts, so the run explains why
its suites did not complete.
- Records the unexpected disconnect and keeps the manager's exit-code
callback active after cleanup starts. Cleanup begins immediately,
without a grace-period timeout. The exit-code message describes an exit
after disconnection, rather than claiming that a cleanup-induced exit
was the original crash.

Intentional shutdown remains silent.

## Tests

Six `BrowserManager` regressions in
`test/commands.shard/hermetic/flutter_web_platform_test.dart` cover
unexpected disconnection, immediate cleanup, exit notifications arriving
after cleanup starts, the process exiting before the connection closes,
intentional shutdown, and normal cleanup without a false crash
diagnostic.

All 43 tests across that file and `test/web.shard/chrome_test.dart`
pass. Formatting, analysis of the changed files, and `git diff --check`
pass.

Also verified with real Chrome using the tool from source: normal
completion, an ordinary assertion failure, a renderer kill, and a
browser-process kill. Neither normal completion nor the assertion
failure produces a false disconnect diagnostic. Both kill paths report
the disconnect and clean up the test browser; the process-kill path
reports exit code -9. Both kill paths reproduce the missing disconnect
diagnostic on unpatched master.

The broader Flutter Tools run attempted 340 test files. It found 16
failures that also reproduce on unpatched master, in widget-preview path
normalization, native-assets filesystem-guard handling, and the
workspace-member tests. A native-assets batch also hung and was stopped.
The full suite is not green; the browser-related tests are.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [AI contribution guidelines] and understand my
responsibilities, or I am not using AI tools.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant in-code documentation (doc comments with
`///`).
- [x] If this PR introduces a new feature or capability, I created and
linked a website documentation issue or PR in [flutter/website] (or
verified none is needed).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [ ] All existing and new tests are passing. Browser-related tests
pass; the broader failures reproduce on unpatched master as noted above.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[AI contribution guidelines]:
https://github.com/flutter/flutter/blob/main/CONTRIBUTING.ai.md
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/website]: https://github.com/flutter/website
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

---------

Co-authored-by: Ben Konyi <[email protected]>
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Nov 3, 2026
Roll Flutter from fcfb4f203eae to 934720261be0 (38 revisions)

flutter/flutter@fcfb4f2...9347202

2026-11-03 [email protected] Roll Skia from 5bceaa033999 to c733ad3e37cb (5 revisions) (flutter/flutter#195872)
2026-11-03 [email protected] Roll Skia from 34d2bf479385 to 5bceaa033999 (9 revisions) (flutter/flutter#195864)
2026-11-03 [email protected] fix: prefer pointer events over semantics tap for mouse clicks on web (flutter/flutter#191529)
2026-11-03 [email protected] [VPAT] Expose a semantics text field value while it is not being edited (flutter/flutter#195445)
2026-11-03 [email protected] [Impeller] Make a GL context current before constructing an AiksContext without a surface (flutter/flutter#195629)
2026-11-03 [email protected] [flutter_tools] Eliminate globals.dart from project.dart (flutter/flutter#194066)
2026-11-03 [email protected] [tool] Complete context runner elimination and migrate AppContext usages to ToolContext (flutter/flutter#190797)
2026-11-03 [email protected] [web] Unify font collections between CanvasKit and Skwasm (flutter/flutter#195539)
2026-11-03 [email protected] [Android] Replay predictive back state for retained engines (flutter/flutter#192286)
2026-11-03 [email protected] [vulkan]: Adds depth/stencil synchronization for subpasses (flutter/flutter#195689)
2026-11-03 [email protected] [web] Honor requested upscaling in instantiateImageCodecWithSize and ImageDescriptor.instantiateCodec (flutter/flutter#195836)
2026-11-03 [email protected] Roll Skia from 866b61ca92c9 to 34d2bf479385 (9 revisions) (flutter/flutter#195848)
2026-11-03 [email protected] Roll Fuchsia Test Scripts from xl1LWBuppzu-BnOzV... to B6zarykZp6PKWRueg... (flutter/flutter#195847)
2026-11-03 [email protected] Remove unused ffi (flutter/flutter#195710)
2026-11-02 [email protected] Revert: [Android] Let the raster thread use every core of a chip without little cores (flutter/flutter#195838)
2026-11-02 [email protected] Run 10 impeller iOS benchmarks as one devicelab task (flutter/flutter#195661)
2026-11-02 [email protected] Use named uniforms instead of setFloat in StretchEffect (flutter/flutter#195372)
2026-11-02 [email protected] [flutter_tools] Report when the browser disconnects during a web test run (flutter/flutter#192198)
2026-11-02 [email protected] fix: Resolve memory leak in image_list example (flutter/flutter#189526)
2026-11-02 [email protected] Only run the common local engine configurations in presubmit (flutter/flutter#195688)
2026-11-02 [email protected] Update gcloud_version to version [email protected] (flutter/flutter#195769)
2026-11-02 [email protected] [Impeller] Order the Vulkan copy-texture-to-texture layout transition after the copy (flutter/flutter#195814)
2026-11-02 [email protected] [Impeller] Order the Vulkan ResizeTexture layout transition after its blit (flutter/flutter#195721)
2026-11-02 [email protected] [web] Fix CanvasKit fragment shader uniforms being freed or mutated after recording (flutter/flutter#195718)
2026-11-02 [email protected] Roll Fuchsia Linux SDK from MqypI6RNyVZ_FLUE-... to T_-6-6X1lDMghhAvX... (flutter/flutter#195809)
2026-11-02 [email protected] [Android] Log why Impeller falls back to OpenGLES (flutter/flutter#194764)
2026-11-02 [email protected] Roll Skia from 3f35fb6e591b to 866b61ca92c9 (11 revisions) (flutter/flutter#195802)
2026-11-02 [email protected] Roll Skia from 3f35fb6e591b to 866b61ca92c9 (11 revisions) (flutter/flutter#195802)
2026-11-02 [email protected] Only run the common local engine configurations in presubmit (flutter/flutter#195688)
2026-11-02 [email protected] Balance tool_integration_tests subshards by expected duration (flutter/flutter#195628)
2026-11-02 [email protected] [web_paragraph] Map code units without a cluster to a cluster (flutter/flutter#195384)
2026-11-02 [email protected] [web] Hide synthetic autofill inputs from assistive technology (flutter/flutter#195618)
2026-11-02 [email protected] Roll Packages from 3742a4e to 3f34a6f (8 revisions) (flutter/flutter#195795)
2026-11-02 [email protected] [web] Unskip web tests that pass on CanvasKit and Skwasm: platform_view_test, route_notification_messages_test (flutter/flutter#195605)
2026-11-02 [email protected] Roll Skia from e5ad3868b8db to 3f35fb6e591b (1 revision) (flutter/flutter#195789)
2026-11-02 [email protected] Roll Skia from f5215e74aa16 to e5ad3868b8db (3 revisions) (flutter/flutter#195779)
2026-11-02 [email protected] Roll Skia from e43478a2f9f8 to f5215e74aa16 (1 revision) (flutter/flutter#195776)
2026-11-01 [email protected] Roll Fuchsia Linux SDK from kW0A0xxNkqf38ljm4... to MqypI6RNyVZ_FLUE-... (flutter/flutter#195768)

If this roll has caused a breakage, revert this CL and set the roller
to dry run mode using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose
...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[flutter_tools] Web test run reports "did not complete" for every remaining test with no diagnostic when the browser connection drops

3 participants