From bf259226b224dec9052d4cd4e9ecb0f40a2efe0d Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Wed, 4 Nov 2020 15:12:44 -0800 Subject: [PATCH] Switch Linux embedding to proc table embedder API (#22280) Switches the Linux embedding from the standard C API to the new proctable version, to allow for unit testing of the embedding layer separately from the embedder APIs implementation. --- shell/platform/linux/BUILD.gn | 17 ++- .../linux/fl_basic_message_channel_test.cc | 47 +++++++- shell/platform/linux/fl_engine.cc | 43 +++++--- shell/platform/linux/fl_engine_private.h | 10 ++ shell/platform/linux/testing/mock_engine.cc | 103 +++++++++++++----- 5 files changed, 168 insertions(+), 52 deletions(-) diff --git a/shell/platform/linux/BUILD.gn b/shell/platform/linux/BUILD.gn index 17914ce1baa..6dcc1779f19 100644 --- a/shell/platform/linux/BUILD.gn +++ b/shell/platform/linux/BUILD.gn @@ -119,7 +119,10 @@ source_set("flutter_linux_sources") { ] # Set flag to stop headers being directly included (library users should not do this) - defines = [ "FLUTTER_LINUX_COMPILATION" ] + defines = [ + "FLUTTER_LINUX_COMPILATION", + "FLUTTER_ENGINE_NO_PROTOTYPES", + ] deps = [ "//flutter/shell/platform/common/cpp:common_cpp_input", @@ -138,6 +141,8 @@ source_set("flutter_linux") { "//third_party/khronos:khronos_headers", ] + defines = [ "FLUTTER_ENGINE_NO_PROTOTYPES" ] + public_deps = [ ":flutter_linux_sources" ] deps = [ "//flutter/shell/platform/embedder:embedder_as_internal_library" ] @@ -182,14 +187,20 @@ executable("flutter_linux_unittests") { "//flutter/shell/platform/linux/config:x11", ] - # Set flag to allow public headers to be directly included (library users should not do this) - defines = [ "FLUTTER_LINUX_COMPILATION" ] + defines = [ + "FLUTTER_ENGINE_NO_PROTOTYPES", + + # Set flag to allow public headers to be directly included + # (library users should not do this) + "FLUTTER_LINUX_COMPILATION", + ] deps = [ ":flutter_linux_fixtures", ":flutter_linux_sources", "//flutter/runtime:libdart", "//flutter/shell/platform/embedder:embedder_headers", + "//flutter/shell/platform/embedder:embedder_test_utils", "//flutter/testing", ] } diff --git a/shell/platform/linux/fl_basic_message_channel_test.cc b/shell/platform/linux/fl_basic_message_channel_test.cc index d4c338836e6..91ac762f9d5 100644 --- a/shell/platform/linux/fl_basic_message_channel_test.cc +++ b/shell/platform/linux/fl_basic_message_channel_test.cc @@ -5,13 +5,54 @@ // Included first as it collides with the X11 headers. #include "gtest/gtest.h" +#include "flutter/shell/platform/linux/public/flutter_linux/fl_basic_message_channel.h" + +#include "flutter/shell/platform/embedder/embedder.h" +#include "flutter/shell/platform/embedder/test_utils/proc_table_replacement.h" #include "flutter/shell/platform/linux/fl_binary_messenger_private.h" #include "flutter/shell/platform/linux/fl_engine_private.h" -#include "flutter/shell/platform/linux/public/flutter_linux/fl_basic_message_channel.h" +#include "flutter/shell/platform/linux/public/flutter_linux/fl_message_codec.h" #include "flutter/shell/platform/linux/public/flutter_linux/fl_standard_message_codec.h" #include "flutter/shell/platform/linux/testing/fl_test.h" #include "flutter/shell/platform/linux/testing/mock_renderer.h" +// Checks sending a message without a reponse works. +TEST(FlBasicMessageChannelTest, SendMessageWithoutResponse) { + g_autoptr(GMainLoop) loop = g_main_loop_new(nullptr, 0); + + g_autoptr(FlEngine) engine = make_mock_engine(); + FlutterEngineProcTable* embedder_api = fl_engine_get_embedder_api(engine); + + bool called = false; + embedder_api->SendPlatformMessage = MOCK_ENGINE_PROC( + SendPlatformMessage, + ([&called](auto engine, const FlutterPlatformMessage* message) { + called = true; + EXPECT_STREQ(message->channel, "test"); + EXPECT_EQ(message->response_handle, nullptr); + + g_autoptr(GBytes) message_bytes = + g_bytes_new(message->message, message->message_size); + g_autoptr(FlStandardMessageCodec) codec = + fl_standard_message_codec_new(); + FlValue* message_value = fl_message_codec_decode_message( + FL_MESSAGE_CODEC(codec), message_bytes, nullptr); + EXPECT_EQ(fl_value_get_type(message_value), FL_VALUE_TYPE_STRING); + EXPECT_STREQ(fl_value_get_string(message_value), "Hello World!"); + + return kSuccess; + })); + + FlBinaryMessenger* messenger = fl_binary_messenger_new(engine); + g_autoptr(FlStandardMessageCodec) codec = fl_standard_message_codec_new(); + g_autoptr(FlBasicMessageChannel) channel = + fl_basic_message_channel_new(messenger, "test", FL_MESSAGE_CODEC(codec)); + g_autoptr(FlValue) message = fl_value_new_string("Hello World!"); + fl_basic_message_channel_send(channel, message, nullptr, nullptr, loop); + + EXPECT_TRUE(called); +} + // Called when the message response is received in the SendMessage test. static void echo_response_cb(GObject* object, GAsyncResult* result, @@ -28,8 +69,8 @@ static void echo_response_cb(GObject* object, g_main_loop_quit(static_cast(user_data)); } -// Checks sending a message works. -TEST(FlBasicMessageChannelTest, SendMessage) { +// Checks sending a message with a response works. +TEST(FlBasicMessageChannelTest, SendMessageWithResponse) { g_autoptr(GMainLoop) loop = g_main_loop_new(nullptr, 0); g_autoptr(FlEngine) engine = make_mock_engine(); diff --git a/shell/platform/linux/fl_engine.cc b/shell/platform/linux/fl_engine.cc index 3dc42694bb8..bed6de5c553 100644 --- a/shell/platform/linux/fl_engine.cc +++ b/shell/platform/linux/fl_engine.cc @@ -32,6 +32,7 @@ struct _FlEngine { FlBinaryMessenger* binary_messenger; FlutterEngineAOTData aot_data; FLUTTER_API_SYMBOL(FlutterEngine) engine; + FlutterEngineProcTable embedder_api; // Function to call when a platform message is received. FlEnginePlatformMessageHandler platform_message_handler; @@ -127,7 +128,7 @@ static void setup_locales(FlEngine* self) { } FlutterLocale** locales = reinterpret_cast(locales_array->pdata); - FlutterEngineResult result = FlutterEngineUpdateLocales( + FlutterEngineResult result = self->embedder_api.UpdateLocales( self->engine, const_cast(locales), locales_array->len); if (result != kSuccess) { @@ -143,7 +144,7 @@ static gboolean flutter_source_dispatch(GSource* source, FlEngine* self = fl_source->engine; FlutterEngineResult result = - FlutterEngineRunTask(self->engine, &fl_source->task); + self->embedder_api.RunTask(self->engine, &fl_source->task); if (result != kSuccess) { g_warning("Failed to run Flutter task\n"); } @@ -302,12 +303,12 @@ static void fl_engine_dispose(GObject* object) { FlEngine* self = FL_ENGINE(object); if (self->engine != nullptr) { - FlutterEngineShutdown(self->engine); + self->embedder_api.Shutdown(self->engine); self->engine = nullptr; } if (self->aot_data != nullptr) { - FlutterEngineCollectAOTData(self->aot_data); + self->embedder_api.CollectAOTData(self->aot_data); self->aot_data = nullptr; } @@ -332,6 +333,9 @@ static void fl_engine_class_init(FlEngineClass* klass) { static void fl_engine_init(FlEngine* self) { self->thread = g_thread_self(); + self->embedder_api.struct_size = sizeof(FlutterEngineProcTable); + FlutterEngineGetProcAddresses(&self->embedder_api); + self->binary_messenger = fl_binary_messenger_new(self); } @@ -400,11 +404,12 @@ gboolean fl_engine_start(FlEngine* self, GError** error) { args.dart_entrypoint_argv = reinterpret_cast(dart_entrypoint_args); - if (FlutterEngineRunsAOTCompiledDartCode()) { + if (self->embedder_api.RunsAOTCompiledDartCode()) { FlutterEngineAOTDataSource source = {}; source.type = kFlutterEngineAOTDataSourceTypeElfPath; source.elf_path = fl_dart_project_get_aot_library_path(self->project); - if (FlutterEngineCreateAOTData(&source, &self->aot_data) != kSuccess) { + if (self->embedder_api.CreateAOTData(&source, &self->aot_data) != + kSuccess) { g_set_error(error, fl_engine_error_quark(), FL_ENGINE_ERROR_FAILED, "Failed to create AOT data"); return FALSE; @@ -412,7 +417,7 @@ gboolean fl_engine_start(FlEngine* self, GError** error) { args.aot_data = self->aot_data; } - FlutterEngineResult result = FlutterEngineInitialize( + FlutterEngineResult result = self->embedder_api.Initialize( FLUTTER_ENGINE_VERSION, &config, &args, self, &self->engine); if (result != kSuccess) { g_set_error(error, fl_engine_error_quark(), FL_ENGINE_ERROR_FAILED, @@ -420,7 +425,7 @@ gboolean fl_engine_start(FlEngine* self, GError** error) { return FALSE; } - result = FlutterEngineRunInitialized(self->engine); + result = self->embedder_api.RunInitialized(self->engine); if (result != kSuccess) { g_set_error(error, fl_engine_error_quark(), FL_ENGINE_ERROR_FAILED, "Failed to run Flutter engine"); @@ -432,6 +437,10 @@ gboolean fl_engine_start(FlEngine* self, GError** error) { return TRUE; } +FlutterEngineProcTable* fl_engine_get_embedder_api(FlEngine* self) { + return &(self->embedder_api); +} + void fl_engine_set_platform_message_handler( FlEngine* self, FlEnginePlatformMessageHandler handler, @@ -470,7 +479,7 @@ gboolean fl_engine_send_platform_message_response( data = static_cast(g_bytes_get_data(response, &data_length)); } - FlutterEngineResult result = FlutterEngineSendPlatformMessageResponse( + FlutterEngineResult result = self->embedder_api.SendPlatformMessageResponse( self->engine, handle, data, data_length); if (result != kSuccess) { @@ -501,9 +510,10 @@ void fl_engine_send_platform_message(FlEngine* self, return; } - FlutterEngineResult result = FlutterPlatformMessageCreateResponseHandle( - self->engine, fl_engine_platform_message_response_cb, task, - &response_handle); + FlutterEngineResult result = + self->embedder_api.PlatformMessageCreateResponseHandle( + self->engine, fl_engine_platform_message_response_cb, task, + &response_handle); if (result != kSuccess) { g_task_return_new_error(task, fl_engine_error_quark(), FL_ENGINE_ERROR_FAILED, @@ -525,7 +535,7 @@ void fl_engine_send_platform_message(FlEngine* self, fl_message.message_size = message != nullptr ? g_bytes_get_size(message) : 0; fl_message.response_handle = response_handle; FlutterEngineResult result = - FlutterEngineSendPlatformMessage(self->engine, &fl_message); + self->embedder_api.SendPlatformMessage(self->engine, &fl_message); if (result != kSuccess && task != nullptr) { g_task_return_new_error(task, fl_engine_error_quark(), @@ -535,7 +545,8 @@ void fl_engine_send_platform_message(FlEngine* self, } if (response_handle != nullptr) { - FlutterPlatformMessageReleaseResponseHandle(self->engine, response_handle); + self->embedder_api.PlatformMessageReleaseResponseHandle(self->engine, + response_handle); } } @@ -563,7 +574,7 @@ void fl_engine_send_window_metrics_event(FlEngine* self, event.width = width; event.height = height; event.pixel_ratio = pixel_ratio; - FlutterEngineSendWindowMetricsEvent(self->engine, &event); + self->embedder_api.SendWindowMetricsEvent(self->engine, &event); } void fl_engine_send_mouse_pointer_event(FlEngine* self, @@ -593,7 +604,7 @@ void fl_engine_send_mouse_pointer_event(FlEngine* self, fl_event.scroll_delta_y = scroll_delta_y; fl_event.device_kind = kFlutterPointerDeviceKindMouse; fl_event.buttons = buttons; - FlutterEngineSendPointerEvent(self->engine, &fl_event, 1); + self->embedder_api.SendPointerEvent(self->engine, &fl_event, 1); } G_MODULE_EXPORT FlBinaryMessenger* fl_engine_get_binary_messenger( diff --git a/shell/platform/linux/fl_engine_private.h b/shell/platform/linux/fl_engine_private.h index 6c912979ba1..ce99004a141 100644 --- a/shell/platform/linux/fl_engine_private.h +++ b/shell/platform/linux/fl_engine_private.h @@ -55,6 +55,16 @@ typedef gboolean (*FlEnginePlatformMessageHandler)( */ FlEngine* fl_engine_new(FlDartProject* project, FlRenderer* renderer); +/** + * fl_engine_get_embedder_api: + * @engine: an #FlEngine. + * + * Gets the embedder API proc table, allowing modificiations for unit testing. + * + * Returns: a mutable pointer to the embedder API proc table. + */ +FlutterEngineProcTable* fl_engine_get_embedder_api(FlEngine* engine); + /** * fl_engine_set_platform_message_handler: * @engine: an #FlEngine. diff --git a/shell/platform/linux/testing/mock_engine.cc b/shell/platform/linux/testing/mock_engine.cc index 60aad75dbea..c498e92f955 100644 --- a/shell/platform/linux/testing/mock_engine.cc +++ b/shell/platform/linux/testing/mock_engine.cc @@ -2,6 +2,13 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +// This file is a historical legacy, predating the proc table API. It has been +// updated to continue to work with the proc table, but new tests should not +// rely on replacements set up here, but instead use test-local replacements +// for any functions relevant to that test. +// +// Over time existing tests should be migrated and this file should be removed. + #include #include "flutter/shell/platform/embedder/embedder.h" @@ -77,6 +84,8 @@ struct _FlutterTaskRunner { } }; +namespace { + // Send a response from the engine. static void send_response( FLUTTER_API_SYMBOL(FlutterEngine) engine, @@ -138,31 +147,6 @@ FlutterEngineResult FlutterEngineCollectAOTData(FlutterEngineAOTData data) { return kSuccess; } -FlutterEngineResult FlutterEngineRun(size_t version, - const FlutterRendererConfig* config, - const FlutterProjectArgs* args, - void* user_data, - FLUTTER_API_SYMBOL(FlutterEngine) * - engine_out) { - EXPECT_NE(config, nullptr); - EXPECT_NE(args, nullptr); - EXPECT_NE(user_data, nullptr); - EXPECT_NE(engine_out, nullptr); - - FlutterEngineResult result = - FlutterEngineInitialize(version, config, args, user_data, engine_out); - if (result != kSuccess) { - return result; - } - return FlutterEngineRunInitialized(*engine_out); -} - -FlutterEngineResult FlutterEngineShutdown(FLUTTER_API_SYMBOL(FlutterEngine) - engine) { - delete engine; - return kSuccess; -} - FlutterEngineResult FlutterEngineInitialize(size_t version, const FlutterRendererConfig* config, const FlutterProjectArgs* args, @@ -189,17 +173,42 @@ FlutterEngineResult FlutterEngineInitialize(size_t version, return kSuccess; } -FlutterEngineResult FlutterEngineDeinitialize(FLUTTER_API_SYMBOL(FlutterEngine) - engine) { - return kSuccess; -} - FlutterEngineResult FlutterEngineRunInitialized( FLUTTER_API_SYMBOL(FlutterEngine) engine) { engine->running = true; return kSuccess; } +FlutterEngineResult FlutterEngineRun(size_t version, + const FlutterRendererConfig* config, + const FlutterProjectArgs* args, + void* user_data, + FLUTTER_API_SYMBOL(FlutterEngine) * + engine_out) { + EXPECT_NE(config, nullptr); + EXPECT_NE(args, nullptr); + EXPECT_NE(user_data, nullptr); + EXPECT_NE(engine_out, nullptr); + + FlutterEngineResult result = + FlutterEngineInitialize(version, config, args, user_data, engine_out); + if (result != kSuccess) { + return result; + } + return FlutterEngineRunInitialized(*engine_out); +} + +FlutterEngineResult FlutterEngineShutdown(FLUTTER_API_SYMBOL(FlutterEngine) + engine) { + delete engine; + return kSuccess; +} + +FlutterEngineResult FlutterEngineDeinitialize(FLUTTER_API_SYMBOL(FlutterEngine) + engine) { + return kSuccess; +} + FlutterEngineResult FlutterEngineSendWindowMetricsEvent( FLUTTER_API_SYMBOL(FlutterEngine) engine, const FlutterWindowMetricsEvent* event) { @@ -401,3 +410,37 @@ FlutterEngineResult FlutterEngineUpdateLocales(FLUTTER_API_SYMBOL(FlutterEngine) size_t locales_count) { return kSuccess; } + +} // namespace + +FlutterEngineResult FlutterEngineGetProcAddresses( + FlutterEngineProcTable* table) { + if (!table) { + return kInvalidArguments; + } + + FlutterEngineProcTable empty_table = {}; + *table = empty_table; + + table->CreateAOTData = &FlutterEngineCreateAOTData; + table->CollectAOTData = &FlutterEngineCollectAOTData; + table->Run = &FlutterEngineRun; + table->Shutdown = &FlutterEngineShutdown; + table->Initialize = &FlutterEngineInitialize; + table->Deinitialize = &FlutterEngineDeinitialize; + table->RunInitialized = &FlutterEngineRunInitialized; + table->SendWindowMetricsEvent = &FlutterEngineSendWindowMetricsEvent; + table->SendPointerEvent = &FlutterEngineSendPointerEvent; + table->SendPlatformMessage = &FlutterEngineSendPlatformMessage; + table->PlatformMessageCreateResponseHandle = + &FlutterPlatformMessageCreateResponseHandle; + table->PlatformMessageReleaseResponseHandle = + &FlutterPlatformMessageReleaseResponseHandle; + table->SendPlatformMessageResponse = + &FlutterEngineSendPlatformMessageResponse; + table->RunTask = &FlutterEngineRunTask; + table->UpdateLocales = &FlutterEngineUpdateLocales; + table->RunsAOTCompiledDartCode = &FlutterEngineRunsAOTCompiledDartCode; + + return kSuccess; +}