[windows] Allow delegation of top-level WindowProc #20613
Conversation
Adds APIs for runners to delegate WindowProc handlers into the Flutter engine, and for plugins to register as possible delegates. This allows for plugins to alter top-level window behavior in ways that can only be done from the WindowProc, such as resize control. This functionality remains entirely on the native side, so is synchronous. Part of flutter/flutter#53168
| std::optional<LRESULT> FlutterViewController::HandleTopLevelWindowProc( | ||
| HWND hwnd, | ||
| UINT message, | ||
| WPARAM wparam, |
There was a problem hiding this comment.
Just a nit but can we give these parameters better names? wparam and lparam is a bit terse. Same elsewhere.
There was a problem hiding this comment.
Not really; this is just how WindowProc works. This is what they are called in the Win32 APIs, and the entire description for both is:
Additional message information. The contents of this parameter depend on the value of the uMsg parameter.
Because the meaning is wildly different depending on the message, there's no meaningful name to give them, and anything other than the names that are always used for them in WindowProc code would just be more confusing.
| LRESULT result; | ||
| bool handled = FlutterDesktopViewControllerHandleTopLevelWindowProc( | ||
| controller_, hwnd, message, wparam, lparam, &result); | ||
| return handled ? result : std::optional<LRESULT>(); |
There was a problem hiding this comment.
Nit: std::nullopt there please. I am never sure whether this means the same same as std::nullopt or an optional with a default initialized value. Maybe that's just me.
There was a problem hiding this comment.
The default constructor is empty; it's the same as nullopt. It seems logical to me (just as, e.g., a std::unique_ptr's default constructor is empty, not a default-initialized object of the pointer's type), but I'm fine with making it explicit.
| // - The application may choose not to delegate WindowProc calls. | ||
| // - If multiple plugins are registered, the first one that returns a value | ||
| // from the delegate message will "win", and others will not be called. | ||
| // The order of delegate calls is not defined. |
There was a problem hiding this comment.
This seems like an unnecessary restriction that can easily be lifted if the Compare template parameter for the map below was specified. Saying either the first or last one wins ought to be a much less surprising API behavior.
There was a problem hiding this comment.
It's not that simple. Plugins on Windows are DLLs (to minimize the possibility of problematic interactions), so in practice each plugin has its own instance of this class, and changing the sort behavior here would do nothing (unless a single plugin registered multiple delegates, which is possible, but unlikely). It's actually the sort order in the engine that would matter most of the time. I could define it there, but then there are bizarre edge cases like: plugin A registers a handler, then plugin B registers a handler, then plugin A registers another handler. Without retooling the API to be more complex just to handle that case, the order of calls would actually be A1 A2 B, not A1 B A2.
I also don't really see that there's any value in such a guarantee, because in practice "first" or "last" aren't really under the control of a plugin developer. Assuming people will generally register this on plugin instantiation, that means that if we make this fully defined it devolves to the order of plugin registration. That's not under the control of a plugin developer, and in practice we don't make any guarantees about that. And even if we did (which I actually filed a bug about, because it's annoying if generated code that's checked in changes for no good reason, which happened recently), then we're basically saying that WindowProc handling is decided by alphabetical ordering of plugin names, which is weird, and could have the effect of people picking odd names for plugins just to try to win conflicts.
Assuming anyone actually tested the interaction between their plugin and other plugins, which I think is going to be very rare at best.
tl;dr: It's non-trivial added complexity, and I don't think it has any real value.
Description
Adds APIs for runners to delegate WindowProc handlers into the Flutter
engine, and for plugins to register as possible delegates.
This allows for plugins to alter top-level window behavior in ways that
can only be done from the WindowProc, such as resize control. This
functionality remains entirely on the native side, so is synchronous.
Related Issues
Part of flutter/flutter#53168
Tests
I added the following tests: Unit tests for the registration and dispatch objects at both the wrapper and implementation levels.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.Breaking Change
Did any tests fail when you ran them? Please read handling breaking changes.