From 099e6d39fe38dbda78dd5efb7d8424bf7fd845a2 Mon Sep 17 00:00:00 2001 From: Matthew Kosarek Date: Thu, 6 Mar 2025 08:42:52 -0500 Subject: [PATCH] [windows] wire the focus request and the focus events through the Windows platform (#164296) ## What's changed? - When a view is focused or unfocused, an event is now sent to the engine. This makes it so the proper view has focus when the corresponding window is focused. Thus, global shortcuts now work across views :tada: - Applications can request for a particular view to be focused, which causes the corresponding window to be focused - Wrote unit tests for all of this ## How To Test 1. Merge `windows/view_focus_event` into [canonical/foundation](https://github.com/canonical/flutter/tree/foundation) 2. Create a new app: ```dart import 'dart:ui'; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; void main() { final RegularWindowController controller1 = RegularWindowController(size: const Size(640, 480)); final RegularWindowController controller2 = RegularWindowController(size: const Size(640, 480)); runWidget(ViewCollection( views: [ RegularWindow(controller: controller1, child: MyApp(otherController: controller2)), RegularWindow(controller: controller2, child: MyApp(otherController: controller1)), ] )); } class IncrementIntent extends Intent { const IncrementIntent(); } class MyApp extends StatelessWidget { const MyApp({super.key, required this.otherController}); final RegularWindowController otherController; @override Widget build(BuildContext context) { return MaterialApp( title: 'Shortcut Counter', theme: ThemeData( primarySwatch: Colors.blue, ), home: CounterPage(otherController: otherController), ); } } class CounterPage extends StatefulWidget { const CounterPage({super.key, required this.otherController}); final RegularWindowController otherController; @override _CounterPageState createState() => _CounterPageState(); } class _CounterPageState extends State { int _counter = 0; void _incrementCounter() { setState(() { _counter++; }); } @override Widget build(BuildContext context) { return Shortcuts( shortcuts: { LogicalKeySet(LogicalKeyboardKey.space): const IncrementIntent(), }, child: Actions( actions: { IncrementIntent: CallbackAction(onInvoke: (intent) { _incrementCounter(); return null; }), }, child: Focus( autofocus: true, child: Scaffold( appBar: AppBar(title: const Text('Shortcut Counter')), body: Center( child: Column(children: [Text( 'Counter: $_counter', style: const TextStyle(fontSize: 24), ), OutlinedButton(onPressed: () { WidgetsBinding.instance.platformDispatcher.requestViewFocusChange( direction: ViewFocusDirection.forward, state: ViewFocusState.focused, viewId: widget.otherController.rootView.viewId, ); }, child: const Text('Focus other window'))]), ), ), ), ), ); } } ``` 3. Run with: ``` flutter run --debug --local-engine-src-path C:/dev/flutter/engine/src/ --local-engine host_debug_unopt --local-engine-host host_debug_unopt lib/main.dart --enable-multi-window ``` 4. Pressing spacebar while either window is focused should make the corresponding counter go up 5. Clicking the button on either window should make the other window become focused ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [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]. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [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 --- .../shell/platform/windows/flutter_window.cc | 24 ++++++++++++++ .../shell/platform/windows/flutter_window.h | 3 ++ .../windows/flutter_window_unittests.cc | 10 ++++++ .../windows/flutter_windows_engine.cc | 25 ++++++++++++++ .../platform/windows/flutter_windows_engine.h | 6 ++++ .../flutter_windows_engine_unittests.cc | 19 +++++++++++ .../platform/windows/flutter_windows_view.cc | 21 +++++++++++- .../platform/windows/flutter_windows_view.h | 12 +++++++ .../windows/flutter_windows_view_unittests.cc | 33 +++++++++++++++++++ .../windows/testing/engine_modifier.h | 4 +++ .../testing/mock_window_binding_handler.h | 1 + .../mock_window_binding_handler_delegate.h | 4 +++ .../platform/windows/window_binding_handler.h | 4 +++ .../windows/window_binding_handler_delegate.h | 7 ++++ 14 files changed, 172 insertions(+), 1 deletion(-) diff --git a/engine/src/flutter/shell/platform/windows/flutter_window.cc b/engine/src/flutter/shell/platform/windows/flutter_window.cc index 82e6cd9c48..fddba02679 100644 --- a/engine/src/flutter/shell/platform/windows/flutter_window.cc +++ b/engine/src/flutter/shell/platform/windows/flutter_window.cc @@ -185,6 +185,20 @@ void FlutterWindow::SetFlutterCursor(HCURSOR cursor) { ::SetCursor(current_cursor_); } +bool FlutterWindow::Focus() { + auto hwnd = GetWindowHandle(); + if (hwnd == nullptr) { + return false; + } + + HWND prevFocus = ::SetFocus(hwnd); + if (prevFocus == nullptr) { + return false; + } + + return true; +} + void FlutterWindow::OnDpiScale(unsigned int dpi) {}; // When DesktopWindow notifies that a WM_Size message has come in @@ -377,9 +391,19 @@ void FlutterWindow::OnWindowStateEvent(WindowStateEvent event) { break; case WindowStateEvent::kFocus: focused_ = true; + if (binding_handler_delegate_) { + binding_handler_delegate_->OnFocus( + FlutterViewFocusState::kFocused, + FlutterViewFocusDirection::kUndefined); + } break; case WindowStateEvent::kUnfocus: focused_ = false; + if (binding_handler_delegate_) { + binding_handler_delegate_->OnFocus( + FlutterViewFocusState::kUnfocused, + FlutterViewFocusDirection::kUndefined); + } break; } HWND hwnd = GetWindowHandle(); diff --git a/engine/src/flutter/shell/platform/windows/flutter_window.h b/engine/src/flutter/shell/platform/windows/flutter_window.h index b8560794ec..1ce6cd5987 100644 --- a/engine/src/flutter/shell/platform/windows/flutter_window.h +++ b/engine/src/flutter/shell/platform/windows/flutter_window.h @@ -187,6 +187,9 @@ class FlutterWindow : public KeyboardManager::WindowDelegate, // |WindowBindingHandler| virtual ui::AXPlatformNodeWin* GetAlert() override; + // [WindowBindingHandler] + virtual bool Focus() override; + // Called to obtain a pointer to the fragment root delegate. virtual ui::AXFragmentRootDelegateWin* GetAxFragmentRootDelegate(); diff --git a/engine/src/flutter/shell/platform/windows/flutter_window_unittests.cc b/engine/src/flutter/shell/platform/windows/flutter_window_unittests.cc index 4b6d076911..8f0d6578fe 100644 --- a/engine/src/flutter/shell/platform/windows/flutter_window_unittests.cc +++ b/engine/src/flutter/shell/platform/windows/flutter_window_unittests.cc @@ -18,6 +18,7 @@ namespace testing { using ::testing::_; using ::testing::AnyNumber; +using ::testing::Eq; using ::testing::Invoke; using ::testing::Return; @@ -372,9 +373,15 @@ TEST_F(FlutterWindowTest, LifecycleFocusMessages) { win32window.InjectWindowMessage(WM_SIZE, 0, MAKEWORD(1, 1)); EXPECT_EQ(last_event, WindowStateEvent::kShow); + EXPECT_CALL(delegate, OnFocus(Eq(FlutterViewFocusState::kFocused), + Eq(FlutterViewFocusDirection::kUndefined))) + .Times(1); win32window.InjectWindowMessage(WM_SETFOCUS, 0, 0); EXPECT_EQ(last_event, WindowStateEvent::kFocus); + EXPECT_CALL(delegate, OnFocus(Eq(FlutterViewFocusState::kUnfocused), + Eq(FlutterViewFocusDirection::kUndefined))) + .Times(1); win32window.InjectWindowMessage(WM_KILLFOCUS, 0, 0); EXPECT_EQ(last_event, WindowStateEvent::kUnfocus); } @@ -407,6 +414,9 @@ TEST_F(FlutterWindowTest, CachedLifecycleMessage) { } }); + EXPECT_CALL(delegate, OnFocus(Eq(FlutterViewFocusState::kFocused), + Eq(FlutterViewFocusDirection::kUndefined))) + .Times(1); win32window.SetView(&delegate); EXPECT_TRUE(focused); EXPECT_TRUE(restored); diff --git a/engine/src/flutter/shell/platform/windows/flutter_windows_engine.cc b/engine/src/flutter/shell/platform/windows/flutter_windows_engine.cc index 6f993167ad..2833fe0fc7 100644 --- a/engine/src/flutter/shell/platform/windows/flutter_windows_engine.cc +++ b/engine/src/flutter/shell/platform/windows/flutter_windows_engine.cc @@ -393,6 +393,11 @@ bool FlutterWindowsEngine::Run(std::string_view entrypoint) { SAFE_ACCESS(update, listening, false)); } }; + args.view_focus_change_request_callback = + [](const FlutterViewFocusChangeRequest* request, void* user_data) { + auto host = static_cast(user_data); + host->OnViewFocusChangeRequest(request); + }; args.custom_task_runners = &custom_task_runners; @@ -711,6 +716,13 @@ void FlutterWindowsEngine::SendKeyEvent(const FlutterKeyEvent& event, } } +void FlutterWindowsEngine::SendViewFocusEvent( + const FlutterViewFocusEvent& event) { + if (engine_) { + embedder_api_.SendViewFocusEvent(engine_, &event); + } +} + bool FlutterWindowsEngine::SendPlatformMessage( const char* channel, const uint8_t* message, @@ -998,6 +1010,19 @@ void FlutterWindowsEngine::OnChannelUpdate(std::string name, bool listening) { } } +void FlutterWindowsEngine::OnViewFocusChangeRequest( + const FlutterViewFocusChangeRequest* request) { + std::shared_lock read_lock(views_mutex_); + + auto iterator = views_.find(request->view_id); + if (iterator == views_.end()) { + return; + } + + FlutterWindowsView* view = iterator->second; + view->Focus(); +} + bool FlutterWindowsEngine::Present(const FlutterPresentViewInfo* info) { // This runs on the raster thread. Lock the views map for the entirety of the // present operation to block the platform thread from destroying the diff --git a/engine/src/flutter/shell/platform/windows/flutter_windows_engine.h b/engine/src/flutter/shell/platform/windows/flutter_windows_engine.h index a2a33e277b..8b68ee15f6 100644 --- a/engine/src/flutter/shell/platform/windows/flutter_windows_engine.h +++ b/engine/src/flutter/shell/platform/windows/flutter_windows_engine.h @@ -185,6 +185,9 @@ class FlutterWindowsEngine { FlutterKeyEventCallback callback, void* user_data); + // Informs the engine of an incoming focus event. + void SendViewFocusEvent(const FlutterViewFocusEvent& event); + KeyboardHandlerBase* keyboard_key_handler() { return keyboard_key_handler_.get(); } @@ -331,6 +334,9 @@ class FlutterWindowsEngine { // channel. virtual void OnChannelUpdate(std::string name, bool listening); + virtual void OnViewFocusChangeRequest( + const FlutterViewFocusChangeRequest* request); + private: // Allows swapping out embedder_api_ calls in tests. friend class EngineModifier; diff --git a/engine/src/flutter/shell/platform/windows/flutter_windows_engine_unittests.cc b/engine/src/flutter/shell/platform/windows/flutter_windows_engine_unittests.cc index 94e8452d4b..bda12c2674 100644 --- a/engine/src/flutter/shell/platform/windows/flutter_windows_engine_unittests.cc +++ b/engine/src/flutter/shell/platform/windows/flutter_windows_engine_unittests.cc @@ -99,6 +99,7 @@ TEST_F(FlutterWindowsEngineTest, RunDoesExpectedInitialization) { EXPECT_NE(args->update_semantics_callback2, nullptr); EXPECT_EQ(args->update_semantics_node_callback, nullptr); EXPECT_EQ(args->update_semantics_custom_action_callback, nullptr); + EXPECT_NE(args->view_focus_change_request_callback, nullptr); args->custom_task_runners->thread_priority_setter( FlutterThreadPriority::kRaster); @@ -661,6 +662,7 @@ class MockFlutterWindowsView : public FlutterWindowsView { (ui::AXPlatformNodeWin*, ax::mojom::Event), (override)); MOCK_METHOD(HWND, GetWindowHandle, (), (const, override)); + MOCK_METHOD(bool, Focus, (), (override)); private: FML_DISALLOW_COPY_AND_ASSIGN(MockFlutterWindowsView); @@ -1346,5 +1348,22 @@ TEST_F(FlutterWindowsEngineTest, MergedUIThread) { ASSERT_EQ(*ui_thread_id, std::this_thread::get_id()); } +TEST_F(FlutterWindowsEngineTest, OnViewFocusChangeRequest) { + FlutterWindowsEngineBuilder builder{GetContext()}; + std::unique_ptr engine = builder.Build(); + auto window_binding_handler = + std::make_unique<::testing::NiceMock>(); + MockFlutterWindowsView view(engine.get(), std::move(window_binding_handler)); + + EngineModifier modifier(engine.get()); + modifier.SetImplicitView(&view); + + FlutterViewFocusChangeRequest request; + request.view_id = kImplicitViewId; + + EXPECT_CALL(view, Focus()).WillOnce(Return(true)); + modifier.OnViewFocusChangeRequest(&request); +} + } // namespace testing } // namespace flutter diff --git a/engine/src/flutter/shell/platform/windows/flutter_windows_view.cc b/engine/src/flutter/shell/platform/windows/flutter_windows_view.cc index dec6a111be..7a99004891 100644 --- a/engine/src/flutter/shell/platform/windows/flutter_windows_view.cc +++ b/engine/src/flutter/shell/platform/windows/flutter_windows_view.cc @@ -315,6 +315,11 @@ void FlutterWindowsView::OnKey(int key, SendKey(key, scancode, action, character, extended, was_down, callback); } +void FlutterWindowsView::OnFocus(FlutterViewFocusState focus_state, + FlutterViewFocusDirection direction) { + SendFocus(focus_state, direction); +} + void FlutterWindowsView::OnComposeBegin() { SendComposeBegin(); } @@ -368,7 +373,7 @@ void FlutterWindowsView::OnResetImeComposing() { binding_handler_->OnResetImeComposing(); } -// Sends new size information to FlutterEngine. +// Sends new size information to FlutterEngine. void FlutterWindowsView::SendWindowMetrics(size_t width, size_t height, double pixel_ratio) const { @@ -557,6 +562,16 @@ void FlutterWindowsView::SendKey(int key, }); } +void FlutterWindowsView::SendFocus(FlutterViewFocusState focus_state, + FlutterViewFocusDirection direction) { + FlutterViewFocusEvent event = {}; + event.struct_size = sizeof(event); + event.view_id = view_id_; + event.state = focus_state; + event.direction = direction; + engine_->SendViewFocusEvent(event); +} + void FlutterWindowsView::SendComposeBegin() { engine_->text_input_plugin()->ComposeBeginHook(); } @@ -826,6 +841,10 @@ void FlutterWindowsView::OnWindowStateEvent(HWND hwnd, WindowStateEvent event) { engine_->OnWindowStateEvent(hwnd, event); } +bool FlutterWindowsView::Focus() { + return binding_handler_->Focus(); +} + bool FlutterWindowsView::NeedsVsync() const { // If the Desktop Window Manager composition is enabled, // the system itself synchronizes with vsync. diff --git a/engine/src/flutter/shell/platform/windows/flutter_windows_view.h b/engine/src/flutter/shell/platform/windows/flutter_windows_view.h index 4adb38bc07..98689daee2 100644 --- a/engine/src/flutter/shell/platform/windows/flutter_windows_view.h +++ b/engine/src/flutter/shell/platform/windows/flutter_windows_view.h @@ -190,6 +190,10 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate { bool was_down, KeyEventCallback callback) override; + // |WindowBindingHandlerDelegate| + void OnFocus(FlutterViewFocusState focus_state, + FlutterViewFocusDirection direction) override; + // |WindowBindingHandlerDelegate| void OnComposeBegin() override; @@ -246,6 +250,10 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate { // |WindowBindingHandlerDelegate| void OnWindowStateEvent(HWND hwnd, WindowStateEvent event) override; + // Focus the view. + // Returns true if the view was focused. + virtual bool Focus(); + protected: virtual void NotifyWinEventWrapper(ui::AXPlatformNodeWin* node, ax::mojom::Event event); @@ -352,6 +360,10 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate { bool was_down, KeyEventCallback callback); + // Reports a focus event to Flutter engine. + void SendFocus(FlutterViewFocusState focus_state, + FlutterViewFocusDirection direction); + // Reports an IME compose begin event. // // Triggered when the user begins editing composing text using a multi-step diff --git a/engine/src/flutter/shell/platform/windows/flutter_windows_view_unittests.cc b/engine/src/flutter/shell/platform/windows/flutter_windows_view_unittests.cc index ed83153cdb..e7ce8f6aaf 100644 --- a/engine/src/flutter/shell/platform/windows/flutter_windows_view_unittests.cc +++ b/engine/src/flutter/shell/platform/windows/flutter_windows_view_unittests.cc @@ -1668,5 +1668,38 @@ TEST(FlutterWindowsViewTest, UpdatesVSyncOnDwmUpdates) { } } +TEST(FlutterWindowsViewTest, FocusTriggersWindowFocus) { + std::unique_ptr engine = GetTestEngine(); + auto window_binding_handler = + std::make_unique>(); + EXPECT_CALL(*window_binding_handler, Focus()).WillOnce(Return(true)); + std::unique_ptr view = + engine->CreateView(std::move(window_binding_handler)); + EXPECT_TRUE(view->Focus()); +} + +TEST(FlutterWindowsViewTest, OnFocusTriggersSendFocusViewEvent) { + std::unique_ptr engine = GetTestEngine(); + auto window_binding_handler = + std::make_unique>(); + std::unique_ptr view = + engine->CreateView(std::move(window_binding_handler)); + + EngineModifier modifier(engine.get()); + bool received_focus_event = false; + modifier.embedder_api().SendViewFocusEvent = MOCK_ENGINE_PROC( + SendViewFocusEvent, [&](FLUTTER_API_SYMBOL(FlutterEngine) raw_engine, + FlutterViewFocusEvent const* event) { + EXPECT_EQ(event->state, FlutterViewFocusState::kFocused); + EXPECT_EQ(event->direction, FlutterViewFocusDirection::kUndefined); + EXPECT_EQ(event->view_id, view->view_id()); + EXPECT_EQ(event->struct_size, sizeof(FlutterViewFocusEvent)); + received_focus_event = true; + return kSuccess; + }); + view->OnFocus(FlutterViewFocusState::kFocused, + FlutterViewFocusDirection::kUndefined); + EXPECT_TRUE(received_focus_event); +} } // namespace testing } // namespace flutter diff --git a/engine/src/flutter/shell/platform/windows/testing/engine_modifier.h b/engine/src/flutter/shell/platform/windows/testing/engine_modifier.h index bbf9925592..7874937a92 100644 --- a/engine/src/flutter/shell/platform/windows/testing/engine_modifier.h +++ b/engine/src/flutter/shell/platform/windows/testing/engine_modifier.h @@ -87,6 +87,10 @@ class EngineModifier { engine_->platform_view_plugin_ = std::move(manager); } + void OnViewFocusChangeRequest(const FlutterViewFocusChangeRequest* request) { + engine_->OnViewFocusChangeRequest(request); + } + private: FlutterWindowsEngine* engine_; diff --git a/engine/src/flutter/shell/platform/windows/testing/mock_window_binding_handler.h b/engine/src/flutter/shell/platform/windows/testing/mock_window_binding_handler.h index b53cfcbc07..9524428500 100644 --- a/engine/src/flutter/shell/platform/windows/testing/mock_window_binding_handler.h +++ b/engine/src/flutter/shell/platform/windows/testing/mock_window_binding_handler.h @@ -38,6 +38,7 @@ class MockWindowBindingHandler : public WindowBindingHandler { MOCK_METHOD(PointerLocation, GetPrimaryPointerLocation, (), (override)); MOCK_METHOD(AlertPlatformNodeDelegate*, GetAlertDelegate, (), (override)); MOCK_METHOD(ui::AXPlatformNodeWin*, GetAlert, (), (override)); + MOCK_METHOD(bool, Focus, (), (override)); private: FML_DISALLOW_COPY_AND_ASSIGN(MockWindowBindingHandler); diff --git a/engine/src/flutter/shell/platform/windows/testing/mock_window_binding_handler_delegate.h b/engine/src/flutter/shell/platform/windows/testing/mock_window_binding_handler_delegate.h index 9cc0ff4d47..7d50fba84d 100644 --- a/engine/src/flutter/shell/platform/windows/testing/mock_window_binding_handler_delegate.h +++ b/engine/src/flutter/shell/platform/windows/testing/mock_window_binding_handler_delegate.h @@ -53,6 +53,10 @@ class MockWindowBindingHandlerDelegate : public WindowBindingHandlerDelegate { OnKey, (int, int, int, char32_t, bool, bool, KeyEventCallback), (override)); + MOCK_METHOD(void, + OnFocus, + (FlutterViewFocusState, FlutterViewFocusDirection), + (override)); MOCK_METHOD(void, OnComposeBegin, (), (override)); MOCK_METHOD(void, OnComposeCommit, (), (override)); MOCK_METHOD(void, OnComposeEnd, (), (override)); diff --git a/engine/src/flutter/shell/platform/windows/window_binding_handler.h b/engine/src/flutter/shell/platform/windows/window_binding_handler.h index d0ef4565ba..bfd24e0871 100644 --- a/engine/src/flutter/shell/platform/windows/window_binding_handler.h +++ b/engine/src/flutter/shell/platform/windows/window_binding_handler.h @@ -90,6 +90,10 @@ class WindowBindingHandler { // Retrieve the alert node. virtual ui::AXPlatformNodeWin* GetAlert() = 0; + + // Focuses the current window. + // Returns true if the window was successfully focused. + virtual bool Focus() = 0; }; } // namespace flutter diff --git a/engine/src/flutter/shell/platform/windows/window_binding_handler_delegate.h b/engine/src/flutter/shell/platform/windows/window_binding_handler_delegate.h index 64694c9c1a..0245a74228 100644 --- a/engine/src/flutter/shell/platform/windows/window_binding_handler_delegate.h +++ b/engine/src/flutter/shell/platform/windows/window_binding_handler_delegate.h @@ -93,6 +93,13 @@ class WindowBindingHandlerDelegate { bool was_down, KeyEventCallback callback) = 0; + /// Notifies the delegate that the backing window has received or + /// lost focus. + /// + /// Typically called by currently configured WindowBindingHandler. + virtual void OnFocus(FlutterViewFocusState focus_state, + FlutterViewFocusDirection direction) = 0; + // Notifies the delegate that IME composing mode has begun. // // Triggered when the user begins editing composing text using a multi-step