Expose platform specific handles for multi-window API#184662
Expose platform specific handles for multi-window API#184662knopp merged 9 commits intoflutter:masterfrom
Conversation
|
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. |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
@knopp Marking this as a draft to get it out of the review queue but feel free to undraft it when you're ready. |
There was a problem hiding this comment.
Code Review
This pull request introduces platform-specific window controller interfaces for Linux, macOS, and Windows to expose native window handles and adds a custom message handling system for Win32. Feedback focuses on adhering to the 80-character line limit, adding missing @OverRide annotations, and removing redundant Set.contains checks. It also suggests removing @internal annotations from window handle methods to ensure they are properly exposed as part of the public API.
13acf0f to
fb85a77
Compare
robert-ancell
left a comment
There was a problem hiding this comment.
Thoughts:
We need to add some clear comments about the responsibilities and risks of accessing these handles. Essentially don't access these unless you absolutely have to and don't call methods without understanding the implications.
Given the above, does it makes sense for the platform specific interfaces to provide some likely methods that will be used? i.e. WindowControllerLinux.setDecorated(). This would reduce the risk of misuse.
WindowsMessageHandler feels like event processing is now split across the native embedder code and here. Is this a special case for Windows or does it suggest that things we're not at the optimal solution?
If you access these (as user) the assumption is you know what you are doing, same way if you are using FFI or doing anything else that goes beyond writing widgets.
Absolutely it does. We have separate controller class on each platform specifically to allow for this and not be restricted to lowest common denominator.
I don't think there is such thing as optimal solution. You can't predict and bake-in every single use case and every framework needs to have an escape hatch for that (i.e. you have NativeEventFilter in Qt). |
Assumptions are dangerous - we should have an explicit comment stating there are risks to using these handles. |
There's no such thing as a perfect solution, but there is always an optimal solution (even if it's not great). Again this might be a documentation thing to note you shouldn't be handling keyboard events in this handler as they are already handled in the embedder. |
There are potentially thousands of different windows messages. If you want to handle keyboard message independently of the embedder there is probably a good a reason for that. I don't think anyone would start listening to keyboard messages instead of using HardwareKeyboard just for the heck of it :) But then again, people do weird things sometimes. It may still be useful to act on a message even if is handled in the embedder. More importantly - this API does not let users prevent embedder from handling the messages. But if we don't provide this, the user can always get a bigger hammer, like subclassing the HWND, replacing the msgproc and actually preventing the embedder from getting the messages at all. So this way at least there is less incentive to do that. And we're still somewhat in control. No matter what you do in your dart handler, you shouldn't be able to break the embedder. |
I really don't mind. Just suggest the right wording and I'll be happy to add it in. |
mattkae
left a comment
There was a problem hiding this comment.
Nice, I like the getter model
Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>
d2a65dc to
da3dc58
Compare
flutter/flutter@2fa45e0...c1b14e9 2026-04-14 15619084+vashworth@users.noreply.github.com Rebuild flutter tool skill (flutter/flutter#184975) 2026-04-14 engine-flutter-autoroll@skia.org Roll Skia from 0851d988db03 to 4c382df6a25a (1 revision) (flutter/flutter#185025) 2026-04-14 engine-flutter-autoroll@skia.org Roll Dart SDK from 5504504b38c2 to ee5afcef0596 (1 revision) (flutter/flutter#185024) 2026-04-14 dacoharkes@google.com [ci] Split up integration.shard record_use_test.dart (flutter/flutter#185022) 2026-04-14 engine-flutter-autoroll@skia.org Roll Skia from d34c84df4c37 to 0851d988db03 (3 revisions) (flutter/flutter#185012) 2026-04-14 dacoharkes@google.com [record_use] Add recorded uses to link hooks (flutter/flutter#184869) 2026-04-14 engine-flutter-autoroll@skia.org Roll Skia from 0e98a9c635bb to d34c84df4c37 (5 revisions) (flutter/flutter#185009) 2026-04-14 engine-flutter-autoroll@skia.org Roll Dart SDK from ef28089d6533 to 5504504b38c2 (3 revisions) (flutter/flutter#185008) 2026-04-14 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from K_2AkZL3Drs6cGE1q... to rB8LAuZL_DwHMssTU... (flutter/flutter#185007) 2026-04-14 rmacnak@google.com [fuchsia] Replace ambient-replace-as-executable with VmexResource. (flutter/flutter#184967) 2026-04-13 chinmaygarde@google.com [Impeller] Commands that don't specify their own viewports get the viewport of the render pass. (flutter/flutter#177473) 2026-04-13 engine-flutter-autoroll@skia.org Roll Skia from bc1df263ff3f to 0e98a9c635bb (1 revision) (flutter/flutter#184995) 2026-04-13 737941+loic-sharma@users.noreply.github.com Update autosubmit guide with the emergency label (flutter/flutter#184993) 2026-04-13 69043738+aNOOBisTheGod@users.noreply.github.com [flutter_tools] Cache pubspec reads and share PackageGraph/PackageConfig across workspace packages during pub get post-processing (flutter/flutter#184528) 2026-04-13 15619084+vashworth@users.noreply.github.com Fix codesign verification test for SwiftPM Add to App (flutter/flutter#184980) 2026-04-13 87962825+kyungilcho@users.noreply.github.com Preprovision Android NDK for flavored builds and reuse matching unflavored NDKs (flutter/flutter#183555) 2026-04-13 15619084+vashworth@users.noreply.github.com Reland "Disable async mode with LLDB" (flutter/flutter#184970) 2026-04-13 engine-flutter-autoroll@skia.org Roll Skia from 55ddd6bb8be5 to bc1df263ff3f (6 revisions) (flutter/flutter#184968) 2026-04-13 matej.knopp@gmail.com Expose platform specific handles for multi-window API (flutter/flutter#184662) 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 boetger@google.com,stuartmorgan@google.com 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
Native windowing API has a very large surface with many platform
specific areas (i.e. window collection behavior on macOS) and we can't
possibly cover it all.
This PR will provide the functionality for users and package authors to
access the underlying platform handles.
For every platform window controller (i.e. RegularWindowControllerMacOS)
a new getter is added:
```dart
ffi.Pointer<ffi.Void> get windowHandle;
```
The interpretation of returned handle depends on platform:
| Platform | Handle Type |
| ---- |----|
| macOS | Pointer to `NSWindow` |
| Windows | `HWND` |
| Linux | Pointer to `GtkWindow` |
Because the handles are platform specific, it is necessary to cast the
window controller to a platform specific controller first to obtain the
handle:
```dart
final RegularWindowController controller = ....
if (controller is WindowControllerWin32) {
final controllerWin32 = controller as WindowControllerWin32;
SetWindowDisplayAffinity(controllerWin32.windowHandle, WDA_EXCLUDEFROMCAPTURE);
}
```
## 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 documentation (doc comments with `///`).
- [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.
- [x] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel
on [Discord].
**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.
<!-- 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/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines
[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/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
---------
Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>
Native windowing API has a very large surface with many platform
specific areas (i.e. window collection behavior on macOS) and we can't
possibly cover it all.
This PR will provide the functionality for users and package authors to
access the underlying platform handles.
For every platform window controller (i.e. RegularWindowControllerMacOS)
a new getter is added:
```dart
ffi.Pointer<ffi.Void> get windowHandle;
```
The interpretation of returned handle depends on platform:
| Platform | Handle Type |
| ---- |----|
| macOS | Pointer to `NSWindow` |
| Windows | `HWND` |
| Linux | Pointer to `GtkWindow` |
Because the handles are platform specific, it is necessary to cast the
window controller to a platform specific controller first to obtain the
handle:
```dart
final RegularWindowController controller = ....
if (controller is WindowControllerWin32) {
final controllerWin32 = controller as WindowControllerWin32;
SetWindowDisplayAffinity(controllerWin32.windowHandle, WDA_EXCLUDEFROMCAPTURE);
}
```
## 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 documentation (doc comments with `///`).
- [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.
- [x] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel
on [Discord].
**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.
<!-- 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/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines
[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/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
---------
Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>
Native windowing API has a very large surface with many platform specific areas (i.e. window collection behavior on macOS) and we can't possibly cover it all.
This PR will provide the functionality for users and package authors to access the underlying platform handles.
For every platform window controller (WindowControllerMacOS, WindowControllerWin32, WindowControllerLinux) a new getter is added:
The interpretation of returned handle depends on platform:
NSWindowHWNDGtkWindowBecause the handles are platform specific, it is necessary to cast the window controller to a platform specific controller first to obtain the handle:
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.