Skip to content

Add BaseWindowController.isDestroyed flag#189061

Merged
auto-submit[bot] merged 8 commits into
flutter:masterfrom
canonical:windowing-is-destroyed
Jul 21, 2026
Merged

Add BaseWindowController.isDestroyed flag#189061
auto-submit[bot] merged 8 commits into
flutter:masterfrom
canonical:windowing-is-destroyed

Conversation

@mattkae

@mattkae mattkae commented Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What's new?

  • Added BaseWindowController.isDestroyed
  • Added BaseWindowController.isDestroyedOf
  • Added BaseWindowController.maybeIsDestroyedOf

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 documentation (doc comments with ///).
  • 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.

@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Jul 7, 2026
@github-actions github-actions Bot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. labels Jul 7, 2026
@mattkae
mattkae requested review from justinmc, knopp, loic-sharma and robert-ancell and removed request for justinmc July 7, 2026 13:25
gemini-code-assist[bot]
gemini-code-assist Bot reviewed Jul 7, 2026
View reviewed changes

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces an isDestroyed getter to BaseWindowController and implements it across Linux, macOS, Win32, and test window controllers to track whether the underlying native window has been destroyed. Feedback suggests adding @override and @internal annotations to the macOS implementation of isDestroyed for consistency with other platforms and to match the API contract.

Comment thread packages/flutter/lib/src/widgets/_window_macos.dart
@mattkae
mattkae force-pushed the windowing-is-destroyed branch from 9abe6f4 to 6e46040 Compare July 7, 2026 13:28
loic-sharma
loic-sharma reviewed Jul 7, 2026
View reviewed changes
Comment thread packages/flutter/lib/src/widgets/_window.dart
loic-sharma
loic-sharma previously approved these changes Jul 7, 2026
View reviewed changes

@loic-sharma loic-sharma left a comment

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.

This looks good to me, but I'd consider updating the integration tests to cover this scenario. See: https://github.com/flutter/flutter/pull/189061/changes#r3540292006

@mattkae
mattkae requested a review from loic-sharma July 8, 2026 14:56
@mattkae mattkae changed the title Add BaseWindowController.isDestroyed flag Fix broken listener code in _window.dart + add BaseWindowController.isDestroyed flag Jul 9, 2026
@mattkae
mattkae force-pushed the windowing-is-destroyed branch from 8773061 to 41d8654 Compare July 9, 2026 18:14
loic-sharma
loic-sharma reviewed Jul 9, 2026
View reviewed changes
Comment thread packages/flutter/lib/src/widgets/_window_linux.dart
loic-sharma
loic-sharma reviewed Jul 9, 2026
View reviewed changes
Comment thread packages/flutter/lib/src/widgets/_window.dart
loic-sharma
loic-sharma reviewed Jul 9, 2026
View reviewed changes
Comment thread packages/flutter/lib/src/widgets/_window_macos.dart Outdated
@mattkae
mattkae marked this pull request as draft July 9, 2026 18:42
@flutter-dashboard

flutter-dashboard Bot commented Jul 9, 2026

Copy link
Copy Markdown

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@loic-sharma loic-sharma mentioned this pull request Jul 9, 2026
Merged
10 tasks
@mattkae
mattkae force-pushed the windowing-is-destroyed branch from 41d8654 to c0c3598 Compare July 9, 2026 19:01
@mattkae mattkae changed the title Fix broken listener code in _window.dart + add BaseWindowController.isDestroyed flag Add BaseWindowController.isDestroyed flag Jul 9, 2026
@mattkae
mattkae force-pushed the windowing-is-destroyed branch from c0c3598 to edee09a Compare July 10, 2026 19:00
@mattkae
mattkae marked this pull request as ready for review July 10, 2026 19:04
@mattkae
mattkae requested a review from loic-sharma July 10, 2026 19:04
gemini-code-assist[bot]
gemini-code-assist Bot reviewed Jul 10, 2026
View reviewed changes

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces an isDestroyed property to BaseWindowController and its platform-specific implementations (Linux, macOS, Win32, and test bindings) to track whether the underlying native window has been destroyed. It also updates WindowScope to expose isDestroyedOf and maybeIsDestroyedOf and adds corresponding integration and widget tests. The review feedback highlights critical issues regarding the lack of guard clauses in the destroy() methods of Linux and Win32 controllers, which could lead to native crashes or accidental destruction of recycled window handles. Additionally, the feedback points out that test window controllers fail to call notifyListeners() upon destruction, creating a discrepancy with production behavior where listening widget trees are not rebuilt.

Comment thread packages/flutter/lib/src/widgets/_window_linux.dart
Comment thread packages/flutter/lib/src/widgets/_window_linux.dart
Comment thread packages/flutter/lib/src/widgets/_window_linux.dart
Comment thread packages/flutter/lib/src/widgets/_window_linux.dart
Comment thread packages/flutter/lib/src/widgets/_window_win32.dart
Comment thread packages/flutter_test/lib/src/binding.dart
Comment thread packages/flutter_test/lib/src/binding.dart
Comment thread packages/flutter_test/lib/src/binding.dart
Comment thread packages/flutter_test/lib/src/binding.dart
Comment thread packages/flutter_test/lib/src/binding.dart
@mattkae mattkae added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 10, 2026
robert-ancell
robert-ancell approved these changes Jul 14, 2026
View reviewed changes

@robert-ancell robert-ancell 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.

👍

@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 20, 2026
@auto-submit

auto-submit Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

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

@mattkae mattkae added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 21, 2026
@auto-submit
auto-submit Bot added this pull request to the merge queue Jul 21, 2026
Merged via the queue into flutter:master with commit 1ac2e82 Jul 21, 2026
20 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 21, 2026
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 21, 2026
Merged
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Jul 21, 2026
@engine-flutter-autoroll
Roll Flutter from cab057d904b6 to 1ac2e82fa65c (23 revisions) (#12256)
796e5b1 
flutter/flutter@cab057d...1ac2e82

2026-07-21 [email protected] Add BaseWindowController.isDestroyed flag (flutter/flutter#189061)
2026-07-21 [email protected] Roll Dart SDK from 666e1e2133b7 to 3b2f5ad7718d (1 revision) (flutter/flutter#189745)
2026-07-21 [email protected] Roll Skia from 3c52d80c960d to 569534e9fa59 (5 revisions) (flutter/flutter#189766)
2026-07-21 [email protected] Roll Skia from 11426cf7aaa1 to 3c52d80c960d (2 revisions) (flutter/flutter#189758)
2026-07-21 [email protected] Roll pub packages (flutter/flutter#189760)
2026-07-21 [email protected] [iOS] Inject DisplayLinkManager into FlutterKeyboardInsetManager (flutter/flutter#189751)
2026-07-21 [email protected] Treat package:stack_trace async-gap marker as asynchronous suspension (flutter/flutter#185791)
2026-07-21 [email protected] [iOS] Inject DisplayLinkManager into VsyncWaiterIOS (flutter/flutter#189749)
2026-07-20 [email protected] Use String builder in `InputConnectionAdaptorTest.java` (flutter/flutter#189281)
2026-07-20 [email protected] Fix non-independant tests in draggable_test.dart (flutter/flutter#186898)
2026-07-20 [email protected] Stop using ReLinker to load libflutter.so on Android 17 (API 37+) (flutter/flutter#189146)
2026-07-20 [email protected] [et] Refactor gn post-processing, fix minor clang[++] regex match bug (flutter/flutter#189685)
2026-07-20 [email protected] Take plugin_test_darwin out of bringup (flutter/flutter#189594)
2026-07-20 [email protected] Roll pub packages (flutter/flutter#189596)
2026-07-20 [email protected] widgets layer re-export ScrollCacheExtent (flutter/flutter#189483)
2026-07-20 [email protected] Roll Skia from 47143b6fa402 to 11426cf7aaa1 (7 revisions) (flutter/flutter#189740)
2026-07-20 [email protected] Sync CHANGELOG.md from stable (flutter/flutter#189735)
2026-07-20 [email protected] [Impeller] In the AHBTextureSourceVK destructor, do not call Vulkan APIs if the VkDevice has been destroyed (flutter/flutter#189666)
2026-07-20 [email protected] Add AutofillHints.emailOTPCode for Android AUTOFILL_HINT_EMAIL_OTP (flutter/flutter#188123)
2026-07-20 [email protected] Add a github workflow to remove flutteractionsbot branches after the PRs are merged or closed. (flutter/flutter#189668)
2026-07-20 [email protected] Roll ANGLE to cc08479fbcc1 (flutter/flutter#189595)
2026-07-20 [email protected] Roll Fuchsia Linux SDK from NL8xtzr8cxr5E8r8E... to GswhlPRO-D1qSNclx... (flutter/flutter#189715)
2026-07-20 [email protected] Roll Skia from ba90f98535de to 47143b6fa402 (4 revisions) (flutter/flutter#189721)

If this roll has caused a breakage, revert this CL and stop the roller
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

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@robert-ancell robert-ancell robert-ancell approved these changes
@knopp knopp Awaiting requested review from knopp
@loic-sharma loic-sharma Awaiting requested review from loic-sharma
+1 more reviewer
@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

a: tests "flutter test", flutter_test, or one of our tests CICD Run CI/CD framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mattkae @robert-ancell @loic-sharma