[Linux] Allow BasicMessageChannel sending and responding to null message

[bleroux]

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 flutter/flutter#128704

Tests

Adds 2 tests.

[stuartmorgan-g]

This will mean calling fl_message_codec_encode_message with null below; does that work? Is there unit testing of it? Does it return a non-null data pointer, such that line 246 won't return an error?

[bleroux]

This will mean calling fl_message_codec_encode_message with null below; does that work?

Yes, I have checked that fl_message_codec_encode_message allow null message, it properly replaces it with fl_value_new_null() :

engine/shell/platform/linux/fl_message_codec.cc

Lines 27 to 31 in ac954ee

	g_autoptr(FlValue) null_value = nullptr;
	if (message == nullptr) {
	null_value = fl_value_new_null();
	message = null_value;
	}

Is there unit testing of it?

I think this is covered by those tests:

engine/shell/platform/linux/fl_standard_message_codec_test.cc

Lines 52 to 61 in ac954ee

	TEST(FlStandardMessageCodecTest, EncodeNullptr) {
	g_autofree gchar* hex_string = encode_message(nullptr);
	EXPECT_STREQ(hex_string, "00");
	}
	TEST(FlStandardMessageCodecTest, EncodeNull) {
	g_autoptr(FlValue) value = fl_value_new_null();
	g_autofree gchar* hex_string = encode_message(value);
	EXPECT_STREQ(hex_string, "00");
	}

[robert-ancell]

Please add '(allow-none)' to the documentation string in public/flutter_linux/fl_basic_message_channel.h

[bleroux]

Please add '(allow-none)' to the documentation string in public/flutter_linux/fl_basic_message_channel.h

Done, thanks for noticing this 👍

[stuartmorgan-g]

@cbracken Null messages strike again :P

How do you think we should handle null messages here: null data, or a value whose value is null? IIRC you found that this was inconsistent in #25064 but I forget whether you cleaned it up later.

[robert-ancell]

You have to return the FlValue object here, because returning NULL implies there was an error (GObject convention).

[robert-ancell]

With the docstring change this looks good, on the assumption that this is the correct behaviour for receiving an empty message.

[stuartmorgan-g]

LGTM, thanks!