From 7a84f1291ee89d35f115509c2f97745e275a043d Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Tue, 24 Mar 2020 19:30:23 -0700 Subject: [PATCH 1/9] Move JSON codecs from wrapper to common internal --- shell/platform/common/cpp/BUILD.gn | 9 ++++++--- shell/platform/common/cpp/client_wrapper/BUILD.gn | 3 --- .../common/cpp/client_wrapper/core_wrapper_files.gni | 5 ----- .../cpp/{client_wrapper => }/json_message_codec.cc | 2 +- .../include/flutter => }/json_message_codec.h | 0 .../common/cpp/{client_wrapper => }/json_method_codec.cc | 4 ++-- .../include/flutter => }/json_method_codec.h | 0 .../cpp/{client_wrapper/include/flutter => }/json_type.h | 0 shell/platform/glfw/key_event_handler.cc | 2 +- shell/platform/glfw/platform_handler.cc | 2 +- shell/platform/glfw/text_input_plugin.cc | 2 +- shell/platform/windows/key_event_handler.cc | 2 +- shell/platform/windows/platform_handler.cc | 2 +- shell/platform/windows/text_input_plugin.cc | 2 +- 14 files changed, 15 insertions(+), 20 deletions(-) rename shell/platform/common/cpp/{client_wrapper => }/json_message_codec.cc (97%) rename shell/platform/common/cpp/{client_wrapper/include/flutter => }/json_message_codec.h (100%) rename shell/platform/common/cpp/{client_wrapper => }/json_method_codec.cc (97%) rename shell/platform/common/cpp/{client_wrapper/include/flutter => }/json_method_codec.h (100%) rename shell/platform/common/cpp/{client_wrapper/include/flutter => }/json_type.h (100%) diff --git a/shell/platform/common/cpp/BUILD.gn b/shell/platform/common/cpp/BUILD.gn index 00db206ed2a70..115e020dc22be 100644 --- a/shell/platform/common/cpp/BUILD.gn +++ b/shell/platform/common/cpp/BUILD.gn @@ -33,6 +33,9 @@ source_set("common_cpp_library_headers") { source_set("common_cpp") { public = [ "incoming_message_dispatcher.h", + "json_message_codec.h", + "json_method_codec.h", + "json_type.h", "text_input_model.h", ] @@ -40,6 +43,8 @@ source_set("common_cpp") { # to the _public_headers above into this target. sources = [ "incoming_message_dispatcher.cc", + "json_message_codec.cc", + "json_method_codec.cc", "text_input_model.cc", ] @@ -49,16 +54,14 @@ source_set("common_cpp") { ":common_cpp_library_headers", "//flutter/shell/platform/common/cpp/client_wrapper:client_wrapper", "//flutter/shell/platform/embedder:embedder_with_symbol_prefix", + "//third_party/rapidjson", ] public_deps = [ ":common_cpp_core", ] - # TODO: Remove once text input model refactor lands, at which point this code - # won't have a JSON dependency. defines = [ "USE_RAPID_JSON" ] - deps += [ "//third_party/rapidjson" ] if (is_win) { # For wstring_conversion. See issue #50053. diff --git a/shell/platform/common/cpp/client_wrapper/BUILD.gn b/shell/platform/common/cpp/client_wrapper/BUILD.gn index a8d30109d2856..f46cd81461ed0 100644 --- a/shell/platform/common/cpp/client_wrapper/BUILD.gn +++ b/shell/platform/common/cpp/client_wrapper/BUILD.gn @@ -12,11 +12,8 @@ source_set("client_wrapper") { deps = [ "//flutter/shell/platform/common/cpp:common_cpp_library_headers", - "//third_party/rapidjson", ] - defines = [ "USE_RAPID_JSON" ] - configs += [ "//flutter/shell/platform/common/cpp:desktop_library_implementation" ] 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 ce91be7866717..2521ded89af70 100644 --- a/shell/platform/common/cpp/client_wrapper/core_wrapper_files.gni +++ b/shell/platform/common/cpp/client_wrapper/core_wrapper_files.gni @@ -8,9 +8,6 @@ core_cpp_client_wrapper_includes = "include/flutter/binary_messenger.h", "include/flutter/encodable_value.h", "include/flutter/engine_method_result.h", - "include/flutter/json_message_codec.h", - "include/flutter/json_method_codec.h", - "include/flutter/json_type.h", "include/flutter/message_codec.h", "include/flutter/method_call.h", "include/flutter/method_channel.h", @@ -31,8 +28,6 @@ core_cpp_client_wrapper_sources = [ "byte_stream_wrappers.h", "engine_method_result.cc", - "json_message_codec.cc", # TODO combine into a single json_codec.cc. - "json_method_codec.cc", # TODO combine into a single json_codec.cc. "plugin_registrar.cc", "standard_codec_serializer.h", "standard_codec.cc", diff --git a/shell/platform/common/cpp/client_wrapper/json_message_codec.cc b/shell/platform/common/cpp/json_message_codec.cc similarity index 97% rename from shell/platform/common/cpp/client_wrapper/json_message_codec.cc rename to shell/platform/common/cpp/json_message_codec.cc index 5610c4762ea83..a88731e75807f 100644 --- a/shell/platform/common/cpp/client_wrapper/json_message_codec.cc +++ b/shell/platform/common/cpp/json_message_codec.cc @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "include/flutter/json_message_codec.h" +#include "flutter/shell/platform/common/cpp/json_message_codec.h" #include #include diff --git a/shell/platform/common/cpp/client_wrapper/include/flutter/json_message_codec.h b/shell/platform/common/cpp/json_message_codec.h similarity index 100% rename from shell/platform/common/cpp/client_wrapper/include/flutter/json_message_codec.h rename to shell/platform/common/cpp/json_message_codec.h diff --git a/shell/platform/common/cpp/client_wrapper/json_method_codec.cc b/shell/platform/common/cpp/json_method_codec.cc similarity index 97% rename from shell/platform/common/cpp/client_wrapper/json_method_codec.cc rename to shell/platform/common/cpp/json_method_codec.cc index 6c9528ff2e304..1647522dcfce2 100644 --- a/shell/platform/common/cpp/client_wrapper/json_method_codec.cc +++ b/shell/platform/common/cpp/json_method_codec.cc @@ -2,9 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "include/flutter/json_method_codec.h" +#include "flutter/shell/platform/common/cpp/json_method_codec.h" -#include "include/flutter/json_message_codec.h" +#include "flutter/shell/platform/common/cpp/json_message_codec.h" namespace flutter { diff --git a/shell/platform/common/cpp/client_wrapper/include/flutter/json_method_codec.h b/shell/platform/common/cpp/json_method_codec.h similarity index 100% rename from shell/platform/common/cpp/client_wrapper/include/flutter/json_method_codec.h rename to shell/platform/common/cpp/json_method_codec.h diff --git a/shell/platform/common/cpp/client_wrapper/include/flutter/json_type.h b/shell/platform/common/cpp/json_type.h similarity index 100% rename from shell/platform/common/cpp/client_wrapper/include/flutter/json_type.h rename to shell/platform/common/cpp/json_type.h diff --git a/shell/platform/glfw/key_event_handler.cc b/shell/platform/glfw/key_event_handler.cc index 2ad013c064462..bdd121091f54e 100644 --- a/shell/platform/glfw/key_event_handler.cc +++ b/shell/platform/glfw/key_event_handler.cc @@ -6,7 +6,7 @@ #include -#include "flutter/shell/platform/common/cpp/client_wrapper/include/flutter/json_message_codec.h" +#include "flutter/shell/platform/common/cpp/json_message_codec.h" static constexpr char kChannelName[] = "flutter/keyevent"; diff --git a/shell/platform/glfw/platform_handler.cc b/shell/platform/glfw/platform_handler.cc index 39c8b4708cd75..6d4046364abc6 100644 --- a/shell/platform/glfw/platform_handler.cc +++ b/shell/platform/glfw/platform_handler.cc @@ -6,7 +6,7 @@ #include -#include "flutter/shell/platform/common/cpp/client_wrapper/include/flutter/json_method_codec.h" +#include "flutter/shell/platform/common/cpp/json_method_codec.h" static constexpr char kChannelName[] = "flutter/platform"; diff --git a/shell/platform/glfw/text_input_plugin.cc b/shell/platform/glfw/text_input_plugin.cc index 94fa0a267cf5e..d1f163b2f1065 100644 --- a/shell/platform/glfw/text_input_plugin.cc +++ b/shell/platform/glfw/text_input_plugin.cc @@ -7,7 +7,7 @@ #include #include -#include "flutter/shell/platform/common/cpp/client_wrapper/include/flutter/json_method_codec.h" +#include "flutter/shell/platform/common/cpp/json_method_codec.h" static constexpr char kSetEditingStateMethod[] = "TextInput.setEditingState"; static constexpr char kClearClientMethod[] = "TextInput.clearClient"; diff --git a/shell/platform/windows/key_event_handler.cc b/shell/platform/windows/key_event_handler.cc index 63af2446b889b..f611210174b75 100644 --- a/shell/platform/windows/key_event_handler.cc +++ b/shell/platform/windows/key_event_handler.cc @@ -8,7 +8,7 @@ #include -#include "flutter/shell/platform/common/cpp/client_wrapper/include/flutter/json_message_codec.h" +#include "flutter/shell/platform/common/cpp/json_message_codec.h" static constexpr char kChannelName[] = "flutter/keyevent"; diff --git a/shell/platform/windows/platform_handler.cc b/shell/platform/windows/platform_handler.cc index c4bf592744345..f1540a9819016 100644 --- a/shell/platform/windows/platform_handler.cc +++ b/shell/platform/windows/platform_handler.cc @@ -8,7 +8,7 @@ #include -#include "flutter/shell/platform/common/cpp/client_wrapper/include/flutter/json_method_codec.h" +#include "flutter/shell/platform/common/cpp/json_method_codec.h" static constexpr char kChannelName[] = "flutter/platform"; diff --git a/shell/platform/windows/text_input_plugin.cc b/shell/platform/windows/text_input_plugin.cc index 6b71ff23e5c3d..0fd4dee9d2442 100644 --- a/shell/platform/windows/text_input_plugin.cc +++ b/shell/platform/windows/text_input_plugin.cc @@ -9,7 +9,7 @@ #include #include -#include "flutter/shell/platform/common/cpp/client_wrapper/include/flutter/json_method_codec.h" +#include "flutter/shell/platform/common/cpp/json_method_codec.h" static constexpr char kSetEditingStateMethod[] = "TextInput.setEditingState"; static constexpr char kClearClientMethod[] = "TextInput.clearClient"; From 733d160a94959131ee3a335d17f351ee78a75513 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Tue, 24 Mar 2020 19:37:09 -0700 Subject: [PATCH 2/9] Remove the jsoncpp codepath --- shell/platform/common/cpp/BUILD.gn | 3 -- .../platform/common/cpp/json_message_codec.cc | 32 +++------------ .../platform/common/cpp/json_message_codec.h | 9 +++-- .../platform/common/cpp/json_method_codec.cc | 39 +++---------------- shell/platform/common/cpp/json_method_codec.h | 13 ++++--- shell/platform/common/cpp/json_type.h | 26 ------------- shell/platform/glfw/BUILD.gn | 2 - shell/platform/windows/BUILD.gn | 2 - 8 files changed, 23 insertions(+), 103 deletions(-) delete mode 100644 shell/platform/common/cpp/json_type.h diff --git a/shell/platform/common/cpp/BUILD.gn b/shell/platform/common/cpp/BUILD.gn index 115e020dc22be..394de5e491d2a 100644 --- a/shell/platform/common/cpp/BUILD.gn +++ b/shell/platform/common/cpp/BUILD.gn @@ -35,7 +35,6 @@ source_set("common_cpp") { "incoming_message_dispatcher.h", "json_message_codec.h", "json_method_codec.h", - "json_type.h", "text_input_model.h", ] @@ -61,8 +60,6 @@ source_set("common_cpp") { ":common_cpp_core", ] - defines = [ "USE_RAPID_JSON" ] - if (is_win) { # For wstring_conversion. See issue #50053. defines += [ "_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING" ] diff --git a/shell/platform/common/cpp/json_message_codec.cc b/shell/platform/common/cpp/json_message_codec.cc index a88731e75807f..d2a43bd806ded 100644 --- a/shell/platform/common/cpp/json_message_codec.cc +++ b/shell/platform/common/cpp/json_message_codec.cc @@ -7,11 +7,9 @@ #include #include -#ifdef USE_RAPID_JSON #include "rapidjson/error/en.h" #include "rapidjson/stringbuffer.h" #include "rapidjson/writer.h" -#endif namespace flutter { @@ -22,8 +20,7 @@ const JsonMessageCodec& JsonMessageCodec::GetInstance() { } std::unique_ptr> JsonMessageCodec::EncodeMessageInternal( - const JsonValueType& message) const { -#ifdef USE_RAPID_JSON + const rapidjson::Document& message) const { // TODO: Look into alternate writers that would avoid the buffer copy. rapidjson::StringBuffer buffer; rapidjson::Writer writer(buffer); @@ -31,37 +28,20 @@ std::unique_ptr> JsonMessageCodec::EncodeMessageInternal( const char* buffer_start = buffer.GetString(); return std::make_unique>( buffer_start, buffer_start + buffer.GetSize()); -#else - Json::StreamWriterBuilder writer_builder; - std::string serialization = Json::writeString(writer_builder, message); - return std::make_unique>(serialization.begin(), - serialization.end()); -#endif } -std::unique_ptr JsonMessageCodec::DecodeMessageInternal( +std::unique_ptr JsonMessageCodec::DecodeMessageInternal( const uint8_t* binary_message, const size_t message_size) const { auto raw_message = reinterpret_cast(binary_message); - auto json_message = std::make_unique(); - std::string parse_errors; - bool parsing_successful = false; -#ifdef USE_RAPID_JSON + auto json_message = std::make_unique(); rapidjson::ParseResult result = json_message->Parse(raw_message, message_size); - parsing_successful = result == rapidjson::ParseErrorCode::kParseErrorNone; - if (!parsing_successful) { - parse_errors = rapidjson::GetParseError_En(result.Code()); - } -#else - Json::CharReaderBuilder reader_builder; - std::unique_ptr parser(reader_builder.newCharReader()); - parsing_successful = parser->parse(raw_message, raw_message + message_size, - json_message.get(), &parse_errors); -#endif + bool parsing_successful = + result == rapidjson::ParseErrorCode::kParseErrorNone; if (!parsing_successful) { std::cerr << "Unable to parse JSON message:" << std::endl - << parse_errors << std::endl; + << rapidjson::GetParseError_En(result.Code()) << std::endl; return nullptr; } return json_message; diff --git a/shell/platform/common/cpp/json_message_codec.h b/shell/platform/common/cpp/json_message_codec.h index ba1aa2ffc3520..aa0819c1a87f4 100644 --- a/shell/platform/common/cpp/json_message_codec.h +++ b/shell/platform/common/cpp/json_message_codec.h @@ -5,14 +5,15 @@ #ifndef FLUTTER_SHELL_PLATFORM_COMMON_CPP_CLIENT_WRAPPER_INCLUDE_FLUTTER_JSON_MESSAGE_CODEC_H_ #define FLUTTER_SHELL_PLATFORM_COMMON_CPP_CLIENT_WRAPPER_INCLUDE_FLUTTER_JSON_MESSAGE_CODEC_H_ -#include "json_type.h" +#include + #include "message_codec.h" namespace flutter { // A message encoding/decoding mechanism for communications to/from the // Flutter engine via JSON channels. -class JsonMessageCodec : public MessageCodec { +class JsonMessageCodec : public MessageCodec { public: // Returns the shared instance of the codec. static const JsonMessageCodec& GetInstance(); @@ -28,13 +29,13 @@ class JsonMessageCodec : public MessageCodec { JsonMessageCodec() = default; // |flutter::MessageCodec| - std::unique_ptr DecodeMessageInternal( + std::unique_ptr DecodeMessageInternal( const uint8_t* binary_message, const size_t message_size) const override; // |flutter::MessageCodec| std::unique_ptr> EncodeMessageInternal( - const JsonValueType& message) const override; + const rapidjson::Document& message) const override; }; } // namespace flutter diff --git a/shell/platform/common/cpp/json_method_codec.cc b/shell/platform/common/cpp/json_method_codec.cc index 1647522dcfce2..26f6a2402082b 100644 --- a/shell/platform/common/cpp/json_method_codec.cc +++ b/shell/platform/common/cpp/json_method_codec.cc @@ -20,16 +20,15 @@ const JsonMethodCodec& JsonMethodCodec::GetInstance() { return sInstance; } -std::unique_ptr> +std::unique_ptr> JsonMethodCodec::DecodeMethodCallInternal(const uint8_t* message, const size_t message_size) const { - std::unique_ptr json_message = + std::unique_ptr json_message = JsonMessageCodec::GetInstance().DecodeMessage(message, message_size); if (!json_message) { return nullptr; } -#if USE_RAPID_JSON auto method_name_iter = json_message->FindMember(kMessageMethodKey); if (method_name_iter == json_message->MemberEnd()) { return nullptr; @@ -57,20 +56,10 @@ JsonMethodCodec::DecodeMethodCallInternal(const uint8_t* message, } return std::make_unique>( method_name, std::move(arguments)); -#else - Json::Value method = (*json_message)[kMessageMethodKey]; - if (method.isNull()) { - return nullptr; - } - return std::make_unique>( - method.asString(), - std::make_unique((*json_message)[kMessageArgumentsKey])); -#endif } std::unique_ptr> JsonMethodCodec::EncodeMethodCallInternal( - const MethodCall& method_call) const { -#if USE_RAPID_JSON + const MethodCall& method_call) const { // TODO: Consider revisiting the codec APIs to avoid the need to copy // everything when doing encoding (e.g., by having a version that takes // owership of the object to encode, so that it can be moved instead). @@ -83,20 +72,13 @@ std::unique_ptr> JsonMethodCodec::EncodeMethodCallInternal( } message.AddMember(kMessageMethodKey, name, allocator); message.AddMember(kMessageArgumentsKey, arguments, allocator); -#else - Json::Value message(Json::objectValue); - message[kMessageMethodKey] = method_call.method_name(); - const Json::Value* arguments = method_call.arguments(); - message[kMessageArgumentsKey] = arguments ? *arguments : Json::Value(); -#endif return JsonMessageCodec::GetInstance().EncodeMessage(message); } std::unique_ptr> JsonMethodCodec::EncodeSuccessEnvelopeInternal( - const JsonValueType* result) const { -#if USE_RAPID_JSON + const rapidjson::Document* result) const { rapidjson::Document envelope; envelope.SetArray(); rapidjson::Value result_value; @@ -104,10 +86,6 @@ JsonMethodCodec::EncodeSuccessEnvelopeInternal( result_value.CopyFrom(*result, envelope.GetAllocator()); } envelope.PushBack(result_value, envelope.GetAllocator()); -#else - Json::Value envelope(Json::arrayValue); - envelope.append(result == nullptr ? Json::Value() : *result); -#endif return JsonMessageCodec::GetInstance().EncodeMessage(envelope); } @@ -116,8 +94,7 @@ std::unique_ptr> JsonMethodCodec::EncodeErrorEnvelopeInternal( const std::string& error_code, const std::string& error_message, - const JsonValueType* error_details) const { -#if USE_RAPID_JSON + const rapidjson::Document* error_details) const { rapidjson::Document envelope(rapidjson::kArrayType); auto& allocator = envelope.GetAllocator(); envelope.PushBack(rapidjson::Value(error_code, allocator), allocator); @@ -127,12 +104,6 @@ JsonMethodCodec::EncodeErrorEnvelopeInternal( details_value.CopyFrom(*error_details, allocator); } envelope.PushBack(details_value, allocator); -#else - Json::Value envelope(Json::arrayValue); - envelope.append(error_code); - envelope.append(error_message.empty() ? Json::Value() : error_message); - envelope.append(error_details == nullptr ? Json::Value() : *error_details); -#endif return JsonMessageCodec::GetInstance().EncodeMessage(envelope); } diff --git a/shell/platform/common/cpp/json_method_codec.h b/shell/platform/common/cpp/json_method_codec.h index 92af82000ac96..044338e959b06 100644 --- a/shell/platform/common/cpp/json_method_codec.h +++ b/shell/platform/common/cpp/json_method_codec.h @@ -5,14 +5,15 @@ #ifndef FLUTTER_SHELL_PLATFORM_COMMON_CPP_CLIENT_WRAPPER_INCLUDE_FLUTTER_JSON_METHOD_CODEC_H_ #define FLUTTER_SHELL_PLATFORM_COMMON_CPP_CLIENT_WRAPPER_INCLUDE_FLUTTER_JSON_METHOD_CODEC_H_ -#include "json_type.h" +#include + #include "method_call.h" #include "method_codec.h" namespace flutter { // An implementation of MethodCodec that uses JSON strings as the serialization. -class JsonMethodCodec : public MethodCodec { +class JsonMethodCodec : public MethodCodec { public: // Returns the shared instance of the codec. static const JsonMethodCodec& GetInstance(); @@ -28,23 +29,23 @@ class JsonMethodCodec : public MethodCodec { JsonMethodCodec() = default; // |flutter::MethodCodec| - std::unique_ptr> DecodeMethodCallInternal( + std::unique_ptr> DecodeMethodCallInternal( const uint8_t* message, const size_t message_size) const override; // |flutter::MethodCodec| std::unique_ptr> EncodeMethodCallInternal( - const MethodCall& method_call) const override; + const MethodCall& method_call) const override; // |flutter::MethodCodec| std::unique_ptr> EncodeSuccessEnvelopeInternal( - const JsonValueType* result) const override; + const rapidjson::Document* result) const override; // |flutter::MethodCodec| std::unique_ptr> EncodeErrorEnvelopeInternal( const std::string& error_code, const std::string& error_message, - const JsonValueType* error_details) const override; + const rapidjson::Document* error_details) const override; }; } // namespace flutter diff --git a/shell/platform/common/cpp/json_type.h b/shell/platform/common/cpp/json_type.h deleted file mode 100644 index 24baf1328d668..0000000000000 --- a/shell/platform/common/cpp/json_type.h +++ /dev/null @@ -1,26 +0,0 @@ -// 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_JSON_TYPE_H_ -#define FLUTTER_SHELL_PLATFORM_COMMON_CPP_CLIENT_WRAPPER_INCLUDE_FLUTTER_JSON_TYPE_H_ - -// By default, the Json codecs use jsoncpp, but a version using RapidJSON is -// implemented as well. To use the latter, set USE_RAPID_JSON. -// -// When writing code using the JSON codec classes, do not use JsonValueType; -// instead use the underlying type for the library you have selected directly. - -#ifdef USE_RAPID_JSON -#include - -// The APIs often pass owning references, which in RapidJSON must include the -// allocator, so the value type for the APIs is Document rather than Value. -using JsonValueType = rapidjson::Document; -#else -#include - -using JsonValueType = Json::Value; -#endif - -#endif // FLUTTER_SHELL_PLATFORM_COMMON_CPP_CLIENT_WRAPPER_INCLUDE_FLUTTER_JSON_TYPE_H_ diff --git a/shell/platform/glfw/BUILD.gn b/shell/platform/glfw/BUILD.gn index 546718bc14158..c78eef6183690 100644 --- a/shell/platform/glfw/BUILD.gn +++ b/shell/platform/glfw/BUILD.gn @@ -42,8 +42,6 @@ source_set("flutter_glfw") { "text_input_plugin.h", ] - defines = [ "USE_RAPID_JSON" ] - configs += [ "//flutter/shell/platform/common/cpp:desktop_library_implementation" ] diff --git a/shell/platform/windows/BUILD.gn b/shell/platform/windows/BUILD.gn index fee227ea7c9e0..88869ace762eb 100644 --- a/shell/platform/windows/BUILD.gn +++ b/shell/platform/windows/BUILD.gn @@ -60,8 +60,6 @@ source_set("flutter_windows_source") { "window_state.h", ] - defines = [ "USE_RAPID_JSON" ] - configs += [ "//flutter/shell/platform/common/cpp:desktop_library_implementation", "//third_party/angle:gl_prototypes", From 4ce7107529415036d307415bdfc6008556fdf0d9 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Tue, 24 Mar 2020 19:38:13 -0700 Subject: [PATCH 3/9] Fix header guards --- shell/platform/common/cpp/json_message_codec.h | 6 +++--- shell/platform/common/cpp/json_method_codec.h | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/shell/platform/common/cpp/json_message_codec.h b/shell/platform/common/cpp/json_message_codec.h index aa0819c1a87f4..04f010af0699b 100644 --- a/shell/platform/common/cpp/json_message_codec.h +++ b/shell/platform/common/cpp/json_message_codec.h @@ -2,8 +2,8 @@ // 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_JSON_MESSAGE_CODEC_H_ -#define FLUTTER_SHELL_PLATFORM_COMMON_CPP_CLIENT_WRAPPER_INCLUDE_FLUTTER_JSON_MESSAGE_CODEC_H_ +#ifndef FLUTTER_SHELL_PLATFORM_COMMON_CPP_JSON_MESSAGE_CODEC_H_ +#define FLUTTER_SHELL_PLATFORM_COMMON_CPP_JSON_MESSAGE_CODEC_H_ #include @@ -40,4 +40,4 @@ class JsonMessageCodec : public MessageCodec { } // namespace flutter -#endif // FLUTTER_SHELL_PLATFORM_COMMON_CPP_CLIENT_WRAPPER_INCLUDE_FLUTTER_JSON_MESSAGE_CODEC_H_ +#endif // FLUTTER_SHELL_PLATFORM_COMMON_CPP_JSON_MESSAGE_CODEC_H_ diff --git a/shell/platform/common/cpp/json_method_codec.h b/shell/platform/common/cpp/json_method_codec.h index 044338e959b06..67056ded596f2 100644 --- a/shell/platform/common/cpp/json_method_codec.h +++ b/shell/platform/common/cpp/json_method_codec.h @@ -2,8 +2,8 @@ // 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_JSON_METHOD_CODEC_H_ -#define FLUTTER_SHELL_PLATFORM_COMMON_CPP_CLIENT_WRAPPER_INCLUDE_FLUTTER_JSON_METHOD_CODEC_H_ +#ifndef FLUTTER_SHELL_PLATFORM_COMMON_CPP_JSON_METHOD_CODEC_H_ +#define FLUTTER_SHELL_PLATFORM_COMMON_CPP_JSON_METHOD_CODEC_H_ #include @@ -50,4 +50,4 @@ class JsonMethodCodec : public MethodCodec { } // namespace flutter -#endif // FLUTTER_SHELL_PLATFORM_COMMON_CPP_CLIENT_WRAPPER_INCLUDE_FLUTTER_JSON_METHOD_CODEC_H_ +#endif // FLUTTER_SHELL_PLATFORM_COMMON_CPP_JSON_METHOD_CODEC_H_ From d925bb61bacf621c5891ed4f0615495dc6eddc0f Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Tue, 24 Mar 2020 23:05:06 -0700 Subject: [PATCH 4/9] Build fixes, unit tests for JSON codecs --- shell/platform/common/cpp/BUILD.gn | 25 +++- .../platform/common/cpp/json_message_codec.h | 2 +- .../cpp/json_message_codec_unittests.cc | 46 ++++++++ shell/platform/common/cpp/json_method_codec.h | 4 +- .../common/cpp/json_method_codec_unittests.cc | 107 ++++++++++++++++++ 5 files changed, 180 insertions(+), 4 deletions(-) create mode 100644 shell/platform/common/cpp/json_message_codec_unittests.cc create mode 100644 shell/platform/common/cpp/json_method_codec_unittests.cc diff --git a/shell/platform/common/cpp/BUILD.gn b/shell/platform/common/cpp/BUILD.gn index 394de5e491d2a..529f0951c51c0 100644 --- a/shell/platform/common/cpp/BUILD.gn +++ b/shell/platform/common/cpp/BUILD.gn @@ -53,11 +53,11 @@ source_set("common_cpp") { ":common_cpp_library_headers", "//flutter/shell/platform/common/cpp/client_wrapper:client_wrapper", "//flutter/shell/platform/embedder:embedder_with_symbol_prefix", - "//third_party/rapidjson", ] public_deps = [ ":common_cpp_core", + "//third_party/rapidjson", ] if (is_win) { @@ -101,6 +101,29 @@ executable("common_cpp_core_unittests") { public_configs = [ "//flutter:config" ] } +test_fixtures("common_cpp_fixtures") { + fixtures = [] +} + +executable("common_cpp_unittests") { + testonly = true + + sources = [ + "json_message_codec_unittests.cc", + "json_method_codec_unittests.cc", + ] + + deps = [ + ":common_cpp", + ":common_cpp_fixtures", + "//flutter/shell/platform/common/cpp/client_wrapper:client_wrapper_library_stubs", + "//flutter/testing", + "//third_party/dart/runtime:libdart_jit", + ] + + public_configs = [ "//flutter:config" ] +} + copy("publish_headers") { sources = _public_headers outputs = [ diff --git a/shell/platform/common/cpp/json_message_codec.h b/shell/platform/common/cpp/json_message_codec.h index 04f010af0699b..81f2502c0c71d 100644 --- a/shell/platform/common/cpp/json_message_codec.h +++ b/shell/platform/common/cpp/json_message_codec.h @@ -7,7 +7,7 @@ #include -#include "message_codec.h" +#include "flutter/shell/platform/common/cpp/client_wrapper/include/flutter/message_codec.h" namespace flutter { diff --git a/shell/platform/common/cpp/json_message_codec_unittests.cc b/shell/platform/common/cpp/json_message_codec_unittests.cc new file mode 100644 index 0000000000000..3354f30a728df --- /dev/null +++ b/shell/platform/common/cpp/json_message_codec_unittests.cc @@ -0,0 +1,46 @@ +// 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/json_message_codec.h" + +#include +#include +#include + +#include "gtest/gtest.h" + +namespace flutter { + +namespace { + +// Validates round-trip encoding and decoding of |value|. +static void CheckEncodeDecode(const rapidjson::Document& value) { + const JsonMessageCodec& codec = JsonMessageCodec::GetInstance(); + auto encoded = codec.EncodeMessage(value); + ASSERT_TRUE(encoded); + auto decoded = codec.DecodeMessage(*encoded); + EXPECT_EQ(value, *decoded); +} + +} // namespace + +// Tests that a JSON document with various data types round-trips correctly. +TEST(JsonMessageCodec, EncodeDecode) { + rapidjson::Document array(rapidjson::kArrayType); + auto& allocator = array.GetAllocator(); + + array.PushBack("string", allocator); + + rapidjson::Value map(rapidjson::kObjectType); + map.AddMember("a", -7, allocator); + map.AddMember("b", std::numeric_limits::max(), allocator); + map.AddMember("c", 3.14159, allocator); + map.AddMember("d", true, allocator); + map.AddMember("e", rapidjson::Value(), allocator); + array.PushBack(map, allocator); + + CheckEncodeDecode(array); +} + +} // namespace flutter diff --git a/shell/platform/common/cpp/json_method_codec.h b/shell/platform/common/cpp/json_method_codec.h index 67056ded596f2..597fb6e6d1f16 100644 --- a/shell/platform/common/cpp/json_method_codec.h +++ b/shell/platform/common/cpp/json_method_codec.h @@ -7,8 +7,8 @@ #include -#include "method_call.h" -#include "method_codec.h" +#include "flutter/shell/platform/common/cpp/client_wrapper/include/flutter/method_call.h" +#include "flutter/shell/platform/common/cpp/client_wrapper/include/flutter/method_codec.h" namespace flutter { diff --git a/shell/platform/common/cpp/json_method_codec_unittests.cc b/shell/platform/common/cpp/json_method_codec_unittests.cc new file mode 100644 index 0000000000000..36bffcff1d104 --- /dev/null +++ b/shell/platform/common/cpp/json_method_codec_unittests.cc @@ -0,0 +1,107 @@ +// 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/json_method_codec.h" + +#include "gtest/gtest.h" + +namespace flutter { + +namespace { + +// Returns true if the given method calls have the same method name, and their +// arguments have equivalent values. +bool MethodCallsAreEqual(const MethodCall& a, + const MethodCall& b) { + if (a.method_name() != b.method_name()) { + return false; + } + // Treat nullptr and Null as equivalent. + if ((!a.arguments() || a.arguments()->IsNull()) && + (!b.arguments() || b.arguments()->IsNull())) { + return true; + } + return *a.arguments() == *b.arguments(); +} + +} // namespace + +TEST(JsonMethodCodec, HandlesMethodCallsWithNullArguments) { + const JsonMethodCodec& codec = JsonMethodCodec::GetInstance(); + MethodCall call("hello", nullptr); + auto encoded = codec.EncodeMethodCall(call); + ASSERT_TRUE(encoded); + std::unique_ptr> decoded = + codec.DecodeMethodCall(*encoded); + ASSERT_TRUE(decoded); + EXPECT_TRUE(MethodCallsAreEqual(call, *decoded)); +} + +TEST(JsonMethodCodec, HandlesMethodCallsWithArgument) { + const JsonMethodCodec& codec = JsonMethodCodec::GetInstance(); + + auto arguments = std::make_unique(rapidjson::kArrayType); + auto& allocator = arguments->GetAllocator(); + arguments->PushBack(42, allocator); + arguments->PushBack("world", allocator); + MethodCall call("hello", std::move(arguments)); + auto encoded = codec.EncodeMethodCall(call); + ASSERT_TRUE(encoded); + std::unique_ptr> decoded = + codec.DecodeMethodCall(*encoded); + ASSERT_TRUE(decoded); + EXPECT_TRUE(MethodCallsAreEqual(call, *decoded)); +} + +TEST(JsonMethodCodec, HandlesSuccessEnvelopesWithNullResult) { + const JsonMethodCodec& codec = JsonMethodCodec::GetInstance(); + auto encoded = codec.EncodeSuccessEnvelope(); + ASSERT_TRUE(encoded); + std::vector bytes = {'[', 'n', 'u', 'l', 'l', ']'}; + EXPECT_EQ(*encoded, bytes); + // TODO: Add round-trip check once decoding replies is implemented. +} + +TEST(JsonMethodCodec, HandlesSuccessEnvelopesWithResult) { + const JsonMethodCodec& codec = JsonMethodCodec::GetInstance(); + rapidjson::Document result; + result.SetInt(42); + auto encoded = codec.EncodeSuccessEnvelope(&result); + ASSERT_TRUE(encoded); + std::vector bytes = {'[', '4', '2', ']'}; + EXPECT_EQ(*encoded, bytes); + // TODO: Add round-trip check once decoding replies is implemented. +} + +TEST(JsonMethodCodec, HandlesErrorEnvelopesWithNulls) { + const JsonMethodCodec& codec = JsonMethodCodec::GetInstance(); + auto encoded = codec.EncodeErrorEnvelope("errorCode"); + ASSERT_TRUE(encoded); + std::vector bytes = { + '[', '"', 'e', 'r', 'r', 'o', 'r', 'C', 'o', 'd', 'e', + '"', ',', '"', '"', ',', 'n', 'u', 'l', 'l', ']', + }; + EXPECT_EQ(*encoded, bytes); + // TODO: Add round-trip check once decoding replies is implemented. +} + +TEST(JsonMethodCodec, HandlesErrorEnvelopesWithDetails) { + const JsonMethodCodec& codec = JsonMethodCodec::GetInstance(); + rapidjson::Document details(rapidjson::kArrayType); + auto& allocator = details.GetAllocator(); + details.PushBack("a", allocator); + details.PushBack(42, allocator); + auto encoded = + codec.EncodeErrorEnvelope("errorCode", "something failed", &details); + ASSERT_NE(encoded.get(), nullptr); + std::vector bytes = { + '[', '"', 'e', 'r', 'r', 'o', 'r', 'C', 'o', 'd', 'e', '"', ',', '"', + 's', 'o', 'm', 'e', 't', 'h', 'i', 'n', 'g', ' ', 'f', 'a', 'i', 'l', + 'e', 'd', '"', ',', '[', '"', 'a', '"', ',', '4', '2', ']', ']', + }; + EXPECT_EQ(*encoded, bytes); + // TODO: Add round-trip check once decoding replies is implemented. +} + +} // namespace flutter From 0366e2f9fc8bc501e487ff13be7c227ec3d389d1 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Tue, 24 Mar 2020 23:16:29 -0700 Subject: [PATCH 5/9] Add new test binary to build and script --- BUILD.gn | 1 + testing/run_tests.py | 2 ++ 2 files changed, 3 insertions(+) diff --git a/BUILD.gn b/BUILD.gn index d35c608ba1d63..fea2a2b5bebf1 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -76,6 +76,7 @@ group("flutter") { "//flutter/runtime:runtime_unittests", "//flutter/shell/common:shell_unittests", "//flutter/shell/platform/common/cpp:common_cpp_core_unittests", + "//flutter/shell/platform/common/cpp:common_cpp_unittests", "//flutter/shell/platform/common/cpp/client_wrapper:client_wrapper_unittests", "//flutter/shell/platform/embedder:embedder_unittests", "//flutter/shell/platform/glfw/client_wrapper:client_wrapper_glfw_unittests", diff --git a/testing/run_tests.py b/testing/run_tests.py index 083117458d448..86412f7ad6ff0 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -110,6 +110,8 @@ def RunCCTests(build_dir, filter): RunEngineExecutable(build_dir, 'common_cpp_core_unittests', filter, shuffle_flags) + RunEngineExecutable(build_dir, 'common_cpp_unittests', filter, shuffle_flags) + RunEngineExecutable(build_dir, 'client_wrapper_unittests', filter, shuffle_flags) # https://github.com/flutter/flutter/issues/36294 From 29808ce7cb86dfa1ea23e954037f90c052773a2d Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Thu, 26 Mar 2020 10:15:05 -0700 Subject: [PATCH 6/9] Fix Windows gn generation --- shell/platform/common/cpp/BUILD.gn | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/common/cpp/BUILD.gn b/shell/platform/common/cpp/BUILD.gn index 529f0951c51c0..8005ee66fc3e6 100644 --- a/shell/platform/common/cpp/BUILD.gn +++ b/shell/platform/common/cpp/BUILD.gn @@ -62,7 +62,7 @@ source_set("common_cpp") { if (is_win) { # For wstring_conversion. See issue #50053. - defines += [ "_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING" ] + defines = [ "_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING" ] } } From 0fd5494cbfe2db2a73d9859a6382f6e1def66e70 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Thu, 26 Mar 2020 15:28:36 -0700 Subject: [PATCH 7/9] gn format --- .../cpp/client_wrapper/core_wrapper_files.gni | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) 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 2521ded89af70..0662cca6be422 100644 --- a/shell/platform/common/cpp/client_wrapper/core_wrapper_files.gni +++ b/shell/platform/common/cpp/client_wrapper/core_wrapper_files.gni @@ -23,13 +23,11 @@ core_cpp_client_wrapper_includes = # TODO: Once the wrapper API is more stable, consolidate to as few files as is # reasonable (without forcing different kinds of clients to take unnecessary # code) to simplify use. -core_cpp_client_wrapper_sources = - get_path_info( - [ - "byte_stream_wrappers.h", - "engine_method_result.cc", - "plugin_registrar.cc", - "standard_codec_serializer.h", - "standard_codec.cc", - ], - "abspath") +core_cpp_client_wrapper_sources = get_path_info([ + "byte_stream_wrappers.h", + "engine_method_result.cc", + "plugin_registrar.cc", + "standard_codec_serializer.h", + "standard_codec.cc", + ], + "abspath") From da189fad26c72873896df2bfe313fbb2ab764890 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Wed, 1 Apr 2020 16:12:59 -0400 Subject: [PATCH 8/9] Remove dart dependency; indirect dependency on embedder provides it --- shell/platform/common/cpp/BUILD.gn | 1 - 1 file changed, 1 deletion(-) diff --git a/shell/platform/common/cpp/BUILD.gn b/shell/platform/common/cpp/BUILD.gn index 8005ee66fc3e6..4d211c8fe8de1 100644 --- a/shell/platform/common/cpp/BUILD.gn +++ b/shell/platform/common/cpp/BUILD.gn @@ -118,7 +118,6 @@ executable("common_cpp_unittests") { ":common_cpp_fixtures", "//flutter/shell/platform/common/cpp/client_wrapper:client_wrapper_library_stubs", "//flutter/testing", - "//third_party/dart/runtime:libdart_jit", ] public_configs = [ "//flutter:config" ] From c61ec3079366dbe9ee9ff9e493443bef75ac8d5f Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Thu, 2 Apr 2020 09:18:00 -0700 Subject: [PATCH 9/9] Licence update --- ci/licenses_golden/licenses_flutter | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 903a33b252e51..724af2afdc5e0 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -778,9 +778,6 @@ FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/include/flutter/ FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/include/flutter/binary_messenger.h FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/include/flutter/encodable_value.h FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/include/flutter/engine_method_result.h -FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/include/flutter/json_message_codec.h -FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/include/flutter/json_method_codec.h -FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/include/flutter/json_type.h FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/include/flutter/message_codec.h FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/include/flutter/method_call.h FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/include/flutter/method_channel.h @@ -790,8 +787,6 @@ FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/include/flutter/ 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/json_message_codec.cc -FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/json_method_codec.cc 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/plugin_registrar.cc @@ -802,6 +797,12 @@ FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/standard_message FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/standard_method_codec_unittests.cc FILE: ../../../flutter/shell/platform/common/cpp/incoming_message_dispatcher.cc FILE: ../../../flutter/shell/platform/common/cpp/incoming_message_dispatcher.h +FILE: ../../../flutter/shell/platform/common/cpp/json_message_codec.cc +FILE: ../../../flutter/shell/platform/common/cpp/json_message_codec.h +FILE: ../../../flutter/shell/platform/common/cpp/json_message_codec_unittests.cc +FILE: ../../../flutter/shell/platform/common/cpp/json_method_codec.cc +FILE: ../../../flutter/shell/platform/common/cpp/json_method_codec.h +FILE: ../../../flutter/shell/platform/common/cpp/json_method_codec_unittests.cc FILE: ../../../flutter/shell/platform/common/cpp/path_utils.cc FILE: ../../../flutter/shell/platform/common/cpp/path_utils.h FILE: ../../../flutter/shell/platform/common/cpp/path_utils_unittests.cc