Skip to content

Commit

Permalink
Revert "Revert "Separate the data required to bootstrap the VM into i…
Browse files Browse the repository at this point in the history
…ts own class. (flutter#8397)" (flutter#8406)" (flutter#8414)

This reverts commit f7b4903.
  • Loading branch information
chinmaygarde authored Apr 3, 2019
1 parent d8bb9d7 commit 7e38b0a
Show file tree
Hide file tree
Showing 24 changed files with 632 additions and 290 deletions.
4 changes: 4 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,10 @@ FILE: ../../../flutter/runtime/dart_snapshot_buffer.cc
FILE: ../../../flutter/runtime/dart_snapshot_buffer.h
FILE: ../../../flutter/runtime/dart_vm.cc
FILE: ../../../flutter/runtime/dart_vm.h
FILE: ../../../flutter/runtime/dart_vm_data.cc
FILE: ../../../flutter/runtime/dart_vm_data.h
FILE: ../../../flutter/runtime/dart_vm_lifecycle.cc
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
Expand Down
2 changes: 2 additions & 0 deletions common/settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ struct Settings {
// Font settings
bool use_test_fonts = false;

bool leak_vm = true;

// Engine settings
TaskObserverAdd task_observer_add;
TaskObserverRemove task_observer_remove;
Expand Down
18 changes: 12 additions & 6 deletions lib/ui/isolate_name_server/isolate_name_server_natives.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ namespace blink {

Dart_Handle IsolateNameServerNatives::LookupPortByName(
const std::string& name) {
IsolateNameServer* name_server =
UIDartState::Current()->GetIsolateNameServer();
auto name_server = UIDartState::Current()->GetIsolateNameServer();
if (!name_server) {
return Dart_Null();
}
Dart_Port port = name_server->LookupIsolatePortByName(name);
if (port == ILLEGAL_PORT) {
return Dart_Null();
Expand All @@ -26,8 +28,10 @@ Dart_Handle IsolateNameServerNatives::LookupPortByName(
Dart_Handle IsolateNameServerNatives::RegisterPortWithName(
Dart_Handle port_handle,
const std::string& name) {
IsolateNameServer* name_server =
UIDartState::Current()->GetIsolateNameServer();
auto name_server = UIDartState::Current()->GetIsolateNameServer();
if (!name_server) {
return Dart_False();
}
Dart_Port port = ILLEGAL_PORT;
Dart_SendPortGetId(port_handle, &port);
if (!name_server->RegisterIsolatePortWithName(port, name)) {
Expand All @@ -38,8 +42,10 @@ Dart_Handle IsolateNameServerNatives::RegisterPortWithName(

Dart_Handle IsolateNameServerNatives::RemovePortNameMapping(
const std::string& name) {
IsolateNameServer* name_server =
UIDartState::Current()->GetIsolateNameServer();
auto name_server = UIDartState::Current()->GetIsolateNameServer();
if (!name_server) {
return Dart_False();
}
if (!name_server->RemoveIsolateNameMapping(name)) {
return Dart_False();
}
Expand Down
6 changes: 3 additions & 3 deletions lib/ui/ui_dart_state.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ UIDartState::UIDartState(
std::string advisory_script_entrypoint,
std::string logger_prefix,
UnhandledExceptionCallback unhandled_exception_callback,
IsolateNameServer* isolate_name_server)
std::shared_ptr<IsolateNameServer> isolate_name_server)
: task_runners_(std::move(task_runners)),
add_callback_(std::move(add_callback)),
remove_callback_(std::move(remove_callback)),
Expand All @@ -33,7 +33,7 @@ UIDartState::UIDartState(
advisory_script_entrypoint_(std::move(advisory_script_entrypoint)),
logger_prefix_(std::move(logger_prefix)),
unhandled_exception_callback_(unhandled_exception_callback),
isolate_name_server_(isolate_name_server) {
isolate_name_server_(std::move(isolate_name_server)) {
AddOrRemoveTaskObserver(true /* add */);
}

Expand Down Expand Up @@ -124,7 +124,7 @@ fml::WeakPtr<GrContext> UIDartState::GetResourceContext() const {
return io_manager_->GetResourceContext();
}

IsolateNameServer* UIDartState::GetIsolateNameServer() {
std::shared_ptr<IsolateNameServer> UIDartState::GetIsolateNameServer() const {
return isolate_name_server_;
}

Expand Down
6 changes: 3 additions & 3 deletions lib/ui/ui_dart_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class UIDartState : public tonic::DartState {

fml::WeakPtr<GrContext> GetResourceContext() const;

IsolateNameServer* GetIsolateNameServer();
std::shared_ptr<IsolateNameServer> GetIsolateNameServer() const;

tonic::DartErrorHandleType GetLastError();

Expand Down Expand Up @@ -81,7 +81,7 @@ class UIDartState : public tonic::DartState {
std::string advisory_script_entrypoint,
std::string logger_prefix,
UnhandledExceptionCallback unhandled_exception_callback,
IsolateNameServer* isolate_name_server);
std::shared_ptr<IsolateNameServer> isolate_name_server);

~UIDartState() override;

Expand All @@ -107,7 +107,7 @@ class UIDartState : public tonic::DartState {
std::unique_ptr<Window> window_;
tonic::DartMicrotaskQueue microtask_queue_;
UnhandledExceptionCallback unhandled_exception_callback_;
IsolateNameServer* isolate_name_server_;
const std::shared_ptr<IsolateNameServer> isolate_name_server_;

void AddOrRemoveTaskObserver(bool add);
};
Expand Down
5 changes: 5 additions & 0 deletions runtime/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ source_set("runtime") {
"dart_snapshot_buffer.h",
"dart_vm.cc",
"dart_vm.h",
"dart_vm_data.cc",
"dart_vm_data.h",
"dart_vm_lifecycle.cc",
"dart_vm_lifecycle.h",
"embedder_resources.cc",
"embedder_resources.h",
"runtime_controller.cc",
Expand Down Expand Up @@ -115,6 +119,7 @@ executable("runtime_unittests") {
"$flutter_root/common",
"$flutter_root/fml",
"$flutter_root/lib/snapshot",
"$flutter_root/shell/common",
"$flutter_root/testing:dart",
"//third_party/skia",
"//third_party/tonic",
Expand Down
89 changes: 44 additions & 45 deletions runtime/dart_isolate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "flutter/lib/ui/dart_ui.h"
#include "flutter/runtime/dart_service_isolate.h"
#include "flutter/runtime/dart_vm.h"
#include "flutter/runtime/dart_vm_lifecycle.h"
#include "third_party/dart/runtime/include/dart_api.h"
#include "third_party/dart/runtime/include/dart_tools_api.h"
#include "third_party/tonic/converter/dart_converter.h"
Expand All @@ -29,9 +30,9 @@
namespace blink {

std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
DartVM* vm,
fml::RefPtr<DartSnapshot> isolate_snapshot,
fml::RefPtr<DartSnapshot> shared_snapshot,
const Settings& settings,
fml::RefPtr<const DartSnapshot> isolate_snapshot,
fml::RefPtr<const DartSnapshot> shared_snapshot,
TaskRunners task_runners,
std::unique_ptr<Window> window,
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
Expand All @@ -50,7 +51,7 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
// isolate lifecycle is entirely managed by the VM).
auto root_embedder_data = std::make_unique<std::shared_ptr<DartIsolate>>(
std::make_shared<DartIsolate>(
vm, // VM
settings, // settings
std::move(isolate_snapshot), // isolate snapshot
std::move(shared_snapshot), // shared snapshot
task_runners, // task runners
Expand Down Expand Up @@ -93,46 +94,41 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
return embedder_isolate;
}

DartIsolate::DartIsolate(DartVM* vm,
fml::RefPtr<DartSnapshot> isolate_snapshot,
fml::RefPtr<DartSnapshot> shared_snapshot,
DartIsolate::DartIsolate(const Settings& settings,
fml::RefPtr<const DartSnapshot> isolate_snapshot,
fml::RefPtr<const DartSnapshot> shared_snapshot,
TaskRunners task_runners,
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
fml::WeakPtr<IOManager> io_manager,
std::string advisory_script_uri,
std::string advisory_script_entrypoint,
ChildIsolatePreparer child_isolate_preparer)
: UIDartState(std::move(task_runners),
vm->GetSettings().task_observer_add,
vm->GetSettings().task_observer_remove,
settings.task_observer_add,
settings.task_observer_remove,
std::move(snapshot_delegate),
std::move(io_manager),
advisory_script_uri,
advisory_script_entrypoint,
vm->GetSettings().log_tag,
vm->GetSettings().unhandled_exception_callback,
vm->GetIsolateNameServer()),
vm_(vm),
settings.log_tag,
settings.unhandled_exception_callback,
DartVMRef::GetIsolateNameServer()),
settings_(settings),
isolate_snapshot_(std::move(isolate_snapshot)),
shared_snapshot_(std::move(shared_snapshot)),
child_isolate_preparer_(std::move(child_isolate_preparer)) {
FML_DCHECK(isolate_snapshot_) << "Must contain a valid isolate snapshot.";

if (vm_ == nullptr) {
return;
}

phase_ = Phase::Uninitialized;
}

DartIsolate::~DartIsolate() = default;

DartIsolate::Phase DartIsolate::GetPhase() const {
return phase_;
const Settings& DartIsolate::GetSettings() const {
return settings_;
}

DartVM* DartIsolate::GetDartVM() const {
return vm_;
DartIsolate::Phase DartIsolate::GetPhase() const {
return phase_;
}

bool DartIsolate::Initialize(Dart_Isolate dart_isolate, bool is_root_isolate) {
Expand Down Expand Up @@ -500,16 +496,16 @@ Dart_Isolate DartIsolate::DartCreateAndStartServiceIsolate(
const char* package_config,
Dart_IsolateFlags* flags,
char** error) {
auto vm = DartVM::ForProcessIfInitialized();
auto vm_data = DartVMRef::GetVMData();

if (!vm) {
if (!vm_data) {
*error = strdup(
"Could not resolve the VM when attempting to create the service "
"isolate.");
"Could not access VM data to initialize isolates. This may be because "
"the VM has initialized shutdown on another thread already.");
return nullptr;
}

const auto& settings = vm->GetSettings();
const auto& settings = vm_data->GetSettings();

if (!settings.enable_observatory) {
FML_DLOG(INFO) << "Observatory is disabled.";
Expand All @@ -524,16 +520,16 @@ Dart_Isolate DartIsolate::DartCreateAndStartServiceIsolate(

std::weak_ptr<DartIsolate> weak_service_isolate =
DartIsolate::CreateRootIsolate(
vm.get(), // vm
vm->GetIsolateSnapshot(), // isolate snapshot
vm->GetSharedSnapshot(), // shared snapshot
null_task_runners, // task runners
nullptr, // window
{}, // snapshot delegate
{}, // IO Manager
DART_VM_SERVICE_ISOLATE_NAME, // script uri
DART_VM_SERVICE_ISOLATE_NAME, // script entrypoint
flags // flags
vm_data->GetSettings(), // settings
vm_data->GetIsolateSnapshot(), // isolate snapshot
vm_data->GetSharedSnapshot(), // shared snapshot
null_task_runners, // task runners
nullptr, // window
{}, // snapshot delegate
{}, // IO Manager
DART_VM_SERVICE_ISOLATE_NAME, // script uri
DART_VM_SERVICE_ISOLATE_NAME, // script entrypoint
flags // flags
);

std::shared_ptr<DartIsolate> service_isolate = weak_service_isolate.lock();
Expand All @@ -556,7 +552,13 @@ Dart_Isolate DartIsolate::DartCreateAndStartServiceIsolate(
return nullptr;
}

vm->GetServiceProtocol().ToggleHooks(true);
if (auto service_protocol = DartVMRef::GetServiceProtocol()) {
service_protocol->ToggleHooks(true);
} else {
FML_DLOG(ERROR)
<< "Could not acquire the service protocol handlers. This might be "
"because the VM has already begun teardown on another thread.";
}

return service_isolate->isolate();
}
Expand Down Expand Up @@ -612,16 +614,13 @@ DartIsolate::CreateDartVMAndEmbedderObjectPair(
std::unique_ptr<std::shared_ptr<DartIsolate>> embedder_isolate(
p_parent_embedder_isolate);

if (embedder_isolate == nullptr ||
(*embedder_isolate)->GetDartVM() == nullptr) {
if (embedder_isolate == nullptr) {
*error =
strdup("Parent isolate did not have embedder specific callback data.");
FML_DLOG(ERROR) << *error;
return {nullptr, {}};
}

DartVM* const vm = (*embedder_isolate)->GetDartVM();

if (!is_root_isolate) {
auto* raw_embedder_isolate = embedder_isolate.release();

Expand All @@ -630,7 +629,7 @@ DartIsolate::CreateDartVMAndEmbedderObjectPair(

embedder_isolate = std::make_unique<std::shared_ptr<DartIsolate>>(
std::make_shared<DartIsolate>(
vm, // vm
(*raw_embedder_isolate)->GetSettings(), // settings
(*raw_embedder_isolate)->GetIsolateSnapshot(), // isolate_snapshot
(*raw_embedder_isolate)->GetSharedSnapshot(), // shared_snapshot
null_task_runners, // task_runners
Expand Down Expand Up @@ -703,11 +702,11 @@ void DartIsolate::DartIsolateCleanupCallback(
delete embedder_isolate;
}

fml::RefPtr<DartSnapshot> DartIsolate::GetIsolateSnapshot() const {
fml::RefPtr<const DartSnapshot> DartIsolate::GetIsolateSnapshot() const {
return isolate_snapshot_;
}

fml::RefPtr<DartSnapshot> DartIsolate::GetSharedSnapshot() const {
fml::RefPtr<const DartSnapshot> DartIsolate::GetSharedSnapshot() const {
return shared_snapshot_;
}

Expand Down
26 changes: 13 additions & 13 deletions runtime/dart_isolate.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ class DartIsolate : public UIDartState {
// bindings. From the VM's perspective, this isolate is not special in any
// way.
static std::weak_ptr<DartIsolate> CreateRootIsolate(
DartVM* vm,
fml::RefPtr<DartSnapshot> isolate_snapshot,
fml::RefPtr<DartSnapshot> shared_snapshot,
const Settings& settings,
fml::RefPtr<const DartSnapshot> isolate_snapshot,
fml::RefPtr<const DartSnapshot> shared_snapshot,
TaskRunners task_runners,
std::unique_ptr<Window> window,
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
Expand All @@ -52,9 +52,9 @@ class DartIsolate : public UIDartState {
std::string advisory_script_entrypoint,
Dart_IsolateFlags* flags = nullptr);

DartIsolate(DartVM* vm,
fml::RefPtr<DartSnapshot> isolate_snapshot,
fml::RefPtr<DartSnapshot> shared_snapshot,
DartIsolate(const Settings& settings,
fml::RefPtr<const DartSnapshot> isolate_snapshot,
fml::RefPtr<const DartSnapshot> shared_snapshot,
TaskRunners task_runners,
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
fml::WeakPtr<IOManager> io_manager,
Expand All @@ -64,6 +64,8 @@ class DartIsolate : public UIDartState {

~DartIsolate() override;

const Settings& GetSettings() const;

Phase GetPhase() const;

FML_WARN_UNUSED_RESULT
Expand All @@ -86,11 +88,9 @@ class DartIsolate : public UIDartState {

void AddIsolateShutdownCallback(fml::closure closure);

DartVM* GetDartVM() const;

fml::RefPtr<DartSnapshot> GetIsolateSnapshot() const;
fml::RefPtr<const DartSnapshot> GetIsolateSnapshot() const;

fml::RefPtr<DartSnapshot> GetSharedSnapshot() const;
fml::RefPtr<const DartSnapshot> GetSharedSnapshot() const;

std::weak_ptr<DartIsolate> GetWeakIsolatePtr();

Expand All @@ -109,10 +109,10 @@ class DartIsolate : public UIDartState {
};
friend class DartVM;

DartVM* const vm_ = nullptr;
Phase phase_ = Phase::Unknown;
const fml::RefPtr<DartSnapshot> isolate_snapshot_;
const fml::RefPtr<DartSnapshot> shared_snapshot_;
const Settings settings_;
const fml::RefPtr<const DartSnapshot> isolate_snapshot_;
const fml::RefPtr<const DartSnapshot> shared_snapshot_;
std::vector<std::shared_ptr<const fml::Mapping>> kernel_buffers_;
std::vector<std::unique_ptr<AutoFireClosure>> shutdown_callbacks_;
ChildIsolatePreparer child_isolate_preparer_ = nullptr;
Expand Down
Loading

0 comments on commit 7e38b0a

Please sign in to comment.