From 9d281b0a8a661cbd36faf9779640e11a88af379b Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 11 Feb 2022 16:01:23 -0800 Subject: [PATCH 1/4] Started looking for the dart plugin registrant at runtime since it no longer is compiled in the root library. --- runtime/dart_isolate.cc | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index 85c8f05f5d022..a9f8e54d696a4 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -709,6 +709,34 @@ static void InvokeDartPluginRegistrantIfAvailable(Dart_Handle library_handle) { tonic::LogIfError(tonic::DartInvokeField(plugin_registrant, "register", {})); } +namespace { +bool EndsWith(const std::string& str, const std::string& ending) { + if (str.size() >= ending.size()) { + return (0 == + str.compare(str.size() - ending.size(), ending.size(), ending)); + } else { + return false; + } +} + +Dart_Handle FindDartPluginRegistrantLibrary() { + // TODO(): Instead of finding this, it could be passed down from the tool. + Dart_Handle libraries = Dart_GetLoadedLibraries(); + intptr_t length = 0; + Dart_ListLength(libraries, &length); + for (intptr_t i = 0; i < length; ++i) { + Dart_Handle library = Dart_ListGetAt(libraries, i); + std::string library_name = + tonic::DartConverter::FromDart(Dart_ToString(library)); + if (EndsWith(library_name, + ".dart_tool/flutter_build/generated_main.dart'")) { + return library; + } + } + return Dart_Null(); +} +} // namespace + bool DartIsolate::RunFromLibrary(std::optional library_name, std::optional entrypoint, const std::vector& args) { @@ -727,7 +755,12 @@ bool DartIsolate::RunFromLibrary(std::optional library_name, ? tonic::ToDart(entrypoint.value().c_str()) : tonic::ToDart("main"); - InvokeDartPluginRegistrantIfAvailable(library_handle); + auto dart_plugin_registrant_library = FindDartPluginRegistrantLibrary(); + if (!Dart_IsNull(dart_plugin_registrant_library)) { + InvokeDartPluginRegistrantIfAvailable(dart_plugin_registrant_library); + } else { + InvokeDartPluginRegistrantIfAvailable(library_handle); + } auto user_entrypoint_function = ::Dart_GetField(library_handle, entrypoint_handle); From 81be313faea2237bbce4e4e70d333b0eed0f58f2 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 14 Feb 2022 09:25:53 -0800 Subject: [PATCH 2/4] renamed to dart_plugin_registrant --- 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 a9f8e54d696a4..46f969a4ff8f6 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -729,7 +729,7 @@ Dart_Handle FindDartPluginRegistrantLibrary() { std::string library_name = tonic::DartConverter::FromDart(Dart_ToString(library)); if (EndsWith(library_name, - ".dart_tool/flutter_build/generated_main.dart'")) { + ".dart_tool/flutter_build/dart_plugin_registrant.dart'")) { return library; } } From 13e9028ec82ee672e69b4c01e8867ba026554d4a Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 28 Feb 2022 10:45:34 -0800 Subject: [PATCH 3/4] added engine unit test --- BUILD.gn | 1 + ci/licenses_golden/licenses_flutter | 2 + runtime/BUILD.gn | 20 +++++ runtime/dart_isolate.cc | 2 +- runtime/dart_plugin_registrant_unittests.cc | 78 +++++++++++++++++++ .../flutter_build/dart_plugin_registrant.dart | 33 ++++++++ 6 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 runtime/dart_plugin_registrant_unittests.cc create mode 100644 runtime/fixtures/dart_tool/flutter_build/dart_plugin_registrant.dart diff --git a/BUILD.gn b/BUILD.gn index 69026888a1460..679559def70a5 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -145,6 +145,7 @@ group("flutter") { "//flutter/lib/spirv/test/supported_glsl_op_shaders:spirv_compile_supported_glsl_shaders", "//flutter/lib/spirv/test/supported_op_shaders:spirv_compile_supported_op_shaders", "//flutter/lib/ui:ui_unittests", + "//flutter/runtime:dart_plugin_registrant_unittests", "//flutter/runtime:no_dart_plugin_registrant_unittests", "//flutter/runtime:runtime_unittests", "//flutter/shell/common:shell_unittests", diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index e7c416d972c44..4e5c531f21f2e 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -978,11 +978,13 @@ 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/dart_tool/flutter_build/dart_plugin_registrant.dart 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/dart_plugin_registrant_unittests.cc FILE: ../../../flutter/runtime/no_dart_plugin_registrant_unittests.cc FILE: ../../../flutter/runtime/platform_data.cc FILE: ../../../flutter/runtime/platform_data.h diff --git a/runtime/BUILD.gn b/runtime/BUILD.gn index 47dcc7ff3633d..9e082cf308077 100644 --- a/runtime/BUILD.gn +++ b/runtime/BUILD.gn @@ -158,4 +158,24 @@ if (enable_unittests) { "//flutter/testing:fixture_test", ] } + + test_fixtures("plugin_registrant") { + dart_main = "fixtures/dart_tool/flutter_build/dart_plugin_registrant.dart" + use_target_as_artifact_prefix = true + } + + executable("dart_plugin_registrant_unittests") { + testonly = true + + sources = [ "dart_plugin_registrant_unittests.cc" ] + + public_configs = [ "//flutter:export_dynamic_symbols" ] + + public_deps = [ + ":plugin_registrant", + "//flutter/fml", + "//flutter/testing", + "//flutter/testing:fixture_test", + ] + } } diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index 46f969a4ff8f6..3aab39c867712 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -729,7 +729,7 @@ Dart_Handle FindDartPluginRegistrantLibrary() { std::string library_name = tonic::DartConverter::FromDart(Dart_ToString(library)); if (EndsWith(library_name, - ".dart_tool/flutter_build/dart_plugin_registrant.dart'")) { + "dart_tool/flutter_build/dart_plugin_registrant.dart'")) { return library; } } diff --git a/runtime/dart_plugin_registrant_unittests.cc b/runtime/dart_plugin_registrant_unittests.cc new file mode 100644 index 0000000000000..2452b1e58e0cd --- /dev/null +++ b/runtime/dart_plugin_registrant_unittests.cc @@ -0,0 +1,78 @@ +// 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/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" + +// CREATE_NATIVE_ENTRY is leaky by design +// NOLINTBEGIN(clang-analyzer-core.StackAddressEscape) + +namespace flutter { +namespace testing { + +const std::string kernel_file_name = "plugin_registrant_kernel_blob.bin"; +const std::string elf_file_name = "plugin_registrant_app_elf_snapshot.so"; + +class DartIsolateTest : public FixtureTest { + public: + DartIsolateTest() : FixtureTest(kernel_file_name, elf_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(); + }))); + + 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(), // + thread, // + thread, // + thread, // + thread // + ); + + auto kernel_path = + fml::paths::JoinPaths({GetFixturesPath(), kernel_file_name}); + auto isolate = + RunDartCodeInIsolate(vm_ref, settings, task_runners, + "mainForPluginRegistrantTest", {}, kernel_path); + + ASSERT_TRUE(isolate); + ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); + + latch.Wait(); + + ASSERT_EQ(messages.size(), 1u); + ASSERT_EQ(messages[0], "_PluginRegistrant.register() was called"); +} + +} // namespace testing +} // namespace flutter + +// NOLINTEND(clang-analyzer-core.StackAddressEscape) diff --git a/runtime/fixtures/dart_tool/flutter_build/dart_plugin_registrant.dart b/runtime/fixtures/dart_tool/flutter_build/dart_plugin_registrant.dart new file mode 100644 index 0000000000000..2a6220f1e5f1e --- /dev/null +++ b/runtime/fixtures/dart_tool/flutter_build/dart_plugin_registrant.dart @@ -0,0 +1,33 @@ +// 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'; + +bool didCallRegistrantBeforeEntrypoint = false; + +// Test the Dart plugin registrant. +@pragma('vm:entry-point') +class _PluginRegistrant { + + @pragma('vm:entry-point') + static void register() { + if (didCallRegistrantBeforeEntrypoint) { + throw '_registerPlugins is being called twice'; + } + didCallRegistrantBeforeEntrypoint = true; + } + +} + + +@pragma('vm:entry-point') +void mainForPluginRegistrantTest() { + if (didCallRegistrantBeforeEntrypoint) { + passMessage('_PluginRegistrant.register() was called'); + } else { + passMessage('_PluginRegistrant.register() was not called'); + } +} + +void main() {} From 87f0eaabe0278b993533cdb07c344e989e55fce2 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 28 Feb 2022 13:17:00 -0800 Subject: [PATCH 4/4] updated licenses --- ci/licenses_golden/licenses_flutter | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 4e5c531f21f2e..4aaa7a87bceef 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -962,6 +962,7 @@ FILE: ../../../flutter/runtime/dart_isolate_group_data.cc FILE: ../../../flutter/runtime/dart_isolate_group_data.h FILE: ../../../flutter/runtime/dart_isolate_unittests.cc FILE: ../../../flutter/runtime/dart_lifecycle_unittests.cc +FILE: ../../../flutter/runtime/dart_plugin_registrant_unittests.cc FILE: ../../../flutter/runtime/dart_service_isolate.cc FILE: ../../../flutter/runtime/dart_service_isolate.h FILE: ../../../flutter/runtime/dart_service_isolate_unittests.cc @@ -984,7 +985,6 @@ 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/dart_plugin_registrant_unittests.cc FILE: ../../../flutter/runtime/no_dart_plugin_registrant_unittests.cc FILE: ../../../flutter/runtime/platform_data.cc FILE: ../../../flutter/runtime/platform_data.h