diff --git a/shell/platform/common/cpp/client_wrapper/BUILD.gn b/shell/platform/common/cpp/client_wrapper/BUILD.gn index 331a0c97d3509..e1adbbaf3e510 100644 --- a/shell/platform/common/cpp/client_wrapper/BUILD.gn +++ b/shell/platform/common/cpp/client_wrapper/BUILD.gn @@ -19,6 +19,23 @@ source_set("client_wrapper") { [ "//flutter/shell/platform/common/cpp:relative_flutter_library_headers" ] } +# Temporary test for the legacy EncodableValue implementation. Remove once the +# legacy version is removed. +source_set("client_wrapper_legacy_encodable_value") { + sources = core_cpp_client_wrapper_sources + public = core_cpp_client_wrapper_includes + + deps = [ "//flutter/shell/platform/common/cpp:common_cpp_library_headers" ] + + configs += + [ "//flutter/shell/platform/common/cpp:desktop_library_implementation" ] + + defines = [ "USE_LEGACY_ENCODABLE_VALUE" ] + + public_configs = + [ "//flutter/shell/platform/common/cpp:relative_flutter_library_headers" ] +} + source_set("client_wrapper_library_stubs") { sources = [ "testing/stub_flutter_api.cc", @@ -56,6 +73,9 @@ executable("client_wrapper_unittests") { ":client_wrapper", ":client_wrapper_fixtures", ":client_wrapper_library_stubs", + + # Build the legacy version as well as a sanity check. + ":client_wrapper_unittests_legacy_encodable_value", "//flutter/testing", # TODO(chunhtai): Consider refactoring flutter_root/testing so that there's a testing @@ -66,3 +86,31 @@ executable("client_wrapper_unittests") { defines = [ "FLUTTER_DESKTOP_LIBRARY" ] } + +# Ensures that the legacy EncodableValue codepath still compiles. +executable("client_wrapper_unittests_legacy_encodable_value") { + testonly = true + + sources = [ + "encodable_value_unittests.cc", + "standard_message_codec_unittests.cc", + "testing/test_codec_extensions.h", + ] + + deps = [ + ":client_wrapper_fixtures", + ":client_wrapper_legacy_encodable_value", + ":client_wrapper_library_stubs", + "//flutter/testing", + + # TODO(chunhtai): Consider refactoring flutter_root/testing so that there's a testing + # target that doesn't require a Dart runtime to be linked in. + # https://github.com/flutter/flutter/issues/41414. + "//third_party/dart/runtime:libdart_jit", + ] + + defines = [ + "FLUTTER_DESKTOP_LIBRARY", + "USE_LEGACY_ENCODABLE_VALUE", + ] +} diff --git a/shell/platform/common/cpp/client_wrapper/byte_buffer_streams.h b/shell/platform/common/cpp/client_wrapper/byte_buffer_streams.h index 86cdfc4551209..e054e00f2d04c 100644 --- a/shell/platform/common/cpp/client_wrapper/byte_buffer_streams.h +++ b/shell/platform/common/cpp/client_wrapper/byte_buffer_streams.h @@ -23,6 +23,8 @@ class ByteBufferStreamReader : public ByteStreamReader { explicit ByteBufferStreamReader(const uint8_t* bytes, size_t size) : bytes_(bytes), size_(size) {} + virtual ~ByteBufferStreamReader() = default; + // |ByteStreamReader| uint8_t ReadByte() override { if (location_ >= size_) { @@ -69,6 +71,8 @@ class ByteBufferStreamWriter : public ByteStreamWriter { assert(buffer); } + virtual ~ByteBufferStreamWriter() = default; + // |ByteStreamWriter| void WriteByte(uint8_t byte) { bytes_->push_back(byte); } diff --git a/shell/platform/common/cpp/client_wrapper/encodable_value_unittests.cc b/shell/platform/common/cpp/client_wrapper/encodable_value_unittests.cc index 17be80b6aeeb4..e64beae8b3a8f 100644 --- a/shell/platform/common/cpp/client_wrapper/encodable_value_unittests.cc +++ b/shell/platform/common/cpp/client_wrapper/encodable_value_unittests.cc @@ -15,6 +15,8 @@ TEST(EncodableValueTest, Null) { value.IsNull(); } +#ifndef USE_LEGACY_ENCODABLE_VALUE + TEST(EncodableValueTest, Bool) { EncodableValue value(false); @@ -280,4 +282,6 @@ TEST(EncodableValueTest, DeepCopy) { EXPECT_EQ(std::get(innermost_map[EncodableValue("a")]), "b"); } +#endif // !LEGACY_ENCODABLE_VALUE + } // namespace flutter diff --git a/shell/platform/common/cpp/client_wrapper/include/flutter/byte_streams.h b/shell/platform/common/cpp/client_wrapper/include/flutter/byte_streams.h index 11f9928cd027a..f5314c0b9b575 100644 --- a/shell/platform/common/cpp/client_wrapper/include/flutter/byte_streams.h +++ b/shell/platform/common/cpp/client_wrapper/include/flutter/byte_streams.h @@ -12,6 +12,9 @@ namespace flutter { // An interface for a class that reads from a byte stream. class ByteStreamReader { public: + explicit ByteStreamReader() = default; + virtual ~ByteStreamReader() = default; + // Reads and returns the next byte from the stream. virtual uint8_t ReadByte() = 0; @@ -48,6 +51,9 @@ class ByteStreamReader { // An interface for a class that writes to a byte stream. class ByteStreamWriter { public: + explicit ByteStreamWriter() = default; + virtual ~ByteStreamWriter() = default; + // Writes |byte| to the stream. virtual void WriteByte(uint8_t byte) = 0; diff --git a/shell/platform/common/cpp/client_wrapper/standard_codec.cc b/shell/platform/common/cpp/client_wrapper/standard_codec.cc index 6ba13b48a67e1..a17845750d8ca 100644 --- a/shell/platform/common/cpp/client_wrapper/standard_codec.cc +++ b/shell/platform/common/cpp/client_wrapper/standard_codec.cc @@ -132,14 +132,14 @@ void StandardCodecSerializer::WriteValue(const EncodableValue& value, // Null and bool are encoded directly in the type. break; case EncodableValue::Type::kInt: - stream->WriteInt32(std::get(value)); + stream->WriteInt32(value.IntValue()); break; - case case EncodableValue::Type::kLong: - stream->WriteInt64(std::get(value)); + case EncodableValue::Type::kLong: + stream->WriteInt64(value.LongValue()); break; case EncodableValue::Type::kDouble: stream->WriteAlignment(8); - stream->WriteDouble(std::get(value)); + stream->WriteDouble(value.DoubleValue()); break; case EncodableValue::Type::kString: { const auto& string_value = value.StringValue(); diff --git a/shell/platform/common/cpp/client_wrapper/standard_message_codec_unittests.cc b/shell/platform/common/cpp/client_wrapper/standard_message_codec_unittests.cc index 3d8c077133eab..03459dc75ddfa 100644 --- a/shell/platform/common/cpp/client_wrapper/standard_message_codec_unittests.cc +++ b/shell/platform/common/cpp/client_wrapper/standard_message_codec_unittests.cc @@ -6,9 +6,12 @@ #include #include "flutter/shell/platform/common/cpp/client_wrapper/include/flutter/standard_message_codec.h" -#include "flutter/shell/platform/common/cpp/client_wrapper/testing/test_codec_extensions.h" #include "gtest/gtest.h" +#ifndef USE_LEGACY_ENCODABLE_VALUE +#include "flutter/shell/platform/common/cpp/client_wrapper/testing/test_codec_extensions.h" +#endif + namespace flutter { // Validates round-trip encoding and decoding of |value|, and checks that the @@ -30,11 +33,21 @@ static void CheckEncodeDecode( EXPECT_EQ(*encoded, expected_encoding); auto decoded = codec.DecodeMessage(*encoded); +#ifdef USE_LEGACY_ENCODABLE_VALUE + // Full equality isn't implemented for the legacy path; just do a sanity test + // of basic types. + if (value.IsNull() || value.IsBool() || value.IsInt() || value.IsLong() || + value.IsDouble() || value.IsString()) { + EXPECT_FALSE(value < *decoded); + EXPECT_FALSE(*decoded < value); + } +#else if (custom_comparator) { EXPECT_TRUE(custom_comparator(value, *decoded)); } else { EXPECT_EQ(value, *decoded); } +#endif } // Validates round-trip encoding and decoding of |value|, and checks that the @@ -46,7 +59,11 @@ static void CheckEncodeDecodeWithEncodePrefix( const EncodableValue& value, const std::vector& expected_encoding_prefix, size_t expected_encoding_length) { +#ifdef USE_LEGACY_ENCODABLE_VALUE + EXPECT_TRUE(value.IsMap()); +#else EXPECT_TRUE(std::holds_alternative(value)); +#endif const StandardMessageCodec& codec = StandardMessageCodec::GetInstance(); auto encoded = codec.EncodeMessage(value); ASSERT_TRUE(encoded); @@ -58,7 +75,12 @@ static void CheckEncodeDecodeWithEncodePrefix( expected_encoding_prefix.begin(), expected_encoding_prefix.end())); auto decoded = codec.DecodeMessage(*encoded); + +#ifdef USE_LEGACY_ENCODABLE_VALUE + EXPECT_NE(decoded, nullptr); +#else EXPECT_EQ(value, *decoded); +#endif } TEST(StandardMessageCodec, CanEncodeAndDecodeNull) { @@ -182,6 +204,8 @@ TEST(StandardMessageCodec, CanEncodeAndDecodeFloat64Array) { CheckEncodeDecode(value, bytes); } +#ifndef USE_LEGACY_ENCODABLE_VALUE + TEST(StandardMessageCodec, CanEncodeAndDecodeSimpleCustomType) { std::vector bytes = {0x80, 0x09, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00}; @@ -217,4 +241,6 @@ TEST(StandardMessageCodec, CanEncodeAndDecodeVariableLengthCustomType) { some_data_comparator); } +#endif // !USE_LEGACY_ENCODABLE_VALUE + } // namespace flutter diff --git a/shell/platform/fuchsia/flutter/BUILD.gn b/shell/platform/fuchsia/flutter/BUILD.gn index c8652cc9a0598..3b1c7e086236b 100644 --- a/shell/platform/fuchsia/flutter/BUILD.gn +++ b/shell/platform/fuchsia/flutter/BUILD.gn @@ -21,98 +21,120 @@ shell_gpu_configuration("fuchsia_legacy_gpu_configuration") { enable_metal = false } -source_set("flutter_runner_sources") { - sources = [ - "accessibility_bridge.cc", - "accessibility_bridge.h", - "component.cc", - "component.h", - "compositor_context.cc", - "compositor_context.h", - "engine.cc", - "engine.h", - "flutter_runner_product_configuration.cc", - "flutter_runner_product_configuration.h", - "fuchsia_intl.cc", - "fuchsia_intl.h", - "isolate_configurator.cc", - "isolate_configurator.h", - "logging.h", - "loop.cc", - "loop.h", - "platform_view.cc", - "platform_view.h", - "runner.cc", - "runner.h", - "session_connection.cc", - "session_connection.h", - "surface.cc", - "surface.h", - "task_observers.cc", - "task_observers.h", - "task_runner_adapter.cc", - "task_runner_adapter.h", - "thread.cc", - "thread.h", - "unique_fdio_ns.h", - "vsync_recorder.cc", - "vsync_recorder.h", - "vsync_waiter.cc", - "vsync_waiter.h", - "vulkan_surface.cc", - "vulkan_surface.h", - "vulkan_surface_pool.cc", - "vulkan_surface_pool.h", - "vulkan_surface_producer.cc", - "vulkan_surface_producer.h", - ] +template("runner_sources") { + assert(defined(invoker.product), "runner_sources must define product") - # The use of these dependencies is temporary and will be moved behind the - # embedder API. - flutter_public_deps = [ - "//flutter/flow:flow_fuchsia_legacy", - "//flutter/lib/ui:ui_fuchsia_legacy", - "//flutter/runtime:runtime_fuchsia_legacy", - "//flutter/shell/common:common_fuchsia_legacy", - ] - flutter_deps = [ - ":fuchsia_legacy_gpu_configuration", - "//flutter/assets", - "//flutter/common", - "//flutter/fml", - "//flutter/vulkan", - ] + extra_defines = [] + if (invoker.product) { + extra_defines += [ "DART_PRODUCT" ] + } - public_deps = [ - "$fuchsia_sdk_root/pkg:scenic_cpp", - "$fuchsia_sdk_root/pkg:sys_cpp", - "//flutter/shell/platform/fuchsia/runtime/dart/utils", - ] + flutter_public_deps + source_set(target_name) { + sources = [ + "accessibility_bridge.cc", + "accessibility_bridge.h", + "component.cc", + "component.h", + "compositor_context.cc", + "compositor_context.h", + "engine.cc", + "engine.h", + "flutter_runner_product_configuration.cc", + "flutter_runner_product_configuration.h", + "fuchsia_intl.cc", + "fuchsia_intl.h", + "isolate_configurator.cc", + "isolate_configurator.h", + "logging.h", + "loop.cc", + "loop.h", + "platform_view.cc", + "platform_view.h", + "runner.cc", + "runner.h", + "session_connection.cc", + "session_connection.h", + "surface.cc", + "surface.h", + "task_observers.cc", + "task_observers.h", + "task_runner_adapter.cc", + "task_runner_adapter.h", + "thread.cc", + "thread.h", + "unique_fdio_ns.h", + "vsync_recorder.cc", + "vsync_recorder.h", + "vsync_waiter.cc", + "vsync_waiter.h", + "vulkan_surface.cc", + "vulkan_surface.h", + "vulkan_surface_pool.cc", + "vulkan_surface_pool.h", + "vulkan_surface_producer.cc", + "vulkan_surface_producer.h", + ] - deps = [ - "$fuchsia_sdk_root/fidl:fuchsia.accessibility.semantics", - "$fuchsia_sdk_root/fidl:fuchsia.fonts", - "$fuchsia_sdk_root/fidl:fuchsia.images", - "$fuchsia_sdk_root/fidl:fuchsia.intl", - "$fuchsia_sdk_root/fidl:fuchsia.io", - "$fuchsia_sdk_root/fidl:fuchsia.sys", - "$fuchsia_sdk_root/fidl:fuchsia.ui.app", - "$fuchsia_sdk_root/fidl:fuchsia.ui.scenic", - "$fuchsia_sdk_root/pkg:async-cpp", - "$fuchsia_sdk_root/pkg:async-default", - "$fuchsia_sdk_root/pkg:async-loop", - "$fuchsia_sdk_root/pkg:async-loop-cpp", - "$fuchsia_sdk_root/pkg:fdio", - "$fuchsia_sdk_root/pkg:fidl_cpp", - "$fuchsia_sdk_root/pkg:syslog", - "$fuchsia_sdk_root/pkg:trace", - "$fuchsia_sdk_root/pkg:trace-engine", - "$fuchsia_sdk_root/pkg:trace-provider-so", - "$fuchsia_sdk_root/pkg:vfs_cpp", - "$fuchsia_sdk_root/pkg:zx", - "//flutter/shell/platform/fuchsia/dart-pkg/fuchsia", - "//flutter/shell/platform/fuchsia/dart-pkg/zircon", - ] + flutter_deps + defines = extra_defines + if (flutter_runtime_mode == "profile") { + defines += [ "FLUTTER_PROFILE" ] + } + + # The use of these dependencies is temporary and will be moved behind the + # embedder API. + flutter_public_deps = [ + "//flutter/flow:flow_fuchsia_legacy", + "//flutter/lib/ui:ui_fuchsia_legacy", + "//flutter/runtime:runtime_fuchsia_legacy", + "//flutter/shell/common:common_fuchsia_legacy", + ] + flutter_deps = [ + ":fuchsia_legacy_gpu_configuration", + "//flutter/assets", + "//flutter/common", + "//flutter/fml", + "//flutter/vulkan", + ] + + public_deps = [ + "$fuchsia_sdk_root/pkg:scenic_cpp", + "$fuchsia_sdk_root/pkg:sys_cpp", + "//flutter/shell/platform/fuchsia/runtime/dart/utils", + ] + flutter_public_deps + + deps = [ + "$fuchsia_sdk_root/fidl:fuchsia.accessibility.semantics", + "$fuchsia_sdk_root/fidl:fuchsia.fonts", + "$fuchsia_sdk_root/fidl:fuchsia.images", + "$fuchsia_sdk_root/fidl:fuchsia.intl", + "$fuchsia_sdk_root/fidl:fuchsia.io", + "$fuchsia_sdk_root/fidl:fuchsia.sys", + "$fuchsia_sdk_root/fidl:fuchsia.ui.app", + "$fuchsia_sdk_root/fidl:fuchsia.ui.scenic", + "$fuchsia_sdk_root/pkg:async-cpp", + "$fuchsia_sdk_root/pkg:async-default", + "$fuchsia_sdk_root/pkg:async-loop", + "$fuchsia_sdk_root/pkg:async-loop-cpp", + "$fuchsia_sdk_root/pkg:fdio", + "$fuchsia_sdk_root/pkg:fidl_cpp", + "$fuchsia_sdk_root/pkg:syslog", + "$fuchsia_sdk_root/pkg:trace", + "$fuchsia_sdk_root/pkg:trace-engine", + "$fuchsia_sdk_root/pkg:trace-provider-so", + "$fuchsia_sdk_root/pkg:vfs_cpp", + "$fuchsia_sdk_root/pkg:zx", + "//flutter/shell/platform/fuchsia/dart-pkg/fuchsia", + "//flutter/shell/platform/fuchsia/dart-pkg/zircon", + ] + flutter_deps + } +} + +runner_sources("flutter_runner_sources") { + product = false +} + +runner_sources("flutter_runner_sources_product") { + product = true } # Things that explicitly being excluded: @@ -142,20 +164,18 @@ template("flutter_runner") { invoker_output_name = invoker.output_name extra_deps = invoker.extra_deps - extra_defines = [] - if (defined(invoker.extra_defines)) { - extra_defines += invoker.extra_defines + product_suffix = "" + if (invoker.product) { + product_suffix = "_product" } executable(target_name) { output_name = invoker_output_name - defines = extra_defines - sources = [ "main.cc" ] deps = [ - ":flutter_runner_sources", + ":flutter_runner_sources${product_suffix}", "$fuchsia_sdk_root/pkg:async-loop-cpp", "$fuchsia_sdk_root/pkg:trace", "$fuchsia_sdk_root/pkg:trace-provider-so", @@ -177,11 +197,6 @@ flutter_runner("jit") { output_name = "flutter_jit_runner" product = false - extra_defines = [] - if (flutter_runtime_mode == "profile") { - extra_defines += [ "FLUTTER_PROFILE" ] - } - extra_deps = [ "//third_party/dart/runtime:libdart_jit", "//third_party/dart/runtime/platform:libdart_platform_jit", @@ -192,8 +207,6 @@ flutter_runner("jit_product") { output_name = "flutter_jit_product_runner" product = true - extra_defines = [ "DART_PRODUCT" ] - extra_deps = [ "//third_party/dart/runtime:libdart_jit_product", "//third_party/dart/runtime/platform:libdart_platform_jit_product", @@ -204,11 +217,6 @@ flutter_runner("aot") { output_name = "flutter_aot_runner" product = false - extra_defines = [] - if (flutter_runtime_mode == "profile") { - extra_defines += [ "FLUTTER_PROFILE" ] - } - extra_deps = [ "//third_party/dart/runtime:libdart_precompiled_runtime", "//third_party/dart/runtime/platform:libdart_platform_precompiled_runtime", @@ -219,8 +227,6 @@ flutter_runner("aot_product") { output_name = "flutter_aot_product_runner" product = true - extra_defines = [ "DART_PRODUCT" ] - extra_deps = [ "//third_party/dart/runtime:libdart_precompiled_runtime_product", "//third_party/dart/runtime/platform:libdart_platform_precompiled_runtime_product",