diff --git a/shell/platform/common/client_wrapper/event_channel_unittests.cc b/shell/platform/common/client_wrapper/event_channel_unittests.cc index 395913870f644..58005fcffffce 100644 --- a/shell/platform/common/client_wrapper/event_channel_unittests.cc +++ b/shell/platform/common/client_wrapper/event_channel_unittests.cc @@ -33,7 +33,9 @@ class TestBinaryMessenger : public BinaryMessenger { return last_message_handler_channel_; } - BinaryMessageHandler last_message_handler() { return last_message_handler_; } + const BinaryMessageHandler& last_message_handler() { + return last_message_handler_; + } private: std::string last_message_handler_channel_; @@ -64,7 +66,7 @@ TEST(EventChannelTest, Registration) { EXPECT_EQ(messenger.last_message_handler_channel(), channel_name); EXPECT_NE(messenger.last_message_handler(), nullptr); - // Send dummy listen message. + // Send test listen message. MethodCall<> call("listen", nullptr); auto message = codec.EncodeMethodCall(call); messenger.last_message_handler()( @@ -121,7 +123,7 @@ TEST(EventChannelTest, Cancel) { EXPECT_EQ(messenger.last_message_handler_channel(), channel_name); EXPECT_NE(messenger.last_message_handler(), nullptr); - // Send dummy listen message. + // Send test listen message. MethodCall<> call_listen("listen", nullptr); auto message = codec.EncodeMethodCall(call_listen); messenger.last_message_handler()( @@ -129,7 +131,7 @@ TEST(EventChannelTest, Cancel) { [](const uint8_t* reply, const size_t reply_size) {}); EXPECT_EQ(on_listen_called, true); - // Send dummy cancel message. + // Send test cancel message. MethodCall<> call_cancel("cancel", nullptr); message = codec.EncodeMethodCall(call_cancel); messenger.last_message_handler()( @@ -165,7 +167,7 @@ TEST(EventChannelTest, ListenNotCancel) { EXPECT_EQ(messenger.last_message_handler_channel(), channel_name); EXPECT_NE(messenger.last_message_handler(), nullptr); - // Send dummy listen message. + // Send test listen message. MethodCall<> call_listen("listen", nullptr); auto message = codec.EncodeMethodCall(call_listen); messenger.last_message_handler()( @@ -205,7 +207,7 @@ TEST(EventChannelTest, ReRegistration) { EXPECT_EQ(messenger.last_message_handler_channel(), channel_name); EXPECT_NE(messenger.last_message_handler(), nullptr); - // Send dummy listen message. + // Send test listen message. MethodCall<> call("listen", nullptr); auto message = codec.EncodeMethodCall(call); messenger.last_message_handler()( @@ -213,7 +215,7 @@ TEST(EventChannelTest, ReRegistration) { [](const uint8_t* reply, const size_t reply_size) {}); EXPECT_EQ(on_listen_called, true); - // Send second dummy message to test StreamHandler's OnCancel + // Send second test message to test StreamHandler's OnCancel // method is called before OnListen method is called. on_listen_called = false; message = codec.EncodeMethodCall(call); @@ -226,4 +228,50 @@ TEST(EventChannelTest, ReRegistration) { EXPECT_EQ(on_listen_called, true); } +// Test that the handler is called even if the event channel is destroyed. +TEST(EventChannelTest, HandlerOutlivesEventChannel) { + TestBinaryMessenger messenger; + const std::string channel_name("some_channel"); + const StandardMethodCodec& codec = StandardMethodCodec::GetInstance(); + + bool on_listen_called = false; + bool on_cancel_called = false; + { + EventChannel channel(&messenger, channel_name, &codec); + auto handler = std::make_unique>( + [&on_listen_called](const EncodableValue* arguments, + std::unique_ptr>&& events) + -> std::unique_ptr> { + on_listen_called = true; + return nullptr; + }, + [&on_cancel_called](const EncodableValue* arguments) + -> std::unique_ptr> { + on_cancel_called = true; + return nullptr; + }); + channel.SetStreamHandler(std::move(handler)); + } + + // The event channel was destroyed but the handler should still be alive. + EXPECT_EQ(messenger.last_message_handler_channel(), channel_name); + EXPECT_NE(messenger.last_message_handler(), nullptr); + + // Send test listen message. + MethodCall<> call_listen("listen", nullptr); + auto message = codec.EncodeMethodCall(call_listen); + messenger.last_message_handler()( + message->data(), message->size(), + [](const uint8_t* reply, const size_t reply_size) {}); + EXPECT_EQ(on_listen_called, true); + + // Send test cancel message. + MethodCall<> call_cancel("cancel", nullptr); + message = codec.EncodeMethodCall(call_cancel); + messenger.last_message_handler()( + message->data(), message->size(), + [](const uint8_t* reply, const size_t reply_size) {}); + EXPECT_EQ(on_cancel_called, true); +} + } // namespace flutter diff --git a/shell/platform/common/client_wrapper/include/flutter/event_channel.h b/shell/platform/common/client_wrapper/include/flutter/event_channel.h index fe0e1414d63da..08c242a2ff441 100644 --- a/shell/platform/common/client_wrapper/include/flutter/event_channel.h +++ b/shell/platform/common/client_wrapper/include/flutter/event_channel.h @@ -47,10 +47,13 @@ class EventChannel { // Registers a stream handler on this channel. // If no handler has been registered, any incoming stream setup requests will // be handled silently by providing an empty stream. + // + // Note that the EventChannel does not own the handler and will not + // unregister it on destruction. The caller is responsible for unregistering + // the handler if it should no longer be called. void SetStreamHandler(std::unique_ptr> handler) { if (!handler) { messenger_->SetMessageHandler(name_, nullptr); - is_listening_ = false; return; } @@ -61,69 +64,75 @@ class EventChannel { const MethodCodec* codec = codec_; const std::string channel_name = name_; const BinaryMessenger* messenger = messenger_; - BinaryMessageHandler binary_handler = [shared_handler, codec, channel_name, - messenger, - this](const uint8_t* message, - const size_t message_size, - BinaryReply reply) { - constexpr char kOnListenMethod[] = "listen"; - constexpr char kOnCancelMethod[] = "cancel"; - - std::unique_ptr> method_call = - codec->DecodeMethodCall(message, message_size); - if (!method_call) { - std::cerr << "Unable to construct method call from message on channel: " - << channel_name << std::endl; - reply(nullptr, 0); - return; - } - - const std::string& method = method_call->method_name(); - if (method.compare(kOnListenMethod) == 0) { - if (is_listening_) { - std::unique_ptr> error = - shared_handler->OnCancel(nullptr); - if (error) { - std::cerr << "Failed to cancel existing stream: " - << (error->error_code) << ", " << (error->error_message) - << ", " << (error->error_details); + BinaryMessageHandler binary_handler = + [shared_handler, codec, channel_name, messenger, + // Mutable state to track the handler's listening status. + is_listening = bool(false)](const uint8_t* message, + const size_t message_size, + BinaryReply reply) mutable { + constexpr char kOnListenMethod[] = "listen"; + constexpr char kOnCancelMethod[] = "cancel"; + + std::unique_ptr> method_call = + codec->DecodeMethodCall(message, message_size); + if (!method_call) { + std::cerr + << "Unable to construct method call from message on channel: " + << channel_name << std::endl; + reply(nullptr, 0); + return; } - } - is_listening_ = true; - - std::unique_ptr> result; - auto sink = std::make_unique( - messenger, channel_name, codec); - std::unique_ptr> error = - shared_handler->OnListen(method_call->arguments(), std::move(sink)); - if (error) { - result = codec->EncodeErrorEnvelope( - error->error_code, error->error_message, error->error_details); - } else { - result = codec->EncodeSuccessEnvelope(); - } - reply(result->data(), result->size()); - } else if (method.compare(kOnCancelMethod) == 0) { - std::unique_ptr> result; - if (is_listening_) { - std::unique_ptr> error = - shared_handler->OnCancel(method_call->arguments()); - if (error) { - result = codec->EncodeErrorEnvelope( - error->error_code, error->error_message, error->error_details); + + const std::string& method = method_call->method_name(); + if (method.compare(kOnListenMethod) == 0) { + if (is_listening) { + std::unique_ptr> error = + shared_handler->OnCancel(nullptr); + if (error) { + std::cerr << "Failed to cancel existing stream: " + << (error->error_code) << ", " + << (error->error_message) << ", " + << (error->error_details); + } + } + is_listening = true; + + std::unique_ptr> result; + auto sink = std::make_unique( + messenger, channel_name, codec); + std::unique_ptr> error = + shared_handler->OnListen(method_call->arguments(), + std::move(sink)); + if (error) { + result = codec->EncodeErrorEnvelope(error->error_code, + error->error_message, + error->error_details); + } else { + result = codec->EncodeSuccessEnvelope(); + } + reply(result->data(), result->size()); + } else if (method.compare(kOnCancelMethod) == 0) { + std::unique_ptr> result; + if (is_listening) { + std::unique_ptr> error = + shared_handler->OnCancel(method_call->arguments()); + if (error) { + result = codec->EncodeErrorEnvelope(error->error_code, + error->error_message, + error->error_details); + } else { + result = codec->EncodeSuccessEnvelope(); + } + is_listening = false; + } else { + result = codec->EncodeErrorEnvelope( + "error", "No active stream to cancel", nullptr); + } + reply(result->data(), result->size()); } else { - result = codec->EncodeSuccessEnvelope(); + reply(nullptr, 0); } - is_listening_ = false; - } else { - result = codec->EncodeErrorEnvelope( - "error", "No active stream to cancel", nullptr); - } - reply(result->data(), result->size()); - } else { - reply(nullptr, 0); - } - }; + }; messenger_->SetMessageHandler(name_, std::move(binary_handler)); } @@ -165,7 +174,6 @@ class EventChannel { BinaryMessenger* messenger_; const std::string name_; const MethodCodec* codec_; - bool is_listening_ = false; }; } // namespace flutter