From 57fa7971d2d59008b4a4edc6145065ce56046531 Mon Sep 17 00:00:00 2001 From: George Wright Date: Mon, 5 Oct 2020 11:54:40 -0700 Subject: [PATCH 1/3] Add dart_entrypoint_argc/argv to the FlutterProjectArgs --- shell/platform/embedder/embedder.cc | 8 ++++++ shell/platform/embedder/embedder.h | 9 ++++++ shell/platform/embedder/fixtures/main.dart | 7 +++++ .../embedder/tests/embedder_unittests.cc | 28 +++++++++++++++++++ 4 files changed, 52 insertions(+) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 971ef22024f59..a8fb8d32be5c0 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -1126,6 +1126,14 @@ FlutterEngineResult FlutterEngineInitialize(size_t version, } } + if (SAFE_ACCESS(args, dart_entrypoint_argc, 0)) { + std::vector arguments(args->dart_entrypoint_argc); + for (int i = 0; i < args->dart_entrypoint_argc; ++i) { + arguments[i] = std::string{args->dart_entrypoint_argv[i]}; + } + settings.dart_entrypoint_args = std::move(arguments); + } + if (!run_configuration.IsValid()) { return LOG_EMBEDDER_ERROR( kInvalidArguments, diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index 8ed2dadfac37f..cf91519e7041f 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -1368,6 +1368,15 @@ typedef struct { /// matches what the platform would natively resolve to as possible. FlutterComputePlatformResolvedLocaleCallback compute_platform_resolved_locale_callback; + + /// The command line argument count for arguments passed through to the Dart + /// entrypoint. + int dart_entrypoint_argc; + + /// The command line arguments passed through to the Dart entrypoint. The + /// strings must be `NULL` terminated. + const char* const* dart_entrypoint_argv; + } FlutterProjectArgs; //------------------------------------------------------------------------------ diff --git a/shell/platform/embedder/fixtures/main.dart b/shell/platform/embedder/fixtures/main.dart index de4cf422b662f..5dae4e494745c 100644 --- a/shell/platform/embedder/fixtures/main.dart +++ b/shell/platform/embedder/fixtures/main.dart @@ -733,3 +733,10 @@ void render_targets_are_recycled() { }; window.scheduleFrame(); } + +void nativeArgumentsCallback(List args) native 'NativeArgumentsCallback'; + +@pragma('vm:entry-point') +void dart_entrypoint_args(List args) { + nativeArgumentsCallback(args); +} diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index 786b7da063e9f..e4e2cbe015de4 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -473,6 +473,34 @@ TEST_F(EmbedderTest, VMShutsDownWhenNoEnginesInProcess) { } } +//------------------------------------------------------------------------------ +/// +TEST_F(EmbedderTest, DartEntrypointArgs) { + auto& context = GetEmbedderContext(ContextType::kSoftwareContext); + EmbedderConfigBuilder builder(context); + builder.SetSoftwareRendererConfig(); + builder.GetProjectArgs().dart_entrypoint_argc = 2; + std::vector args = {"foo", "bar"}; + builder.GetProjectArgs().dart_entrypoint_argv = args.data(); + builder.SetDartEntrypoint("dart_entrypoint_args"); + fml::AutoResetWaitableEvent callbackLatch; + std::vector callbackArgs; + auto nativeArgumentsCallback = [&callbackArgs, + &callbackLatch](Dart_NativeArguments args) { + Dart_Handle exception = nullptr; + callbackArgs = + tonic::DartConverter>::FromArguments( + args, 0, exception); + callbackLatch.Signal(); + }; + context.AddNativeCallback("NativeArgumentsCallback", + CREATE_NATIVE_ENTRY(nativeArgumentsCallback)); + auto engine = builder.LaunchEngine(); + callbackLatch.Wait(); + ASSERT_EQ(callbackArgs[0], "foo"); + ASSERT_EQ(callbackArgs[1], "bar"); +} + //------------------------------------------------------------------------------ /// These snapshots may be materialized from symbols and the size field may not /// be relevant. Since this information is redundant, engine launch should not From 5f4d17e1e63b7fae0ef68de83eaf88d5ab297262 Mon Sep 17 00:00:00 2001 From: George Wright Date: Fri, 9 Oct 2020 13:22:42 -0700 Subject: [PATCH 2/3] review updates --- shell/platform/embedder/embedder.cc | 3 ++- shell/platform/embedder/embedder.h | 3 +++ .../embedder/tests/embedder_config_builder.cc | 25 +++++++++++++++++++ .../embedder/tests/embedder_config_builder.h | 3 +++ .../embedder/tests/embedder_unittests.cc | 23 ++++++++--------- 5 files changed, 44 insertions(+), 13 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index a8fb8d32be5c0..e8578b140bc42 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -1126,7 +1126,8 @@ FlutterEngineResult FlutterEngineInitialize(size_t version, } } - if (SAFE_ACCESS(args, dart_entrypoint_argc, 0)) { + if (SAFE_ACCESS(args, dart_entrypoint_argc, 0) > 0 && + SAFE_ACCESS(args, dart_entrypoint_argv, nullptr) != nullptr) { std::vector arguments(args->dart_entrypoint_argc); for (int i = 0; i < args->dart_entrypoint_argc; ++i) { arguments[i] = std::string{args->dart_entrypoint_argv[i]}; diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index cf91519e7041f..a4caf7f9be58e 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -1375,6 +1375,9 @@ typedef struct { /// The command line arguments passed through to the Dart entrypoint. The /// strings must be `NULL` terminated. + /// + /// The strings will be copied out and so any strings passed in here can + /// be safely collected after initializing the engine with `FlutterProjectArgs`. const char* const* dart_entrypoint_argv; } FlutterProjectArgs; diff --git a/shell/platform/embedder/tests/embedder_config_builder.cc b/shell/platform/embedder/tests/embedder_config_builder.cc index 5c1f741996257..93d4c6af916f2 100644 --- a/shell/platform/embedder/tests/embedder_config_builder.cc +++ b/shell/platform/embedder/tests/embedder_config_builder.cc @@ -217,6 +217,14 @@ void EmbedderConfigBuilder::AddCommandLineArgument(std::string arg) { command_line_arguments_.emplace_back(std::move(arg)); } +void EmbedderConfigBuilder::AddDartEntrypointArgument(std::string arg) { + if (arg.size() == 0) { + return; + } + + dart_entrypoint_arguments_.emplace_back(std::move(arg)); +} + void EmbedderConfigBuilder::SetPlatformTaskRunner( const FlutterTaskRunnerDescription* runner) { if (runner == nullptr) { @@ -317,6 +325,23 @@ UniqueEngine EmbedderConfigBuilder::SetupEngine(bool run) const { project_args.command_line_argc = 0; } + std::vector dart_args; + dart_args.reserve(dart_entrypoint_arguments_.size()); + + for (const auto& arg : dart_entrypoint_arguments_) { + dart_args.push_back(arg.c_str()); + } + + if (dart_args.size() > 0) { + project_args.dart_entrypoint_argv = dart_args.data(); + project_args.dart_entrypoint_argc = dart_args.size(); + } else { + // Clear it out in case this is not the first engine launch from the + // embedder config builder. + project_args.dart_entrypoint_argv = nullptr; + project_args.dart_entrypoint_argc = 0; + } + auto result = run ? FlutterEngineRun(FLUTTER_ENGINE_VERSION, &renderer_config_, &project_args, &context_, &engine) diff --git a/shell/platform/embedder/tests/embedder_config_builder.h b/shell/platform/embedder/tests/embedder_config_builder.h index 29dc4ef65ad11..4c37d644bc7ac 100644 --- a/shell/platform/embedder/tests/embedder_config_builder.h +++ b/shell/platform/embedder/tests/embedder_config_builder.h @@ -76,6 +76,8 @@ class EmbedderConfigBuilder { void AddCommandLineArgument(std::string arg); + void AddDartEntrypointArgument(std::string arg); + void SetPlatformTaskRunner(const FlutterTaskRunnerDescription* runner); void SetRenderTaskRunner(const FlutterTaskRunnerDescription* runner); @@ -106,6 +108,7 @@ class EmbedderConfigBuilder { FlutterCustomTaskRunners custom_task_runners_ = {}; FlutterCompositor compositor_ = {}; std::vector command_line_arguments_; + std::vector dart_entrypoint_arguments_; UniqueEngine SetupEngine(bool run) const; diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index e4e2cbe015de4..01309bb7366f3 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -479,26 +479,25 @@ TEST_F(EmbedderTest, DartEntrypointArgs) { auto& context = GetEmbedderContext(ContextType::kSoftwareContext); EmbedderConfigBuilder builder(context); builder.SetSoftwareRendererConfig(); - builder.GetProjectArgs().dart_entrypoint_argc = 2; - std::vector args = {"foo", "bar"}; - builder.GetProjectArgs().dart_entrypoint_argv = args.data(); + builder.AddDartEntrypointArgument("foo"); + builder.AddDartEntrypointArgument("bar"); builder.SetDartEntrypoint("dart_entrypoint_args"); - fml::AutoResetWaitableEvent callbackLatch; - std::vector callbackArgs; - auto nativeArgumentsCallback = [&callbackArgs, - &callbackLatch](Dart_NativeArguments args) { + fml::AutoResetWaitableEvent callback_latch; + std::vector callback_args; + auto nativeArgumentsCallback = [&callback_args, + &callback_latch](Dart_NativeArguments args) { Dart_Handle exception = nullptr; - callbackArgs = + callback_args = tonic::DartConverter>::FromArguments( args, 0, exception); - callbackLatch.Signal(); + callback_latch.Signal(); }; context.AddNativeCallback("NativeArgumentsCallback", CREATE_NATIVE_ENTRY(nativeArgumentsCallback)); auto engine = builder.LaunchEngine(); - callbackLatch.Wait(); - ASSERT_EQ(callbackArgs[0], "foo"); - ASSERT_EQ(callbackArgs[1], "bar"); + callback_latch.Wait(); + ASSERT_EQ(callback_args[0], "foo"); + ASSERT_EQ(callback_args[1], "bar"); } //------------------------------------------------------------------------------ From dba4dd4fb30e69e1b2f0dabeeaeab10232ba64d5 Mon Sep 17 00:00:00 2001 From: George Wright Date: Fri, 9 Oct 2020 14:37:44 -0700 Subject: [PATCH 3/3] review update --- shell/platform/embedder/embedder.cc | 9 +++++++-- shell/platform/embedder/embedder.h | 3 ++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index e8578b140bc42..06b45ac5372e1 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -1126,8 +1126,13 @@ FlutterEngineResult FlutterEngineInitialize(size_t version, } } - if (SAFE_ACCESS(args, dart_entrypoint_argc, 0) > 0 && - SAFE_ACCESS(args, dart_entrypoint_argv, nullptr) != nullptr) { + if (SAFE_ACCESS(args, dart_entrypoint_argc, 0) > 0) { + if (SAFE_ACCESS(args, dart_entrypoint_argv, nullptr) == nullptr) { + return LOG_EMBEDDER_ERROR(kInvalidArguments, + "Could not determine Dart entrypoint arguments " + "as dart_entrypoint_argc " + "was set, but dart_entrypoint_argv was null."); + } std::vector arguments(args->dart_entrypoint_argc); for (int i = 0; i < args->dart_entrypoint_argc; ++i) { arguments[i] = std::string{args->dart_entrypoint_argv[i]}; diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index a4caf7f9be58e..51d832bfe5397 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -1377,7 +1377,8 @@ typedef struct { /// strings must be `NULL` terminated. /// /// The strings will be copied out and so any strings passed in here can - /// be safely collected after initializing the engine with `FlutterProjectArgs`. + /// be safely collected after initializing the engine with + /// `FlutterProjectArgs`. const char* const* dart_entrypoint_argv; } FlutterProjectArgs;