From b4c0ba222b4213299bdb6bd5197c43c540a643ff Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 1 Apr 2021 13:00:41 -0700 Subject: [PATCH 01/16] Revert "Revert "Call Dart plugin registrant if available (#23813)" (#25221)" This reverts commit 15b20ee52841279d009edb0ea0db9be93108cb5c. --- lib/ui/hooks.dart | 4 ++++ runtime/dart_isolate.cc | 32 ++++++++++++++++++++++++---- runtime/dart_isolate_unittests.cc | 34 ++++++++++++++++++++++++++++++ runtime/fixtures/runtime_test.dart | 24 +++++++++++++++++++-- 4 files changed, 88 insertions(+), 6 deletions(-) diff --git a/lib/ui/hooks.dart b/lib/ui/hooks.dart index 79299a60ed3aa..102694398f841 100644 --- a/lib/ui/hooks.dart +++ b/lib/ui/hooks.dart @@ -132,10 +132,14 @@ typedef _ListStringArgFunction(List args); @pragma('vm:entry-point') // ignore: unused_element void _runMainZoned(Function startMainIsolateFunction, + Function? dartPluginRegistrant, Function userMainFunction, List args) { startMainIsolateFunction(() { runZonedGuarded(() { + if (dartPluginRegistrant != null) { + dartPluginRegistrant(); + } if (userMainFunction is _ListStringArgFunction) { (userMainFunction as dynamic)(args); } else { diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index 658423f178c3a..de933bca67fc4 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -700,6 +700,7 @@ bool DartIsolate::MarkIsolateRunnable() { [[nodiscard]] static bool InvokeMainEntrypoint( Dart_Handle user_entrypoint_function, + Dart_Handle plugin_registrant_function, Dart_Handle args) { if (tonic::LogIfError(user_entrypoint_function)) { FML_LOG(ERROR) << "Could not resolve main entrypoint function."; @@ -717,7 +718,8 @@ bool DartIsolate::MarkIsolateRunnable() { if (tonic::LogIfError(tonic::DartInvokeField( Dart_LookupLibrary(tonic::ToDart("dart:ui")), "_runMainZoned", - {start_main_isolate_function, user_entrypoint_function, args}))) { + {start_main_isolate_function, plugin_registrant_function, + user_entrypoint_function, args}))) { FML_LOG(ERROR) << "Could not invoke the main entrypoint."; return false; } @@ -742,12 +744,34 @@ bool DartIsolate::RunFromLibrary(std::optional library_name, auto entrypoint_handle = entrypoint.has_value() && !entrypoint.value().empty() ? tonic::ToDart(entrypoint.value().c_str()) : tonic::ToDart("main"); + auto entrypoint_args = tonic::ToDart(args); auto user_entrypoint_function = ::Dart_GetField(library_handle, entrypoint_handle); - auto entrypoint_args = tonic::ToDart(args); - - if (!InvokeMainEntrypoint(user_entrypoint_function, entrypoint_args)) { + // The Dart plugin registrant is a function named `_registerPlugins` + // generated by the Flutter tool. + // + // This function binds a plugin implementation to their platform + // interface based on the configuration of the app's pubpec.yaml, and the + // plugin's pubspec.yaml. + // + // Since this function may or may not be defined. Check that it is a top + // level function, and call it in hooks.dart before the main entrypoint + // function. + // + // If it's not defined, then just call the main entrypoint function + // as usual. + // + // This allows embeddings to change the name of the entrypoint function. + auto plugin_registrant_function = + ::Dart_GetField(library_handle, tonic::ToDart("_registerPlugins")); + + if (Dart_IsError(plugin_registrant_function)) { + plugin_registrant_function = Dart_Null(); + } + + if (!InvokeMainEntrypoint(user_entrypoint_function, + plugin_registrant_function, entrypoint_args)) { return false; } diff --git a/runtime/dart_isolate_unittests.cc b/runtime/dart_isolate_unittests.cc index bb6ada9e3e98d..2bc890ddd5949 100644 --- a/runtime/dart_isolate_unittests.cc +++ b/runtime/dart_isolate_unittests.cc @@ -613,5 +613,39 @@ TEST_F(DartIsolateTest, DISABLED_ValidLoadingUnitSucceeds) { Wait(); } +TEST_F(DartIsolateTest, DartPluginRegistrantIsCalled) { + ASSERT_FALSE(DartVMRef::IsInstanceRunning()); + + std::vector messages; + fml::AutoResetWaitableEvent latch; + + AddNativeCallback( + "PassMessage", + CREATE_NATIVE_ENTRY(([&latch, &messages](Dart_NativeArguments args) { + auto message = tonic::DartConverter::FromDart( + Dart_GetNativeArgument(args, 0)); + messages.push_back(message); + latch.Signal(); + }))); + + const auto settings = CreateSettingsForFixture(); + auto vm_ref = DartVMRef::Create(settings); + auto thread = CreateNewThread(); + TaskRunners task_runners(GetCurrentTestName(), // + thread, // + thread, // + thread, // + thread // + ); + auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, + "mainForPluginRegistrantTest", {}, + GetFixturesPath()); + ASSERT_TRUE(isolate); + ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); + latch.Wait(); + ASSERT_EQ(messages.size(), 1u); + ASSERT_EQ(messages[0], "_registerPlugins was called"); +} + } // namespace testing } // namespace flutter diff --git a/runtime/fixtures/runtime_test.dart b/runtime/fixtures/runtime_test.dart index 9e137e1fb6acb..9a0169fb0d1ed 100644 --- a/runtime/fixtures/runtime_test.dart +++ b/runtime/fixtures/runtime_test.dart @@ -10,8 +10,7 @@ import 'dart:ui'; import 'split_lib_test.dart' deferred as splitlib; -void main() { -} +void main() {} @pragma('vm:entry-point') void sayHi() { @@ -115,3 +114,24 @@ void testCanConvertListOfInts(List args){ args[2] == 3 && args[3] == 4); } + +bool didCallRegistrantBeforeEntrypoint = false; + +// Test the Dart plugin registrant. +// _registerPlugins requires the entrypoint annotation, so the compiler doesn't tree shake it. +@pragma('vm:entry-point') +void _registerPlugins() { // ignore: unused_element + if (didCallRegistrantBeforeEntrypoint) { + throw '_registerPlugins is being called twice'; + } + didCallRegistrantBeforeEntrypoint = true; +} + +@pragma('vm:entry-point') +void mainForPluginRegistrantTest() { // ignore: unused_element + if (didCallRegistrantBeforeEntrypoint) { + passMessage('_registerPlugins was called'); + } else { + passMessage('_registerPlugins was not called'); + } +} From d0f084ed0dd15f8adbdfc5ef07583ccabd6bf06e Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 8 Apr 2021 10:29:40 -0700 Subject: [PATCH 02/16] Rework plugin registrant --- lib/ui/hooks.dart | 4 ---- runtime/dart_isolate.cc | 37 ++++++++++++++---------------- runtime/dart_isolate_unittests.cc | 2 +- runtime/fixtures/runtime_test.dart | 20 ++++++++++------ 4 files changed, 31 insertions(+), 32 deletions(-) diff --git a/lib/ui/hooks.dart b/lib/ui/hooks.dart index 102694398f841..79299a60ed3aa 100644 --- a/lib/ui/hooks.dart +++ b/lib/ui/hooks.dart @@ -132,14 +132,10 @@ typedef _ListStringArgFunction(List args); @pragma('vm:entry-point') // ignore: unused_element void _runMainZoned(Function startMainIsolateFunction, - Function? dartPluginRegistrant, Function userMainFunction, List args) { startMainIsolateFunction(() { runZonedGuarded(() { - if (dartPluginRegistrant != null) { - dartPluginRegistrant(); - } if (userMainFunction is _ListStringArgFunction) { (userMainFunction as dynamic)(args); } else { diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index de933bca67fc4..1f718dba164cd 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -718,8 +718,7 @@ bool DartIsolate::MarkIsolateRunnable() { if (tonic::LogIfError(tonic::DartInvokeField( Dart_LookupLibrary(tonic::ToDart("dart:ui")), "_runMainZoned", - {start_main_isolate_function, plugin_registrant_function, - user_entrypoint_function, args}))) { + {start_main_isolate_function, user_entrypoint_function, args}))) { FML_LOG(ERROR) << "Could not invoke the main entrypoint."; return false; } @@ -748,30 +747,28 @@ bool DartIsolate::RunFromLibrary(std::optional library_name, auto user_entrypoint_function = ::Dart_GetField(library_handle, entrypoint_handle); - // The Dart plugin registrant is a function named `_registerPlugins` - // generated by the Flutter tool. + // The Dart plugin registrant is a static with signature `void register()` + // within the class `_PluginRegistrant` generated by the Flutter tool. // - // This function binds a plugin implementation to their platform + // This method binds a plugin implementation to their platform // interface based on the configuration of the app's pubpec.yaml, and the // plugin's pubspec.yaml. // - // Since this function may or may not be defined. Check that it is a top - // level function, and call it in hooks.dart before the main entrypoint - // function. - // - // If it's not defined, then just call the main entrypoint function - // as usual. - // - // This allows embeddings to change the name of the entrypoint function. - auto plugin_registrant_function = - ::Dart_GetField(library_handle, tonic::ToDart("_registerPlugins")); - - if (Dart_IsError(plugin_registrant_function)) { - plugin_registrant_function = Dart_Null(); + // Since this method may or may not be defined, check if the class is defined + // in the default library before calling the method. + Dart_Handle classRegistrant = + ::Dart_GetClass(library_handle, tonic::ToDart("_PluginRegistrant")); + + if (!Dart_IsError(classRegistrant)) { + Dart_Handle result = + tonic::DartInvokeField(classRegistrant, "register", {}); + if (tonic::LogIfError(result)) { + FML_LOG(ERROR) << "Could not invoke the main entrypoint."; + } } - if (!InvokeMainEntrypoint(user_entrypoint_function, - plugin_registrant_function, entrypoint_args)) { + if (!InvokeMainEntrypoint(user_entrypoint_function, classRegistrant, + entrypoint_args)) { return false; } diff --git a/runtime/dart_isolate_unittests.cc b/runtime/dart_isolate_unittests.cc index 2bc890ddd5949..c2ed61f8e614c 100644 --- a/runtime/dart_isolate_unittests.cc +++ b/runtime/dart_isolate_unittests.cc @@ -644,7 +644,7 @@ TEST_F(DartIsolateTest, DartPluginRegistrantIsCalled) { ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); latch.Wait(); ASSERT_EQ(messages.size(), 1u); - ASSERT_EQ(messages[0], "_registerPlugins was called"); + ASSERT_EQ(messages[0], "_PluginRegistrant.register() was called"); } } // namespace testing diff --git a/runtime/fixtures/runtime_test.dart b/runtime/fixtures/runtime_test.dart index 9a0169fb0d1ed..4b8ada1cd15e5 100644 --- a/runtime/fixtures/runtime_test.dart +++ b/runtime/fixtures/runtime_test.dart @@ -118,20 +118,26 @@ void testCanConvertListOfInts(List args){ bool didCallRegistrantBeforeEntrypoint = false; // Test the Dart plugin registrant. -// _registerPlugins requires the entrypoint annotation, so the compiler doesn't tree shake it. +// _PluginRegistrant requires the entrypoint annotation, so the compiler doesn't tree shake it. @pragma('vm:entry-point') -void _registerPlugins() { // ignore: unused_element - if (didCallRegistrantBeforeEntrypoint) { - throw '_registerPlugins is being called twice'; +class _PluginRegistrant { + + @pragma('vm:entry-point') + static void register() { + if (didCallRegistrantBeforeEntrypoint) { + throw '_registerPlugins is being called twice'; + } + didCallRegistrantBeforeEntrypoint = true; } - didCallRegistrantBeforeEntrypoint = true; + } + @pragma('vm:entry-point') void mainForPluginRegistrantTest() { // ignore: unused_element if (didCallRegistrantBeforeEntrypoint) { - passMessage('_registerPlugins was called'); + passMessage('_PluginRegistrant.register() was called'); } else { - passMessage('_registerPlugins was not called'); + passMessage('_PluginRegistrant.register() was not called'); } } From ab50fec9447bb21436d355cb543739459218af99 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 8 Apr 2021 10:44:39 -0700 Subject: [PATCH 03/16] Fix error message --- runtime/dart_isolate.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index 1f718dba164cd..4349ed3dee2eb 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -763,7 +763,7 @@ bool DartIsolate::RunFromLibrary(std::optional library_name, Dart_Handle result = tonic::DartInvokeField(classRegistrant, "register", {}); if (tonic::LogIfError(result)) { - FML_LOG(ERROR) << "Could not invoke the main entrypoint."; + FML_LOG(ERROR) << "Could not invoke the Dart plugin registrant."; } } From 47bc3da15c6fd6208d78cb870c226a9e7f3ed686 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 8 Apr 2021 10:46:54 -0700 Subject: [PATCH 04/16] snake case --- runtime/dart_isolate.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index 4349ed3dee2eb..801e78be45468 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -756,18 +756,18 @@ bool DartIsolate::RunFromLibrary(std::optional library_name, // // Since this method may or may not be defined, check if the class is defined // in the default library before calling the method. - Dart_Handle classRegistrant = + Dart_Handle plugin_registrant = ::Dart_GetClass(library_handle, tonic::ToDart("_PluginRegistrant")); - if (!Dart_IsError(classRegistrant)) { + if (!Dart_IsError(plugin_registrant)) { Dart_Handle result = - tonic::DartInvokeField(classRegistrant, "register", {}); + tonic::DartInvokeField(plugin_registrant, "register", {}); if (tonic::LogIfError(result)) { FML_LOG(ERROR) << "Could not invoke the Dart plugin registrant."; } } - if (!InvokeMainEntrypoint(user_entrypoint_function, classRegistrant, + if (!InvokeMainEntrypoint(user_entrypoint_function, plugin_registrant, entrypoint_args)) { return false; } From 222b0aa1645933c0c4930f859c12af345caf8498 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 8 Apr 2021 10:49:35 -0700 Subject: [PATCH 05/16] Remove arg --- runtime/dart_isolate.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index 801e78be45468..1ae20e2095c5c 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -700,7 +700,6 @@ bool DartIsolate::MarkIsolateRunnable() { [[nodiscard]] static bool InvokeMainEntrypoint( Dart_Handle user_entrypoint_function, - Dart_Handle plugin_registrant_function, Dart_Handle args) { if (tonic::LogIfError(user_entrypoint_function)) { FML_LOG(ERROR) << "Could not resolve main entrypoint function."; @@ -767,8 +766,7 @@ bool DartIsolate::RunFromLibrary(std::optional library_name, } } - if (!InvokeMainEntrypoint(user_entrypoint_function, plugin_registrant, - entrypoint_args)) { + if (!InvokeMainEntrypoint(user_entrypoint_function, entrypoint_args)) { return false; } From a62131d386a7949495705a279348a60c2240e410 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 8 Apr 2021 10:56:01 -0700 Subject: [PATCH 06/16] Remove useless comment --- runtime/fixtures/runtime_test.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/runtime/fixtures/runtime_test.dart b/runtime/fixtures/runtime_test.dart index 4b8ada1cd15e5..dfc01f2819779 100644 --- a/runtime/fixtures/runtime_test.dart +++ b/runtime/fixtures/runtime_test.dart @@ -118,7 +118,6 @@ void testCanConvertListOfInts(List args){ bool didCallRegistrantBeforeEntrypoint = false; // Test the Dart plugin registrant. -// _PluginRegistrant requires the entrypoint annotation, so the compiler doesn't tree shake it. @pragma('vm:entry-point') class _PluginRegistrant { From 53c0304c4c1640c5c8060877c980bfb1b4020596 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 8 Apr 2021 15:54:40 -0700 Subject: [PATCH 07/16] Move to a separate method --- runtime/dart_isolate.cc | 45 ++++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index 1ae20e2095c5c..baf952838aacb 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -725,6 +725,31 @@ bool DartIsolate::MarkIsolateRunnable() { return true; } +static void InvokeDartPluginRegistrantIfAvailable(Dart_Handle library_handle) { + TRACE_EVENT0("flutter", "InvokeDartPluginRegistrantIfAvailable"); + + // The Dart plugin registrant is a static with signature `void register()` + // within the class `_PluginRegistrant` generated by the Flutter tool. + // + // This method binds a plugin implementation to their platform + // interface based on the configuration of the app's pubpec.yaml, and the + // plugin's pubspec.yaml. + // + // Since this method may or may not be defined, check if the class is defined + // in the default library before calling the method. + Dart_Handle plugin_registrant = + ::Dart_GetClass(library_handle, tonic::ToDart("_PluginRegistrant")); + + if (Dart_IsError(plugin_registrant)) { + return; + } + Dart_Handle result = + tonic::DartInvokeField(plugin_registrant, "register", {}); + if (tonic::LogIfError(result)) { + FML_LOG(ERROR) << "The Dart plugin registrant produced an error"; + } +} + bool DartIsolate::RunFromLibrary(std::optional library_name, std::optional entrypoint, const std::vector& args) { @@ -746,25 +771,7 @@ bool DartIsolate::RunFromLibrary(std::optional library_name, auto user_entrypoint_function = ::Dart_GetField(library_handle, entrypoint_handle); - // The Dart plugin registrant is a static with signature `void register()` - // within the class `_PluginRegistrant` generated by the Flutter tool. - // - // This method binds a plugin implementation to their platform - // interface based on the configuration of the app's pubpec.yaml, and the - // plugin's pubspec.yaml. - // - // Since this method may or may not be defined, check if the class is defined - // in the default library before calling the method. - Dart_Handle plugin_registrant = - ::Dart_GetClass(library_handle, tonic::ToDart("_PluginRegistrant")); - - if (!Dart_IsError(plugin_registrant)) { - Dart_Handle result = - tonic::DartInvokeField(plugin_registrant, "register", {}); - if (tonic::LogIfError(result)) { - FML_LOG(ERROR) << "Could not invoke the Dart plugin registrant."; - } - } + InvokeDartPluginRegistrantIfAvailable(library_handle); if (!InvokeMainEntrypoint(user_entrypoint_function, entrypoint_args)) { return false; From 3d0765ec3585508e574cbb1e3700c57cf4e153cb Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 8 Apr 2021 16:50:12 -0700 Subject: [PATCH 08/16] Clean up --- runtime/dart_isolate.cc | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index baf952838aacb..8be193062a0c4 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -728,8 +728,9 @@ bool DartIsolate::MarkIsolateRunnable() { static void InvokeDartPluginRegistrantIfAvailable(Dart_Handle library_handle) { TRACE_EVENT0("flutter", "InvokeDartPluginRegistrantIfAvailable"); - // The Dart plugin registrant is a static with signature `void register()` - // within the class `_PluginRegistrant` generated by the Flutter tool. + // The Dart plugin registrant is a static method with signature `void + // register()` within the class `_PluginRegistrant` generated by the Flutter + // tool. // // This method binds a plugin implementation to their platform // interface based on the configuration of the app's pubpec.yaml, and the @@ -767,11 +768,13 @@ bool DartIsolate::RunFromLibrary(std::optional library_name, auto entrypoint_handle = entrypoint.has_value() && !entrypoint.value().empty() ? tonic::ToDart(entrypoint.value().c_str()) : tonic::ToDart("main"); - auto entrypoint_args = tonic::ToDart(args); + + InvokeDartPluginRegistrantIfAvailable(library_handle); + auto user_entrypoint_function = ::Dart_GetField(library_handle, entrypoint_handle); - InvokeDartPluginRegistrantIfAvailable(library_handle); + auto entrypoint_args = tonic::ToDart(args); if (!InvokeMainEntrypoint(user_entrypoint_function, entrypoint_args)) { return false; From 1b426a77d9687e9b0d94d6ed09959eb86cb52cc9 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 8 Apr 2021 16:54:29 -0700 Subject: [PATCH 09/16] Remove log --- runtime/dart_isolate.cc | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index 8be193062a0c4..5714100188e27 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -744,11 +744,7 @@ static void InvokeDartPluginRegistrantIfAvailable(Dart_Handle library_handle) { if (Dart_IsError(plugin_registrant)) { return; } - Dart_Handle result = - tonic::DartInvokeField(plugin_registrant, "register", {}); - if (tonic::LogIfError(result)) { - FML_LOG(ERROR) << "The Dart plugin registrant produced an error"; - } + tonic::LogIfError(tonic::DartInvokeField(plugin_registrant, "register", {})); } bool DartIsolate::RunFromLibrary(std::optional library_name, From 89a2c96657cb70f1d7fce794286ba0a3552079e1 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 8 Apr 2021 22:57:53 -0700 Subject: [PATCH 10/16] Explicitly test no dart plugin registrant --- BUILD.gn | 1 + lib/ui/painting/image_decoder_unittests.cc | 6 +- lib/ui/ui_benchmarks.cc | 2 +- runtime/BUILD.gn | 20 ++++++ runtime/dart_isolate_unittests.cc | 30 ++++----- .../no_dart_plugin_registrant_test.dart | 9 +++ .../no_dart_plugin_registrant_unittests.cc | 65 +++++++++++++++++++ runtime/type_conversions_unittests.cc | 2 +- testing/dart_isolate_runner.cc | 9 +-- testing/fixture_test.cc | 7 +- testing/fixture_test.h | 2 + testing/testing.cc | 5 ++ testing/testing.gni | 12 +++- testing/testing.h | 8 +++ .../dart_weak_persistent_handle_unittest.cc | 2 +- 15 files changed, 150 insertions(+), 30 deletions(-) create mode 100644 runtime/fixtures/no_dart_plugin_registrant_test.dart create mode 100644 runtime/no_dart_plugin_registrant_unittests.cc diff --git a/BUILD.gn b/BUILD.gn index dd14c6d29aa26..9449d7acb442a 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -103,6 +103,7 @@ group("flutter") { "//flutter/flow:flow_unittests", "//flutter/fml:fml_unittests", "//flutter/lib/ui:ui_unittests", + "//flutter/runtime:no_dart_plugin_registrant_unittests", "//flutter/runtime:runtime_unittests", "//flutter/shell/common:shell_unittests", "//flutter/shell/platform/embedder:embedder_proctable_unittests", diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index d9cdbfab51380..15fd1992e9dfd 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -637,9 +637,9 @@ TEST_F(ImageDecoderFixtureTest, }); latch.Wait(); - auto isolate = - RunDartCodeInIsolate(vm_ref, settings, runners, "main", {}, - GetFixturesPath(), io_manager->GetWeakIOManager()); + auto isolate = RunDartCodeInIsolate(vm_ref, settings, runners, "main", {}, + GetDefaultKernelFilePath(), + io_manager->GetWeakIOManager()); // Latch the IO task runner. runners.GetIOTaskRunner()->PostTask([&]() { io_latch.Wait(); }); diff --git a/lib/ui/ui_benchmarks.cc b/lib/ui/ui_benchmarks.cc index 739885710d7a2..e659b30b942a7 100644 --- a/lib/ui/ui_benchmarks.cc +++ b/lib/ui/ui_benchmarks.cc @@ -32,7 +32,7 @@ static void BM_PlatformMessageResponseDartComplete(benchmark::State& state) { auto vm_ref = DartVMRef::Create(settings); auto isolate = testing::RunDartCodeInIsolate(vm_ref, settings, task_runners, "main", {}, - testing::GetFixturesPath(), {}); + testing::GetDefaultKernelFilePath(), {}); while (state.KeepRunning()) { state.PauseTiming(); diff --git a/runtime/BUILD.gn b/runtime/BUILD.gn index 074c040a721db..1e5731ad22be0 100644 --- a/runtime/BUILD.gn +++ b/runtime/BUILD.gn @@ -136,4 +136,24 @@ if (enable_unittests) { "//third_party/skia", ] } + + test_fixtures("no_dart_plugin_registrant_test") { + dart_main = "fixtures/no_dart_plugin_registrant_test.dart" + dart_kernel = "$target_gen_dir/assets/no_plugin_registrant_kernel_blob.bin" + } + + executable("no_dart_plugin_registrant_unittests") { + testonly = true + + sources = [ "no_dart_plugin_registrant_unittests.cc" ] + + public_configs = [ "//flutter:export_dynamic_symbols" ] + + public_deps = [ + ":no_dart_plugin_registrant_test", + "//flutter/fml", + "//flutter/testing", + "//flutter/testing:fixture_test", + ] + } } diff --git a/runtime/dart_isolate_unittests.cc b/runtime/dart_isolate_unittests.cc index c2ed61f8e614c..0d35ebe983921 100644 --- a/runtime/dart_isolate_unittests.cc +++ b/runtime/dart_isolate_unittests.cc @@ -214,7 +214,7 @@ TEST_F(DartIsolateTest, IsolateCanLoadAndRunDartCode) { GetCurrentTaskRunner() // ); auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, "main", - {}, GetFixturesPath()); + {}, GetDefaultKernelFilePath()); ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); } @@ -231,7 +231,7 @@ TEST_F(DartIsolateTest, IsolateCannotLoadAndRunUnknownDartEntrypoint) { ); auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, "thisShouldNotExist", - {}, GetFixturesPath()); + {}, GetDefaultKernelFilePath()); ASSERT_FALSE(isolate); } @@ -246,7 +246,7 @@ TEST_F(DartIsolateTest, CanRunDartCodeCodeSynchronously) { GetCurrentTaskRunner() // ); auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, "main", - {}, GetFixturesPath()); + {}, GetDefaultKernelFilePath()); ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); @@ -275,9 +275,9 @@ TEST_F(DartIsolateTest, CanRegisterNativeCallback) { thread, // thread // ); - auto isolate = - RunDartCodeInIsolate(vm_ref, settings, task_runners, - "canRegisterNativeCallback", {}, GetFixturesPath()); + auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, + "canRegisterNativeCallback", {}, + GetDefaultKernelFilePath()); ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); Wait(); @@ -307,7 +307,7 @@ TEST_F(DartIsolateTest, CanSaveCompilationTrace) { ); auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, "testCanSaveCompilationTrace", {}, - GetFixturesPath()); + GetDefaultKernelFilePath()); ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); @@ -367,7 +367,7 @@ TEST_F(DartSecondaryIsolateTest, CanLaunchSecondaryIsolates) { ); auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, "testCanLaunchSecondaryIsolate", {}, - GetFixturesPath()); + GetDefaultKernelFilePath()); ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); ChildShutdownWait(); // wait for child isolate to shutdown first @@ -395,7 +395,7 @@ TEST_F(DartIsolateTest, CanRecieveArguments) { ); auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, "testCanRecieveArguments", {"arg1"}, - GetFixturesPath()); + GetDefaultKernelFilePath()); ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); @@ -477,7 +477,7 @@ TEST_F(DartIsolateTest, ); { auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, "main", - {}, GetFixturesPath()); + {}, GetDefaultKernelFilePath()); ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); } @@ -505,7 +505,7 @@ TEST_F(DartIsolateTest, ); { auto isolate = RunDartCodeInIsolate(vm_ref, instance_settings, task_runners, - "main", {}, GetFixturesPath()); + "main", {}, GetDefaultKernelFilePath()); ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); } @@ -596,9 +596,9 @@ TEST_F(DartIsolateTest, DISABLED_ValidLoadingUnitSucceeds) { thread, // thread // ); - auto isolate = - RunDartCodeInIsolate(vm_ref, settings, task_runners, - "canCallDeferredLibrary", {}, GetFixturesPath()); + auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, + "canCallDeferredLibrary", {}, + GetDefaultKernelFilePath()); ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); Wait(); @@ -639,7 +639,7 @@ TEST_F(DartIsolateTest, DartPluginRegistrantIsCalled) { ); auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, "mainForPluginRegistrantTest", {}, - GetFixturesPath()); + GetDefaultKernelFilePath()); ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); latch.Wait(); diff --git a/runtime/fixtures/no_dart_plugin_registrant_test.dart b/runtime/fixtures/no_dart_plugin_registrant_test.dart new file mode 100644 index 0000000000000..b4fb5c59eb0fc --- /dev/null +++ b/runtime/fixtures/no_dart_plugin_registrant_test.dart @@ -0,0 +1,9 @@ +// 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. + +void passMessage(String message) native 'PassMessage'; + +void main() { + passMessage('main() was called'); +} diff --git a/runtime/no_dart_plugin_registrant_unittests.cc b/runtime/no_dart_plugin_registrant_unittests.cc new file mode 100644 index 0000000000000..38a5b733bf930 --- /dev/null +++ b/runtime/no_dart_plugin_registrant_unittests.cc @@ -0,0 +1,65 @@ +// 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/runtime/dart_isolate.h" + +#include "flutter/fml/paths.h" +#include "flutter/testing/fixture_test.h" +#include "flutter/runtime/dart_vm.h" +#include "flutter/runtime/dart_vm_lifecycle.h" +#include "flutter/testing/dart_isolate_runner.h" +#include "flutter/testing/testing.h" + +namespace flutter { +namespace testing { + +const std::string kernel_file_name = "no_plugin_registrant_kernel_blob.bin"; + +class DartIsolateTest : public FixtureTest { + public: + DartIsolateTest() : FixtureTest(kernel_file_name) {} +}; + +TEST_F(DartIsolateTest, DartPluginRegistrantIsNotPresent) { + ASSERT_FALSE(DartVMRef::IsInstanceRunning()); + + std::vector messages; + fml::AutoResetWaitableEvent latch; + + AddNativeCallback( + "PassMessage", + CREATE_NATIVE_ENTRY(([&latch, &messages](Dart_NativeArguments args) { + auto message = tonic::DartConverter::FromDart( + Dart_GetNativeArgument(args, 0)); + messages.push_back(message); + latch.Signal(); + }))); + + const auto settings = CreateSettingsForFixture(); + auto vm_ref = DartVMRef::Create(settings); + auto thread = CreateNewThread(); + TaskRunners task_runners(GetCurrentTestName(), // + thread, // + thread, // + thread, // + thread // + ); + + + auto kernel_file_path = fml::paths::JoinPaths({GetFixturesPath(), kernel_file_name}); + auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, + "main", {}, + kernel_file_path); + + ASSERT_TRUE(isolate); + ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); + + latch.Wait(); + + ASSERT_EQ(messages.size(), 1u); + ASSERT_EQ(messages[0], "main() was called"); +} + +} // namespace testing +} // namespace flutter diff --git a/runtime/type_conversions_unittests.cc b/runtime/type_conversions_unittests.cc index f1ed831e174cc..ea4aed66d5d4d 100644 --- a/runtime/type_conversions_unittests.cc +++ b/runtime/type_conversions_unittests.cc @@ -28,7 +28,7 @@ class TypeConversionsTest : public FixtureTest { thread, thread, thread); auto isolate = RunDartCodeInIsolate(vm_, settings_, single_threaded_task_runner, - entrypoint, {}, GetFixturesPath()); + entrypoint, {}, GetDefaultKernelFilePath()); if (!isolate || isolate->get()->GetPhase() != DartIsolate::Phase::Running) { return false; } diff --git a/testing/dart_isolate_runner.cc b/testing/dart_isolate_runner.cc index 4ce157ee8ec43..2ab17585a8cb0 100644 --- a/testing/dart_isolate_runner.cc +++ b/testing/dart_isolate_runner.cc @@ -55,7 +55,7 @@ std::unique_ptr RunDartCodeInIsolateOnUITaskRunner( const TaskRunners& task_runners, std::string entrypoint, const std::vector& args, - const std::string& fixtures_path, + const std::string& kernel_file_path, fml::WeakPtr io_manager, std::shared_ptr volatile_path_tracker) { FML_CHECK(task_runners.GetUITaskRunner()->RunsTasksOnCurrentThread()); @@ -75,9 +75,6 @@ std::unique_ptr RunDartCodeInIsolateOnUITaskRunner( settings.dart_entrypoint_args = args; if (!DartVM::IsRunningPrecompiledCode()) { - auto kernel_file_path = - fml::paths::JoinPaths({fixtures_path, "kernel_blob.bin"}); - if (!fml::IsFile(kernel_file_path)) { FML_LOG(ERROR) << "Could not locate kernel file."; return nullptr; @@ -147,7 +144,7 @@ std::unique_ptr RunDartCodeInIsolate( const TaskRunners& task_runners, std::string entrypoint, const std::vector& args, - const std::string& fixtures_path, + const std::string& kernel_file_path, fml::WeakPtr io_manager, std::shared_ptr volatile_path_tracker) { std::unique_ptr result; @@ -155,7 +152,7 @@ std::unique_ptr RunDartCodeInIsolate( fml::TaskRunner::RunNowOrPostTask( task_runners.GetUITaskRunner(), fml::MakeCopyable([&]() mutable { result = RunDartCodeInIsolateOnUITaskRunner( - vm_ref, settings, task_runners, entrypoint, args, fixtures_path, + vm_ref, settings, task_runners, entrypoint, args, kernel_file_path, io_manager, std::move(volatile_path_tracker)); latch.Signal(); })); diff --git a/testing/fixture_test.cc b/testing/fixture_test.cc index 5e8f369b4ae3c..3386ae7940012 100644 --- a/testing/fixture_test.cc +++ b/testing/fixture_test.cc @@ -7,9 +7,12 @@ namespace flutter { namespace testing { -FixtureTest::FixtureTest() +FixtureTest::FixtureTest() : FixtureTest("kernel_blob.bin") {} + +FixtureTest::FixtureTest(std::string kernel_file) : native_resolver_(std::make_shared()), split_aot_symbols_(LoadELFSplitSymbolFromFixturesIfNeccessary()), + kernel_file_(kernel_file), assets_dir_(fml::OpenDirectory(GetFixturesPath(), false, fml::FilePermission::kRead)), @@ -44,7 +47,7 @@ void FixtureTest::SetSnapshotsAndAssets(Settings& settings) { settings.application_kernels = [this]() -> Mappings { std::vector> kernel_mappings; auto kernel_mapping = - fml::FileMapping::CreateReadOnly(assets_dir_, "kernel_blob.bin"); + fml::FileMapping::CreateReadOnly(assets_dir_, kernel_file_); if (!kernel_mapping || !kernel_mapping->IsValid()) { FML_LOG(ERROR) << "Could not find kernel blob for test fixture not " "running in precompiled mode."; diff --git a/testing/fixture_test.h b/testing/fixture_test.h index 82a82cb8e2f66..2ad701f93f3a9 100644 --- a/testing/fixture_test.h +++ b/testing/fixture_test.h @@ -20,6 +20,7 @@ namespace testing { class FixtureTest : public ThreadTest { public: FixtureTest(); + FixtureTest(std::string kernel_file); virtual Settings CreateSettingsForFixture(); @@ -33,6 +34,7 @@ class FixtureTest : public ThreadTest { ELFAOTSymbols split_aot_symbols_; private: + std::string kernel_file_; fml::UniqueFD assets_dir_; ELFAOTSymbols aot_symbols_; diff --git a/testing/testing.cc b/testing/testing.cc index 95852ed149e21..fb7b4fc06b0c6 100644 --- a/testing/testing.cc +++ b/testing/testing.cc @@ -5,6 +5,7 @@ #include "testing.h" #include "flutter/fml/file.h" +#include "flutter/fml/paths.h" namespace flutter { namespace testing { @@ -13,6 +14,10 @@ std::string GetCurrentTestName() { return ::testing::UnitTest::GetInstance()->current_test_info()->name(); } +std::string GetDefaultKernelFilePath() { + return fml::paths::JoinPaths({GetFixturesPath(), "kernel_blob.bin"}); +} + fml::UniqueFD OpenFixturesDirectory() { auto fixtures_directory = OpenDirectory(GetFixturesPath(), // path diff --git a/testing/testing.gni b/testing/testing.gni index 89cfc2932ad43..a9f35023ae145 100644 --- a/testing/testing.gni +++ b/testing/testing.gni @@ -156,7 +156,13 @@ template("dart_snapshot") { testonly = true dart_snapshot_kernel_target_name = "_dsk_$target_name" - dart_snapshot_kernel_path = "$target_gen_dir/assets/kernel_blob.bin" + + if (defined(invoker.dart_kernel)) { + dart_snapshot_kernel_path = invoker.dart_kernel + } else { + dart_snapshot_kernel_path = "$target_gen_dir/assets/kernel_blob.bin" + } + dart_snapshot_kernel(dart_snapshot_kernel_target_name) { dart_main = invoker.dart_main dart_kernel = dart_snapshot_kernel_path @@ -257,7 +263,11 @@ template("test_fixtures") { dart_snapshot_target_name = "_ds_$target_name" dart_snapshot(dart_snapshot_target_name) { dart_main = invoker.dart_main + if (defined(invoker.dart_kernel)) { + dart_kernel = invoker.dart_kernel + } } + test_public_deps += [ ":$dart_snapshot_target_name" ] } diff --git a/testing/testing.h b/testing/testing.h index 79d407e2faaf5..99a8e1bbe7e0d 100644 --- a/testing/testing.h +++ b/testing/testing.h @@ -27,6 +27,14 @@ namespace testing { /// const char* GetFixturesPath(); +//------------------------------------------------------------------------------ +/// @brief Returns the default path to kernel_blob.bin. This file is within +/// the directory returned by `GetFixturesPath()`. +/// +/// @return The kernel file path. +/// +std::string GetDefaultKernelFilePath(); + //------------------------------------------------------------------------------ /// @brief Opens the fixtures directory for the unit-test harness. /// diff --git a/third_party/tonic/tests/dart_weak_persistent_handle_unittest.cc b/third_party/tonic/tests/dart_weak_persistent_handle_unittest.cc index 65d5e116cd230..604d21b0bd7af 100644 --- a/third_party/tonic/tests/dart_weak_persistent_handle_unittest.cc +++ b/third_party/tonic/tests/dart_weak_persistent_handle_unittest.cc @@ -25,7 +25,7 @@ class DartWeakPersistentHandle : public FixtureTest { thread, thread, thread); auto isolate = RunDartCodeInIsolate(vm_, settings_, single_threaded_task_runner, - entrypoint, {}, GetFixturesPath()); + entrypoint, {}, GetDefaultKernelFilePath()); if (!isolate || isolate->get()->GetPhase() != DartIsolate::Phase::Running) { return false; } From 6ed1ed8de8d447512fd10403fb978a71b5dbb798 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 8 Apr 2021 23:11:12 -0700 Subject: [PATCH 11/16] Ensure no exception --- .../no_dart_plugin_registrant_unittests.cc | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/runtime/no_dart_plugin_registrant_unittests.cc b/runtime/no_dart_plugin_registrant_unittests.cc index 38a5b733bf930..287c276598929 100644 --- a/runtime/no_dart_plugin_registrant_unittests.cc +++ b/runtime/no_dart_plugin_registrant_unittests.cc @@ -5,10 +5,10 @@ #include "flutter/runtime/dart_isolate.h" #include "flutter/fml/paths.h" -#include "flutter/testing/fixture_test.h" #include "flutter/runtime/dart_vm.h" #include "flutter/runtime/dart_vm_lifecycle.h" #include "flutter/testing/dart_isolate_runner.h" +#include "flutter/testing/fixture_test.h" #include "flutter/testing/testing.h" namespace flutter { @@ -17,8 +17,8 @@ namespace testing { const std::string kernel_file_name = "no_plugin_registrant_kernel_blob.bin"; class DartIsolateTest : public FixtureTest { - public: - DartIsolateTest() : FixtureTest(kernel_file_name) {} + public: + DartIsolateTest() : FixtureTest(kernel_file_name) {} }; TEST_F(DartIsolateTest, DartPluginRegistrantIsNotPresent) { @@ -36,7 +36,14 @@ TEST_F(DartIsolateTest, DartPluginRegistrantIsNotPresent) { latch.Signal(); }))); - const auto settings = CreateSettingsForFixture(); + auto settings = CreateSettingsForFixture(); + auto did_throw_exception = false; + settings.unhandled_exception_callback = [&](const std::string& error, + const std::string& stack_trace) { + did_throw_exception = true; + return true; + }; + auto vm_ref = DartVMRef::Create(settings); auto thread = CreateNewThread(); TaskRunners task_runners(GetCurrentTestName(), // @@ -46,11 +53,10 @@ TEST_F(DartIsolateTest, DartPluginRegistrantIsNotPresent) { thread // ); - - auto kernel_file_path = fml::paths::JoinPaths({GetFixturesPath(), kernel_file_name}); - auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, - "main", {}, - kernel_file_path); + auto kernel_file_path = + fml::paths::JoinPaths({GetFixturesPath(), kernel_file_name}); + auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, "main", + {}, kernel_file_path); ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); @@ -59,6 +65,7 @@ TEST_F(DartIsolateTest, DartPluginRegistrantIsNotPresent) { ASSERT_EQ(messages.size(), 1u); ASSERT_EQ(messages[0], "main() was called"); + ASSERT_FALSE(did_throw_exception); } } // namespace testing From 22ceaff6c7016ea62e4b2469e3ab5ce6d68e497a Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 9 Apr 2021 12:57:04 -0700 Subject: [PATCH 12/16] Add flag use_target_as_artifact_prefix --- ci/licenses_golden/licenses_flutter | 2 ++ runtime/BUILD.gn | 6 ++-- .../no_dart_plugin_registrant_unittests.cc | 7 +++-- runtime/type_conversions_unittests.cc | 4 ++- shell/common/shell_benchmarks.cc | 3 +- .../embedder/tests/embedder_test_context.cc | 7 +++-- .../embedder/tests/embedder_unittests.cc | 2 +- testing/elf_loader.cc | 9 +++--- testing/elf_loader.h | 14 ++++++--- testing/fixture_test.cc | 18 ++++++++---- testing/fixture_test.h | 10 +++++-- testing/testing.gni | 29 ++++++++++++++----- .../dart_weak_persistent_handle_unittest.cc | 4 ++- 13 files changed, 78 insertions(+), 37 deletions(-) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 8045bc9e5d84e..093f8c362cfd8 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -609,10 +609,12 @@ FILE: ../../../flutter/runtime/dart_vm_lifecycle.h FILE: ../../../flutter/runtime/dart_vm_unittests.cc FILE: ../../../flutter/runtime/embedder_resources.cc FILE: ../../../flutter/runtime/embedder_resources.h +FILE: ../../../flutter/runtime/fixtures/no_dart_plugin_registrant_test.dart FILE: ../../../flutter/runtime/fixtures/runtime_test.dart FILE: ../../../flutter/runtime/fixtures/split_lib_test.dart FILE: ../../../flutter/runtime/isolate_configuration.cc FILE: ../../../flutter/runtime/isolate_configuration.h +FILE: ../../../flutter/runtime/no_dart_plugin_registrant_unittests.cc FILE: ../../../flutter/runtime/platform_data.cc FILE: ../../../flutter/runtime/platform_data.h FILE: ../../../flutter/runtime/ptrace_check.cc diff --git a/runtime/BUILD.gn b/runtime/BUILD.gn index 1e5731ad22be0..e2e9478e23ba7 100644 --- a/runtime/BUILD.gn +++ b/runtime/BUILD.gn @@ -137,9 +137,9 @@ if (enable_unittests) { ] } - test_fixtures("no_dart_plugin_registrant_test") { + test_fixtures("no_plugin_registrant") { dart_main = "fixtures/no_dart_plugin_registrant_test.dart" - dart_kernel = "$target_gen_dir/assets/no_plugin_registrant_kernel_blob.bin" + use_target_as_artifact_prefix = true } executable("no_dart_plugin_registrant_unittests") { @@ -150,7 +150,7 @@ if (enable_unittests) { public_configs = [ "//flutter:export_dynamic_symbols" ] public_deps = [ - ":no_dart_plugin_registrant_test", + ":no_plugin_registrant", "//flutter/fml", "//flutter/testing", "//flutter/testing:fixture_test", diff --git a/runtime/no_dart_plugin_registrant_unittests.cc b/runtime/no_dart_plugin_registrant_unittests.cc index 287c276598929..7869830a1b7d0 100644 --- a/runtime/no_dart_plugin_registrant_unittests.cc +++ b/runtime/no_dart_plugin_registrant_unittests.cc @@ -15,10 +15,11 @@ namespace flutter { namespace testing { const std::string kernel_file_name = "no_plugin_registrant_kernel_blob.bin"; +const std::string elf_file_name = "no_plugin_registrant_app_elf_snapshot.so"; class DartIsolateTest : public FixtureTest { public: - DartIsolateTest() : FixtureTest(kernel_file_name) {} + DartIsolateTest() : FixtureTest(kernel_file_name, elf_file_name, "") {} }; TEST_F(DartIsolateTest, DartPluginRegistrantIsNotPresent) { @@ -53,10 +54,10 @@ TEST_F(DartIsolateTest, DartPluginRegistrantIsNotPresent) { thread // ); - auto kernel_file_path = + auto kernel_path = fml::paths::JoinPaths({GetFixturesPath(), kernel_file_name}); auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, "main", - {}, kernel_file_path); + {}, kernel_path); ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); diff --git a/runtime/type_conversions_unittests.cc b/runtime/type_conversions_unittests.cc index ea4aed66d5d4d..d69d60b10caf3 100644 --- a/runtime/type_conversions_unittests.cc +++ b/runtime/type_conversions_unittests.cc @@ -26,9 +26,11 @@ class TypeConversionsTest : public FixtureTest { auto thread = CreateNewThread(); TaskRunners single_threaded_task_runner(GetCurrentTestName(), thread, thread, thread, thread); + auto kernel_path = fml::paths::JoinPaths( + {GetFixturesPath(), "runtime_fixtures_kernel_blob.bin"}); auto isolate = RunDartCodeInIsolate(vm_, settings_, single_threaded_task_runner, - entrypoint, {}, GetDefaultKernelFilePath()); + entrypoint, {}, kernel_path); if (!isolate || isolate->get()->GetPhase() != DartIsolate::Phase::Running) { return false; } diff --git a/shell/common/shell_benchmarks.cc b/shell/common/shell_benchmarks.cc index 21cf71147f575..cf73ab352fcf7 100644 --- a/shell/common/shell_benchmarks.cc +++ b/shell/common/shell_benchmarks.cc @@ -29,7 +29,8 @@ static void StartupAndShutdownShell(benchmark::State& state, settings.task_observer_remove = [](intptr_t) {}; if (DartVM::IsRunningPrecompiledCode()) { - aot_symbols = testing::LoadELFSymbolFromFixturesIfNeccessary(); + aot_symbols = testing::LoadELFSymbolFromFixturesIfNeccessary( + testing::kDefaultAOTAppELFFileName); FML_CHECK( testing::PrepareSettingsForAOTWithSymbols(settings, aot_symbols)) << "Could not set up settings with AOT symbols."; diff --git a/shell/platform/embedder/tests/embedder_test_context.cc b/shell/platform/embedder/tests/embedder_test_context.cc index 71503ae4ef31a..78a1c1b50e0d6 100644 --- a/shell/platform/embedder/tests/embedder_test_context.cc +++ b/shell/platform/embedder/tests/embedder_test_context.cc @@ -17,7 +17,8 @@ namespace testing { EmbedderTestContext::EmbedderTestContext(std::string assets_path) : assets_path_(std::move(assets_path)), - aot_symbols_(LoadELFSymbolFromFixturesIfNeccessary()), + aot_symbols_( + LoadELFSymbolFromFixturesIfNeccessary(kDefaultAOTAppELFFileName)), native_resolver_(std::make_shared()) { SetupAOTMappingsIfNecessary(); SetupAOTDataIfNecessary(); @@ -53,8 +54,8 @@ void EmbedderTestContext::SetupAOTDataIfNecessary() { FlutterEngineAOTDataSource data_in = {}; FlutterEngineAOTData data_out = nullptr; - const auto elf_path = - fml::paths::JoinPaths({GetFixturesPath(), kAOTAppELFFileName}); + const auto elf_path = fml::paths::JoinPaths( + {GetFixturesPath(), testing::kDefaultAOTAppELFFileName}); data_in.type = kFlutterEngineAOTDataSourceTypeElfPath; data_in.elf_path = elf_path.c_str(); diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index 6175daa820dce..05f208026dcbd 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -1154,7 +1154,7 @@ TEST_F(EmbedderTest, CanCreateAndCollectAValidElfSource) { ASSERT_EQ(FlutterEngineCollectAOTData(data_out), kSuccess); const auto elf_path = - fml::paths::JoinPaths({GetFixturesPath(), kAOTAppELFFileName}); + fml::paths::JoinPaths({GetFixturesPath(), kDefaultAOTAppELFFileName}); data_in.type = kFlutterEngineAOTDataSourceTypeElfPath; data_in.elf_path = elf_path.c_str(); diff --git a/testing/elf_loader.cc b/testing/elf_loader.cc index e4fbd48d39f62..50b6e43960bec 100644 --- a/testing/elf_loader.cc +++ b/testing/elf_loader.cc @@ -12,13 +12,13 @@ namespace flutter { namespace testing { -ELFAOTSymbols LoadELFSymbolFromFixturesIfNeccessary() { +ELFAOTSymbols LoadELFSymbolFromFixturesIfNeccessary(std::string elf_filename) { if (!DartVM::IsRunningPrecompiledCode()) { return {}; } const auto elf_path = - fml::paths::JoinPaths({GetFixturesPath(), kAOTAppELFFileName}); + fml::paths::JoinPaths({GetFixturesPath(), elf_filename}); if (!fml::IsFile(elf_path)) { FML_LOG(ERROR) << "App AOT file does not exist for this fixture. Attempts " @@ -60,13 +60,14 @@ ELFAOTSymbols LoadELFSymbolFromFixturesIfNeccessary() { return symbols; } -ELFAOTSymbols LoadELFSplitSymbolFromFixturesIfNeccessary() { +ELFAOTSymbols LoadELFSplitSymbolFromFixturesIfNeccessary( + std::string elf_split_filename) { if (!DartVM::IsRunningPrecompiledCode()) { return {}; } const auto elf_path = - fml::paths::JoinPaths({GetFixturesPath(), kAOTAppELFSplitFileName}); + fml::paths::JoinPaths({GetFixturesPath(), elf_split_filename}); if (!fml::IsFile(elf_path)) { // We do not log here, as there is no expectation for a split library to diff --git a/testing/elf_loader.h b/testing/elf_loader.h index d5861f04dee43..fb9ff47b08182 100644 --- a/testing/elf_loader.h +++ b/testing/elf_loader.h @@ -14,14 +14,14 @@ namespace flutter { namespace testing { -inline constexpr const char* kAOTAppELFFileName = "app_elf_snapshot.so"; +inline constexpr const char* kDefaultAOTAppELFFileName = "app_elf_snapshot.so"; // This file name is what gen_snapshot defaults to. It is based off of the // name of the base file, with the `2` indicating that this split corresponds // to loading unit id of 2. The base module id is 1 and is omitted as it is not // considered a split. If dart changes the naming convention, this should be // changed to match, however, this is considered unlikely to happen. -inline constexpr const char* kAOTAppELFSplitFileName = +inline constexpr const char* kDefaultAOTAppELFSplitFileName = "app_elf_snapshot.so-2.part.so"; struct LoadedELFDeleter { @@ -44,9 +44,11 @@ struct ELFAOTSymbols { /// generator. This only returns valid symbols when the VM is /// configured for AOT. /// +/// @param[in] elf_filename The AOT ELF filename from the fixtures generator. +/// /// @return The loaded ELF symbols. /// -ELFAOTSymbols LoadELFSymbolFromFixturesIfNeccessary(); +ELFAOTSymbols LoadELFSymbolFromFixturesIfNeccessary(std::string elf_filename); //------------------------------------------------------------------------------ /// @brief Attempts to resolve split loading unit AOT symbols from the @@ -55,9 +57,13 @@ ELFAOTSymbols LoadELFSymbolFromFixturesIfNeccessary(); /// This only returns valid symbols when the VM is configured for /// AOT. /// +/// @param[in] elf_split_filename The split AOT ELF filename from the fixtures +/// generator. +/// /// @return The loaded ELF symbols. /// -ELFAOTSymbols LoadELFSplitSymbolFromFixturesIfNeccessary(); +ELFAOTSymbols LoadELFSplitSymbolFromFixturesIfNeccessary( + std::string elf_split_filename); //------------------------------------------------------------------------------ /// @brief Prepare the settings objects various AOT mappings resolvers with diff --git a/testing/fixture_test.cc b/testing/fixture_test.cc index 3386ae7940012..490d70e3a4ad4 100644 --- a/testing/fixture_test.cc +++ b/testing/fixture_test.cc @@ -7,16 +7,22 @@ namespace flutter { namespace testing { -FixtureTest::FixtureTest() : FixtureTest("kernel_blob.bin") {} +FixtureTest::FixtureTest() + : FixtureTest("kernel_blob.bin", + kDefaultAOTAppELFFileName, + kDefaultAOTAppELFSplitFileName) {} -FixtureTest::FixtureTest(std::string kernel_file) +FixtureTest::FixtureTest(std::string kernel_filename, + std::string elf_filename, + std::string elf_split_filename) : native_resolver_(std::make_shared()), - split_aot_symbols_(LoadELFSplitSymbolFromFixturesIfNeccessary()), - kernel_file_(kernel_file), + split_aot_symbols_( + LoadELFSplitSymbolFromFixturesIfNeccessary(elf_split_filename)), + kernel_filename_(kernel_filename), assets_dir_(fml::OpenDirectory(GetFixturesPath(), false, fml::FilePermission::kRead)), - aot_symbols_(LoadELFSymbolFromFixturesIfNeccessary()) {} + aot_symbols_(LoadELFSymbolFromFixturesIfNeccessary(elf_filename)) {} Settings FixtureTest::CreateSettingsForFixture() { Settings settings; @@ -47,7 +53,7 @@ void FixtureTest::SetSnapshotsAndAssets(Settings& settings) { settings.application_kernels = [this]() -> Mappings { std::vector> kernel_mappings; auto kernel_mapping = - fml::FileMapping::CreateReadOnly(assets_dir_, kernel_file_); + fml::FileMapping::CreateReadOnly(assets_dir_, kernel_filename_); if (!kernel_mapping || !kernel_mapping->IsValid()) { FML_LOG(ERROR) << "Could not find kernel blob for test fixture not " "running in precompiled mode."; diff --git a/testing/fixture_test.h b/testing/fixture_test.h index 2ad701f93f3a9..49442ecf8bd7f 100644 --- a/testing/fixture_test.h +++ b/testing/fixture_test.h @@ -19,8 +19,13 @@ namespace testing { class FixtureTest : public ThreadTest { public: + // Uses kernel_blob.bin as the kernel file name. FixtureTest(); - FixtureTest(std::string kernel_file); + + // Allows to customize the kernel, ELF and split ELF filenames. + explicit FixtureTest(std::string kernel_filename, + std::string elf_filename, + std::string elf_split_filename); virtual Settings CreateSettingsForFixture(); @@ -34,7 +39,8 @@ class FixtureTest : public ThreadTest { ELFAOTSymbols split_aot_symbols_; private: - std::string kernel_file_; + std::string kernel_filename_; + std::string elf_filename_; fml::UniqueFD assets_dir_; ELFAOTSymbols aot_symbols_; diff --git a/testing/testing.gni b/testing/testing.gni index a9f35023ae145..ecac446b02915 100644 --- a/testing/testing.gni +++ b/testing/testing.gni @@ -116,6 +116,9 @@ template("dart_snapshot_aot") { assert(defined(invoker.dart_kernel), "The Dart Kernel file location must be specified") + assert(defined(invoker.dart_elf_filename), + "The main Dart ELF filename must be specified.") + compiled_action(target_name) { testonly = true @@ -124,7 +127,7 @@ template("dart_snapshot_aot") { inputs = [ invoker.dart_kernel ] # Custom ELF loader is used for Mac and Windows. - elf_object = "$target_gen_dir/assets/app_elf_snapshot.so" + elf_object = "$target_gen_dir/assets/${invoker.dart_elf_filename}" loading_unit_manifest = "$target_gen_dir/assets/loading_unit_manifest.json" @@ -152,16 +155,15 @@ template("dart_snapshot_aot") { # dart_main (required): The path to the main Dart file. template("dart_snapshot") { assert(defined(invoker.dart_main), "The main Dart file must be specified.") + assert(defined(invoker.dart_kernel_filename), + "The main Dart kernel filename must be specified.") testonly = true dart_snapshot_kernel_target_name = "_dsk_$target_name" - if (defined(invoker.dart_kernel)) { - dart_snapshot_kernel_path = invoker.dart_kernel - } else { - dart_snapshot_kernel_path = "$target_gen_dir/assets/kernel_blob.bin" - } + dart_snapshot_kernel_path = + "$target_gen_dir/assets/${invoker.dart_kernel_filename}" dart_snapshot_kernel(dart_snapshot_kernel_target_name) { dart_main = invoker.dart_main @@ -172,9 +174,13 @@ template("dart_snapshot") { snapshot_public_deps = [ ":$dart_snapshot_kernel_target_name" ] if (is_aot_test) { + assert(defined(invoker.dart_elf_filename), + "The main Dart ELF filename must be specified.") + dart_snapshot_aot_target_name = "_dsa_$target_name" dart_snapshot_aot(dart_snapshot_aot_target_name) { dart_kernel = dart_snapshot_kernel_path + dart_elf_filename = invoker.dart_elf_filename deps = [ ":$dart_snapshot_kernel_target_name" ] } snapshot_deps += [ ":$dart_snapshot_aot_target_name" ] @@ -260,11 +266,18 @@ template("test_fixtures") { # If a Dart file is specified, snapshot it and place it in the generated # assets directory. if (defined(invoker.dart_main)) { + if (defined(invoker.use_target_as_artifact_prefix) && + invoker.use_target_as_artifact_prefix) { + artifact_prefix = "${target_name}_" + } else { + artifact_prefix = "" + } dart_snapshot_target_name = "_ds_$target_name" dart_snapshot(dart_snapshot_target_name) { dart_main = invoker.dart_main - if (defined(invoker.dart_kernel)) { - dart_kernel = invoker.dart_kernel + dart_kernel_filename = "${artifact_prefix}kernel_blob.bin" + if (is_aot_test) { + dart_elf_filename = "${artifact_prefix}app_elf_snapshot.so" } } diff --git a/third_party/tonic/tests/dart_weak_persistent_handle_unittest.cc b/third_party/tonic/tests/dart_weak_persistent_handle_unittest.cc index 604d21b0bd7af..5ed65c3b83b34 100644 --- a/third_party/tonic/tests/dart_weak_persistent_handle_unittest.cc +++ b/third_party/tonic/tests/dart_weak_persistent_handle_unittest.cc @@ -23,9 +23,11 @@ class DartWeakPersistentHandle : public FixtureTest { auto thread = CreateNewThread(); TaskRunners single_threaded_task_runner(GetCurrentTestName(), thread, thread, thread, thread); + auto kernel_file = fml::paths::JoinPaths( + {GetFixturesPath(), "tonic_fixtures_kernel_blob.bin"}); auto isolate = RunDartCodeInIsolate(vm_, settings_, single_threaded_task_runner, - entrypoint, {}, GetDefaultKernelFilePath()); + entrypoint, {}, kernel_file); if (!isolate || isolate->get()->GetPhase() != DartIsolate::Phase::Running) { return false; } From 63194ab9398ea784644bb5bdfbd6727e543f0bf4 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 9 Apr 2021 13:03:29 -0700 Subject: [PATCH 13/16] Fix comment --- testing/fixture_test.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/fixture_test.h b/testing/fixture_test.h index 49442ecf8bd7f..f5e3934762f38 100644 --- a/testing/fixture_test.h +++ b/testing/fixture_test.h @@ -19,7 +19,7 @@ namespace testing { class FixtureTest : public ThreadTest { public: - // Uses kernel_blob.bin as the kernel file name. + // Uses the default filenames from the fixtures generator. FixtureTest(); // Allows to customize the kernel, ELF and split ELF filenames. From b82b1029a9ad758e7908398d0c774880e7d1abe9 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 9 Apr 2021 14:03:12 -0700 Subject: [PATCH 14/16] Fix test --- runtime/type_conversions_unittests.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/runtime/type_conversions_unittests.cc b/runtime/type_conversions_unittests.cc index d69d60b10caf3..ea4aed66d5d4d 100644 --- a/runtime/type_conversions_unittests.cc +++ b/runtime/type_conversions_unittests.cc @@ -26,11 +26,9 @@ class TypeConversionsTest : public FixtureTest { auto thread = CreateNewThread(); TaskRunners single_threaded_task_runner(GetCurrentTestName(), thread, thread, thread, thread); - auto kernel_path = fml::paths::JoinPaths( - {GetFixturesPath(), "runtime_fixtures_kernel_blob.bin"}); auto isolate = RunDartCodeInIsolate(vm_, settings_, single_threaded_task_runner, - entrypoint, {}, kernel_path); + entrypoint, {}, GetDefaultKernelFilePath()); if (!isolate || isolate->get()->GetPhase() != DartIsolate::Phase::Running) { return false; } From 3b8721502996124c348301d98e2df332328b18fb Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 9 Apr 2021 14:44:36 -0700 Subject: [PATCH 15/16] Comments --- testing/testing.gni | 8 ++++++++ .../tonic/tests/dart_weak_persistent_handle_unittest.cc | 4 +--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/testing/testing.gni b/testing/testing.gni index ecac446b02915..acf2cf20fe5e6 100644 --- a/testing/testing.gni +++ b/testing/testing.gni @@ -110,6 +110,8 @@ template("dart_snapshot_kernel") { # Arguments: # # dart_kernel (required): The path to the kernel snapshot. +# +# dart_elf_filename (required): The filename of the AOT ELF snapshot. template("dart_snapshot_aot") { testonly = true @@ -153,6 +155,10 @@ template("dart_snapshot_aot") { # Arguments: # # dart_main (required): The path to the main Dart file. +# +# dart_kernel_filename (required): The filename of the kernel blob. +# +# dart_elf_filename (required): The filename of the AOT ELF snapshot only if is_aot_test is true. template("dart_snapshot") { assert(defined(invoker.dart_main), "The main Dart file must be specified.") assert(defined(invoker.dart_kernel_filename), @@ -240,6 +246,8 @@ template("copy_fixtures") { # # dart_main (optional): The path to the main Dart file. If specified, it is # snapshotted. +# +# use_target_as_artifact_prefix(optional): If true, adds the target name as prefix of the kernel and AOT ELF snapshot filename. template("test_fixtures") { # Even if no fixtures are present, the location of the fixtures directory # must always be known to tests. diff --git a/third_party/tonic/tests/dart_weak_persistent_handle_unittest.cc b/third_party/tonic/tests/dart_weak_persistent_handle_unittest.cc index 5ed65c3b83b34..604d21b0bd7af 100644 --- a/third_party/tonic/tests/dart_weak_persistent_handle_unittest.cc +++ b/third_party/tonic/tests/dart_weak_persistent_handle_unittest.cc @@ -23,11 +23,9 @@ class DartWeakPersistentHandle : public FixtureTest { auto thread = CreateNewThread(); TaskRunners single_threaded_task_runner(GetCurrentTestName(), thread, thread, thread, thread); - auto kernel_file = fml::paths::JoinPaths( - {GetFixturesPath(), "tonic_fixtures_kernel_blob.bin"}); auto isolate = RunDartCodeInIsolate(vm_, settings_, single_threaded_task_runner, - entrypoint, {}, kernel_file); + entrypoint, {}, GetDefaultKernelFilePath()); if (!isolate || isolate->get()->GetPhase() != DartIsolate::Phase::Running) { return false; } From 2d988d3928dff21eed54eaf9bf509019a9ed769e Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 9 Apr 2021 16:19:37 -0700 Subject: [PATCH 16/16] comments --- testing/fixture_test.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/testing/fixture_test.h b/testing/fixture_test.h index f5e3934762f38..4b95a8b8289a3 100644 --- a/testing/fixture_test.h +++ b/testing/fixture_test.h @@ -23,9 +23,9 @@ class FixtureTest : public ThreadTest { FixtureTest(); // Allows to customize the kernel, ELF and split ELF filenames. - explicit FixtureTest(std::string kernel_filename, - std::string elf_filename, - std::string elf_split_filename); + FixtureTest(std::string kernel_filename, + std::string elf_filename, + std::string elf_split_filename); virtual Settings CreateSettingsForFixture();