diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 08d65c11b2a61..266c9cf5de4d9 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -783,12 +783,14 @@ FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/include/flutter/ FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/include/flutter/method_channel.h FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/include/flutter/method_codec.h FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/include/flutter/method_result.h +FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/include/flutter/method_result_functions.h FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/include/flutter/plugin_registrar.h FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/include/flutter/plugin_registry.h FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/include/flutter/standard_message_codec.h FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/include/flutter/standard_method_codec.h FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/method_call_unittests.cc FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/method_channel_unittests.cc +FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/method_result_functions_unittests.cc FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/plugin_registrar.cc FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/plugin_registrar_unittests.cc FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/standard_codec.cc diff --git a/shell/platform/common/cpp/BUILD.gn b/shell/platform/common/cpp/BUILD.gn index 4d211c8fe8de1..f3bcde6b2442d 100644 --- a/shell/platform/common/cpp/BUILD.gn +++ b/shell/platform/common/cpp/BUILD.gn @@ -116,6 +116,7 @@ executable("common_cpp_unittests") { deps = [ ":common_cpp", ":common_cpp_fixtures", + "//flutter/shell/platform/common/cpp/client_wrapper:client_wrapper", "//flutter/shell/platform/common/cpp/client_wrapper:client_wrapper_library_stubs", "//flutter/testing", ] diff --git a/shell/platform/common/cpp/client_wrapper/BUILD.gn b/shell/platform/common/cpp/client_wrapper/BUILD.gn index ed0bdf9e87536..94e68dc86084a 100644 --- a/shell/platform/common/cpp/client_wrapper/BUILD.gn +++ b/shell/platform/common/cpp/client_wrapper/BUILD.gn @@ -41,12 +41,12 @@ test_fixtures("client_wrapper_fixtures") { executable("client_wrapper_unittests") { testonly = true - # TODO: Add more unit tests. sources = [ "basic_message_channel_unittests.cc", "encodable_value_unittests.cc", "method_call_unittests.cc", "method_channel_unittests.cc", + "method_result_functions_unittests.cc", "plugin_registrar_unittests.cc", "standard_message_codec_unittests.cc", "standard_method_codec_unittests.cc", diff --git a/shell/platform/common/cpp/client_wrapper/basic_message_channel_unittests.cc b/shell/platform/common/cpp/client_wrapper/basic_message_channel_unittests.cc index 3e47b80ceca45..5098e89b30dd6 100644 --- a/shell/platform/common/cpp/client_wrapper/basic_message_channel_unittests.cc +++ b/shell/platform/common/cpp/client_wrapper/basic_message_channel_unittests.cc @@ -17,10 +17,6 @@ namespace { class TestBinaryMessenger : public BinaryMessenger { public: - void Send(const std::string& channel, - const uint8_t* message, - const size_t message_size) const override {} - void Send(const std::string& channel, const uint8_t* message, const size_t message_size, diff --git a/shell/platform/common/cpp/client_wrapper/core_wrapper_files.gni b/shell/platform/common/cpp/client_wrapper/core_wrapper_files.gni index 0662cca6be422..86c3280ea3699 100644 --- a/shell/platform/common/cpp/client_wrapper/core_wrapper_files.gni +++ b/shell/platform/common/cpp/client_wrapper/core_wrapper_files.gni @@ -12,6 +12,7 @@ core_cpp_client_wrapper_includes = "include/flutter/method_call.h", "include/flutter/method_channel.h", "include/flutter/method_codec.h", + "include/flutter/method_result_functions.h", "include/flutter/method_result.h", "include/flutter/plugin_registrar.h", "include/flutter/plugin_registry.h", diff --git a/shell/platform/common/cpp/client_wrapper/include/flutter/binary_messenger.h b/shell/platform/common/cpp/client_wrapper/include/flutter/binary_messenger.h index 08d074c86e744..8dd2f78ad8f76 100644 --- a/shell/platform/common/cpp/client_wrapper/include/flutter/binary_messenger.h +++ b/shell/platform/common/cpp/client_wrapper/include/flutter/binary_messenger.h @@ -8,21 +8,19 @@ #include #include -// TODO: Consider adding absl as a dependency and using absl::Span for all of -// the message/message_size pairs. namespace flutter { // A binary message reply callback. // // Used for submitting a binary reply back to a Flutter message sender. -typedef std::function +typedef std::function BinaryReply; // A message handler callback. // // Used for receiving messages from Flutter and providing an asynchronous reply. typedef std::function< - void(const uint8_t* message, const size_t message_size, BinaryReply reply)> + void(const uint8_t* message, size_t message_size, BinaryReply reply)> BinaryMessageHandler; // A protocol for a class that handles communication of binary data on named @@ -31,18 +29,14 @@ class BinaryMessenger { public: virtual ~BinaryMessenger() = default; - // Sends a binary message to the Flutter side on the specified channel, - // expecting no reply. - virtual void Send(const std::string& channel, - const uint8_t* message, - const size_t message_size) const = 0; - - // Sends a binary message to the Flutter side on the specified channel, - // expecting a reply. + // Sends a binary message to the Flutter engine on the specified channel. + // + // If |reply| is provided, it will be called back with the response from the + // engine. virtual void Send(const std::string& channel, const uint8_t* message, - const size_t message_size, - BinaryReply reply) const = 0; + size_t message_size, + BinaryReply reply = nullptr) const = 0; // Registers a message handler for incoming binary messages from the Flutter // side on the specified channel. diff --git a/shell/platform/common/cpp/client_wrapper/include/flutter/method_channel.h b/shell/platform/common/cpp/client_wrapper/include/flutter/method_channel.h index 96658af96c3c4..0fe94701c963b 100644 --- a/shell/platform/common/cpp/client_wrapper/include/flutter/method_channel.h +++ b/shell/platform/common/cpp/client_wrapper/include/flutter/method_channel.h @@ -44,21 +44,46 @@ class MethodChannel { MethodChannel& operator=(MethodChannel const&) = delete; // Sends a message to the Flutter engine on this channel. - void InvokeMethod(const std::string& method, std::unique_ptr arguments) { - MethodCall method_call(method, std::move(arguments)); - std::unique_ptr> message = - codec_->EncodeMethodCall(method_call); - messenger_->Send(name_, message->data(), message->size(), nullptr); - } - - // Sends a message to the Flutter engine on this channel expecting a reply. + // + // If |result| is provided, one of its methods will be invoked with the + // response from the engine. void InvokeMethod(const std::string& method, std::unique_ptr arguments, - flutter::BinaryReply reply) { + std::unique_ptr> result = nullptr) { MethodCall method_call(method, std::move(arguments)); std::unique_ptr> message = codec_->EncodeMethodCall(method_call); - messenger_->Send(name_, message->data(), message->size(), reply); + if (!result) { + messenger_->Send(name_, message->data(), message->size(), nullptr); + return; + } + + // std::function requires a copyable lambda, so convert to a shared pointer. + // This is safe since only one copy of the shared_pointer will ever be + // accessed. + std::shared_ptr> shared_result(result.release()); + const auto* codec = codec_; + std::string channel_name = name_; + BinaryReply reply_handler = [shared_result, codec, channel_name]( + const uint8_t* reply, size_t reply_size) { + if (reply_size == 0) { + shared_result->NotImplemented(); + return; + } + // Use this channel's codec to decode and handle the + // reply. + bool decoded = codec->DecodeAndProcessResponseEnvelope( + reply, reply_size, shared_result.get()); + if (!decoded) { + std::cerr << "Unable to decode reply to method " + "invocation on channel " + << channel_name << std::endl; + shared_result->NotImplemented(); + } + }; + + messenger_->Send(name_, message->data(), message->size(), + std::move(reply_handler)); } // Registers a handler that should be called any time a method call is @@ -76,7 +101,7 @@ class MethodChannel { std::string channel_name = name_; BinaryMessageHandler binary_handler = [handler, codec, channel_name]( const uint8_t* message, - const size_t message_size, + size_t message_size, BinaryReply reply) { // Use this channel's codec to decode the call and build a result handler. auto result = diff --git a/shell/platform/common/cpp/client_wrapper/include/flutter/method_codec.h b/shell/platform/common/cpp/client_wrapper/include/flutter/method_codec.h index 6f9e09ac24775..959e298c5984a 100644 --- a/shell/platform/common/cpp/client_wrapper/include/flutter/method_codec.h +++ b/shell/platform/common/cpp/client_wrapper/include/flutter/method_codec.h @@ -10,6 +10,7 @@ #include #include "method_call.h" +#include "method_result.h" namespace flutter { @@ -28,9 +29,8 @@ class MethodCodec { // Returns the MethodCall encoded in |message|, or nullptr if it cannot be // decoded. - std::unique_ptr> DecodeMethodCall( - const uint8_t* message, - const size_t message_size) const { + std::unique_ptr> DecodeMethodCall(const uint8_t* message, + size_t message_size) const { return std::move(DecodeMethodCallInternal(message, message_size)); } @@ -67,11 +67,23 @@ class MethodCodec { EncodeErrorEnvelopeInternal(error_code, error_message, error_details)); } + // Decodes the response envelope encoded in |response|, calling the + // appropriate method on |result|. + // + // Returns false if |response| cannot be decoded. In that case the caller is + // responsible for calling a |result| method. + bool DecodeAndProcessResponseEnvelope(const uint8_t* response, + size_t response_size, + MethodResult* result) const { + return DecodeAndProcessResponseEnvelopeInternal(response, response_size, + result); + } + protected: // Implementation of the public interface, to be provided by subclasses. virtual std::unique_ptr> DecodeMethodCallInternal( const uint8_t* message, - const size_t message_size) const = 0; + size_t message_size) const = 0; // Implementation of the public interface, to be provided by subclasses. virtual std::unique_ptr> EncodeMethodCallInternal( @@ -86,6 +98,12 @@ class MethodCodec { const std::string& error_code, const std::string& error_message, const T* error_details) const = 0; + + // Implementation of the public interface, to be provided by subclasses. + virtual bool DecodeAndProcessResponseEnvelopeInternal( + const uint8_t* response, + size_t response_size, + MethodResult* result) const = 0; }; } // namespace flutter diff --git a/shell/platform/common/cpp/client_wrapper/include/flutter/method_result.h b/shell/platform/common/cpp/client_wrapper/include/flutter/method_result.h index e3bf572996eb4..1e9c822e253e8 100644 --- a/shell/platform/common/cpp/client_wrapper/include/flutter/method_result.h +++ b/shell/platform/common/cpp/client_wrapper/include/flutter/method_result.h @@ -9,8 +9,8 @@ namespace flutter { -// Encapsulates a result sent back to the Flutter engine in response to a -// MethodCall. Only one method should be called on any given instance. +// Encapsulates a result returned from a MethodCall. Only one method should be +// called on any given instance. template class MethodResult { public: diff --git a/shell/platform/common/cpp/client_wrapper/include/flutter/method_result_functions.h b/shell/platform/common/cpp/client_wrapper/include/flutter/method_result_functions.h new file mode 100644 index 0000000000000..762128f444324 --- /dev/null +++ b/shell/platform/common/cpp/client_wrapper/include/flutter/method_result_functions.h @@ -0,0 +1,77 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_SHELL_PLATFORM_COMMON_CPP_CLIENT_WRAPPER_INCLUDE_FLUTTER_METHOD_RESULT_FUNCTIONS_H_ +#define FLUTTER_SHELL_PLATFORM_COMMON_CPP_CLIENT_WRAPPER_INCLUDE_FLUTTER_METHOD_RESULT_FUNCTIONS_H_ + +#include +#include + +#include "method_result.h" + +namespace flutter { + +// Handler types for each of the MethodResult outcomes. +template +using ResultHandlerSuccess = std::function; +template +using ResultHandlerError = std::function; +template +using ResultHandlerNotImplemented = std::function; + +// An implementation of MethodResult that pass calls through to provided +// function objects, for ease of constructing one-off result handlers. +template +class MethodResultFunctions : public MethodResult { + public: + // Creates a result object that calls the provided functions for the + // corresponding MethodResult outcomes. + MethodResultFunctions(ResultHandlerSuccess on_success, + ResultHandlerError on_error, + ResultHandlerNotImplemented on_not_implemented) + : on_success_(on_success), + on_error_(on_error), + on_not_implemented_(on_not_implemented) {} + + virtual ~MethodResultFunctions() = default; + + // Prevent copying. + MethodResultFunctions(MethodResultFunctions const&) = delete; + MethodResultFunctions& operator=(MethodResultFunctions const&) = delete; + + protected: + // |flutter::MethodResult| + void SuccessInternal(const T* result) override { + if (on_success_) { + on_success_(result); + } + } + + // |flutter::MethodResult| + void ErrorInternal(const std::string& error_code, + const std::string& error_message, + const T* error_details) override { + if (on_error_) { + on_error_(error_code, error_message, error_details); + } + } + + // |flutter::MethodResult| + void NotImplementedInternal() override { + if (on_not_implemented_) { + on_not_implemented_(); + } + } + + private: + ResultHandlerSuccess on_success_; + ResultHandlerError on_error_; + ResultHandlerNotImplemented on_not_implemented_; +}; + +} // namespace flutter + +#endif // FLUTTER_SHELL_PLATFORM_COMMON_CPP_CLIENT_WRAPPER_INCLUDE_FLUTTER_METHOD_RESULT_FUNCTIONS_H_ diff --git a/shell/platform/common/cpp/client_wrapper/include/flutter/standard_method_codec.h b/shell/platform/common/cpp/client_wrapper/include/flutter/standard_method_codec.h index 3b474110613cb..ef40897893183 100644 --- a/shell/platform/common/cpp/client_wrapper/include/flutter/standard_method_codec.h +++ b/shell/platform/common/cpp/client_wrapper/include/flutter/standard_method_codec.h @@ -30,7 +30,7 @@ class StandardMethodCodec : public MethodCodec { // |flutter::MethodCodec| std::unique_ptr> DecodeMethodCallInternal( const uint8_t* message, - const size_t message_size) const override; + size_t message_size) const override; // |flutter::MethodCodec| std::unique_ptr> EncodeMethodCallInternal( @@ -45,6 +45,12 @@ class StandardMethodCodec : public MethodCodec { const std::string& error_code, const std::string& error_message, const EncodableValue* error_details) const override; + + // |flutter::MethodCodec| + bool DecodeAndProcessResponseEnvelopeInternal( + const uint8_t* response, + size_t response_size, + MethodResult* result) const override; }; } // namespace flutter diff --git a/shell/platform/common/cpp/client_wrapper/method_channel_unittests.cc b/shell/platform/common/cpp/client_wrapper/method_channel_unittests.cc index 549c1023a4ab0..ee62b526eaf12 100644 --- a/shell/platform/common/cpp/client_wrapper/method_channel_unittests.cc +++ b/shell/platform/common/cpp/client_wrapper/method_channel_unittests.cc @@ -8,6 +8,7 @@ #include #include "flutter/shell/platform/common/cpp/client_wrapper/include/flutter/binary_messenger.h" +#include "flutter/shell/platform/common/cpp/client_wrapper/include/flutter/method_result_functions.h" #include "flutter/shell/platform/common/cpp/client_wrapper/include/flutter/standard_method_codec.h" #include "gtest/gtest.h" @@ -19,12 +20,11 @@ class TestBinaryMessenger : public BinaryMessenger { public: void Send(const std::string& channel, const uint8_t* message, - const size_t message_size) const override {} - - void Send(const std::string& channel, - const uint8_t* message, - const size_t message_size, - BinaryReply reply) const override {} + size_t message_size, + BinaryReply reply) const override { + send_called_ = true; + last_reply_handler_ = reply; + } void SetMessageHandler(const std::string& channel, BinaryMessageHandler handler) override { @@ -32,6 +32,10 @@ class TestBinaryMessenger : public BinaryMessenger { last_message_handler_ = handler; } + bool send_called() { return send_called_; } + + BinaryReply last_reply_handler() { return last_reply_handler_; } + std::string last_message_handler_channel() { return last_message_handler_channel_; } @@ -39,6 +43,8 @@ class TestBinaryMessenger : public BinaryMessenger { BinaryMessageHandler last_message_handler() { return last_message_handler_; } private: + mutable bool send_called_ = false; + mutable BinaryReply last_reply_handler_; std::string last_message_handler_channel_; BinaryMessageHandler last_message_handler_; }; @@ -71,8 +77,8 @@ TEST(MethodChannelTest, Registration) { messenger.last_message_handler()( message->data(), message->size(), - [](const uint8_t* reply, const size_t reply_size) {}); - EXPECT_EQ(callback_called, true); + [](const uint8_t* reply, size_t reply_size) {}); + EXPECT_TRUE(callback_called); } // Tests that SetMethodCallHandler with a null handler unregisters the handler. @@ -91,4 +97,63 @@ TEST(MethodChannelTest, Unregistration) { EXPECT_EQ(messenger.last_message_handler(), nullptr); } +TEST(MethodChannelTest, InvokeWithoutResponse) { + TestBinaryMessenger messenger; + const std::string channel_name("some_channel"); + MethodChannel channel(&messenger, channel_name, + &flutter::StandardMethodCodec::GetInstance()); + + channel.InvokeMethod("foo", nullptr); + EXPECT_TRUE(messenger.send_called()); + EXPECT_EQ(messenger.last_reply_handler(), nullptr); +} + +TEST(MethodChannelTest, InvokeWithResponse) { + TestBinaryMessenger messenger; + const std::string channel_name("some_channel"); + MethodChannel channel(&messenger, channel_name, + &flutter::StandardMethodCodec::GetInstance()); + + bool received_reply = false; + const std::string reply = "bar"; + auto result_handler = std::make_unique>( + [&received_reply, reply](const EncodableValue* success_value) { + received_reply = true; + EXPECT_EQ(success_value->StringValue(), reply); + }, + nullptr, nullptr); + + channel.InvokeMethod("foo", nullptr, std::move(result_handler)); + EXPECT_TRUE(messenger.send_called()); + ASSERT_NE(messenger.last_reply_handler(), nullptr); + + // Call the underlying reply handler to ensure it's processed correctly. + EncodableValue reply_value(reply); + std::unique_ptr> encoded_reply = + flutter::StandardMethodCodec::GetInstance().EncodeSuccessEnvelope( + &reply_value); + messenger.last_reply_handler()(encoded_reply->data(), encoded_reply->size()); + EXPECT_TRUE(received_reply); +} + +TEST(MethodChannelTest, InvokeNotImplemented) { + TestBinaryMessenger messenger; + const std::string channel_name("some_channel"); + MethodChannel channel(&messenger, channel_name, + &flutter::StandardMethodCodec::GetInstance()); + + bool received_not_implemented = false; + auto result_handler = std::make_unique>( + nullptr, nullptr, + [&received_not_implemented]() { received_not_implemented = true; }); + + channel.InvokeMethod("foo", nullptr, std::move(result_handler)); + EXPECT_EQ(messenger.send_called(), true); + ASSERT_NE(messenger.last_reply_handler(), nullptr); + + // Call the underlying reply handler to ensure it's reported as unimplemented. + messenger.last_reply_handler()(nullptr, 0); + EXPECT_TRUE(received_not_implemented); +} + } // namespace flutter diff --git a/shell/platform/common/cpp/client_wrapper/method_result_functions_unittests.cc b/shell/platform/common/cpp/client_wrapper/method_result_functions_unittests.cc new file mode 100644 index 0000000000000..98750dbba244b --- /dev/null +++ b/shell/platform/common/cpp/client_wrapper/method_result_functions_unittests.cc @@ -0,0 +1,66 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/shell/platform/common/cpp/client_wrapper/include/flutter/method_result_functions.h" + +#include +#include + +#include "gtest/gtest.h" + +namespace flutter { + +// Tests that unset handlers don't cause crashes. +TEST(MethodChannelTest, NoHandlers) { + MethodResultFunctions result(nullptr, nullptr, nullptr); + result.Success(); + result.Error("error"); + result.NotImplemented(); +} + +// Tests that Success calls through to handler. +TEST(MethodChannelTest, Success) { + bool called = false; + int value = 1; + MethodResultFunctions result( + [&called, value](const int* i) { + called = true; + EXPECT_EQ(*i, value); + }, + nullptr, nullptr); + result.Success(&value); + EXPECT_TRUE(called); +} + +// Tests that Error calls through to handler. +TEST(MethodChannelTest, Error) { + bool called = false; + std::string error_code = "a"; + std::string error_message = "b"; + int error_details = 1; + MethodResultFunctions result( + nullptr, + [&called, error_code, error_message, error_details]( + const std::string& code, const std::string& message, + const int* details) { + called = true; + EXPECT_EQ(code, error_code); + EXPECT_EQ(message, error_message); + EXPECT_EQ(*details, error_details); + }, + nullptr); + result.Error(error_code, error_message, &error_details); + EXPECT_TRUE(called); +} + +// Tests that NotImplemented calls through to handler. +TEST(MethodChannelTest, NotImplemented) { + bool called = false; + MethodResultFunctions result(nullptr, nullptr, + [&called]() { called = true; }); + result.NotImplemented(); + EXPECT_TRUE(called); +} + +} // namespace flutter diff --git a/shell/platform/common/cpp/client_wrapper/plugin_registrar.cc b/shell/platform/common/cpp/client_wrapper/plugin_registrar.cc index 92584a93ee903..9527dd73ead3d 100644 --- a/shell/platform/common/cpp/client_wrapper/plugin_registrar.cc +++ b/shell/platform/common/cpp/client_wrapper/plugin_registrar.cc @@ -26,7 +26,7 @@ void ForwardToHandler(FlutterDesktopMessengerRef messenger, auto* response_handle = message->response_handle; BinaryReply reply_handler = [messenger, response_handle]( const uint8_t* reply, - const size_t reply_size) mutable { + size_t reply_size) mutable { if (!response_handle) { std::cerr << "Error: Response can be set only once. Ignoring " "duplicate response." @@ -65,12 +65,7 @@ class BinaryMessengerImpl : public BinaryMessenger { // |flutter::BinaryMessenger| void Send(const std::string& channel, const uint8_t* message, - const size_t message_size) const override; - - // |flutter::BinaryMessenger| - void Send(const std::string& channel, - const uint8_t* message, - const size_t message_size, + size_t message_size, BinaryReply reply) const override; // |flutter::BinaryMessenger| @@ -88,14 +83,7 @@ class BinaryMessengerImpl : public BinaryMessenger { void BinaryMessengerImpl::Send(const std::string& channel, const uint8_t* message, - const size_t message_size) const { - FlutterDesktopMessengerSend(messenger_, channel.c_str(), message, - message_size); -} - -void BinaryMessengerImpl::Send(const std::string& channel, - const uint8_t* message, - const size_t message_size, + size_t message_size, BinaryReply reply) const { if (reply == nullptr) { FlutterDesktopMessengerSend(messenger_, channel.c_str(), message, diff --git a/shell/platform/common/cpp/client_wrapper/standard_codec.cc b/shell/platform/common/cpp/client_wrapper/standard_codec.cc index 137a3e849002c..d6e38c6bf4b79 100644 --- a/shell/platform/common/cpp/client_wrapper/standard_codec.cc +++ b/shell/platform/common/cpp/client_wrapper/standard_codec.cc @@ -286,7 +286,7 @@ StandardMessageCodec::~StandardMessageCodec() = default; std::unique_ptr StandardMessageCodec::DecodeMessageInternal( const uint8_t* binary_message, - const size_t message_size) const { + size_t message_size) const { StandardCodecSerializer serializer; ByteBufferStreamReader stream(binary_message, message_size); return std::make_unique(serializer.ReadValue(&stream)); @@ -312,7 +312,7 @@ const StandardMethodCodec& StandardMethodCodec::GetInstance() { std::unique_ptr> StandardMethodCodec::DecodeMethodCallInternal(const uint8_t* message, - const size_t message_size) const { + size_t message_size) const { StandardCodecSerializer serializer; ByteBufferStreamReader stream(message, message_size); EncodableValue method_name = serializer.ReadValue(&stream); @@ -380,4 +380,31 @@ StandardMethodCodec::EncodeErrorEnvelopeInternal( return encoded; } +bool StandardMethodCodec::DecodeAndProcessResponseEnvelopeInternal( + const uint8_t* response, + size_t response_size, + MethodResult* result) const { + StandardCodecSerializer serializer; + ByteBufferStreamReader stream(response, response_size); + uint8_t flag = stream.ReadByte(); + switch (flag) { + case 0: { + EncodableValue value = serializer.ReadValue(&stream); + result->Success(value.IsNull() ? nullptr : &value); + return true; + } + case 1: { + EncodableValue code = serializer.ReadValue(&stream); + EncodableValue message = serializer.ReadValue(&stream); + EncodableValue details = serializer.ReadValue(&stream); + result->Error(code.StringValue(), + message.IsNull() ? "" : message.StringValue(), + details.IsNull() ? nullptr : &details); + return true; + } + default: + return false; + } +} + } // namespace flutter diff --git a/shell/platform/common/cpp/client_wrapper/standard_method_codec_unittests.cc b/shell/platform/common/cpp/client_wrapper/standard_method_codec_unittests.cc index e3f4b15de77a8..4b43b40229038 100644 --- a/shell/platform/common/cpp/client_wrapper/standard_method_codec_unittests.cc +++ b/shell/platform/common/cpp/client_wrapper/standard_method_codec_unittests.cc @@ -4,6 +4,7 @@ #include "flutter/shell/platform/common/cpp/client_wrapper/include/flutter/standard_method_codec.h" +#include "flutter/shell/platform/common/cpp/client_wrapper/include/flutter/method_result_functions.h" #include "flutter/shell/platform/common/cpp/client_wrapper/testing/encodable_value_utils.h" #include "gtest/gtest.h" @@ -60,7 +61,17 @@ TEST(StandardMethodCodec, HandlesSuccessEnvelopesWithNullResult) { ASSERT_NE(encoded.get(), nullptr); std::vector bytes = {0x00, 0x00}; EXPECT_EQ(*encoded, bytes); - // TODO: Add round-trip check once decoding replies is implemented. + + bool decoded_successfully = false; + MethodResultFunctions result_handler( + [&decoded_successfully](const EncodableValue* result) { + decoded_successfully = true; + EXPECT_EQ(result, nullptr); + }, + nullptr, nullptr); + codec.DecodeAndProcessResponseEnvelope(encoded->data(), encoded->size(), + &result_handler); + EXPECT_TRUE(decoded_successfully); } TEST(StandardMethodCodec, HandlesSuccessEnvelopesWithResult) { @@ -70,7 +81,17 @@ TEST(StandardMethodCodec, HandlesSuccessEnvelopesWithResult) { ASSERT_NE(encoded.get(), nullptr); std::vector bytes = {0x00, 0x03, 0x2a, 0x00, 0x00, 0x00}; EXPECT_EQ(*encoded, bytes); - // TODO: Add round-trip check once decoding replies is implemented. + + bool decoded_successfully = false; + MethodResultFunctions result_handler( + [&decoded_successfully](const EncodableValue* result) { + decoded_successfully = true; + EXPECT_EQ(result->IntValue(), 42); + }, + nullptr, nullptr); + codec.DecodeAndProcessResponseEnvelope(encoded->data(), encoded->size(), + &result_handler); + EXPECT_TRUE(decoded_successfully); } TEST(StandardMethodCodec, HandlesErrorEnvelopesWithNulls) { @@ -80,7 +101,22 @@ TEST(StandardMethodCodec, HandlesErrorEnvelopesWithNulls) { std::vector bytes = {0x01, 0x07, 0x09, 0x65, 0x72, 0x72, 0x6f, 0x72, 0x43, 0x6f, 0x64, 0x65, 0x00, 0x00}; EXPECT_EQ(*encoded, bytes); - // TODO: Add round-trip check once decoding replies is implemented. + + bool decoded_successfully = false; + MethodResultFunctions result_handler( + nullptr, + [&decoded_successfully](const std::string& code, + const std::string& message, + const EncodableValue* details) { + decoded_successfully = true; + EXPECT_EQ(code, "errorCode"); + EXPECT_EQ(message, ""); + EXPECT_EQ(details, nullptr); + }, + nullptr); + codec.DecodeAndProcessResponseEnvelope(encoded->data(), encoded->size(), + &result_handler); + EXPECT_TRUE(decoded_successfully); } TEST(StandardMethodCodec, HandlesErrorEnvelopesWithDetails) { @@ -99,7 +135,24 @@ TEST(StandardMethodCodec, HandlesErrorEnvelopesWithDetails) { 0x0c, 0x02, 0x07, 0x01, 0x61, 0x03, 0x2a, 0x00, 0x00, 0x00, }; EXPECT_EQ(*encoded, bytes); - // TODO: Add round-trip check once decoding replies is implemented. + + bool decoded_successfully = false; + MethodResultFunctions result_handler( + nullptr, + [&decoded_successfully](const std::string& code, + const std::string& message, + const EncodableValue* details) { + decoded_successfully = true; + EXPECT_EQ(code, "errorCode"); + EXPECT_EQ(message, "something failed"); + EXPECT_TRUE(details->IsList()); + EXPECT_EQ(details->ListValue()[0].StringValue(), "a"); + EXPECT_EQ(details->ListValue()[1].IntValue(), 42); + }, + nullptr); + codec.DecodeAndProcessResponseEnvelope(encoded->data(), encoded->size(), + &result_handler); + EXPECT_TRUE(decoded_successfully); } } // namespace flutter diff --git a/shell/platform/common/cpp/json_method_codec.cc b/shell/platform/common/cpp/json_method_codec.cc index 26f6a2402082b..4a37eb4950d33 100644 --- a/shell/platform/common/cpp/json_method_codec.cc +++ b/shell/platform/common/cpp/json_method_codec.cc @@ -9,9 +9,27 @@ namespace flutter { namespace { + // Keys used in MethodCall encoding. constexpr char kMessageMethodKey[] = "method"; constexpr char kMessageArgumentsKey[] = "args"; + +// Returns a new document containing only |element|, which must be an element +// in |document|. This is a move rather than a copy, so it is efficient but +// destructive to the data in |document|. +std::unique_ptr ExtractElement( + rapidjson::Document* document, + rapidjson::Value* subtree) { + auto extracted = std::make_unique(); + // Pull the subtree up to the root of the document. + document->Swap(*subtree); + // Swap the entire document into |extracted|. Unlike the swap above this moves + // the allocator ownership, so the data won't be deleted when |document| is + // destroyed. + extracted->Swap(*document); + return extracted; +} + } // namespace // static @@ -22,7 +40,7 @@ const JsonMethodCodec& JsonMethodCodec::GetInstance() { std::unique_ptr> JsonMethodCodec::DecodeMethodCallInternal(const uint8_t* message, - const size_t message_size) const { + size_t message_size) const { std::unique_ptr json_message = JsonMessageCodec::GetInstance().DecodeMessage(message, message_size); if (!json_message) { @@ -40,19 +58,7 @@ JsonMethodCodec::DecodeMethodCallInternal(const uint8_t* message, auto arguments_iter = json_message->FindMember(kMessageArgumentsKey); std::unique_ptr arguments; if (arguments_iter != json_message->MemberEnd()) { - // Pull the arguments subtree up to the root of json_message. This is - // destructive to json_message, but the full value is no longer needed, and - // this avoids a subtree copy. - // Note: The static_cast is for compatibility with RapidJSON 1.1; master - // already allows swapping a Document with a Value directly. Once there is - // a new RapidJSON release (at which point clients can be expected to have - // that change in the version they depend on) remove the cast. - static_cast(json_message.get()) - ->Swap(arguments_iter->value); - // Swap it into |arguments|. This moves the allocator ownership, so that - // the data won't be deleted when json_message goes out of scope. - arguments = std::make_unique(); - arguments->Swap(*json_message); + arguments = ExtractElement(json_message.get(), &(arguments_iter->value)); } return std::make_unique>( method_name, std::move(arguments)); @@ -108,4 +114,36 @@ JsonMethodCodec::EncodeErrorEnvelopeInternal( return JsonMessageCodec::GetInstance().EncodeMessage(envelope); } +bool JsonMethodCodec::DecodeAndProcessResponseEnvelopeInternal( + const uint8_t* response, + size_t response_size, + MethodResult* result) const { + std::unique_ptr json_response = + JsonMessageCodec::GetInstance().DecodeMessage(response, response_size); + if (!json_response) { + return false; + } + if (!json_response->IsArray()) { + return false; + } + switch (json_response->Size()) { + case 1: { + std::unique_ptr value = + ExtractElement(json_response.get(), &((*json_response)[0])); + result->Success(value->IsNull() ? nullptr : value.get()); + return true; + } + case 3: { + std::string code = (*json_response)[0].GetString(); + std::string message = (*json_response)[1].GetString(); + std::unique_ptr details = + ExtractElement(json_response.get(), &((*json_response)[2])); + result->Error(code, message, details->IsNull() ? nullptr : details.get()); + return true; + } + default: + return false; + } +} + } // namespace flutter diff --git a/shell/platform/common/cpp/json_method_codec.h b/shell/platform/common/cpp/json_method_codec.h index 597fb6e6d1f16..25e3963fcb03d 100644 --- a/shell/platform/common/cpp/json_method_codec.h +++ b/shell/platform/common/cpp/json_method_codec.h @@ -46,6 +46,12 @@ class JsonMethodCodec : public MethodCodec { const std::string& error_code, const std::string& error_message, const rapidjson::Document* error_details) const override; + + // |flutter::MethodCodec| + bool DecodeAndProcessResponseEnvelopeInternal( + const uint8_t* response, + const size_t response_size, + MethodResult* result) const override; }; } // namespace flutter diff --git a/shell/platform/common/cpp/json_method_codec_unittests.cc b/shell/platform/common/cpp/json_method_codec_unittests.cc index 36bffcff1d104..f9d2ec882fc45 100644 --- a/shell/platform/common/cpp/json_method_codec_unittests.cc +++ b/shell/platform/common/cpp/json_method_codec_unittests.cc @@ -4,6 +4,7 @@ #include "flutter/shell/platform/common/cpp/json_method_codec.h" +#include "flutter/shell/platform/common/cpp/client_wrapper/include/flutter/method_result_functions.h" #include "gtest/gtest.h" namespace flutter { @@ -60,7 +61,17 @@ TEST(JsonMethodCodec, HandlesSuccessEnvelopesWithNullResult) { ASSERT_TRUE(encoded); std::vector bytes = {'[', 'n', 'u', 'l', 'l', ']'}; EXPECT_EQ(*encoded, bytes); - // TODO: Add round-trip check once decoding replies is implemented. + + bool decoded_successfully = false; + MethodResultFunctions result_handler( + [&decoded_successfully](const rapidjson::Document* result) { + decoded_successfully = true; + EXPECT_EQ(result, nullptr); + }, + nullptr, nullptr); + codec.DecodeAndProcessResponseEnvelope(encoded->data(), encoded->size(), + &result_handler); + EXPECT_TRUE(decoded_successfully); } TEST(JsonMethodCodec, HandlesSuccessEnvelopesWithResult) { @@ -71,7 +82,17 @@ TEST(JsonMethodCodec, HandlesSuccessEnvelopesWithResult) { ASSERT_TRUE(encoded); std::vector bytes = {'[', '4', '2', ']'}; EXPECT_EQ(*encoded, bytes); - // TODO: Add round-trip check once decoding replies is implemented. + + bool decoded_successfully = false; + MethodResultFunctions result_handler( + [&decoded_successfully](const rapidjson::Document* result) { + decoded_successfully = true; + EXPECT_EQ(result->GetInt(), 42); + }, + nullptr, nullptr); + codec.DecodeAndProcessResponseEnvelope(encoded->data(), encoded->size(), + &result_handler); + EXPECT_TRUE(decoded_successfully); } TEST(JsonMethodCodec, HandlesErrorEnvelopesWithNulls) { @@ -83,7 +104,22 @@ TEST(JsonMethodCodec, HandlesErrorEnvelopesWithNulls) { '"', ',', '"', '"', ',', 'n', 'u', 'l', 'l', ']', }; EXPECT_EQ(*encoded, bytes); - // TODO: Add round-trip check once decoding replies is implemented. + + bool decoded_successfully = false; + MethodResultFunctions result_handler( + nullptr, + [&decoded_successfully](const std::string& code, + const std::string& message, + const rapidjson::Document* details) { + decoded_successfully = true; + EXPECT_EQ(code, "errorCode"); + EXPECT_EQ(message, ""); + EXPECT_EQ(details, nullptr); + }, + nullptr); + codec.DecodeAndProcessResponseEnvelope(encoded->data(), encoded->size(), + &result_handler); + EXPECT_TRUE(decoded_successfully); } TEST(JsonMethodCodec, HandlesErrorEnvelopesWithDetails) { @@ -101,7 +137,24 @@ TEST(JsonMethodCodec, HandlesErrorEnvelopesWithDetails) { 'e', 'd', '"', ',', '[', '"', 'a', '"', ',', '4', '2', ']', ']', }; EXPECT_EQ(*encoded, bytes); - // TODO: Add round-trip check once decoding replies is implemented. + + bool decoded_successfully = false; + MethodResultFunctions result_handler( + nullptr, + [&decoded_successfully](const std::string& code, + const std::string& message, + const rapidjson::Document* details) { + decoded_successfully = true; + EXPECT_EQ(code, "errorCode"); + EXPECT_EQ(message, "something failed"); + EXPECT_TRUE(details->IsArray()); + EXPECT_EQ(std::string((*details)[0].GetString()), "a"); + EXPECT_EQ((*details)[1].GetInt(), 42); + }, + nullptr); + codec.DecodeAndProcessResponseEnvelope(encoded->data(), encoded->size(), + &result_handler); + EXPECT_TRUE(decoded_successfully); } } // namespace flutter