From 79b136534a5255a3a6849ecf0f928d85fce2cd29 Mon Sep 17 00:00:00 2001 From: Robert Ancell Date: Wed, 27 May 2020 13:38:45 +1200 Subject: [PATCH] Add GDestroyNotify callbacks on handlers (#18546) --- .../linux/fl_basic_message_channel.cc | 45 +++++++++++++++++-- shell/platform/linux/fl_binary_messenger.cc | 30 ++++++++++--- shell/platform/linux/fl_engine.cc | 15 ++++++- shell/platform/linux/fl_engine_private.h | 5 ++- shell/platform/linux/fl_method_channel.cc | 44 ++++++++++++++++-- .../flutter_linux/fl_basic_message_channel.h | 15 +++++-- .../flutter_linux/fl_binary_messenger.h | 13 ++++-- .../public/flutter_linux/fl_method_channel.h | 18 +++++--- 8 files changed, 160 insertions(+), 25 deletions(-) diff --git a/shell/platform/linux/fl_basic_message_channel.cc b/shell/platform/linux/fl_basic_message_channel.cc index 0c27513f5..be5ad98b1 100644 --- a/shell/platform/linux/fl_basic_message_channel.cc +++ b/shell/platform/linux/fl_basic_message_channel.cc @@ -12,6 +12,9 @@ struct _FlBasicMessageChannel { // Messenger to communicate on FlBinaryMessenger* messenger; + // TRUE if the channel has been closed + gboolean channel_closed; + // Channel name gchar* name; @@ -21,6 +24,7 @@ struct _FlBasicMessageChannel { // Function called when a message is received FlBasicMessageChannelMessageHandler message_handler; gpointer message_handler_data; + GDestroyNotify message_handler_destroy_notify; }; struct _FlBasicMessageChannelResponseHandle { @@ -106,17 +110,37 @@ static void message_response_cb(GObject* object, g_task_return_pointer(task, result, g_object_unref); } +// Called when the channel handler is closed +static void channel_closed_cb(gpointer user_data) { + g_autoptr(FlBasicMessageChannel) self = FL_BASIC_MESSAGE_CHANNEL(user_data); + + self->channel_closed = TRUE; + + // Disconnect handler + if (self->message_handler_destroy_notify != nullptr) + self->message_handler_destroy_notify(self->message_handler_data); + self->message_handler = nullptr; + self->message_handler_data = nullptr; + self->message_handler_destroy_notify = nullptr; +} + static void fl_basic_message_channel_dispose(GObject* object) { FlBasicMessageChannel* self = FL_BASIC_MESSAGE_CHANNEL(object); if (self->messenger != nullptr) fl_binary_messenger_set_message_handler_on_channel( - self->messenger, self->name, nullptr, nullptr); + self->messenger, self->name, nullptr, nullptr, nullptr); g_clear_object(&self->messenger); g_clear_pointer(&self->name, g_free); g_clear_object(&self->codec); + if (self->message_handler_destroy_notify != nullptr) + self->message_handler_destroy_notify(self->message_handler_data); + self->message_handler = nullptr; + self->message_handler_data = nullptr; + self->message_handler_destroy_notify = nullptr; + G_OBJECT_CLASS(fl_basic_message_channel_parent_class)->dispose(object); } @@ -143,7 +167,8 @@ G_MODULE_EXPORT FlBasicMessageChannel* fl_basic_message_channel_new( self->codec = FL_MESSAGE_CODEC(g_object_ref(codec)); fl_binary_messenger_set_message_handler_on_channel( - self->messenger, self->name, message_cb, self); + self->messenger, self->name, message_cb, g_object_ref(self), + channel_closed_cb); return self; } @@ -151,11 +176,25 @@ G_MODULE_EXPORT FlBasicMessageChannel* fl_basic_message_channel_new( G_MODULE_EXPORT void fl_basic_message_channel_set_message_handler( FlBasicMessageChannel* self, FlBasicMessageChannelMessageHandler handler, - gpointer user_data) { + gpointer user_data, + GDestroyNotify destroy_notify) { g_return_if_fail(FL_IS_BASIC_MESSAGE_CHANNEL(self)); + // Don't set handler if channel closed + if (self->channel_closed) { + g_warning( + "Attempted to set message handler on closed FlBasicMessageChannel"); + if (destroy_notify != nullptr) + destroy_notify(user_data); + return; + } + + if (self->message_handler_destroy_notify != nullptr) + self->message_handler_destroy_notify(self->message_handler_data); + self->message_handler = handler; self->message_handler_data = user_data; + self->message_handler_destroy_notify = destroy_notify; } G_MODULE_EXPORT gboolean fl_basic_message_channel_respond( diff --git a/shell/platform/linux/fl_binary_messenger.cc b/shell/platform/linux/fl_binary_messenger.cc index 319c38b95..50a8c21f9 100644 --- a/shell/platform/linux/fl_binary_messenger.cc +++ b/shell/platform/linux/fl_binary_messenger.cc @@ -76,26 +76,34 @@ static FlBinaryMessengerResponseHandle* fl_binary_messenger_response_handle_new( typedef struct { FlBinaryMessengerMessageHandler message_handler; gpointer message_handler_data; + GDestroyNotify message_handler_destroy_notify; } PlatformMessageHandler; static PlatformMessageHandler* platform_message_handler_new( FlBinaryMessengerMessageHandler handler, - gpointer user_data) { + gpointer user_data, + GDestroyNotify destroy_notify) { PlatformMessageHandler* self = static_cast( g_malloc0(sizeof(PlatformMessageHandler))); self->message_handler = handler; self->message_handler_data = user_data; + self->message_handler_destroy_notify = destroy_notify; return self; } static void platform_message_handler_free(gpointer data) { PlatformMessageHandler* self = static_cast(data); + if (self->message_handler_destroy_notify) + self->message_handler_destroy_notify(self->message_handler_data); g_free(self); } static void engine_weak_notify_cb(gpointer user_data, GObject* object) { FlBinaryMessenger* self = FL_BINARY_MESSENGER(user_data); self->engine = nullptr; + + // Disconnect any handlers + g_hash_table_remove_all(self->platform_message_handlers); } static gboolean fl_binary_messenger_platform_message_cb( @@ -151,7 +159,7 @@ FlBinaryMessenger* fl_binary_messenger_new(FlEngine* engine) { g_object_weak_ref(G_OBJECT(engine), engine_weak_notify_cb, self); fl_engine_set_platform_message_handler( - engine, fl_binary_messenger_platform_message_cb, self); + engine, fl_binary_messenger_platform_message_cb, self, NULL); return self; } @@ -160,13 +168,25 @@ G_MODULE_EXPORT void fl_binary_messenger_set_message_handler_on_channel( FlBinaryMessenger* self, const gchar* channel, FlBinaryMessengerMessageHandler handler, - gpointer user_data) { + gpointer user_data, + GDestroyNotify destroy_notify) { g_return_if_fail(FL_IS_BINARY_MESSENGER(self)); g_return_if_fail(channel != nullptr); + // Don't set handlers if engine already gone + if (self->engine == nullptr) { + g_warning( + "Attempted to set message handler on closed FlBinaryMessenger without " + "engine"); + if (destroy_notify != nullptr) + destroy_notify(user_data); + return; + } + if (handler != nullptr) - g_hash_table_replace(self->platform_message_handlers, g_strdup(channel), - platform_message_handler_new(handler, user_data)); + g_hash_table_replace( + self->platform_message_handlers, g_strdup(channel), + platform_message_handler_new(handler, user_data, destroy_notify)); else g_hash_table_remove(self->platform_message_handlers, channel); } diff --git a/shell/platform/linux/fl_engine.cc b/shell/platform/linux/fl_engine.cc index d40b14da1..df27c56c8 100644 --- a/shell/platform/linux/fl_engine.cc +++ b/shell/platform/linux/fl_engine.cc @@ -29,6 +29,7 @@ struct _FlEngine { // Function to call when a platform message is received FlEnginePlatformMessageHandler platform_message_handler; gpointer platform_message_handler_data; + GDestroyNotify platform_message_handler_destroy_notify; }; G_DEFINE_QUARK(fl_engine_error_quark, fl_engine_error) @@ -161,6 +162,12 @@ static void fl_engine_dispose(GObject* object) { g_clear_object(&self->renderer); g_clear_object(&self->binary_messenger); + if (self->platform_message_handler_destroy_notify) + self->platform_message_handler_destroy_notify( + self->platform_message_handler_data); + self->platform_message_handler_data = nullptr; + self->platform_message_handler_destroy_notify = nullptr; + G_OBJECT_CLASS(fl_engine_parent_class)->dispose(object); } @@ -240,12 +247,18 @@ gboolean fl_engine_start(FlEngine* self, GError** error) { void fl_engine_set_platform_message_handler( FlEngine* self, FlEnginePlatformMessageHandler handler, - gpointer user_data) { + gpointer user_data, + GDestroyNotify destroy_notify) { g_return_if_fail(FL_IS_ENGINE(self)); g_return_if_fail(handler != nullptr); + if (self->platform_message_handler_destroy_notify) + self->platform_message_handler_destroy_notify( + self->platform_message_handler_data); + self->platform_message_handler = handler; self->platform_message_handler_data = user_data; + self->platform_message_handler_destroy_notify = destroy_notify; } gboolean fl_engine_send_platform_message_response( diff --git a/shell/platform/linux/fl_engine_private.h b/shell/platform/linux/fl_engine_private.h index 1007cfe75..0d5216f16 100644 --- a/shell/platform/linux/fl_engine_private.h +++ b/shell/platform/linux/fl_engine_private.h @@ -60,6 +60,8 @@ FlEngine* fl_engine_new(FlDartProject* project, FlRenderer* renderer); * @engine: an #FlEngine. * @handler: function to call when a platform message is received. * @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 a platform message is reveived. Call * fl_engine_send_platform_message_response() with the response to this message. @@ -70,7 +72,8 @@ FlEngine* fl_engine_new(FlDartProject* project, FlRenderer* renderer); void fl_engine_set_platform_message_handler( FlEngine* engine, FlEnginePlatformMessageHandler handler, - gpointer user_data); + gpointer user_data, + GDestroyNotify destroy_notify); /** * fl_engine_start: diff --git a/shell/platform/linux/fl_method_channel.cc b/shell/platform/linux/fl_method_channel.cc index c8d61fa1e..b2ea8dcb6 100644 --- a/shell/platform/linux/fl_method_channel.cc +++ b/shell/platform/linux/fl_method_channel.cc @@ -16,6 +16,9 @@ struct _FlMethodChannel { // Messenger to communicate on FlBinaryMessenger* messenger; + // TRUE if the channel has been closed + gboolean channel_closed; + // Channel name gchar* name; @@ -25,6 +28,7 @@ struct _FlMethodChannel { // Function called when a method call is received FlMethodChannelMethodCallHandler method_call_handler; gpointer method_call_handler_data; + GDestroyNotify method_call_handler_destroy_notify; }; // Added here to stop the compiler from optimising this function away @@ -65,17 +69,37 @@ static void message_response_cb(GObject* object, g_task_return_pointer(task, result, g_object_unref); } +// Called when the channel handler is closed +static void channel_closed_cb(gpointer user_data) { + g_autoptr(FlMethodChannel) self = FL_METHOD_CHANNEL(user_data); + + self->channel_closed = TRUE; + + // Disconnect handler + if (self->method_call_handler_destroy_notify != nullptr) + self->method_call_handler_destroy_notify(self->method_call_handler_data); + self->method_call_handler = nullptr; + self->method_call_handler_data = nullptr; + self->method_call_handler_destroy_notify = nullptr; +} + static void fl_method_channel_dispose(GObject* object) { FlMethodChannel* self = FL_METHOD_CHANNEL(object); if (self->messenger != nullptr) fl_binary_messenger_set_message_handler_on_channel( - self->messenger, self->name, nullptr, nullptr); + self->messenger, self->name, nullptr, nullptr, nullptr); g_clear_object(&self->messenger); g_clear_pointer(&self->name, g_free); g_clear_object(&self->codec); + if (self->method_call_handler_destroy_notify != nullptr) + self->method_call_handler_destroy_notify(self->method_call_handler_data); + self->method_call_handler = nullptr; + self->method_call_handler_data = nullptr; + self->method_call_handler_destroy_notify = nullptr; + G_OBJECT_CLASS(fl_method_channel_parent_class)->dispose(object); } @@ -101,7 +125,8 @@ G_MODULE_EXPORT FlMethodChannel* fl_method_channel_new( self->codec = FL_METHOD_CODEC(g_object_ref(codec)); fl_binary_messenger_set_message_handler_on_channel( - self->messenger, self->name, message_cb, self); + self->messenger, self->name, message_cb, g_object_ref(self), + channel_closed_cb); return self; } @@ -109,11 +134,24 @@ G_MODULE_EXPORT FlMethodChannel* fl_method_channel_new( G_MODULE_EXPORT void fl_method_channel_set_method_call_handler( FlMethodChannel* self, FlMethodChannelMethodCallHandler handler, - gpointer user_data) { + gpointer user_data, + GDestroyNotify destroy_notify) { g_return_if_fail(FL_IS_METHOD_CHANNEL(self)); + // Don't set handler if channel closed + if (self->channel_closed) { + g_warning("Attempted to set method call handler on closed FlMethodChannel"); + if (destroy_notify != nullptr) + destroy_notify(user_data); + return; + } + + if (self->method_call_handler_destroy_notify != nullptr) + self->method_call_handler_destroy_notify(self->method_call_handler_data); + self->method_call_handler = handler; self->method_call_handler_data = user_data; + self->method_call_handler_destroy_notify = destroy_notify; } G_MODULE_EXPORT void fl_method_channel_invoke_method( diff --git a/shell/platform/linux/public/flutter_linux/fl_basic_message_channel.h b/shell/platform/linux/public/flutter_linux/fl_basic_message_channel.h index 870b09cdf..b9fec0160 100644 --- a/shell/platform/linux/public/flutter_linux/fl_basic_message_channel.h +++ b/shell/platform/linux/public/flutter_linux/fl_basic_message_channel.h @@ -70,7 +70,8 @@ G_DECLARE_FINAL_TYPE(FlBasicMessageChannelResponseHandle, * g_autoptr(FlStandardMessageCodec) codec = fl_standard_message_codec_new (); * channel = fl_basic_message_channel_new (messenger, "flutter/foo", * FL_MESSAGE_CODEC (codec)); - * fl_basic_message_channel_set_message_handler (channel, message_cb, NULL); + * fl_basic_message_channel_set_message_handler (channel, message_cb, NULL, + * NULL); * * g_autoptr(FlValue) message = fl_value_new_string ("Hello World"); * fl_basic_message_channel_send (channel, message, NULL, @@ -131,13 +132,21 @@ FlBasicMessageChannel* fl_basic_message_channel_new( * @handler: (allow-none): function to call when a message is received on this * channel or %NULL to disable the handler. * @user_data: (closure): user data to pass to @handler. + * @destroy_notify: (allow-none): a function which gets called to free + * @user_data, or %NULL. * - * Sets the function called when a message is received. + * Sets the function called when a message is received from the Dart side of the + * channel. See #FlBasicMessageChannelMessageHandler for details on how to + * respond to messages. + * + * The handler is removed if the channel is closed or is replaced by another + * handler, set @destroy_notify if you want to detect this. */ void fl_basic_message_channel_set_message_handler( FlBasicMessageChannel* channel, FlBasicMessageChannelMessageHandler handler, - gpointer user_data); + gpointer user_data, + GDestroyNotify destroy_notify); /** * fl_basic_message_channel_respond: diff --git a/shell/platform/linux/public/flutter_linux/fl_binary_messenger.h b/shell/platform/linux/public/flutter_linux/fl_binary_messenger.h index 6792fcddc..4f7398654 100644 --- a/shell/platform/linux/public/flutter_linux/fl_binary_messenger.h +++ b/shell/platform/linux/public/flutter_linux/fl_binary_messenger.h @@ -83,17 +83,22 @@ typedef void (*FlBinaryMessengerMessageHandler)( * @handler: (allow-none): function to call when a message is received on this * channel or %NULL to disable a handler * @user_data: (closure): user data to pass to @handler. + * @destroy_notify: (allow-none): a function which gets called to free + * @user_data, or %NULL. * * Sets the function called when a platform message is received on the given - * channel. Call fl_binary_messenger_send_response() when the message is - * handled. Ownership of #FlBinaryMessengerResponseHandle is transferred to the - * caller, and the call must be responded to to avoid memory leaks. + * channel. See #FlBinaryMessengerMessageHandler for details on how to respond + * to messages. + * + * The handler is removed if the channel is closed or is replaced by another + * handler, set @destroy_notify if you want to detect this. */ void fl_binary_messenger_set_message_handler_on_channel( FlBinaryMessenger* messenger, const gchar* channel, FlBinaryMessengerMessageHandler handler, - gpointer user_data); + gpointer user_data, + GDestroyNotify destroy_notify); /** * fl_binary_messenger_send_response: diff --git a/shell/platform/linux/public/flutter_linux/fl_method_channel.h b/shell/platform/linux/public/flutter_linux/fl_method_channel.h index 0ef04ff15..f4102ed29 100644 --- a/shell/platform/linux/public/flutter_linux/fl_method_channel.h +++ b/shell/platform/linux/public/flutter_linux/fl_method_channel.h @@ -84,7 +84,8 @@ G_DECLARE_FINAL_TYPE(FlMethodChannel, * g_autoptr(FlStandardMethodCodec) codec = fl_standard_method_codec_new (); * channel = * fl_method_channel_new(messenger, "flutter/foo", FL_METHOD_CODEC (codec)); - * fl_method_channel_set_method_call_handler (channel, method_call_cb, NULL); + * fl_method_channel_set_method_call_handler (channel, method_call_cb, NULL, + * NULL); * * g_autoptr(FlValue) args = fl_value_new_string ("Hello World"); * fl_method_channel_invoke_method (channel, "Foo.foo", args, @@ -105,7 +106,7 @@ G_DECLARE_FINAL_TYPE(FlMethodChannel, * Function called when a method call is received. Respond to the method call * with fl_method_call_respond(). If the response is not occurring in this * callback take a reference to @method_call and release that once it has been - * responded to.Failing to respond before the last reference to @method_call is + * responded to. Failing to respond before the last reference to @method_call is * dropped is a programming error. */ typedef void (*FlMethodChannelMethodCallHandler)(FlMethodChannel* channel, @@ -132,14 +133,21 @@ FlMethodChannel* fl_method_channel_new(FlBinaryMessenger* messenger, * @channel: an #FlMethodChannel. * @handler: function to call when a method call is received on this channel. * @user_data: (closure): user data to pass to @handler. + * @destroy_notify: (allow-none): a function which gets called to free + * @user_data, or %NULL. * - * Sets the function called when a method is called. Call - * fl_method_call_respond() when the method completes. + * Sets the function called when a method call is received from the Dart side of + * the channel. See #FlMethodChannelMethodCallHandler for details on how to + * respond to method calls. + * + * The handler is removed if the channel is closed or is replaced by another + * handler, set @destroy_notify if you want to detect this. */ void fl_method_channel_set_method_call_handler( FlMethodChannel* channel, FlMethodChannelMethodCallHandler handler, - gpointer user_data); + gpointer user_data, + GDestroyNotify destroy_notify); /** * fl_method_channel_invoke_method: -- GitLab