Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Reland Dart plugin registrant #25496

Merged
merged 17 commits into from
Apr 12, 2021
1 change: 1 addition & 0 deletions BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 2 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions lib/ui/painting/image_decoder_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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(); });
Expand Down
2 changes: 1 addition & 1 deletion lib/ui/ui_benchmarks.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
20 changes: 20 additions & 0 deletions runtime/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -136,4 +136,24 @@ if (enable_unittests) {
"//third_party/skia",
]
}

test_fixtures("no_plugin_registrant") {
dart_main = "fixtures/no_dart_plugin_registrant_test.dart"
use_target_as_artifact_prefix = true
}

executable("no_dart_plugin_registrant_unittests") {
testonly = true

sources = [ "no_dart_plugin_registrant_unittests.cc" ]

public_configs = [ "//flutter:export_dynamic_symbols" ]

public_deps = [
":no_plugin_registrant",
"//flutter/fml",
"//flutter/testing",
"//flutter/testing:fixture_test",
]
}
}
25 changes: 25 additions & 0 deletions runtime/dart_isolate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,28 @@ bool DartIsolate::MarkIsolateRunnable() {
return true;
}

static void InvokeDartPluginRegistrantIfAvailable(Dart_Handle library_handle) {
TRACE_EVENT0("flutter", "InvokeDartPluginRegistrantIfAvailable");

// 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
// 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;
}
tonic::LogIfError(tonic::DartInvokeField(plugin_registrant, "register", {}));
}

bool DartIsolate::RunFromLibrary(std::optional<std::string> library_name,
std::optional<std::string> entrypoint,
const std::vector<std::string>& args) {
Expand All @@ -742,6 +764,9 @@ bool DartIsolate::RunFromLibrary(std::optional<std::string> library_name,
auto entrypoint_handle = entrypoint.has_value() && !entrypoint.value().empty()
? tonic::ToDart(entrypoint.value().c_str())
: tonic::ToDart("main");

InvokeDartPluginRegistrantIfAvailable(library_handle);

auto user_entrypoint_function =
::Dart_GetField(library_handle, entrypoint_handle);

Expand Down
62 changes: 48 additions & 14 deletions runtime/dart_isolate_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -231,7 +231,7 @@ TEST_F(DartIsolateTest, IsolateCannotLoadAndRunUnknownDartEntrypoint) {
);
auto isolate =
RunDartCodeInIsolate(vm_ref, settings, task_runners, "thisShouldNotExist",
{}, GetFixturesPath());
{}, GetDefaultKernelFilePath());
ASSERT_FALSE(isolate);
}

Expand All @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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();
Expand All @@ -613,5 +613,39 @@ TEST_F(DartIsolateTest, DISABLED_ValidLoadingUnitSucceeds) {
Wait();
}

TEST_F(DartIsolateTest, DartPluginRegistrantIsCalled) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to add an explicit test that there is no error when the class is absent?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would require a different location for the test. Since such tests require a new Dart main file, the Dart tooling complains about duplicated output file:

ERROR at //third_party/dart/build/dart/dart_action.gni:45:3: Duplicate output file.
  action(target_name) {
  ^--------------------
Two or more targets generate the same output:
  gen/flutter/runtime/assets/kernel_blob.bin

This is can often be fixed by changing one of the target names, or by
setting an output_name on one of them.

Collisions:
  //flutter/runtime:_dsk__ds_no_plugin_registrant_fixtures
  //flutter/runtime:_dsk__ds_runtime_fixtures

See //third_party/dart/build/dart/dart_action.gni:45:3: Collision.
  action(target_name) {
  ^--------------------

Any suggestion @chinmaygarde ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could try to update //testing/testing.gni to support multiple Dart snapshot targets. Or just create another testing target if it's easier - e.g. executable("runtime_plugin_missing_unittests") { .... } that depends on its own test_fixtures("runtime_plugin_missing_fixtures") which specifies a different Dart file.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it might be enough to just add another test_fixtures target and have the existing runtime executable depend on it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh I see, that is probably what you did already. You'll want to update testing.gni so that you can tell it to use a different file name at line 159.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Done

ASSERT_FALSE(DartVMRef::IsInstanceRunning());

std::vector<std::string> messages;
fml::AutoResetWaitableEvent latch;

AddNativeCallback(
"PassMessage",
CREATE_NATIVE_ENTRY(([&latch, &messages](Dart_NativeArguments args) {
auto message = tonic::DartConverter<std::string>::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", {},
GetDefaultKernelFilePath());
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
9 changes: 9 additions & 0 deletions runtime/fixtures/no_dart_plugin_registrant_test.dart
Original file line number Diff line number Diff line change
@@ -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');
}
29 changes: 27 additions & 2 deletions runtime/fixtures/runtime_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -115,3 +114,29 @@ void testCanConvertListOfInts(List<int> args){
args[2] == 3 &&
args[3] == 4);
}

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() { // ignore: unused_element
if (didCallRegistrantBeforeEntrypoint) {
passMessage('_PluginRegistrant.register() was called');
} else {
passMessage('_PluginRegistrant.register() was not called');
}
}
73 changes: 73 additions & 0 deletions runtime/no_dart_plugin_registrant_unittests.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// 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"

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, elf_file_name, "") {}
};

TEST_F(DartIsolateTest, DartPluginRegistrantIsNotPresent) {
ASSERT_FALSE(DartVMRef::IsInstanceRunning());

std::vector<std::string> messages;
fml::AutoResetWaitableEvent latch;

AddNativeCallback(
"PassMessage",
CREATE_NATIVE_ENTRY(([&latch, &messages](Dart_NativeArguments args) {
auto message = tonic::DartConverter<std::string>::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, "main",
{}, kernel_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");
ASSERT_FALSE(did_throw_exception);
}

} // namespace testing
} // namespace flutter
2 changes: 1 addition & 1 deletion runtime/type_conversions_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
3 changes: 2 additions & 1 deletion shell/common/shell_benchmarks.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.";
Expand Down
Loading