From d4326f7b086e33b929d620ceabcfd3ce3320ce68 Mon Sep 17 00:00:00 2001 From: Bruno Leroux Date: Thu, 15 Jun 2023 15:04:00 +0200 Subject: [PATCH] [Linux] Allow BasicMessageChannel sending and responding to null message (flutter/engine#42808) ## Description This PR changes how a null message is handled by the Linux engine for a basic message channel. **Before**: - when receiving a null message a warning was emitted. - `fl_binary_messenger_send_response` was called but failed and the application exited. ``` ** (bug:9866): WARNING **: 23:13:42.109: Failed to decode message: Unexpected end of data ** (bug:9866): CRITICAL **: 23:13:42.109: gboolean send_response(FlBinaryMessenger *, FlBinaryMessengerResponseHandle *, GBytes *, GError **): assertion 'response_handle->response_handle != nullptr' failed ``` **After**: - Receiving a null message is handled as expected from the framework side documentation: https://github.com/flutter/flutter/blob/9287e81d52d69560222c14170a2fd7def613fe61/packages/flutter/lib/src/services/platform_channel.dart#L149-L150 ## Related Issue Fixes https://github.com/flutter/flutter/issues/128704 ## Tests Adds 2 tests. --- .../linux/fl_basic_message_channel.cc | 1 - .../linux/fl_basic_message_channel_test.cc | 35 ++++++++++++++++++- .../linux/fl_standard_message_codec.cc | 4 +++ .../linux/fl_standard_message_codec_test.cc | 13 +++++++ .../flutter_linux/fl_basic_message_channel.h | 3 +- 5 files changed, 53 insertions(+), 3 deletions(-) diff --git a/engine/src/flutter/shell/platform/linux/fl_basic_message_channel.cc b/engine/src/flutter/shell/platform/linux/fl_basic_message_channel.cc index 52670f6c1a..7b7c35a4ab 100644 --- a/engine/src/flutter/shell/platform/linux/fl_basic_message_channel.cc +++ b/engine/src/flutter/shell/platform/linux/fl_basic_message_channel.cc @@ -232,7 +232,6 @@ G_MODULE_EXPORT void fl_basic_message_channel_send(FlBasicMessageChannel* self, GAsyncReadyCallback callback, gpointer user_data) { g_return_if_fail(FL_IS_BASIC_MESSAGE_CHANNEL(self)); - g_return_if_fail(message != nullptr); g_autoptr(GTask) task = callback != nullptr ? g_task_new(self, cancellable, callback, user_data) diff --git a/engine/src/flutter/shell/platform/linux/fl_basic_message_channel_test.cc b/engine/src/flutter/shell/platform/linux/fl_basic_message_channel_test.cc index 8811fa388a..5a8eb635af 100644 --- a/engine/src/flutter/shell/platform/linux/fl_basic_message_channel_test.cc +++ b/engine/src/flutter/shell/platform/linux/fl_basic_message_channel_test.cc @@ -62,7 +62,8 @@ TEST(FlBasicMessageChannelTest, SendMessageWithoutResponse) { } // NOLINTEND(clang-analyzer-core.StackAddressEscape) -// Called when the message response is received in the SendMessage test. +// Called when the message response is received in the SendMessageWithResponse +// test. static void echo_response_cb(GObject* object, GAsyncResult* result, gpointer user_data) { @@ -189,3 +190,35 @@ TEST(FlBasicMessageChannelTest, ReceiveMessage) { // Blocks here until response_cb is called. g_main_loop_run(loop); } + +// Called when the message response is received in the +// SendNullMessageWithResponse test. +static void null_message_response_cb(GObject* object, + GAsyncResult* result, + gpointer user_data) { + g_autoptr(GError) error = nullptr; + g_autoptr(FlValue) message = fl_basic_message_channel_send_finish( + FL_BASIC_MESSAGE_CHANNEL(object), result, &error); + EXPECT_NE(message, nullptr); + EXPECT_EQ(error, nullptr); + + EXPECT_EQ(fl_value_get_type(message), FL_VALUE_TYPE_NULL); + + g_main_loop_quit(static_cast(user_data)); +} + +// Checks sending a null message with a response works. +TEST(FlBasicMessageChannelTest, SendNullMessageWithResponse) { + g_autoptr(GMainLoop) loop = g_main_loop_new(nullptr, 0); + + g_autoptr(FlEngine) engine = make_mock_engine(); + FlBinaryMessenger* messenger = fl_binary_messenger_new(engine); + g_autoptr(FlStandardMessageCodec) codec = fl_standard_message_codec_new(); + g_autoptr(FlBasicMessageChannel) channel = fl_basic_message_channel_new( + messenger, "test/echo", FL_MESSAGE_CODEC(codec)); + fl_basic_message_channel_send(channel, nullptr, nullptr, + null_message_response_cb, loop); + + // Blocks here until null_message_response_cb is called. + g_main_loop_run(loop); +} diff --git a/engine/src/flutter/shell/platform/linux/fl_standard_message_codec.cc b/engine/src/flutter/shell/platform/linux/fl_standard_message_codec.cc index 9fb31d1cc8..deeccb4397 100644 --- a/engine/src/flutter/shell/platform/linux/fl_standard_message_codec.cc +++ b/engine/src/flutter/shell/platform/linux/fl_standard_message_codec.cc @@ -420,6 +420,10 @@ static GBytes* fl_standard_message_codec_encode_message(FlMessageCodec* codec, static FlValue* fl_standard_message_codec_decode_message(FlMessageCodec* codec, GBytes* message, GError** error) { + if (g_bytes_get_size(message) == 0) { + return fl_value_new_null(); + } + FlStandardMessageCodec* self = reinterpret_cast(codec); diff --git a/engine/src/flutter/shell/platform/linux/fl_standard_message_codec_test.cc b/engine/src/flutter/shell/platform/linux/fl_standard_message_codec_test.cc index b3b13a5a5c..2e9e34b272 100644 --- a/engine/src/flutter/shell/platform/linux/fl_standard_message_codec_test.cc +++ b/engine/src/flutter/shell/platform/linux/fl_standard_message_codec_test.cc @@ -60,6 +60,19 @@ TEST(FlStandardMessageCodecTest, EncodeNull) { EXPECT_STREQ(hex_string, "00"); } +TEST(FlStandardMessageCodecTest, DecodeNull) { + // Regression test for https://github.com/flutter/flutter/issues/128704. + + g_autoptr(FlStandardMessageCodec) codec = fl_standard_message_codec_new(); + g_autoptr(GBytes) data = g_bytes_new(nullptr, 0); + g_autoptr(GError) error = nullptr; + g_autoptr(FlValue) value = + fl_message_codec_decode_message(FL_MESSAGE_CODEC(codec), data, &error); + + EXPECT_FALSE(value == nullptr); + EXPECT_EQ(fl_value_get_type(value), FL_VALUE_TYPE_NULL); +} + static gchar* encode_bool(gboolean value) { g_autoptr(FlValue) v = fl_value_new_bool(value); return encode_message(v); diff --git a/engine/src/flutter/shell/platform/linux/public/flutter_linux/fl_basic_message_channel.h b/engine/src/flutter/shell/platform/linux/public/flutter_linux/fl_basic_message_channel.h index b9fec0160c..9e7f4e46bc 100644 --- a/engine/src/flutter/shell/platform/linux/public/flutter_linux/fl_basic_message_channel.h +++ b/engine/src/flutter/shell/platform/linux/public/flutter_linux/fl_basic_message_channel.h @@ -171,7 +171,8 @@ gboolean fl_basic_message_channel_respond( /** * fl_basic_message_channel_send: * @channel: an #FlBasicMessageChannel. - * @message: message to send, must match what the #FlMessageCodec supports. + * @message: (allow-none): message to send, must match what the #FlMessageCodec + * supports. * @cancellable: (allow-none): a #GCancellable or %NULL. * @callback: (scope async): (allow-none): a #GAsyncReadyCallback to call when * the request is satisfied or %NULL to ignore the response.