From cd64782026bb5df59ceaf9eb270a7b33c5a943bb Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Sun, 26 Sep 2021 13:09:56 -0700 Subject: [PATCH 1/6] Restart --- shell/platform/embedder/embedder.cc | 8 ++++ shell/platform/embedder/embedder.h | 4 ++ .../embedder/platform_view_embedder.cc | 7 +++ .../embedder/platform_view_embedder.h | 5 ++ shell/platform/linux/fl_engine.cc | 37 +++++++++++++++ shell/platform/linux/fl_engine_private.h | 11 +++++ shell/platform/linux/fl_keyboard_manager.cc | 25 ++++------ .../linux/fl_keyboard_manager_test.cc | 33 +++++++++++-- shell/platform/linux/fl_view.cc | 47 +++++++++++++------ 9 files changed, 143 insertions(+), 34 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index e3aae4df40c4a..2cb4f67984d81 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -1168,6 +1168,13 @@ FlutterEngineResult FlutterEngineInitialize(size_t version, }; } + flutter::PlatformViewEmbedder::OnPreEngineRestartCallback on_pre_engine_restart_callback = nullptr; + if (SAFE_ACCESS(args, on_pre_engine_restart_callback, nullptr) != nullptr) { + on_pre_engine_restart_callback = [ptr = args->on_pre_engine_restart_callback, user_data]() { + return ptr(user_data); + }; + } + auto external_view_embedder_result = InferExternalViewEmbedderFromArgs(SAFE_ACCESS(args, compositor, nullptr)); if (external_view_embedder_result.second) { @@ -1182,6 +1189,7 @@ FlutterEngineResult FlutterEngineInitialize(size_t version, platform_message_response_callback, // vsync_callback, // compute_platform_resolved_locale_callback, // + on_pre_engine_restart_callback, // }; auto on_create_platform_view = InferPlatformViewCreationCallback( diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index 0a63b5580e680..3e3f25f9afa4f 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -313,6 +313,7 @@ typedef bool (*TextureFrameCallback)(void* /* user data */, size_t /* height */, FlutterOpenGLTexture* /* texture out */); typedef void (*VsyncCallback)(void* /* user data */, intptr_t /* baton */); +typedef void (*OnPreEngineRestartCallback)(void* /* user data */); /// A structure to represent the width and height. typedef struct { @@ -1577,6 +1578,9 @@ typedef struct { // or component name to embedder's logger. This string will be passed to to // callbacks on `log_message_callback`. Defaults to "flutter" if unspecified. const char* log_tag; + + /// TODO + OnPreEngineRestartCallback on_pre_engine_restart_callback; } FlutterProjectArgs; #ifndef FLUTTER_ENGINE_NO_PROTOTYPES diff --git a/shell/platform/embedder/platform_view_embedder.cc b/shell/platform/embedder/platform_view_embedder.cc index 2cd020719944d..2a2752446f5e2 100644 --- a/shell/platform/embedder/platform_view_embedder.cc +++ b/shell/platform/embedder/platform_view_embedder.cc @@ -130,4 +130,11 @@ PlatformViewEmbedder::ComputePlatformResolvedLocales( return out; } +// |PlatformView| +void PlatformViewEmbedder::OnPreEngineRestart() const { + if (platform_dispatch_table_.on_pre_engine_restart_callback != nullptr) { + platform_dispatch_table_.on_pre_engine_restart_callback(); + } +} + } // namespace flutter diff --git a/shell/platform/embedder/platform_view_embedder.h b/shell/platform/embedder/platform_view_embedder.h index 3ecc634b46b71..22f4d2dee6ea0 100644 --- a/shell/platform/embedder/platform_view_embedder.h +++ b/shell/platform/embedder/platform_view_embedder.h @@ -36,6 +36,7 @@ class PlatformViewEmbedder final : public PlatformView { using ComputePlatformResolvedLocaleCallback = std::function>( const std::vector& supported_locale_data)>; + using OnPreEngineRestartCallback = std::function; struct PlatformDispatchTable { UpdateSemanticsNodesCallback update_semantics_nodes_callback; // optional @@ -46,6 +47,7 @@ class PlatformViewEmbedder final : public PlatformView { VsyncWaiterEmbedder::VsyncCallback vsync_callback; // optional ComputePlatformResolvedLocaleCallback compute_platform_resolved_locale_callback; + OnPreEngineRestartCallback on_pre_engine_restart_callback; // optional }; // Create a platform view that sets up a software rasterizer. @@ -104,6 +106,9 @@ class PlatformViewEmbedder final : public PlatformView { // |PlatformView| std::unique_ptr CreateVSyncWaiter() override; + // |PlatformView| + void OnPreEngineRestart() const override; + // |PlatformView| std::unique_ptr> ComputePlatformResolvedLocales( const std::vector& supported_locale_data) override; diff --git a/shell/platform/linux/fl_engine.cc b/shell/platform/linux/fl_engine.cc index 05e3d084e3e9f..64675ae28b797 100644 --- a/shell/platform/linux/fl_engine.cc +++ b/shell/platform/linux/fl_engine.cc @@ -47,6 +47,11 @@ struct _FlEngine { FlEngineUpdateSemanticsNodeHandler update_semantics_node_handler; gpointer update_semantics_node_handler_data; GDestroyNotify update_semantics_node_handler_destroy_notify; + + // TODO + FlEngineOnPreEngineRestartHandler on_pre_engine_restart_handler; + gpointer on_pre_engine_restart_handler_data; + GDestroyNotify on_pre_engine_restart_handler_destroy_notify; }; G_DEFINE_QUARK(fl_engine_error_quark, fl_engine_error) @@ -283,6 +288,19 @@ static void fl_engine_update_semantics_node_cb(const FlutterSemanticsNode* node, } } +// TODO +static void fl_engine_on_pre_engine_restart_cb(void* user_data) { + FlEngine* self = FL_ENGINE(user_data); + + // Do more + + if (self->on_pre_engine_restart_handler != nullptr) { + self->on_pre_engine_restart_handler( + self, self->on_pre_engine_restart_handler_data); + } +} + + // Called when a response to a sent platform message is received from the // engine. static void fl_engine_platform_message_response_cb(const uint8_t* data, @@ -425,6 +443,7 @@ gboolean fl_engine_start(FlEngine* self, GError** error) { args.update_semantics_node_callback = fl_engine_update_semantics_node_cb; args.custom_task_runners = &custom_task_runners; args.shutdown_dart_vm_when_done = true; + args.on_pre_engine_restart_callback = fl_engine_on_pre_engine_restart_cb; args.dart_entrypoint_argc = dart_entrypoint_args != nullptr ? g_strv_length(dart_entrypoint_args) : 0; args.dart_entrypoint_argv = @@ -519,6 +538,24 @@ void fl_engine_set_update_semantics_node_handler( self->update_semantics_node_handler_destroy_notify = destroy_notify; } +void fl_engine_set_on_pre_engine_restart_handler( + FlEngine* self, + FlEngineOnPreEngineRestartHandler handler, + gpointer user_data, + GDestroyNotify destroy_notify) { + g_return_if_fail(FL_IS_ENGINE(self)); + + if (self->on_pre_engine_restart_handler_destroy_notify) { + self->on_pre_engine_restart_handler_destroy_notify( + self->on_pre_engine_restart_handler_data); + } + + self->on_pre_engine_restart_handler = handler; + self->on_pre_engine_restart_handler_data = user_data; + self->on_pre_engine_restart_handler_destroy_notify = destroy_notify; +} + + gboolean fl_engine_send_platform_message_response( FlEngine* self, const FlutterPlatformMessageResponseHandle* handle, diff --git a/shell/platform/linux/fl_engine_private.h b/shell/platform/linux/fl_engine_private.h index fdc35ac54b5c4..c7f80fde128db 100644 --- a/shell/platform/linux/fl_engine_private.h +++ b/shell/platform/linux/fl_engine_private.h @@ -58,6 +58,10 @@ typedef void (*FlEngineUpdateSemanticsNodeHandler)( const FlutterSemanticsNode* node, gpointer user_data); +// TODO +typedef void (*FlEngineOnPreEngineRestartHandler)(FlEngine* engine, + gpointer user_data); + /** * fl_engine_new: * @project: an #FlDartProject. @@ -115,6 +119,13 @@ void fl_engine_set_update_semantics_node_handler( gpointer user_data, GDestroyNotify destroy_notify); +// TODO +void fl_engine_set_on_pre_engine_restart_handler( + FlEngine* engine, + FlEngineOnPreEngineRestartHandler handler, + gpointer user_data, + GDestroyNotify destroy_notify); + /** * fl_engine_start: * @engine: an #FlEngine. diff --git a/shell/platform/linux/fl_keyboard_manager.cc b/shell/platform/linux/fl_keyboard_manager.cc index ad3dc7f6d102c..7ace3dd4e5aa3 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); @@ -343,7 +342,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 +398,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..f28bce2aa99a9 100644 --- a/shell/platform/linux/fl_keyboard_manager_test.cc +++ b/shell/platform/linux/fl_keyboard_manager_test.cc @@ -219,6 +219,30 @@ static void store_redispatched_event(gpointer event) { g_ptr_array_add(redispatched_events(), new_event); } +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 +337,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 +393,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 +461,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 +494,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 +520,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..94120a953db69 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,28 @@ 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))); +} + +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 +189,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 +220,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 +303,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); From 1b8d1e3fb6215ab19612efabc374c513a6909e8e Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Sun, 26 Sep 2021 13:12:38 -0700 Subject: [PATCH 2/6] Format --- shell/platform/embedder/embedder.cc | 9 +++++---- shell/platform/linux/fl_engine.cc | 2 -- shell/platform/linux/fl_keyboard_manager_test.cc | 1 - shell/platform/linux/fl_view.cc | 6 +++--- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 2cb4f67984d81..dfe40017dfb41 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -1168,11 +1168,12 @@ FlutterEngineResult FlutterEngineInitialize(size_t version, }; } - flutter::PlatformViewEmbedder::OnPreEngineRestartCallback on_pre_engine_restart_callback = nullptr; + flutter::PlatformViewEmbedder::OnPreEngineRestartCallback + on_pre_engine_restart_callback = nullptr; if (SAFE_ACCESS(args, on_pre_engine_restart_callback, nullptr) != nullptr) { - on_pre_engine_restart_callback = [ptr = args->on_pre_engine_restart_callback, user_data]() { - return ptr(user_data); - }; + on_pre_engine_restart_callback = [ptr = + args->on_pre_engine_restart_callback, + user_data]() { return ptr(user_data); }; } auto external_view_embedder_result = diff --git a/shell/platform/linux/fl_engine.cc b/shell/platform/linux/fl_engine.cc index 64675ae28b797..c5334e378477a 100644 --- a/shell/platform/linux/fl_engine.cc +++ b/shell/platform/linux/fl_engine.cc @@ -300,7 +300,6 @@ static void fl_engine_on_pre_engine_restart_cb(void* user_data) { } } - // Called when a response to a sent platform message is received from the // engine. static void fl_engine_platform_message_response_cb(const uint8_t* data, @@ -555,7 +554,6 @@ void fl_engine_set_on_pre_engine_restart_handler( self->on_pre_engine_restart_handler_destroy_notify = destroy_notify; } - gboolean fl_engine_send_platform_message_response( FlEngine* self, const FlutterPlatformMessageResponseHandle* handle, diff --git a/shell/platform/linux/fl_keyboard_manager_test.cc b/shell/platform/linux/fl_keyboard_manager_test.cc index f28bce2aa99a9..7c0d9de4e8beb 100644 --- a/shell/platform/linux/fl_keyboard_manager_test.cc +++ b/shell/platform/linux/fl_keyboard_manager_test.cc @@ -242,7 +242,6 @@ TEST(FlKeyboardManagerTest, DisposeWithUnresolvedPends) { g_ptr_array_unref(call_records); } - TEST(FlKeyboardManagerTest, SingleDelegateWithAsyncResponds) { GPtrArray* call_records = g_ptr_array_new_with_free_func(g_object_unref); FlKeyboardCallRecord* record; diff --git a/shell/platform/linux/fl_view.cc b/shell/platform/linux/fl_view.cc index 94120a953db69..4eae550e47be8 100644 --- a/shell/platform/linux/fl_view.cc +++ b/shell/platform/linux/fl_view.cc @@ -91,8 +91,7 @@ static void fl_view_update_semantics_node_cb(FlEngine* engine, 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), + 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( @@ -103,7 +102,8 @@ static void fl_view_init_keyboard(FlView* self) { FL_KEY_RESPONDER(fl_key_channel_responder_new(messenger))); } -static void fl_view_on_pre_engine_restart_cb(FlEngine* engine, gpointer user_data) { +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); From 765d415c32b2e46dc78ea8a0b5b56f7676e5bdf4 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 28 Sep 2021 13:46:14 -0700 Subject: [PATCH 3/6] Doc --- shell/platform/embedder/embedder.h | 9 ++++++++- shell/platform/linux/fl_engine.cc | 10 ++++++---- shell/platform/linux/fl_engine_private.h | 19 +++++++++++++++++-- 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index 3e3f25f9afa4f..d12d2a5a396fe 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -1579,7 +1579,14 @@ typedef struct { // callbacks on `log_message_callback`. Defaults to "flutter" if unspecified. const char* log_tag; - /// TODO + // A callback that is invoked when the engine is restarted. + // + // This optional callback is typically used to reset states to as if the + // engine has just been started, and usually indicates the user has requested + // a hot restart (Shift-R in the Flutter CLI.) It is not called the first time + // the engine starts. + // + // The first argument is the `user_data` from `FlutterEngineInitialize`. OnPreEngineRestartCallback on_pre_engine_restart_callback; } FlutterProjectArgs; diff --git a/shell/platform/linux/fl_engine.cc b/shell/platform/linux/fl_engine.cc index c5334e378477a..59f0edb7c6292 100644 --- a/shell/platform/linux/fl_engine.cc +++ b/shell/platform/linux/fl_engine.cc @@ -48,7 +48,7 @@ struct _FlEngine { gpointer update_semantics_node_handler_data; GDestroyNotify update_semantics_node_handler_destroy_notify; - // TODO + // Function to call when the engine is restarted. FlEngineOnPreEngineRestartHandler on_pre_engine_restart_handler; gpointer on_pre_engine_restart_handler_data; GDestroyNotify on_pre_engine_restart_handler_destroy_notify; @@ -288,12 +288,14 @@ static void fl_engine_update_semantics_node_cb(const FlutterSemanticsNode* node, } } -// TODO +// Called when the engine is restarted. +// +// This method should reset states to as if the engine has just been started, +// and usually indicates the user has requested a hot restart (Shift-R in the +// Flutter CLI.) static void fl_engine_on_pre_engine_restart_cb(void* user_data) { FlEngine* self = FL_ENGINE(user_data); - // Do more - if (self->on_pre_engine_restart_handler != nullptr) { self->on_pre_engine_restart_handler( self, self->on_pre_engine_restart_handler_data); diff --git a/shell/platform/linux/fl_engine_private.h b/shell/platform/linux/fl_engine_private.h index c7f80fde128db..a576a64e9608c 100644 --- a/shell/platform/linux/fl_engine_private.h +++ b/shell/platform/linux/fl_engine_private.h @@ -58,7 +58,13 @@ typedef void (*FlEngineUpdateSemanticsNodeHandler)( const FlutterSemanticsNode* node, gpointer user_data); -// TODO +/** + * FlEngineOnPreEngineRestartHandler: + * @engine: an #FlEngine. + * @user_data: semantic node information. + * + * @user_data: (closure): data provided when registering this handler. + */ typedef void (*FlEngineOnPreEngineRestartHandler)(FlEngine* engine, gpointer user_data); @@ -119,7 +125,16 @@ void fl_engine_set_update_semantics_node_handler( gpointer user_data, GDestroyNotify destroy_notify); -// TODO +/** + * fl_engine_set_on_pre_engine_restart_handler: + * @engine: an #FlEngine. + * @handler: function to call when the engine is restarted. + * @user_data: (closure): user data to pass to @handler. + * @destroy_notify: (allow-none): a function which gets called to free + * @user_data, or %NULL. + * + * Registers the function called when the engine is restarted. + */ void fl_engine_set_on_pre_engine_restart_handler( FlEngine* engine, FlEngineOnPreEngineRestartHandler handler, From b721b6ddb06afa12e6bcfb3fa768517623319df6 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 28 Sep 2021 16:07:07 -0700 Subject: [PATCH 4/6] Test --- shell/common/platform_view.cc | 2 - shell/common/platform_view.h | 2 - shell/platform/linux/fl_engine.cc | 9 +++- shell/platform/linux/fl_engine_test.cc | 59 ++++++++++++++++++++++++++ shell/platform/linux/fl_view.cc | 5 +++ 5 files changed, 72 insertions(+), 5 deletions(-) diff --git a/shell/common/platform_view.cc b/shell/common/platform_view.cc index d48442604ff5c..cd3fada30afb4 100644 --- a/shell/common/platform_view.cc +++ b/shell/common/platform_view.cc @@ -8,8 +8,6 @@ #include "flutter/fml/make_copyable.h" #include "flutter/fml/synchronization/waitable_event.h" -#include "flutter/shell/common/rasterizer.h" -#include "flutter/shell/common/shell.h" #include "flutter/shell/common/vsync_waiter_fallback.h" #include "third_party/skia/include/gpu/gl/GrGLInterface.h" diff --git a/shell/common/platform_view.h b/shell/common/platform_view.h index 1cbf73ce4abea..48f4648d665ac 100644 --- a/shell/common/platform_view.h +++ b/shell/common/platform_view.h @@ -29,8 +29,6 @@ namespace flutter { -class Shell; - //------------------------------------------------------------------------------ /// @brief Platform views are created by the shell on the platform task /// runner. Unless explicitly specified, all platform view methods diff --git a/shell/platform/linux/fl_engine.cc b/shell/platform/linux/fl_engine.cc index 59f0edb7c6292..0033a3195dd17 100644 --- a/shell/platform/linux/fl_engine.cc +++ b/shell/platform/linux/fl_engine.cc @@ -291,7 +291,7 @@ static void fl_engine_update_semantics_node_cb(const FlutterSemanticsNode* node, // Called when the engine is restarted. // // This method should reset states to as if the engine has just been started, -// and usually indicates the user has requested a hot restart (Shift-R in the +// which usually indicates the user has requested a hot restart (Shift-R in the // Flutter CLI.) static void fl_engine_on_pre_engine_restart_cb(void* user_data) { FlEngine* self = FL_ENGINE(user_data); @@ -361,6 +361,13 @@ static void fl_engine_dispose(GObject* object) { self->update_semantics_node_handler_data = nullptr; self->update_semantics_node_handler_destroy_notify = nullptr; + if (self->on_pre_engine_restart_handler_destroy_notify) { + self->on_pre_engine_restart_handler_destroy_notify( + self->on_pre_engine_restart_handler_data); + } + self->on_pre_engine_restart_handler_data = nullptr; + self->on_pre_engine_restart_handler_destroy_notify = nullptr; + G_OBJECT_CLASS(fl_engine_parent_class)->dispose(object); } diff --git a/shell/platform/linux/fl_engine_test.cc b/shell/platform/linux/fl_engine_test.cc index 688e2e6883cc4..fb9454e1ab399 100644 --- a/shell/platform/linux/fl_engine_test.cc +++ b/shell/platform/linux/fl_engine_test.cc @@ -225,6 +225,65 @@ TEST(FlEngineTest, SettingsPlugin) { EXPECT_TRUE(called); } +void on_pre_engine_restart_cb(FlEngine* engine, gpointer user_data) { + int* count = reinterpret_cast(user_data); + *count += 1; +} + +void on_pre_engine_restart_destroy_notify(gpointer user_data) { + int* count = reinterpret_cast(user_data); + *count += 10; +} + +// Checks restarting the engine invokes the correct callback. +TEST(FlEngineTest, OnPreEngineRestart) { + g_autoptr(FlEngine) engine = make_mock_engine(); + FlutterEngineProcTable* embedder_api = fl_engine_get_embedder_api(engine); + + OnPreEngineRestartCallback callback; + void* callback_user_data; + + bool called = false; + embedder_api->Initialize = MOCK_ENGINE_PROC( + Initialize, ([&callback, &callback_user_data, &called]( + size_t version, const FlutterRendererConfig* config, + const FlutterProjectArgs* args, void* user_data, + FLUTTER_API_SYMBOL(FlutterEngine) * engine_out) { + called = true; + callback = args->on_pre_engine_restart_callback; + callback_user_data = user_data; + + return kSuccess; + })); + + g_autoptr(GError) error = nullptr; + EXPECT_TRUE(fl_engine_start(engine, &error)); + EXPECT_EQ(error, nullptr); + + EXPECT_TRUE(called); + EXPECT_NE(callback, nullptr); + + // The following call has no effect but should not crash. + callback(callback_user_data); + + int count = 0; + + // Set a handler, and the call should has an effect. + fl_engine_set_on_pre_engine_restart_handler( + engine, + on_pre_engine_restart_cb, + &count, + on_pre_engine_restart_destroy_notify + ); + + callback(callback_user_data); + EXPECT_EQ(count, 1); + + // Disposal should call the destroy notify. + g_object_unref(engine); + EXPECT_EQ(count, 11); +} + TEST(FlEngineTest, DartEntrypointArgs) { g_autoptr(FlDartProject) project = fl_dart_project_new(); diff --git a/shell/platform/linux/fl_view.cc b/shell/platform/linux/fl_view.cc index 4eae550e47be8..652c54742e26c 100644 --- a/shell/platform/linux/fl_view.cc +++ b/shell/platform/linux/fl_view.cc @@ -102,6 +102,11 @@ static void fl_view_init_keyboard(FlView* self) { FL_KEY_RESPONDER(fl_key_channel_responder_new(messenger))); } +// Called when the engine is restarted. +// +// This method should reset states to as if the engine has 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); From c94fb27cfdc5e56a38ef1f92a1dd190152f9fd56 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 28 Sep 2021 16:08:58 -0700 Subject: [PATCH 5/6] Format --- shell/platform/linux/fl_engine_test.cc | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/shell/platform/linux/fl_engine_test.cc b/shell/platform/linux/fl_engine_test.cc index fb9454e1ab399..082f936ee8fe7 100644 --- a/shell/platform/linux/fl_engine_test.cc +++ b/shell/platform/linux/fl_engine_test.cc @@ -270,11 +270,8 @@ TEST(FlEngineTest, OnPreEngineRestart) { // Set a handler, and the call should has an effect. fl_engine_set_on_pre_engine_restart_handler( - engine, - on_pre_engine_restart_cb, - &count, - on_pre_engine_restart_destroy_notify - ); + engine, on_pre_engine_restart_cb, &count, + on_pre_engine_restart_destroy_notify); callback(callback_user_data); EXPECT_EQ(count, 1); From ba2c93e826fdfb74f56c79f6a5176c9e627e68a5 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 28 Sep 2021 16:12:20 -0700 Subject: [PATCH 6/6] Revert keyboard --- shell/platform/linux/fl_keyboard_manager.cc | 25 +++++---- .../linux/fl_keyboard_manager_test.cc | 32 ++---------- shell/platform/linux/fl_view.cc | 52 ++++++------------- 3 files changed, 34 insertions(+), 75 deletions(-) diff --git a/shell/platform/linux/fl_keyboard_manager.cc b/shell/platform/linux/fl_keyboard_manager.cc index 7ace3dd4e5aa3..ad3dc7f6d102c 100644 --- a/shell/platform/linux/fl_keyboard_manager.cc +++ b/shell/platform/linux/fl_keyboard_manager.cc @@ -184,16 +184,13 @@ struct _FlKeyboardManager { // automatically released on dispose. GPtrArray* responder_list; - // 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. + // An array of #FlKeyboardPendingEvent. FlKeyboardManager must manually + // release the elements unless it is transferring them to + // pending_redispatches. GPtrArray* pending_responds; - // An array of #FlKeyboardPendingEvent. - // - // Its elements are unreferenced when removed. + // An array of #FlKeyboardPendingEvent. FlKeyboardManager must manually + // release the elements. GPtrArray* pending_redispatches; // The last sequence ID used. Increased by 1 by every use. @@ -210,14 +207,18 @@ 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, g_object_unref); + g_ptr_array_set_free_func(self->pending_responds, + fl_key_event_destroy_notify); 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); @@ -342,7 +343,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_with_free_func(g_object_unref); + self->pending_redispatches = g_ptr_array_new(); self->last_sequence_id = 1; @@ -398,3 +399,7 @@ 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 7c0d9de4e8beb..b822f30e52d04 100644 --- a/shell/platform/linux/fl_keyboard_manager_test.cc +++ b/shell/platform/linux/fl_keyboard_manager_test.cc @@ -219,29 +219,6 @@ static void store_redispatched_event(gpointer event) { g_ptr_array_add(redispatched_events(), new_event); } -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; @@ -336,7 +313,7 @@ TEST(FlKeyboardManagerTest, SingleDelegateWithAsyncResponds) { g_ptr_array_clear(redispatched_events()); EXPECT_TRUE(fl_keyboard_manager_is_state_clear(manager)); - g_ptr_array_unref(call_records); + g_ptr_array_clear(call_records); } TEST(FlKeyboardManagerTest, SingleDelegateWithSyncResponds) { @@ -392,7 +369,7 @@ TEST(FlKeyboardManagerTest, SingleDelegateWithSyncResponds) { EXPECT_TRUE(fl_keyboard_manager_is_state_clear(manager)); g_ptr_array_clear(redispatched_events()); - g_ptr_array_unref(call_records); + g_ptr_array_clear(call_records); } TEST(FlKeyboardManagerTest, WithTwoAsyncDelegates) { @@ -460,7 +437,6 @@ TEST(FlKeyboardManagerTest, WithTwoAsyncDelegates) { g_ptr_array_clear(call_records); g_ptr_array_clear(redispatched_events()); - g_ptr_array_unref(call_records); } TEST(FlKeyboardManagerTest, TextInputPluginReturnsFalse) { @@ -493,7 +469,7 @@ TEST(FlKeyboardManagerTest, TextInputPluginReturnsFalse) { g_ptr_array_clear(redispatched_events()); EXPECT_TRUE(fl_keyboard_manager_is_state_clear(manager)); - g_ptr_array_unref(call_records); + g_ptr_array_clear(call_records); } TEST(FlKeyboardManagerTest, TextInputPluginReturnsTrue) { @@ -519,7 +495,7 @@ TEST(FlKeyboardManagerTest, TextInputPluginReturnsTrue) { EXPECT_EQ(redispatched_events()->len, 0u); EXPECT_TRUE(fl_keyboard_manager_is_state_clear(manager)); - g_ptr_array_unref(call_records); + g_ptr_array_clear(call_records); } } // namespace diff --git a/shell/platform/linux/fl_view.cc b/shell/platform/linux/fl_view.cc index 652c54742e26c..01d25ea0cf764 100644 --- a/shell/platform/linux/fl_view.cc +++ b/shell/platform/linux/fl_view.cc @@ -67,11 +67,6 @@ 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, @@ -88,33 +83,6 @@ 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 as if the engine has 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) { @@ -194,6 +162,11 @@ 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); @@ -225,13 +198,20 @@ 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); - fl_view_init_keyboard(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))); self->mouse_cursor_plugin = fl_mouse_cursor_plugin_new(messenger, self); self->platform_plugin = fl_platform_plugin_new(messenger); @@ -308,8 +288,6 @@ 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);