From bb8679e23f854511d95c9c69936bd88d197f61cf Mon Sep 17 00:00:00 2001 From: Robert Ancell Date: Wed, 23 Oct 2024 22:05:09 +1300 Subject: [PATCH] Move send_key_event from FlKeyboardViewDelegate to FlKeyboardManager. (flutter/engine#56020) Ideally the tests would mock FlEngine, but I wasn't able to get it working so I've added fl_keyboard_manager_set_send_key_event_handler for now. --- .../platform/linux/fl_keyboard_manager.cc | 36 ++++- .../platform/linux/fl_keyboard_manager.h | 23 ++- .../linux/fl_keyboard_manager_test.cc | 137 ++++++++---------- .../linux/fl_keyboard_view_delegate.cc | 11 -- .../linux/fl_keyboard_view_delegate.h | 22 --- .../flutter/shell/platform/linux/fl_view.cc | 11 +- 6 files changed, 113 insertions(+), 127 deletions(-) diff --git a/engine/src/flutter/shell/platform/linux/fl_keyboard_manager.cc b/engine/src/flutter/shell/platform/linux/fl_keyboard_manager.cc index 2f6f240a0a..53e950a21f 100644 --- a/engine/src/flutter/shell/platform/linux/fl_keyboard_manager.cc +++ b/engine/src/flutter/shell/platform/linux/fl_keyboard_manager.cc @@ -116,8 +116,13 @@ static FlKeyboardManagerUserData* fl_keyboard_manager_user_data_new( struct _FlKeyboardManager { GObject parent_instance; + GWeakRef engine; + GWeakRef view_delegate; + FlKeyboardManagerSendKeyEventHandler send_key_event_handler; + gpointer send_key_event_handler_user_data; + FlKeyboardManagerLookupKeyHandler lookup_key_handler; gpointer lookup_key_handler_user_data; @@ -410,6 +415,7 @@ static void guarantee_layout(FlKeyboardManager* self, FlKeyEvent* event) { static void fl_keyboard_manager_dispose(GObject* object) { FlKeyboardManager* self = FL_KEYBOARD_MANAGER(object); + g_weak_ref_clear(&self->engine); g_weak_ref_clear(&self->view_delegate); self->keycode_to_goals.reset(); @@ -458,29 +464,34 @@ static void fl_keyboard_manager_init(FlKeyboardManager* self) { } FlKeyboardManager* fl_keyboard_manager_new( - FlBinaryMessenger* messenger, + FlEngine* engine, FlKeyboardViewDelegate* view_delegate) { g_return_val_if_fail(FL_IS_KEYBOARD_VIEW_DELEGATE(view_delegate), nullptr); FlKeyboardManager* self = FL_KEYBOARD_MANAGER( g_object_new(fl_keyboard_manager_get_type(), nullptr)); + g_weak_ref_init(&self->engine, engine); g_weak_ref_init(&self->view_delegate, view_delegate); self->key_embedder_responder = fl_key_embedder_responder_new( [](const FlutterKeyEvent* event, FlutterKeyEventCallback callback, void* callback_user_data, void* send_key_event_user_data) { FlKeyboardManager* self = FL_KEYBOARD_MANAGER(send_key_event_user_data); - g_autoptr(FlKeyboardViewDelegate) view_delegate = - FL_KEYBOARD_VIEW_DELEGATE(g_weak_ref_get(&self->view_delegate)); - if (view_delegate == nullptr) { - return; + if (self->send_key_event_handler != nullptr) { + self->send_key_event_handler(event, callback, callback_user_data, + self->send_key_event_handler_user_data); + } else { + g_autoptr(FlEngine) engine = FL_ENGINE(g_weak_ref_get(&self->engine)); + if (engine != nullptr) { + fl_engine_send_key_event(engine, event, callback, + callback_user_data); + } } - fl_keyboard_view_delegate_send_key_event(view_delegate, event, callback, - callback_user_data); }, self); - self->key_channel_responder = fl_key_channel_responder_new(messenger); + self->key_channel_responder = + fl_key_channel_responder_new(fl_engine_get_binary_messenger(engine)); return self; } @@ -536,6 +547,15 @@ GHashTable* fl_keyboard_manager_get_pressed_state(FlKeyboardManager* self) { self->key_embedder_responder); } +void fl_keyboard_manager_set_send_key_event_handler( + FlKeyboardManager* self, + FlKeyboardManagerSendKeyEventHandler send_key_event_handler, + gpointer user_data) { + g_return_if_fail(FL_IS_KEYBOARD_MANAGER(self)); + self->send_key_event_handler = send_key_event_handler; + self->send_key_event_handler_user_data = user_data; +} + void fl_keyboard_manager_set_lookup_key_handler( FlKeyboardManager* self, FlKeyboardManagerLookupKeyHandler lookup_key_handler, diff --git a/engine/src/flutter/shell/platform/linux/fl_keyboard_manager.h b/engine/src/flutter/shell/platform/linux/fl_keyboard_manager.h index 9701342091..667fcb8b77 100644 --- a/engine/src/flutter/shell/platform/linux/fl_keyboard_manager.h +++ b/engine/src/flutter/shell/platform/linux/fl_keyboard_manager.h @@ -8,7 +8,7 @@ #include #include "flutter/shell/platform/linux/fl_keyboard_view_delegate.h" -#include "flutter/shell/platform/linux/public/flutter_linux/fl_binary_messenger.h" +#include "flutter/shell/platform/linux/public/flutter_linux/fl_engine.h" G_BEGIN_DECLS @@ -36,7 +36,7 @@ G_DECLARE_FINAL_TYPE(FlKeyboardManager, /** * fl_keyboard_manager_new: - * @messenger: an #FlBinaryMessenger. + * @engine: an #FlEngine. * @view_delegate: An interface that the manager requires to communicate with * the platform. Usually implemented by FlView. * @@ -45,7 +45,7 @@ G_DECLARE_FINAL_TYPE(FlKeyboardManager, * Returns: a new #FlKeyboardManager. */ FlKeyboardManager* fl_keyboard_manager_new( - FlBinaryMessenger* messenger, + FlEngine* engine, FlKeyboardViewDelegate* view_delegate); /** @@ -94,6 +94,23 @@ void fl_keyboard_manager_sync_modifier_if_needed(FlKeyboardManager* manager, */ GHashTable* fl_keyboard_manager_get_pressed_state(FlKeyboardManager* manager); +typedef void (*FlKeyboardManagerSendKeyEventHandler)( + const FlutterKeyEvent* event, + FlutterKeyEventCallback callback, + void* callback_user_data, + gpointer user_data); + +/** + * fl_keyboard_manager_set_send_key_event_handler: + * @manager: the #FlKeyboardManager self. + * + * Set the handler for sending events, for testing purposes only. + */ +void fl_keyboard_manager_set_send_key_event_handler( + FlKeyboardManager* manager, + FlKeyboardManagerSendKeyEventHandler send_key_event_handler, + gpointer user_data); + typedef guint (*FlKeyboardManagerLookupKeyHandler)(const GdkKeymapKey* key, gpointer user_data); diff --git a/engine/src/flutter/shell/platform/linux/fl_keyboard_manager_test.cc b/engine/src/flutter/shell/platform/linux/fl_keyboard_manager_test.cc index 7718acbad0..cdb22ff9cf 100644 --- a/engine/src/flutter/shell/platform/linux/fl_keyboard_manager_test.cc +++ b/engine/src/flutter/shell/platform/linux/fl_keyboard_manager_test.cc @@ -233,6 +233,10 @@ static void fl_mock_binary_messenger_set_warns_on_channel_overflow( // Mock implementation. Do nothing. } +static void fl_mock_binary_messenger_shutdown(FlBinaryMessenger* messenger) { + // Mock implementation. Do nothing. +} + static void fl_mock_key_binary_messenger_iface_init( FlBinaryMessengerInterface* iface) { iface->set_message_handler_on_channel = @@ -257,6 +261,7 @@ static void fl_mock_key_binary_messenger_iface_init( iface->resize_channel = fl_mock_binary_messenger_resize_channel; iface->set_warns_on_channel_overflow = fl_mock_binary_messenger_set_warns_on_channel_overflow; + iface->shutdown = fl_mock_binary_messenger_shutdown; } static FlMockKeyBinaryMessenger* fl_mock_key_binary_messenger_new() { @@ -279,9 +284,6 @@ static void fl_mock_key_binary_messenger_set_callback_handler( struct _FlMockViewDelegate { GObject parent_instance; - - FlMockKeyBinaryMessenger* messenger; - EmbedderCallHandler embedder_handler; bool text_filter_result; }; @@ -297,30 +299,7 @@ G_DEFINE_TYPE_WITH_CODE( static void fl_mock_view_delegate_init(FlMockViewDelegate* self) {} -static void fl_mock_view_delegate_dispose(GObject* object) { - FlMockViewDelegate* self = FL_MOCK_VIEW_DELEGATE(object); - - g_clear_object(&self->messenger); - - G_OBJECT_CLASS(fl_mock_view_delegate_parent_class)->dispose(object); -} - -static void fl_mock_view_delegate_class_init(FlMockViewDelegateClass* klass) { - G_OBJECT_CLASS(klass)->dispose = fl_mock_view_delegate_dispose; -} - -static void fl_mock_view_keyboard_send_key_event( - FlKeyboardViewDelegate* view_delegate, - const FlutterKeyEvent* event, - FlutterKeyEventCallback callback, - void* user_data) { - FlMockViewDelegate* self = FL_MOCK_VIEW_DELEGATE(view_delegate); - self->embedder_handler(event, [callback, user_data](bool handled) { - if (callback != nullptr) { - callback(handled, user_data); - } - }); -} +static void fl_mock_view_delegate_class_init(FlMockViewDelegateClass* klass) {} static gboolean fl_mock_view_keyboard_text_filter_key_press( FlKeyboardViewDelegate* view_delegate, @@ -331,7 +310,6 @@ static gboolean fl_mock_view_keyboard_text_filter_key_press( static void fl_mock_view_keyboard_delegate_iface_init( FlKeyboardViewDelegateInterface* iface) { - iface->send_key_event = fl_mock_view_keyboard_send_key_event; iface->text_filter_key_press = fl_mock_view_keyboard_text_filter_key_press; } @@ -342,16 +320,9 @@ static FlMockViewDelegate* fl_mock_view_delegate_new() { // Added to stop compiler complaining about an unused function. FL_IS_MOCK_VIEW_DELEGATE(self); - self->messenger = fl_mock_key_binary_messenger_new(); - return self; } -static void fl_mock_view_set_embedder_handler(FlMockViewDelegate* self, - EmbedderCallHandler handler) { - self->embedder_handler = std::move(handler); -} - static void fl_mock_view_set_text_filter_result(FlMockViewDelegate* self, bool result) { self->text_filter_result = result; @@ -362,14 +333,31 @@ static void fl_mock_view_set_text_filter_result(FlMockViewDelegate* self, class KeyboardTester { public: KeyboardTester() { + messenger_ = fl_mock_key_binary_messenger_new(); + view_ = fl_mock_view_delegate_new(); respondToEmbedderCallsWith(false); respondToChannelCallsWith(false); respondToTextInputWith(false); setLayout(kLayoutUs); - manager_ = fl_keyboard_manager_new(FL_BINARY_MESSENGER(view_->messenger), - FL_KEYBOARD_VIEW_DELEGATE(view_)); + engine_ = FL_ENGINE(g_object_new(fl_engine_get_type(), "binary-messenger", + FL_BINARY_MESSENGER(messenger_), nullptr)); + manager_ = + fl_keyboard_manager_new(engine_, FL_KEYBOARD_VIEW_DELEGATE(view_)); + fl_keyboard_manager_set_send_key_event_handler( + manager_, + [](const FlutterKeyEvent* event, FlutterKeyEventCallback callback, + void* callback_user_data, gpointer user_data) { + KeyboardTester* self = reinterpret_cast(user_data); + self->embedder_handler_(event, + [callback, callback_user_data](bool handled) { + if (callback != nullptr) { + callback(handled, callback_user_data); + } + }); + }, + this); fl_keyboard_manager_set_lookup_key_handler( manager_, [](const GdkKeymapKey* key, gpointer user_data) { @@ -394,6 +382,8 @@ class KeyboardTester { ~KeyboardTester() { g_clear_object(&view_); + g_clear_object(&messenger_); + g_clear_object(&engine_); g_clear_object(&manager_); g_clear_pointer(&redispatched_events_, g_ptr_array_unref); } @@ -432,53 +422,51 @@ class KeyboardTester { } void respondToEmbedderCallsWith(bool response) { - fl_mock_view_set_embedder_handler( - view_, [response, this](const FlutterKeyEvent* event, - const AsyncKeyCallback& callback) { - EXPECT_FALSE(during_redispatch_); - callback(response); - }); + embedder_handler_ = [response, this](const FlutterKeyEvent* event, + const AsyncKeyCallback& callback) { + EXPECT_FALSE(during_redispatch_); + callback(response); + }; } void recordEmbedderCallsTo(std::vector& storage) { - fl_mock_view_set_embedder_handler( - view_, [&storage, this](const FlutterKeyEvent* event, - AsyncKeyCallback callback) { - EXPECT_FALSE(during_redispatch_); - auto new_event = std::make_unique(*event); - char* new_event_character = cloneString(event->character); - new_event->character = new_event_character; - storage.push_back(CallRecord{ - .type = CallRecord::kKeyCallEmbedder, - .callback = std::move(callback), - .event = std::move(new_event), - .event_character = std::unique_ptr(new_event_character), - }); - }); + embedder_handler_ = [&storage, this](const FlutterKeyEvent* event, + AsyncKeyCallback callback) { + EXPECT_FALSE(during_redispatch_); + auto new_event = std::make_unique(*event); + char* new_event_character = cloneString(event->character); + new_event->character = new_event_character; + storage.push_back(CallRecord{ + .type = CallRecord::kKeyCallEmbedder, + .callback = std::move(callback), + .event = std::move(new_event), + .event_character = std::unique_ptr(new_event_character), + }); + }; } void respondToEmbedderCallsWithAndRecordsTo( bool response, std::vector& storage) { - fl_mock_view_set_embedder_handler( - view_, [&storage, response, this](const FlutterKeyEvent* event, - const AsyncKeyCallback& callback) { - EXPECT_FALSE(during_redispatch_); - auto new_event = std::make_unique(*event); - char* new_event_character = cloneString(event->character); - new_event->character = new_event_character; - storage.push_back(CallRecord{ - .type = CallRecord::kKeyCallEmbedder, - .event = std::move(new_event), - .event_character = std::unique_ptr(new_event_character), - }); - callback(response); - }); + embedder_handler_ = [&storage, response, this]( + const FlutterKeyEvent* event, + const AsyncKeyCallback& callback) { + EXPECT_FALSE(during_redispatch_); + auto new_event = std::make_unique(*event); + char* new_event_character = cloneString(event->character); + new_event->character = new_event_character; + storage.push_back(CallRecord{ + .type = CallRecord::kKeyCallEmbedder, + .event = std::move(new_event), + .event_character = std::unique_ptr(new_event_character), + }); + callback(response); + }; } void respondToChannelCallsWith(bool response) { fl_mock_key_binary_messenger_set_callback_handler( - view_->messenger, [response, this](const AsyncKeyCallback& callback) { + messenger_, [response, this](const AsyncKeyCallback& callback) { EXPECT_FALSE(during_redispatch_); callback(response); }); @@ -486,7 +474,7 @@ class KeyboardTester { void recordChannelCallsTo(std::vector& storage) { fl_mock_key_binary_messenger_set_callback_handler( - view_->messenger, [&storage, this](AsyncKeyCallback callback) { + messenger_, [&storage, this](AsyncKeyCallback callback) { EXPECT_FALSE(during_redispatch_); storage.push_back(CallRecord{ .type = CallRecord::kKeyCallChannel, @@ -519,10 +507,13 @@ class KeyboardTester { private: FlMockViewDelegate* view_; + FlMockKeyBinaryMessenger* messenger_ = nullptr; + FlEngine* engine_ = nullptr; FlKeyboardManager* manager_ = nullptr; GPtrArray* redispatched_events_ = nullptr; bool during_redispatch_ = false; const MockLayoutData* layout_data_; + EmbedderCallHandler embedder_handler_; static gboolean _flushChannelMessagesCb(gpointer data) { g_autoptr(GMainLoop) loop = reinterpret_cast(data); diff --git a/engine/src/flutter/shell/platform/linux/fl_keyboard_view_delegate.cc b/engine/src/flutter/shell/platform/linux/fl_keyboard_view_delegate.cc index effdd39808..22bb871495 100644 --- a/engine/src/flutter/shell/platform/linux/fl_keyboard_view_delegate.cc +++ b/engine/src/flutter/shell/platform/linux/fl_keyboard_view_delegate.cc @@ -11,17 +11,6 @@ G_DEFINE_INTERFACE(FlKeyboardViewDelegate, static void fl_keyboard_view_delegate_default_init( FlKeyboardViewDelegateInterface* iface) {} -void fl_keyboard_view_delegate_send_key_event(FlKeyboardViewDelegate* self, - const FlutterKeyEvent* event, - FlutterKeyEventCallback callback, - void* user_data) { - g_return_if_fail(FL_IS_KEYBOARD_VIEW_DELEGATE(self)); - g_return_if_fail(event != nullptr); - - FL_KEYBOARD_VIEW_DELEGATE_GET_IFACE(self)->send_key_event( - self, event, callback, user_data); -} - gboolean fl_keyboard_view_delegate_text_filter_key_press( FlKeyboardViewDelegate* self, FlKeyEvent* event) { diff --git a/engine/src/flutter/shell/platform/linux/fl_keyboard_view_delegate.h b/engine/src/flutter/shell/platform/linux/fl_keyboard_view_delegate.h index 4e7ca39069..faa8bbbe76 100644 --- a/engine/src/flutter/shell/platform/linux/fl_keyboard_view_delegate.h +++ b/engine/src/flutter/shell/platform/linux/fl_keyboard_view_delegate.h @@ -34,34 +34,12 @@ G_DECLARE_INTERFACE(FlKeyboardViewDelegate, struct _FlKeyboardViewDelegateInterface { GTypeInterface g_iface; - void (*send_key_event)(FlKeyboardViewDelegate* delegate, - const FlutterKeyEvent* event, - FlutterKeyEventCallback callback, - void* user_data); - gboolean (*text_filter_key_press)(FlKeyboardViewDelegate* delegate, FlKeyEvent* event); GHashTable* (*get_keyboard_state)(FlKeyboardViewDelegate* delegate); }; -/** - * fl_keyboard_view_delegate_send_key_event: - * - * Handles `FlKeyboardHandler`'s request to send a `FlutterKeyEvent` through the - * embedder API to the framework. - * - * The ownership of the `event` is kept by the keyboard handler, and the `event` - * might be immediately destroyed after this function returns. - * - * The `callback` must eventually be called exactly once with the event result - * and the `user_data`. - */ -void fl_keyboard_view_delegate_send_key_event(FlKeyboardViewDelegate* delegate, - const FlutterKeyEvent* event, - FlutterKeyEventCallback callback, - void* user_data); - /** * fl_keyboard_view_delegate_text_filter_key_press: * diff --git a/engine/src/flutter/shell/platform/linux/fl_view.cc b/engine/src/flutter/shell/platform/linux/fl_view.cc index 58d234601f..51950db817 100644 --- a/engine/src/flutter/shell/platform/linux/fl_view.cc +++ b/engine/src/flutter/shell/platform/linux/fl_view.cc @@ -151,7 +151,7 @@ static void init_keyboard(FlView* self) { fl_keyboard_handler_new(messenger, FL_KEYBOARD_VIEW_DELEGATE(self)); g_clear_object(&self->keyboard_manager); self->keyboard_manager = - fl_keyboard_manager_new(messenger, FL_KEYBOARD_VIEW_DELEGATE(self)); + fl_keyboard_manager_new(self->engine, FL_KEYBOARD_VIEW_DELEGATE(self)); } static void init_scrolling(FlView* self) { @@ -364,15 +364,6 @@ static void fl_view_plugin_registry_iface_init( static void fl_view_keyboard_delegate_iface_init( FlKeyboardViewDelegateInterface* iface) { - iface->send_key_event = - [](FlKeyboardViewDelegate* view_delegate, const FlutterKeyEvent* event, - FlutterKeyEventCallback callback, void* user_data) { - FlView* self = FL_VIEW(view_delegate); - if (self->engine != nullptr) { - fl_engine_send_key_event(self->engine, event, callback, user_data); - }; - }; - iface->text_filter_key_press = [](FlKeyboardViewDelegate* view_delegate, FlKeyEvent* event) { FlView* self = FL_VIEW(view_delegate);