Repository navigation
Share the common Linux window controller code in mixins - #191930
Merged
Merged
Conversation
The four Linux window controllers were largely copies of each other: every one repeated the destroyed flag, the content size, destroy(), the windowing-enabled guard, setConstraints() and the block that creates the FlView and looks up its FlutterView. On top of that the tooltip and popup controllers each carried an identical ~55 line copy of updatePosition() and its two enum conversion helpers, and the regular and dialog controllers each carried identical copies of the title, activation and minimization members. Move that code into three mixins: * BaseWindowControllerLinux now owns the window, view and monitors along with the members common to all four controllers. * _ToplevelWindowControllerLinux covers the members shared by the regular and dialog windows. * _PositionedWindowControllerLinux covers the members shared by the tooltip and popup windows, which are placed relative to a rectangle in a parent window. BaseWindowControllerLinux cannot use an `on BaseWindowController` clause because BaseWindowController is sealed and so cannot be named outside its own library. The members it needs from the base class are therefore declared abstract, and the mixin application supplies them. This is a refactor with two incidental changes: the dialog, tooltip and popup controllers now spell their unbounded constraint as _kMaxWindowDimensions rather than hardcoding the identical 0x7fffffff, and the GtkWindow is now created after the windowing-enabled check rather than before, so a disabled build no longer leaks one.
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
github-actions
Bot
added
the
framework
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors Linux window controllers in _window_linux.dart by extracting shared logic into mixins (BaseWindowControllerLinux, _ToplevelWindowControllerLinux, and _PositionedWindowControllerLinux) to reduce code duplication across top-level and positioned windows. Feedback suggests throwing a StateError instead of a generic Exception when a parent window cannot be found.
Failing to find the parent window means the controller has been asked to place a window against a parent that is not registered, which is an invalid state rather than an expected failure. Every other throw in this file and in _window.dart already uses a typed error, and _window.dart raises a StateError for the analogous case of an unexpected controller in the window hierarchy. Also applies this to the dialog controller, which had the same generic Exception.
This was referenced Aug 28, 2026
auto-submit Bot
pushed a commit
to flutter/packages
that referenced
this pull request
Aug 28, 2026
flutter/flutter@e8dca90...8d8c1aa 2026-08-28 [email protected] Roll Packages from bd3cbc1 to cd4cdd0 (15 revisions) (flutter/flutter#191965) 2026-08-28 [email protected] Bump customer testing version to pick up forui tests (flutter/flutter#191931) 2026-08-28 [email protected] Use leading scroll offset instead of infinite offset to reveal pinned slivers (flutter/flutter#191329) 2026-08-28 [email protected] Roll Skia from 21f71051e749 to fe0b9a5261f9 (1 revision) (flutter/flutter#191960) 2026-08-28 [email protected] Share the common Linux window controller code in mixins (flutter/flutter#191930) 2026-08-28 [email protected] Roll Dart SDK from 2cf2f69fd9f0 to ae63a6eca3f1 (1 revision) (flutter/flutter#191953) 2026-08-28 [email protected] Roll Skia from 11e3a7cb0fd0 to 21f71051e749 (3 revisions) (flutter/flutter#191942) 2026-08-28 [email protected] Roll Dart SDK from e030e77d5413 to 2cf2f69fd9f0 (2 revisions) (flutter/flutter#191934) 2026-08-28 [email protected] Remove no-shuffle in flutter tools overall experience test (flutter/flutter#191861) 2026-08-28 [email protected] [flutter_tools] Gracefully handle Windows Application Control and security policy execution blocks (flutter/flutter#191912) 2026-08-28 [email protected] [tool] Migrate ConfigCommand to modular dependency injection (flutter/flutter#190743) 2026-08-28 [email protected] Roll Skia from 37459da0a755 to 11e3a7cb0fd0 (9 revisions) (flutter/flutter#191929) 2026-08-28 [email protected] Connect to moved-to-rect after the window is realized (flutter/flutter#191773) 2026-08-28 [email protected] macOS: Compare accessibility strings as NSStrings in tests (flutter/flutter#191853) 2026-08-28 [email protected] macOS: Record test accessibility notifications in a dictionary (flutter/flutter#191854) 2026-08-28 [email protected] Roll Fuchsia Test Scripts from 6SXRz6wsA2Zm_rAqP... to u_fSMsPU22VvUsvSo... (flutter/flutter#191927) 2026-08-28 [email protected] Roll Dart SDK from cb4c39d067f3 to e030e77d5413 (3 revisions) (flutter/flutter#191923) 2026-08-27 [email protected] Migrate ios driver tests batch 2 to simulators (flutter/flutter#190930) 2026-08-27 [email protected] [flutter_test] Ensure LiveTestWidgetsFlutterBinding executes frame and pointer callbacks in the test zone (flutter/flutter#191595) 2026-08-27 [email protected] Deprecate legacy gen_defaults tool (flutter/flutter#191911) 2026-08-27 [email protected] Fix black window in the subsurface view renderer (flutter/flutter#191850) 2026-08-27 [email protected] Roll Skia from 82e972ea84b1 to 37459da0a755 (10 revisions) (flutter/flutter#191906) 2026-08-27 [email protected] iOS: Key the platform view factory registries by NSString (flutter/flutter#191855) 2026-08-27 [email protected] Sync CHANGELOG.md from stable (flutter/flutter#191895) 2026-08-27 [email protected] [flutter_tools] Honor configured build-dir across flutter project and build targets (flutter/flutter#191592) 2026-08-27 [email protected] Revert: Handle linear and radial gradients in UberSDF (flutter/flutter#191913) 2026-08-27 [email protected] Explicitly disable HCPP in hybrid_android_views integration test (flutter/flutter#191894) 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
victorsanni
pushed a commit
to victorsanni/packages
that referenced
this pull request
Sep 9, 2026
…r#12679) flutter/flutter@e8dca90...8d8c1aa 2026-08-28 [email protected] Roll Packages from bd3cbc1 to cd4cdd0 (15 revisions) (flutter/flutter#191965) 2026-08-28 [email protected] Bump customer testing version to pick up forui tests (flutter/flutter#191931) 2026-08-28 [email protected] Use leading scroll offset instead of infinite offset to reveal pinned slivers (flutter/flutter#191329) 2026-08-28 [email protected] Roll Skia from 21f71051e749 to fe0b9a5261f9 (1 revision) (flutter/flutter#191960) 2026-08-28 [email protected] Share the common Linux window controller code in mixins (flutter/flutter#191930) 2026-08-28 [email protected] Roll Dart SDK from 2cf2f69fd9f0 to ae63a6eca3f1 (1 revision) (flutter/flutter#191953) 2026-08-28 [email protected] Roll Skia from 11e3a7cb0fd0 to 21f71051e749 (3 revisions) (flutter/flutter#191942) 2026-08-28 [email protected] Roll Dart SDK from e030e77d5413 to 2cf2f69fd9f0 (2 revisions) (flutter/flutter#191934) 2026-08-28 [email protected] Remove no-shuffle in flutter tools overall experience test (flutter/flutter#191861) 2026-08-28 [email protected] [flutter_tools] Gracefully handle Windows Application Control and security policy execution blocks (flutter/flutter#191912) 2026-08-28 [email protected] [tool] Migrate ConfigCommand to modular dependency injection (flutter/flutter#190743) 2026-08-28 [email protected] Roll Skia from 37459da0a755 to 11e3a7cb0fd0 (9 revisions) (flutter/flutter#191929) 2026-08-28 [email protected] Connect to moved-to-rect after the window is realized (flutter/flutter#191773) 2026-08-28 [email protected] macOS: Compare accessibility strings as NSStrings in tests (flutter/flutter#191853) 2026-08-28 [email protected] macOS: Record test accessibility notifications in a dictionary (flutter/flutter#191854) 2026-08-28 [email protected] Roll Fuchsia Test Scripts from 6SXRz6wsA2Zm_rAqP... to u_fSMsPU22VvUsvSo... (flutter/flutter#191927) 2026-08-28 [email protected] Roll Dart SDK from cb4c39d067f3 to e030e77d5413 (3 revisions) (flutter/flutter#191923) 2026-08-27 [email protected] Migrate ios driver tests batch 2 to simulators (flutter/flutter#190930) 2026-08-27 [email protected] [flutter_test] Ensure LiveTestWidgetsFlutterBinding executes frame and pointer callbacks in the test zone (flutter/flutter#191595) 2026-08-27 [email protected] Deprecate legacy gen_defaults tool (flutter/flutter#191911) 2026-08-27 [email protected] Fix black window in the subsurface view renderer (flutter/flutter#191850) 2026-08-27 [email protected] Roll Skia from 82e972ea84b1 to 37459da0a755 (10 revisions) (flutter/flutter#191906) 2026-08-27 [email protected] iOS: Key the platform view factory registries by NSString (flutter/flutter#191855) 2026-08-27 [email protected] Sync CHANGELOG.md from stable (flutter/flutter#191895) 2026-08-27 [email protected] [flutter_tools] Honor configured build-dir across flutter project and build targets (flutter/flutter#191592) 2026-08-27 [email protected] Revert: Handle linear and radial gradients in UberSDF (flutter/flutter#191913) 2026-08-27 [email protected] Explicitly disable HCPP in hybrid_android_views integration test (flutter/flutter#191894) 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
victorsanni
pushed a commit
to victorsanni/packages
that referenced
this pull request
Sep 9, 2026
…r#12679) flutter/flutter@e8dca90...8d8c1aa 2026-08-28 [email protected] Roll Packages from bd3cbc1 to cd4cdd0 (15 revisions) (flutter/flutter#191965) 2026-08-28 [email protected] Bump customer testing version to pick up forui tests (flutter/flutter#191931) 2026-08-28 [email protected] Use leading scroll offset instead of infinite offset to reveal pinned slivers (flutter/flutter#191329) 2026-08-28 [email protected] Roll Skia from 21f71051e749 to fe0b9a5261f9 (1 revision) (flutter/flutter#191960) 2026-08-28 [email protected] Share the common Linux window controller code in mixins (flutter/flutter#191930) 2026-08-28 [email protected] Roll Dart SDK from 2cf2f69fd9f0 to ae63a6eca3f1 (1 revision) (flutter/flutter#191953) 2026-08-28 [email protected] Roll Skia from 11e3a7cb0fd0 to 21f71051e749 (3 revisions) (flutter/flutter#191942) 2026-08-28 [email protected] Roll Dart SDK from e030e77d5413 to 2cf2f69fd9f0 (2 revisions) (flutter/flutter#191934) 2026-08-28 [email protected] Remove no-shuffle in flutter tools overall experience test (flutter/flutter#191861) 2026-08-28 [email protected] [flutter_tools] Gracefully handle Windows Application Control and security policy execution blocks (flutter/flutter#191912) 2026-08-28 [email protected] [tool] Migrate ConfigCommand to modular dependency injection (flutter/flutter#190743) 2026-08-28 [email protected] Roll Skia from 37459da0a755 to 11e3a7cb0fd0 (9 revisions) (flutter/flutter#191929) 2026-08-28 [email protected] Connect to moved-to-rect after the window is realized (flutter/flutter#191773) 2026-08-28 [email protected] macOS: Compare accessibility strings as NSStrings in tests (flutter/flutter#191853) 2026-08-28 [email protected] macOS: Record test accessibility notifications in a dictionary (flutter/flutter#191854) 2026-08-28 [email protected] Roll Fuchsia Test Scripts from 6SXRz6wsA2Zm_rAqP... to u_fSMsPU22VvUsvSo... (flutter/flutter#191927) 2026-08-28 [email protected] Roll Dart SDK from cb4c39d067f3 to e030e77d5413 (3 revisions) (flutter/flutter#191923) 2026-08-27 [email protected] Migrate ios driver tests batch 2 to simulators (flutter/flutter#190930) 2026-08-27 [email protected] [flutter_test] Ensure LiveTestWidgetsFlutterBinding executes frame and pointer callbacks in the test zone (flutter/flutter#191595) 2026-08-27 [email protected] Deprecate legacy gen_defaults tool (flutter/flutter#191911) 2026-08-27 [email protected] Fix black window in the subsurface view renderer (flutter/flutter#191850) 2026-08-27 [email protected] Roll Skia from 82e972ea84b1 to 37459da0a755 (10 revisions) (flutter/flutter#191906) 2026-08-27 [email protected] iOS: Key the platform view factory registries by NSString (flutter/flutter#191855) 2026-08-27 [email protected] Sync CHANGELOG.md from stable (flutter/flutter#191895) 2026-08-27 [email protected] [flutter_tools] Honor configured build-dir across flutter project and build targets (flutter/flutter#191592) 2026-08-27 [email protected] Revert: Handle linear and radial gradients in UberSDF (flutter/flutter#191913) 2026-08-27 [email protected] Explicitly disable HCPP in hybrid_android_views integration test (flutter/flutter#191894) 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
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
2 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.
The four Linux window controllers were largely copies of each other: every one repeated the destroyed flag, the content size, destroy(), the windowing-enabled guard, setConstraints() and the block that creates the FlView and looks up its FlutterView. On top of that the tooltip and popup controllers each carried an identical ~55 line copy of updatePosition() and its two enum conversion helpers, and the regular and dialog controllers each carried identical copies of the title, activation and minimization members.
Move that code into three mixins:
BaseWindowControllerLinux cannot use an
on BaseWindowControllerclause because BaseWindowController is sealed and so cannot be named outside its own library. The members it needs from the base class are therefore declared abstract, and the mixin application supplies them.This is a refactor with two incidental changes: the dialog, tooltip and popup controllers now spell their unbounded constraint as _kMaxWindowDimensions rather than hardcoding the identical 0x7fffffff, and the GtkWindow is now created after the windowing-enabled check rather than before, so a disabled build no longer leaks one.