Skip to content

Commit

Permalink
Allow native bindings in secondary isolates. (flutter#8658)
Browse files Browse the repository at this point in the history
The callbacks can be wired in via the Settings object. Both runtime and shell unit-tests have been patched to test this.
  • Loading branch information
chinmaygarde authored Apr 20, 2019
1 parent b0cbce4 commit 1239df9
Show file tree
Hide file tree
Showing 18 changed files with 268 additions and 86 deletions.
4 changes: 2 additions & 2 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ 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/simple_main.dart
FILE: ../../../flutter/runtime/fixtures/runtime_test.dart
FILE: ../../../flutter/runtime/runtime_controller.cc
FILE: ../../../flutter/runtime/runtime_controller.h
FILE: ../../../flutter/runtime/runtime_delegate.cc
Expand All @@ -409,7 +409,7 @@ FILE: ../../../flutter/shell/common/animator.cc
FILE: ../../../flutter/shell/common/animator.h
FILE: ../../../flutter/shell/common/engine.cc
FILE: ../../../flutter/shell/common/engine.h
FILE: ../../../flutter/shell/common/fixtures/main.dart
FILE: ../../../flutter/shell/common/fixtures/shell_test.dart
FILE: ../../../flutter/shell/common/isolate_configuration.cc
FILE: ../../../flutter/shell/common/isolate_configuration.h
FILE: ../../../flutter/shell/common/persistent_cache.cc
Expand Down
2 changes: 2 additions & 0 deletions common/settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,11 @@ struct Settings {
// The main isolate is current when this callback is made. This is a good spot
// to perform native Dart bindings for libraries not built in.
fml::closure root_isolate_create_callback;
fml::closure isolate_create_callback;
// The isolate is not current and may have already been destroyed when this
// call is made.
fml::closure root_isolate_shutdown_callback;
fml::closure isolate_shutdown_callback;
// The callback made on the UI thread in an isolate scope when the engine
// detects that the framework is idle. The VM also uses this time to perform
// tasks suitable when idling. Due to this, embedders are still advised to be
Expand Down
2 changes: 1 addition & 1 deletion runtime/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ source_set("runtime") {
}

test_fixtures("runtime_fixtures") {
fixtures = [ "fixtures/simple_main.dart" ]
fixtures = [ "fixtures/runtime_test.dart" ]
}

source_set("runtime_unittests_common") {
Expand Down
46 changes: 39 additions & 7 deletions runtime/dart_isolate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
fml::WeakPtr<IOManager> io_manager,
std::string advisory_script_uri,
std::string advisory_script_entrypoint,
Dart_IsolateFlags* flags) {
Dart_IsolateFlags* flags,
fml::closure isolate_create_callback,
fml::closure isolate_shutdown_callback) {
TRACE_EVENT0("flutter", "DartIsolate::CreateRootIsolate");
Dart_Isolate vm_isolate = nullptr;
std::weak_ptr<DartIsolate> embedder_isolate;
Expand All @@ -49,6 +51,9 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
// Since this is the root isolate, we fake a parent embedder data object. We
// cannot use unique_ptr here because the destructor is private (since the
// isolate lifecycle is entirely managed by the VM).
//
// The child isolate preparer is null but will be set when the isolate is
// being prepared to run.
auto root_embedder_data = std::make_unique<std::shared_ptr<DartIsolate>>(
std::make_shared<DartIsolate>(
settings, // settings
Expand All @@ -59,8 +64,9 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
std::move(io_manager), // IO manager
advisory_script_uri, // advisory URI
advisory_script_entrypoint, // advisory entrypoint
nullptr // child isolate preparer will be set when this isolate is
// prepared to run
nullptr, // child isolate preparer
isolate_create_callback, // isolate create callback
isolate_shutdown_callback // isolate shutdown callback
));

std::tie(vm_isolate, embedder_isolate) = CreateDartVMAndEmbedderObjectPair(
Expand Down Expand Up @@ -102,7 +108,9 @@ DartIsolate::DartIsolate(const Settings& settings,
fml::WeakPtr<IOManager> io_manager,
std::string advisory_script_uri,
std::string advisory_script_entrypoint,
ChildIsolatePreparer child_isolate_preparer)
ChildIsolatePreparer child_isolate_preparer,
fml::closure isolate_create_callback,
fml::closure isolate_shutdown_callback)
: UIDartState(std::move(task_runners),
settings.task_observer_add,
settings.task_observer_remove,
Expand All @@ -116,7 +124,9 @@ DartIsolate::DartIsolate(const Settings& settings,
settings_(settings),
isolate_snapshot_(std::move(isolate_snapshot)),
shared_snapshot_(std::move(shared_snapshot)),
child_isolate_preparer_(std::move(child_isolate_preparer)) {
child_isolate_preparer_(std::move(child_isolate_preparer)),
isolate_create_callback_(isolate_create_callback),
isolate_shutdown_callback_(isolate_shutdown_callback) {
FML_DCHECK(isolate_snapshot_) << "Must contain a valid isolate snapshot.";
phase_ = Phase::Uninitialized;
}
Expand Down Expand Up @@ -279,6 +289,11 @@ bool DartIsolate::PrepareForRunningFromPrecompiledCode() {
child_isolate_preparer_ = [](DartIsolate* isolate) {
return isolate->PrepareForRunningFromPrecompiledCode();
};

if (isolate_create_callback_) {
isolate_create_callback_();
}

phase_ = Phase::Ready;
return true;
}
Expand Down Expand Up @@ -365,7 +380,13 @@ bool DartIsolate::PrepareForRunningFromKernel(
return true;
};
}

if (isolate_create_callback_) {
isolate_create_callback_();
}

phase_ = Phase::Ready;

return true;
}

Expand Down Expand Up @@ -563,7 +584,9 @@ Dart_Isolate DartIsolate::DartCreateAndStartServiceIsolate(
{}, // IO Manager
DART_VM_SERVICE_ISOLATE_NAME, // script uri
DART_VM_SERVICE_ISOLATE_NAME, // script entrypoint
flags // flags
flags, // flags
nullptr, // isolate create callback
nullptr // isolate shutdown callback
);

std::shared_ptr<DartIsolate> service_isolate = weak_service_isolate.lock();
Expand Down Expand Up @@ -662,6 +685,7 @@ DartIsolate::CreateDartVMAndEmbedderObjectPair(
TaskRunners null_task_runners(advisory_script_uri, nullptr, nullptr,
nullptr, nullptr);

// Copy most fields from the parent to the child.
embedder_isolate = std::make_unique<std::shared_ptr<DartIsolate>>(
std::make_shared<DartIsolate>(
(*raw_embedder_isolate)->GetSettings(), // settings
Expand All @@ -672,7 +696,12 @@ DartIsolate::CreateDartVMAndEmbedderObjectPair(
fml::WeakPtr<IOManager>{}, // io_manager
advisory_script_uri, // advisory_script_uri
advisory_script_entrypoint, // advisory_script_entrypoint
(*raw_embedder_isolate)->child_isolate_preparer_));
(*raw_embedder_isolate)->child_isolate_preparer_, // preparer
(*raw_embedder_isolate)->isolate_create_callback_, // on create
(*raw_embedder_isolate)->isolate_shutdown_callback_ // on shutdown
)

);
}

// Create the Dart VM isolate and give it the embedder object as the baton.
Expand Down Expand Up @@ -755,6 +784,9 @@ void DartIsolate::AddIsolateShutdownCallback(fml::closure closure) {

void DartIsolate::OnShutdownCallback() {
shutdown_callbacks_.clear();
if (isolate_shutdown_callback_) {
isolate_shutdown_callback_();
}
}

DartIsolate::AutoFireClosure::AutoFireClosure(fml::closure closure)
Expand Down
14 changes: 10 additions & 4 deletions runtime/dart_isolate.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ class DartIsolate : public UIDartState {
fml::WeakPtr<IOManager> io_manager,
std::string advisory_script_uri,
std::string advisory_script_entrypoint,
Dart_IsolateFlags* flags = nullptr);
Dart_IsolateFlags* flags,
fml::closure isolate_create_callback,
fml::closure isolate_shutdown_callback);

DartIsolate(const Settings& settings,
fml::RefPtr<const DartSnapshot> isolate_snapshot,
Expand All @@ -61,7 +63,9 @@ class DartIsolate : public UIDartState {
fml::WeakPtr<IOManager> io_manager,
std::string advisory_script_uri,
std::string advisory_script_entrypoint,
ChildIsolatePreparer child_isolate_preparer);
ChildIsolatePreparer child_isolate_preparer,
fml::closure isolate_create_callback,
fml::closure isolate_shutdown_callback);

~DartIsolate() override;

Expand Down Expand Up @@ -128,9 +132,11 @@ class DartIsolate : public UIDartState {
std::vector<std::unique_ptr<AutoFireClosure>> shutdown_callbacks_;
ChildIsolatePreparer child_isolate_preparer_ = nullptr;
fml::RefPtr<fml::TaskRunner> message_handling_task_runner_;
const fml::closure isolate_create_callback_;
const fml::closure isolate_shutdown_callback_;

FML_WARN_UNUSED_RESULT
bool Initialize(Dart_Isolate isolate, bool is_root_isolate);
FML_WARN_UNUSED_RESULT bool Initialize(Dart_Isolate isolate,
bool is_root_isolate);

void SetMessageHandlingTaskRunner(fml::RefPtr<fml::TaskRunner> runner,
bool is_root_isolate);
Expand Down
87 changes: 60 additions & 27 deletions runtime/dart_isolate_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "flutter/fml/make_copyable.h"
#include "flutter/fml/mapping.h"
#include "flutter/fml/paths.h"
#include "flutter/fml/synchronization/count_down_latch.h"
#include "flutter/fml/synchronization/waitable_event.h"
#include "flutter/fml/thread.h"
#include "flutter/runtime/dart_isolate.h"
Expand Down Expand Up @@ -35,15 +36,18 @@ TEST_F(DartIsolateTest, RootIsolateCreationAndShutdown) {
GetCurrentTaskRunner() //
);
auto weak_isolate = DartIsolate::CreateRootIsolate(
vm_data->GetSettings(), // settings
vm_data->GetIsolateSnapshot(), // isolate snapshot
vm_data->GetSharedSnapshot(), // shared snapshot
std::move(task_runners), // task runners
nullptr, // window
{}, // snapshot delegate
{}, // io manager
"main.dart", // advisory uri
"main" // advisory entrypoint
vm_data->GetSettings(), // settings
vm_data->GetIsolateSnapshot(), // isolate snapshot
vm_data->GetSharedSnapshot(), // shared snapshot
std::move(task_runners), // task runners
nullptr, // window
{}, // snapshot delegate
{}, // io manager
"main.dart", // advisory uri
"main", // advisory entrypoint,
nullptr, // flags
settings.isolate_create_callback, // isolate create callback
settings.isolate_shutdown_callback // isolate shutdown callback
);
auto root_isolate = weak_isolate.lock();
ASSERT_TRUE(root_isolate);
Expand All @@ -65,15 +69,18 @@ TEST_F(DartIsolateTest, IsolateShutdownCallbackIsInIsolateScope) {
GetCurrentTaskRunner() //
);
auto weak_isolate = DartIsolate::CreateRootIsolate(
vm_data->GetSettings(), // settings
vm_data->GetIsolateSnapshot(), // isolate snapshot
vm_data->GetSharedSnapshot(), // shared snapshot
std::move(task_runners), // task runners
nullptr, // window
{}, // snapshot delegate
{}, // io manager
"main.dart", // advisory uri
"main" // advisory entrypoint
vm_data->GetSettings(), // settings
vm_data->GetIsolateSnapshot(), // isolate snapshot
vm_data->GetSharedSnapshot(), // shared snapshot
std::move(task_runners), // task runners
nullptr, // window
{}, // snapshot delegate
{}, // io manager
"main.dart", // advisory uri
"main", // advisory entrypoint
nullptr, // flags
settings.isolate_create_callback, // isolate create callback
settings.isolate_shutdown_callback // isolate shutdown callback
);
auto root_isolate = weak_isolate.lock();
ASSERT_TRUE(root_isolate);
Expand Down Expand Up @@ -171,15 +178,18 @@ static void RunDartCodeInIsolate(DartVMRef& vm_ref,
}

auto weak_isolate = DartIsolate::CreateRootIsolate(
vm_data->GetSettings(), // settings
vm_data->GetIsolateSnapshot(), // isolate snapshot
vm_data->GetSharedSnapshot(), // shared snapshot
std::move(task_runners), // task runners
nullptr, // window
{}, // snapshot delegate
{}, // io manager
"main.dart", // advisory uri
"main" // advisory entrypoint
vm_data->GetSettings(), // settings
vm_data->GetIsolateSnapshot(), // isolate snapshot
vm_data->GetSharedSnapshot(), // shared snapshot
std::move(task_runners), // task runners
nullptr, // window
{}, // snapshot delegate
{}, // io manager
"main.dart", // advisory uri
"main", // advisory entrypoint
nullptr, // flags
settings.isolate_create_callback, // isolate create callback
settings.isolate_shutdown_callback // isolate shutdown callback
);

auto root_isolate =
Expand Down Expand Up @@ -345,5 +355,28 @@ TEST_F(DartIsolateTest, CanSaveCompilationTrace) {
latch.Wait();
}

TEST_F(DartIsolateTest, CanLaunchSecondaryIsolates) {
fml::CountDownLatch latch(3);
AddNativeCallback("NotifyNative",
CREATE_NATIVE_ENTRY(([&latch](Dart_NativeArguments args) {
latch.CountDown();
})));
AddNativeCallback(
"PassMessage", CREATE_NATIVE_ENTRY(([&latch](Dart_NativeArguments args) {
auto message = tonic::DartConverter<std::string>::FromDart(
Dart_GetNativeArgument(args, 0));
ASSERT_EQ("Hello from code is secondary isolate.", message);
latch.CountDown();
})));
const auto settings = CreateSettingsForFixture();
auto vm_ref = DartVMRef::Create(settings);
auto isolate = RunDartCodeInIsolate(vm_ref, settings, GetThreadTaskRunner(),
"testCanLaunchSecondaryIsolate");
ASSERT_TRUE(isolate);
ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running);

latch.Wait();
}

} // namespace testing
} // namespace flutter
22 changes: 12 additions & 10 deletions runtime/dart_lifecycle_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,18 @@ static std::shared_ptr<DartIsolate> CreateAndRunRootIsolate(
TaskRunners runners("io.flutter.test", task_runner, task_runner, task_runner,
task_runner);
auto isolate_weak = DartIsolate::CreateRootIsolate(
vm.GetSettings(), // settings
vm.GetIsolateSnapshot(), // isolate_snapshot
vm.GetSharedSnapshot(), // shared_snapshot
runners, // task_runners
{}, // window
{}, // snapshot_delegate
{}, // io_manager
"main.dart", // advisory_script_uri
entrypoint.c_str(), // advisory_script_entrypoint
nullptr // flags
vm.GetSettings(), // settings
vm.GetIsolateSnapshot(), // isolate_snapshot
vm.GetSharedSnapshot(), // shared_snapshot
runners, // task_runners
{}, // window
{}, // snapshot_delegate
{}, // io_manager
"main.dart", // advisory_script_uri
entrypoint.c_str(), // advisory_script_entrypoint
nullptr, // flags
settings.isolate_create_callback, // isolate create callback
settings.isolate_shutdown_callback // isolate shutdown callback
);

auto isolate = isolate_weak.lock();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,16 @@ void testCanSaveCompilationTrace() {
}

void NotifyResult(bool success) native "NotifyNative";
void PassMessage(String message) native "PassMessage";

void secondaryIsolateMain(String message) {
print("Secondary isolate got message: " + message);
PassMessage("Hello from code is secondary isolate.");
NotifyNative();
}

@pragma('vm:entry-point')
void testCanLaunchSecondaryIsolate() {
Isolate.spawn(secondaryIsolateMain, "Hello from root isolate.");
NotifyNative();
}
Loading

0 comments on commit 1239df9

Please sign in to comment.