From 071068bfc20e4bb3f0430f5f429f2ecd6c453c25 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Sun, 26 Sep 2021 13:25:39 -0700 Subject: [PATCH 1/3] Impl --- shell/platform/linux/fl_method_channel.cc | 4 + .../platform/linux/fl_method_channel_test.cc | 140 ++++++++++++++++++ 2 files changed, 144 insertions(+) diff --git a/shell/platform/linux/fl_method_channel.cc b/shell/platform/linux/fl_method_channel.cc index cb705d243733b..d4c45ed920a7c 100644 --- a/shell/platform/linux/fl_method_channel.cc +++ b/shell/platform/linux/fl_method_channel.cc @@ -75,6 +75,10 @@ static void channel_closed_cb(gpointer user_data) { g_autoptr(FlMethodChannel) self = FL_METHOD_CHANNEL(user_data); self->channel_closed = TRUE; + // Clear the messenger so that disposing the channel will not clear the + // messenger's mapped channel, since `channel_closed_cb` means the messenger + // has abandoned this channel. + self->messenger = nullptr; // Disconnect handler. if (self->method_call_handler_destroy_notify != nullptr) { diff --git a/shell/platform/linux/fl_method_channel_test.cc b/shell/platform/linux/fl_method_channel_test.cc index 0d9fed34a624c..782b6dc327800 100644 --- a/shell/platform/linux/fl_method_channel_test.cc +++ b/shell/platform/linux/fl_method_channel_test.cc @@ -603,3 +603,143 @@ TEST(FlMethodChannelTest, ReceiveMethodCallRespondErrorError) { // Blocks here until method_call_error_error_cb is called. g_main_loop_run(loop); } + +struct UserDataReassignMethod { + GMainLoop* loop; + int count; +}; + +static void reassign_method_cb(FlMethodChannel* channel, + FlMethodCall* method_call, + gpointer raw_user_data) { + UserDataReassignMethod* user_data = static_cast(raw_user_data); + user_data->count += 1; + + g_autoptr(FlValue) result = fl_value_new_string("Polo!"); + g_autoptr(GError) error = nullptr; + EXPECT_TRUE(fl_method_call_respond_success(method_call, result, &error)); + EXPECT_EQ(error, nullptr); + + g_main_loop_quit(user_data->loop); +} + +TEST(FlMethodChannelTest, ReassignMethodChannelShouldWork) { + const char* method_name = "test/standard-method"; + 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(FlStandardMethodCodec) codec = fl_standard_method_codec_new(); + + // Create the first channel and test if sending messages work. + UserDataReassignMethod user_data1{ + .loop = loop, + .count = 100, + }; + FlMethodChannel* channel1 = fl_method_channel_new( + messenger, method_name, FL_METHOD_CODEC(codec)); + fl_method_channel_set_method_call_handler(channel1, reassign_method_cb, + &user_data1, nullptr); + + // Trigger the engine to make a method call. + g_autoptr(FlValue) args = fl_value_new_list(); + fl_value_append_take(args, fl_value_new_string(method_name)); + fl_value_append_take(args, fl_value_new_string("FOO")); + fl_value_append_take(args, fl_value_new_string("BAR")); + + fl_method_channel_invoke_method(channel1, "InvokeMethod", args, nullptr, + nullptr, nullptr); + + // Blocks here until method_call_error_error_cb is called. + g_main_loop_run(loop); + + EXPECT_EQ(user_data1.count, 101); + + // g_autoptr(GMainLoop) loop2 = g_main_loop_new(nullptr, 0); + // Create the second channel on the same name and test if sending messages + // work. + UserDataReassignMethod user_data2{ + .loop = loop, + .count = 100, + }; + g_autoptr(FlMethodChannel) channel2 = fl_method_channel_new( + messenger, method_name, FL_METHOD_CODEC(codec)); + fl_method_channel_set_method_call_handler(channel2, reassign_method_cb, + &user_data2, nullptr); + + fl_method_channel_invoke_method(channel2, "InvokeMethod", args, nullptr, + nullptr, nullptr); + g_main_loop_run(loop); + + EXPECT_EQ(user_data1.count, 101); + EXPECT_EQ(user_data2.count, 101); + + g_object_unref(channel1); + + fl_method_channel_invoke_method(channel2, "InvokeMethod", args, nullptr, + nullptr, nullptr); + g_main_loop_run(loop); + + EXPECT_EQ(user_data1.count, 101); + EXPECT_EQ(user_data2.count, 102); +} + +TEST(FlMethodChannelTest, LateReassignMethodChannelShouldWork) { + const char* method_name = "test/standard-method"; + 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(FlStandardMethodCodec) codec = fl_standard_method_codec_new(); + + // Create the first channel and test if sending messages work. + UserDataReassignMethod user_data1{ + .loop = loop, + .count = 100, + }; + FlMethodChannel* channel1 = fl_method_channel_new( + messenger, method_name, FL_METHOD_CODEC(codec)); + fl_method_channel_set_method_call_handler(channel1, reassign_method_cb, + &user_data1, nullptr); + + // Trigger the engine to make a method call. + g_autoptr(FlValue) args = fl_value_new_list(); + fl_value_append_take(args, fl_value_new_string(method_name)); + fl_value_append_take(args, fl_value_new_string("FOO")); + fl_value_append_take(args, fl_value_new_string("BAR")); + + fl_method_channel_invoke_method(channel1, "InvokeMethod", args, nullptr, + nullptr, nullptr); + + // Blocks here until method_call_error_error_cb is called. + g_main_loop_run(loop); + + EXPECT_EQ(user_data1.count, 101); + + // Create the second channel on the same name and test if sending messages + // work. + UserDataReassignMethod user_data2{ + .loop = loop, + .count = 100, + }; + g_autoptr(FlMethodChannel) channel2 = fl_method_channel_new( + messenger, method_name, FL_METHOD_CODEC(codec)); + fl_method_channel_set_method_call_handler(channel2, reassign_method_cb, + &user_data2, nullptr); + + fl_method_channel_invoke_method(channel2, "InvokeMethod", args, nullptr, + nullptr, nullptr); + g_main_loop_run(loop); + + EXPECT_EQ(user_data1.count, 101); + EXPECT_EQ(user_data2.count, 101); + + g_object_unref(channel1); + + fl_method_channel_invoke_method(channel2, "InvokeMethod", args, nullptr, + nullptr, nullptr); + g_main_loop_run(loop); + + EXPECT_EQ(user_data1.count, 101); + EXPECT_EQ(user_data2.count, 102); +} From ddc68fb978b313c4df585c38174399c05b6c2c7e Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Sun, 26 Sep 2021 13:26:08 -0700 Subject: [PATCH 2/3] Format --- .../platform/linux/fl_method_channel_test.cc | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/shell/platform/linux/fl_method_channel_test.cc b/shell/platform/linux/fl_method_channel_test.cc index 782b6dc327800..37691a495878d 100644 --- a/shell/platform/linux/fl_method_channel_test.cc +++ b/shell/platform/linux/fl_method_channel_test.cc @@ -612,7 +612,8 @@ struct UserDataReassignMethod { static void reassign_method_cb(FlMethodChannel* channel, FlMethodCall* method_call, gpointer raw_user_data) { - UserDataReassignMethod* user_data = static_cast(raw_user_data); + UserDataReassignMethod* user_data = + static_cast(raw_user_data); user_data->count += 1; g_autoptr(FlValue) result = fl_value_new_string("Polo!"); @@ -633,11 +634,11 @@ TEST(FlMethodChannelTest, ReassignMethodChannelShouldWork) { // Create the first channel and test if sending messages work. UserDataReassignMethod user_data1{ - .loop = loop, - .count = 100, + .loop = loop, + .count = 100, }; - FlMethodChannel* channel1 = fl_method_channel_new( - messenger, method_name, FL_METHOD_CODEC(codec)); + FlMethodChannel* channel1 = + fl_method_channel_new(messenger, method_name, FL_METHOD_CODEC(codec)); fl_method_channel_set_method_call_handler(channel1, reassign_method_cb, &user_data1, nullptr); @@ -659,11 +660,11 @@ TEST(FlMethodChannelTest, ReassignMethodChannelShouldWork) { // Create the second channel on the same name and test if sending messages // work. UserDataReassignMethod user_data2{ - .loop = loop, - .count = 100, + .loop = loop, + .count = 100, }; - g_autoptr(FlMethodChannel) channel2 = fl_method_channel_new( - messenger, method_name, FL_METHOD_CODEC(codec)); + g_autoptr(FlMethodChannel) channel2 = + fl_method_channel_new(messenger, method_name, FL_METHOD_CODEC(codec)); fl_method_channel_set_method_call_handler(channel2, reassign_method_cb, &user_data2, nullptr); @@ -694,11 +695,11 @@ TEST(FlMethodChannelTest, LateReassignMethodChannelShouldWork) { // Create the first channel and test if sending messages work. UserDataReassignMethod user_data1{ - .loop = loop, - .count = 100, + .loop = loop, + .count = 100, }; - FlMethodChannel* channel1 = fl_method_channel_new( - messenger, method_name, FL_METHOD_CODEC(codec)); + FlMethodChannel* channel1 = + fl_method_channel_new(messenger, method_name, FL_METHOD_CODEC(codec)); fl_method_channel_set_method_call_handler(channel1, reassign_method_cb, &user_data1, nullptr); @@ -719,11 +720,11 @@ TEST(FlMethodChannelTest, LateReassignMethodChannelShouldWork) { // Create the second channel on the same name and test if sending messages // work. UserDataReassignMethod user_data2{ - .loop = loop, - .count = 100, + .loop = loop, + .count = 100, }; - g_autoptr(FlMethodChannel) channel2 = fl_method_channel_new( - messenger, method_name, FL_METHOD_CODEC(codec)); + g_autoptr(FlMethodChannel) channel2 = + fl_method_channel_new(messenger, method_name, FL_METHOD_CODEC(codec)); fl_method_channel_set_method_call_handler(channel2, reassign_method_cb, &user_data2, nullptr); From aed18353e853df5c7df7f0a6538c9212dc61dff7 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Mon, 27 Sep 2021 11:08:29 -0700 Subject: [PATCH 3/3] Doc --- .../platform/linux/fl_method_channel_test.cc | 80 +++++++++---------- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/shell/platform/linux/fl_method_channel_test.cc b/shell/platform/linux/fl_method_channel_test.cc index 37691a495878d..74b9294c60e03 100644 --- a/shell/platform/linux/fl_method_channel_test.cc +++ b/shell/platform/linux/fl_method_channel_test.cc @@ -609,6 +609,8 @@ struct UserDataReassignMethod { int count; }; +// This callback parses the user data as UserDataReassignMethod, +// increases its `count`, and quits `loop`. static void reassign_method_cb(FlMethodChannel* channel, FlMethodCall* method_call, gpointer raw_user_data) { @@ -624,15 +626,28 @@ static void reassign_method_cb(FlMethodChannel* channel, g_main_loop_quit(user_data->loop); } -TEST(FlMethodChannelTest, ReassignMethodChannelShouldWork) { +// Make sure that the following steps will work properly: +// +// 1. Register a method channel. +// 2. Dispose the method channel, and it's unregistered. +// 3. Register a new channel with the same name. +// +// This is a regression test to https://github.com/flutter/flutter/issues/90817. +TEST(FlMethodChannelTest, ReplaceADisposedMethodChannel) { const char* method_name = "test/standard-method"; + // The loop is used to pause the main process until the callback is fully + // executed. 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(FlStandardMethodCodec) codec = fl_standard_method_codec_new(); - // Create the first channel and test if sending messages work. + g_autoptr(FlValue) args = fl_value_new_list(); + fl_value_append_take(args, fl_value_new_string(method_name)); + fl_value_append_take(args, fl_value_new_string("FOO")); + fl_value_append_take(args, fl_value_new_string("BAR")); + + // Register the first channel and test if it works. UserDataReassignMethod user_data1{ .loop = loop, .count = 100, @@ -642,23 +657,15 @@ TEST(FlMethodChannelTest, ReassignMethodChannelShouldWork) { fl_method_channel_set_method_call_handler(channel1, reassign_method_cb, &user_data1, nullptr); - // Trigger the engine to make a method call. - g_autoptr(FlValue) args = fl_value_new_list(); - fl_value_append_take(args, fl_value_new_string(method_name)); - fl_value_append_take(args, fl_value_new_string("FOO")); - fl_value_append_take(args, fl_value_new_string("BAR")); - fl_method_channel_invoke_method(channel1, "InvokeMethod", args, nullptr, nullptr, nullptr); - - // Blocks here until method_call_error_error_cb is called. g_main_loop_run(loop); - EXPECT_EQ(user_data1.count, 101); - // g_autoptr(GMainLoop) loop2 = g_main_loop_new(nullptr, 0); - // Create the second channel on the same name and test if sending messages - // work. + // Dispose the first channel. + g_object_unref(channel1); + + // Register the second channel and test if it works. UserDataReassignMethod user_data2{ .loop = loop, .count = 100, @@ -674,26 +681,30 @@ TEST(FlMethodChannelTest, ReassignMethodChannelShouldWork) { EXPECT_EQ(user_data1.count, 101); EXPECT_EQ(user_data2.count, 101); - - g_object_unref(channel1); - - fl_method_channel_invoke_method(channel2, "InvokeMethod", args, nullptr, - nullptr, nullptr); - g_main_loop_run(loop); - - EXPECT_EQ(user_data1.count, 101); - EXPECT_EQ(user_data2.count, 102); } -TEST(FlMethodChannelTest, LateReassignMethodChannelShouldWork) { +// Make sure that the following steps will work properly: +// +// 1. Register a method channel. +// 2. Register the same name with a new channel. +// 3. Dispose the previous method channel. +// +// This is a regression test to https://github.com/flutter/flutter/issues/90817. +TEST(FlMethodChannelTest, DisposeAReplacedMethodChannel) { const char* method_name = "test/standard-method"; + // The loop is used to pause the main process until the callback is fully + // executed. 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(FlStandardMethodCodec) codec = fl_standard_method_codec_new(); - // Create the first channel and test if sending messages work. + g_autoptr(FlValue) args = fl_value_new_list(); + fl_value_append_take(args, fl_value_new_string(method_name)); + fl_value_append_take(args, fl_value_new_string("FOO")); + fl_value_append_take(args, fl_value_new_string("BAR")); + + // Register the first channel and test if it works. UserDataReassignMethod user_data1{ .loop = loop, .count = 100, @@ -703,22 +714,12 @@ TEST(FlMethodChannelTest, LateReassignMethodChannelShouldWork) { fl_method_channel_set_method_call_handler(channel1, reassign_method_cb, &user_data1, nullptr); - // Trigger the engine to make a method call. - g_autoptr(FlValue) args = fl_value_new_list(); - fl_value_append_take(args, fl_value_new_string(method_name)); - fl_value_append_take(args, fl_value_new_string("FOO")); - fl_value_append_take(args, fl_value_new_string("BAR")); - fl_method_channel_invoke_method(channel1, "InvokeMethod", args, nullptr, nullptr, nullptr); - - // Blocks here until method_call_error_error_cb is called. g_main_loop_run(loop); - EXPECT_EQ(user_data1.count, 101); - // Create the second channel on the same name and test if sending messages - // work. + // Register a new channel to the same name. UserDataReassignMethod user_data2{ .loop = loop, .count = 100, @@ -731,16 +732,15 @@ TEST(FlMethodChannelTest, LateReassignMethodChannelShouldWork) { fl_method_channel_invoke_method(channel2, "InvokeMethod", args, nullptr, nullptr, nullptr); g_main_loop_run(loop); - EXPECT_EQ(user_data1.count, 101); EXPECT_EQ(user_data2.count, 101); + // Dispose the first channel. The new channel should keep working. g_object_unref(channel1); fl_method_channel_invoke_method(channel2, "InvokeMethod", args, nullptr, nullptr, nullptr); g_main_loop_run(loop); - EXPECT_EQ(user_data1.count, 101); EXPECT_EQ(user_data2.count, 102); }