diff --git a/shell/platform/linux/fl_keyboard_manager.cc b/shell/platform/linux/fl_keyboard_manager.cc index ad3dc7f6d102c..9926dd44caf63 100644 --- a/shell/platform/linux/fl_keyboard_manager.cc +++ b/shell/platform/linux/fl_keyboard_manager.cc @@ -184,13 +184,16 @@ struct _FlKeyboardManager { // automatically released on dispose. GPtrArray* responder_list; - // An array of #FlKeyboardPendingEvent. FlKeyboardManager must manually - // release the elements unless it is transferring them to - // pending_redispatches. + // An array of #FlKeyboardPendingEvent. + // + // Its elements are *not* unreferenced when removed. When FlKeyboardManager is + // disposed, this array will be set with a free_func so that the elements are + // unreferenced when removed. GPtrArray* pending_responds; - // An array of #FlKeyboardPendingEvent. FlKeyboardManager must manually - // release the elements. + // An array of #FlKeyboardPendingEvent. + // + // Its elements are unreferenced when removed. GPtrArray* pending_redispatches; // The last sequence ID used. Increased by 1 by every use. @@ -207,18 +210,14 @@ static void fl_keyboard_manager_class_init(FlKeyboardManagerClass* klass) { static void fl_keyboard_manager_init(FlKeyboardManager* self) {} -static void fl_key_event_destroy_notify(gpointer event); static void fl_keyboard_manager_dispose(GObject* object) { FlKeyboardManager* self = FL_KEYBOARD_MANAGER(object); if (self->text_input_plugin != nullptr) g_clear_object(&self->text_input_plugin); g_ptr_array_free(self->responder_list, TRUE); - g_ptr_array_set_free_func(self->pending_responds, - fl_key_event_destroy_notify); + g_ptr_array_set_free_func(self->pending_responds, g_object_unref); g_ptr_array_free(self->pending_responds, TRUE); - g_ptr_array_set_free_func(self->pending_redispatches, - fl_key_event_destroy_notify); g_ptr_array_free(self->pending_redispatches, TRUE); G_OBJECT_CLASS(fl_keyboard_manager_parent_class)->dispose(object); @@ -278,11 +277,8 @@ static bool fl_keyboard_manager_remove_redispatched(FlKeyboardManager* self, self->pending_redispatches, static_cast(&hash), compare_pending_by_hash, &result_index); if (found) { - FlKeyboardPendingEvent* removed = - FL_KEYBOARD_PENDING_EVENT(g_ptr_array_remove_index_fast( - self->pending_redispatches, result_index)); - g_return_val_if_fail(removed != nullptr, TRUE); - g_object_unref(removed); + // The removed object is freed due to `pending_redispatches`'s free_func. + g_ptr_array_remove_index_fast(self->pending_redispatches, result_index); return TRUE; } else { return FALSE; @@ -343,7 +339,7 @@ FlKeyboardManager* fl_keyboard_manager_new( self->responder_list = g_ptr_array_new_with_free_func(g_object_unref); self->pending_responds = g_ptr_array_new(); - self->pending_redispatches = g_ptr_array_new(); + self->pending_redispatches = g_ptr_array_new_with_free_func(g_object_unref); self->last_sequence_id = 1; @@ -399,7 +395,3 @@ gboolean fl_keyboard_manager_is_state_clear(FlKeyboardManager* self) { return self->pending_responds->len == 0 && self->pending_redispatches->len == 0; } - -static void fl_key_event_destroy_notify(gpointer event) { - fl_key_event_dispose(reinterpret_cast(event)); -} diff --git a/shell/platform/linux/fl_keyboard_manager_test.cc b/shell/platform/linux/fl_keyboard_manager_test.cc index b822f30e52d04..d42294f4cb453 100644 --- a/shell/platform/linux/fl_keyboard_manager_test.cc +++ b/shell/platform/linux/fl_keyboard_manager_test.cc @@ -219,6 +219,31 @@ static void store_redispatched_event(gpointer event) { g_ptr_array_add(redispatched_events(), new_event); } +// Make sure that the keyboard can be disposed without crashes when there are +// unresolved pending events. +TEST(FlKeyboardManagerTest, DisposeWithUnresolvedPends) { + FlKeyboardManager* manager = + fl_keyboard_manager_new(nullptr, store_redispatched_event); + + GPtrArray* call_records = g_ptr_array_new_with_free_func(g_object_unref); + FlKeyMockResponder* responder = fl_key_mock_responder_new(call_records, 1); + fl_keyboard_manager_add_responder(manager, FL_KEY_RESPONDER(responder)); + + responder->callback_handler = dont_respond; + fl_keyboard_manager_handle_event( + manager, + fl_key_event_new_by_mock(true, GDK_KEY_a, kKeyCodeKeyA, 0x10, false)); + + responder->callback_handler = respond_true; + fl_keyboard_manager_handle_event( + manager, + fl_key_event_new_by_mock(true, GDK_KEY_a, kKeyCodeKeyA, 0x10, false)); + + // Passes if the cleanup of `manager` does not crash. + g_object_unref(manager); + g_ptr_array_unref(call_records); +} + TEST(FlKeyboardManagerTest, SingleDelegateWithAsyncResponds) { GPtrArray* call_records = g_ptr_array_new_with_free_func(g_object_unref); FlKeyboardCallRecord* record; @@ -313,7 +338,7 @@ TEST(FlKeyboardManagerTest, SingleDelegateWithAsyncResponds) { g_ptr_array_clear(redispatched_events()); EXPECT_TRUE(fl_keyboard_manager_is_state_clear(manager)); - g_ptr_array_clear(call_records); + g_ptr_array_unref(call_records); } TEST(FlKeyboardManagerTest, SingleDelegateWithSyncResponds) { @@ -369,7 +394,7 @@ TEST(FlKeyboardManagerTest, SingleDelegateWithSyncResponds) { EXPECT_TRUE(fl_keyboard_manager_is_state_clear(manager)); g_ptr_array_clear(redispatched_events()); - g_ptr_array_clear(call_records); + g_ptr_array_unref(call_records); } TEST(FlKeyboardManagerTest, WithTwoAsyncDelegates) { @@ -437,6 +462,7 @@ TEST(FlKeyboardManagerTest, WithTwoAsyncDelegates) { g_ptr_array_clear(call_records); g_ptr_array_clear(redispatched_events()); + g_ptr_array_unref(call_records); } TEST(FlKeyboardManagerTest, TextInputPluginReturnsFalse) { @@ -469,7 +495,7 @@ TEST(FlKeyboardManagerTest, TextInputPluginReturnsFalse) { g_ptr_array_clear(redispatched_events()); EXPECT_TRUE(fl_keyboard_manager_is_state_clear(manager)); - g_ptr_array_clear(call_records); + g_ptr_array_unref(call_records); } TEST(FlKeyboardManagerTest, TextInputPluginReturnsTrue) { @@ -495,7 +521,7 @@ TEST(FlKeyboardManagerTest, TextInputPluginReturnsTrue) { EXPECT_EQ(redispatched_events()->len, 0u); EXPECT_TRUE(fl_keyboard_manager_is_state_clear(manager)); - g_ptr_array_clear(call_records); + g_ptr_array_unref(call_records); } } // namespace diff --git a/shell/platform/linux/fl_view.cc b/shell/platform/linux/fl_view.cc index 01d25ea0cf764..1fbdd633348ef 100644 --- a/shell/platform/linux/fl_view.cc +++ b/shell/platform/linux/fl_view.cc @@ -67,6 +67,11 @@ enum { PROP_FLUTTER_PROJECT = 1, PROP_LAST }; static void fl_view_plugin_registry_iface_init( FlPluginRegistryInterface* iface); +static gboolean text_input_im_filter_by_gtk(GtkIMContext* im_context, + gpointer gdk_event); + +static void redispatch_key_event_by_gtk(gpointer gdk_event); + G_DEFINE_TYPE_WITH_CODE( FlView, fl_view, @@ -83,6 +88,33 @@ static void fl_view_update_semantics_node_cb(FlEngine* engine, self->accessibility_plugin, node); } +static void fl_view_init_keyboard(FlView* self) { + FlBinaryMessenger* messenger = fl_engine_get_binary_messenger(self->engine); + self->keyboard_manager = fl_keyboard_manager_new( + fl_text_input_plugin_new(messenger, self, text_input_im_filter_by_gtk), + redispatch_key_event_by_gtk); + // The embedder responder must be added before the channel responder. + fl_keyboard_manager_add_responder( + self->keyboard_manager, + FL_KEY_RESPONDER(fl_key_embedder_responder_new(self->engine))); + fl_keyboard_manager_add_responder( + self->keyboard_manager, + FL_KEY_RESPONDER(fl_key_channel_responder_new(messenger))); +} + +// Called when the engine is restarted. +// +// This method should reset states to be as if the engine had just been started, +// which usually indicates the user has requested a hot restart (Shift-R in the +// Flutter CLI.) +static void fl_view_on_pre_engine_restart_cb(FlEngine* engine, + gpointer user_data) { + FlView* self = FL_VIEW(user_data); + + g_clear_object(&self->keyboard_manager); + fl_view_init_keyboard(self); +} + // Converts a GDK button event into a Flutter event and sends it to the engine. static gboolean fl_view_send_pointer_button_event(FlView* self, GdkEventButton* event) { @@ -162,11 +194,6 @@ static void fl_view_plugin_registry_iface_init( iface->get_registrar_for_plugin = fl_view_get_registrar_for_plugin; } -static void redispatch_key_event_by_gtk(gpointer gdk_event); - -static gboolean text_input_im_filter_by_gtk(GtkIMContext* im_context, - gpointer gdk_event); - static gboolean event_box_button_release_event(GtkWidget* widget, GdkEventButton* event, FlView* view); @@ -198,20 +225,13 @@ static void fl_view_constructed(GObject* object) { self->engine = fl_engine_new(self->project, self->renderer); fl_engine_set_update_semantics_node_handler( self->engine, fl_view_update_semantics_node_cb, self, nullptr); + fl_engine_set_on_pre_engine_restart_handler( + self->engine, fl_view_on_pre_engine_restart_cb, self, nullptr); // Create system channel handlers. FlBinaryMessenger* messenger = fl_engine_get_binary_messenger(self->engine); self->accessibility_plugin = fl_accessibility_plugin_new(self); - self->keyboard_manager = fl_keyboard_manager_new( - fl_text_input_plugin_new(messenger, self, text_input_im_filter_by_gtk), - redispatch_key_event_by_gtk); - // The embedder responder must be added before the channel responder. - fl_keyboard_manager_add_responder( - self->keyboard_manager, - FL_KEY_RESPONDER(fl_key_embedder_responder_new(self->engine))); - fl_keyboard_manager_add_responder( - self->keyboard_manager, - FL_KEY_RESPONDER(fl_key_channel_responder_new(messenger))); + fl_view_init_keyboard(self); self->mouse_cursor_plugin = fl_mouse_cursor_plugin_new(messenger, self); self->platform_plugin = fl_platform_plugin_new(messenger); @@ -288,6 +308,8 @@ static void fl_view_dispose(GObject* object) { if (self->engine != nullptr) { fl_engine_set_update_semantics_node_handler(self->engine, nullptr, nullptr, nullptr); + fl_engine_set_on_pre_engine_restart_handler(self->engine, nullptr, nullptr, + nullptr); } g_clear_object(&self->project);