Skip to content

Commit

Permalink
Revert "Create root isolate asynchronously (flutter#20142)" (flutter#…
Browse files Browse the repository at this point in the history 
…20937)

This reverts commit 95f2b72.
  • Loading branch information
@liyuqian
liyuqian authored Sep 2, 2020
1 parent 96efe39 commit 5585ed9
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 149 deletions.
142 changes: 49 additions & 93 deletions runtime/runtime_controller.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@ namespace flutter {

RuntimeController::RuntimeController(RuntimeDelegate& client,
TaskRunners p_task_runners)
: client_(client),
vm_(nullptr),
task_runners_(p_task_runners),
weak_factory_(this) {}
: client_(client), vm_(nullptr), task_runners_(p_task_runners) {}

RuntimeController::RuntimeController(
RuntimeDelegate& p_client,
Expand Down Expand Up @@ -53,80 +50,48 @@ RuntimeController::RuntimeController(
platform_data_(std::move(p_platform_data)),
isolate_create_callback_(p_isolate_create_callback),
isolate_shutdown_callback_(p_isolate_shutdown_callback),
persistent_isolate_data_(std::move(p_persistent_isolate_data)),
weak_factory_(this) {
// Create the root isolate as soon as the runtime controller is initialized,
// but not using a synchronous way to avoid blocking the platform thread a
// long time as it is waiting while creating `Shell` on that platform thread.
persistent_isolate_data_(std::move(p_persistent_isolate_data)) {
// Create the root isolate as soon as the runtime controller is initialized.
// It will be run at a later point when the engine provides a run
// configuration and then runs the isolate.
create_and_config_root_isolate_ =
std::async(std::launch::deferred, [self = weak_factory_.GetWeakPtr()]() {
if (!self) {
return;
}

auto strong_root_isolate =
DartIsolate::CreateRootIsolate(
self->vm_->GetVMData()->GetSettings(), //
self->isolate_snapshot_, //
self->task_runners_, //
std::make_unique<PlatformConfiguration>(self.get()), //
self->snapshot_delegate_, //
self->io_manager_, //
self->unref_queue_, //
self->image_decoder_, //
self->advisory_script_uri_, //
self->advisory_script_entrypoint_, //
nullptr, //
self->isolate_create_callback_, //
self->isolate_shutdown_callback_ //
)
.lock();

FML_CHECK(strong_root_isolate) << "Could not create root isolate.";

// The root isolate ivar is weak.
self->root_isolate_ = strong_root_isolate;

strong_root_isolate->SetReturnCodeCallback([self](uint32_t code) {
if (!self) {
return;
}

self->root_isolate_return_code_ = {true, code};
});

if (auto* platform_configuration =
self->GetPlatformConfigurationIfAvailable()) {
tonic::DartState::Scope scope(strong_root_isolate);
platform_configuration->DidCreateIsolate();
if (!self->FlushRuntimeStateToIsolate()) {
FML_DLOG(ERROR) << "Could not setup initial isolate state.";
}
} else {
FML_DCHECK(false)
<< "RuntimeController created without window binding.";
}

FML_DCHECK(Dart_CurrentIsolate() == nullptr);

self->client_.OnRootIsolateCreated();
return;
});

// We're still trying to create the root isolate as soon as possible here on
// the UI thread although it's deferred a little bit by
// std::async(std::launch::deferred, ...). So the callers of `GetRootIsolate`
// should get a quick return after this UI thread task.
task_runners_.GetUITaskRunner()->PostTask(
[self = weak_factory_.GetWeakPtr()]() {
if (!self) {
return;
}

self->GetRootIsolate();
});
auto strong_root_isolate =
DartIsolate::CreateRootIsolate(
vm_->GetVMData()->GetSettings(), //
isolate_snapshot_, //
task_runners_, //
std::make_unique<PlatformConfiguration>(this), //
snapshot_delegate_, //
io_manager_, //
unref_queue_, //
image_decoder_, //
p_advisory_script_uri, //
p_advisory_script_entrypoint, //
nullptr, //
isolate_create_callback_, //
isolate_shutdown_callback_ //
)
.lock();

FML_CHECK(strong_root_isolate) << "Could not create root isolate.";

// The root isolate ivar is weak.
root_isolate_ = strong_root_isolate;

strong_root_isolate->SetReturnCodeCallback([this](uint32_t code) {
root_isolate_return_code_ = {true, code};
});

if (auto* platform_configuration = GetPlatformConfigurationIfAvailable()) {
tonic::DartState::Scope scope(strong_root_isolate);
platform_configuration->DidCreateIsolate();
if (!FlushRuntimeStateToIsolate()) {
FML_DLOG(ERROR) << "Could not setup initial isolate state.";
}
} else {
FML_DCHECK(false) << "RuntimeController created without window binding.";
}

FML_DCHECK(Dart_CurrentIsolate() == nullptr);
}

RuntimeController::~RuntimeController() {
Expand All @@ -142,8 +107,8 @@ RuntimeController::~RuntimeController() {
}
}

bool RuntimeController::IsRootIsolateRunning() {
std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
bool RuntimeController::IsRootIsolateRunning() const {
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
if (root_isolate) {
return root_isolate->GetPhase() == DartIsolate::Phase::Running;
}
Expand Down Expand Up @@ -269,7 +234,7 @@ bool RuntimeController::ReportTimings(std::vector<int64_t> timings) {
}

bool RuntimeController::NotifyIdle(int64_t deadline) {
std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
if (!root_isolate) {
return false;
}
Expand Down Expand Up @@ -326,7 +291,7 @@ bool RuntimeController::DispatchSemanticsAction(int32_t id,

PlatformConfiguration*
RuntimeController::GetPlatformConfigurationIfAvailable() {
std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
return root_isolate ? root_isolate->platform_configuration() : nullptr;
}

Expand Down Expand Up @@ -388,17 +353,17 @@ RuntimeController::ComputePlatformResolvedLocale(
}

Dart_Port RuntimeController::GetMainPort() {
std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
return root_isolate ? root_isolate->main_port() : ILLEGAL_PORT;
}

std::string RuntimeController::GetIsolateName() {
std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
return root_isolate ? root_isolate->debug_name() : "";
}

bool RuntimeController::HasLivePorts() {
std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
if (!root_isolate) {
return false;
}
Expand All @@ -407,20 +372,11 @@ bool RuntimeController::HasLivePorts() {
}

tonic::DartErrorHandleType RuntimeController::GetLastError() {
std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
return root_isolate ? root_isolate->GetLastError() : tonic::kNoError;
}

std::weak_ptr<DartIsolate> RuntimeController::GetRootIsolate() {
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
if (root_isolate) {
return root_isolate_;
}

// Root isolate is not yet created, get it and do some configuration.
FML_DCHECK(create_and_config_root_isolate_.valid());
create_and_config_root_isolate_.get();

return root_isolate_;
}

Expand Down
13 changes: 2 additions & 11 deletions runtime/runtime_controller.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#ifndef FLUTTER_RUNTIME_RUNTIME_CONTROLLER_H_
#define FLUTTER_RUNTIME_RUNTIME_CONTROLLER_H_

#include <future>
#include <memory>
#include <vector>

Expand Down Expand Up @@ -341,7 +340,7 @@ class RuntimeController : public PlatformConfigurationClient {
///
/// @return True if root isolate running, False otherwise.
///
virtual bool IsRootIsolateRunning();
virtual bool IsRootIsolateRunning() const;

//----------------------------------------------------------------------------
/// @brief Dispatch the specified platform message to running root
Expand Down Expand Up @@ -423,10 +422,7 @@ class RuntimeController : public PlatformConfigurationClient {
/// @brief Get a weak pointer to the root Dart isolate. This isolate may
/// only be locked on the UI task runner. Callers use this
/// accessor to transition to the root isolate to the running
/// phase. Note that it might take times if the isolate is not yet
/// created, which should be done in a subsequence task after
/// constructing `RuntimeController`, or it should get a quick
/// return otherwise.
/// phase.
///
/// @return The root isolate reference.
///
Expand Down Expand Up @@ -475,16 +471,11 @@ class RuntimeController : public PlatformConfigurationClient {
std::string advisory_script_entrypoint_;
std::function<void(int64_t)> idle_notification_callback_;
PlatformData platform_data_;
std::future<void> create_and_config_root_isolate_;
// Note that `root_isolate_` is created asynchronously from the constructor of
// `RuntimeController`, be careful to use it directly while it might have not
// been created yet. Call `GetRootIsolate()` instead which guarantees that.
std::weak_ptr<DartIsolate> root_isolate_;
std::pair<bool, uint32_t> root_isolate_return_code_ = {false, 0};
const fml::closure isolate_create_callback_;
const fml::closure isolate_shutdown_callback_;
std::shared_ptr<const fml::Mapping> persistent_isolate_data_;
fml::WeakPtrFactory<RuntimeController> weak_factory_;

PlatformConfiguration* GetPlatformConfigurationIfAvailable();

Expand Down
2 changes: 0 additions & 2 deletions runtime/runtime_delegate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ class RuntimeDelegate {

virtual FontCollection& GetFontCollection() = 0;

virtual void OnRootIsolateCreated() = 0;

virtual void UpdateIsolateDescription(const std::string isolate_name,
int64_t isolate_port) = 0;

Expand Down
4 changes: 0 additions & 4 deletions shell/common/engine.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -495,10 +495,6 @@ void Engine::HandlePlatformMessage(fml::RefPtr<PlatformMessage> message) {
}
}

void Engine::OnRootIsolateCreated() {
delegate_.OnRootIsolateCreated();
}

void Engine::UpdateIsolateDescription(const std::string isolate_name,
int64_t isolate_port) {
delegate_.UpdateIsolateDescription(isolate_name, isolate_port);
Expand Down
12 changes: 0 additions & 12 deletions shell/common/engine.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,15 +185,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
///
virtual void OnPreEngineRestart() = 0;

//--------------------------------------------------------------------------
/// @brief Notifies the shell that the root isolate is created.
/// Currently, this information is to add to the service
/// protocol list of available root isolates running in the VM
/// and their names so that the appropriate isolate can be
/// selected in the tools for debugging and instrumentation.
///
virtual void OnRootIsolateCreated() = 0;

//--------------------------------------------------------------------------
/// @brief Notifies the shell of the name of the root isolate and its
/// port when that isolate is launched, restarted (in the
Expand Down Expand Up @@ -821,9 +812,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
// |RuntimeDelegate|
void HandlePlatformMessage(fml::RefPtr<PlatformMessage> message) override;

// |RuntimeDelegate|
void OnRootIsolateCreated() override;

// |RuntimeDelegate|
void UpdateIsolateDescription(const std::string isolate_name,
int64_t isolate_port) override;
Expand Down
4 changes: 1 addition & 3 deletions shell/common/engine_unittests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ class MockDelegate : public Engine::Delegate {
MOCK_METHOD1(OnEngineHandlePlatformMessage,
void(fml::RefPtr<PlatformMessage>));
MOCK_METHOD0(OnPreEngineRestart, void());
MOCK_METHOD0(OnRootIsolateCreated, void());
MOCK_METHOD2(UpdateIsolateDescription, void(const std::string, int64_t));
MOCK_METHOD1(SetNeedsReportTimings, void(bool));
MOCK_METHOD1(ComputePlatformResolvedLocale,
Expand All @@ -47,7 +46,6 @@ class MockRuntimeDelegate : public RuntimeDelegate {
void(SemanticsNodeUpdates, CustomAccessibilityActionUpdates));
MOCK_METHOD1(HandlePlatformMessage, void(fml::RefPtr<PlatformMessage>));
MOCK_METHOD0(GetFontCollection, FontCollection&());
MOCK_METHOD0(OnRootIsolateCreated, void());
MOCK_METHOD2(UpdateIsolateDescription, void(const std::string, int64_t));
MOCK_METHOD1(SetNeedsReportTimings, void(bool));
MOCK_METHOD1(ComputePlatformResolvedLocale,
Expand All @@ -59,7 +57,7 @@ class MockRuntimeController : public RuntimeController {
public:
MockRuntimeController(RuntimeDelegate& client, TaskRunners p_task_runners)
: RuntimeController(client, p_task_runners) {}
MOCK_METHOD0(IsRootIsolateRunning, bool());
MOCK_CONST_METHOD0(IsRootIsolateRunning, bool());
MOCK_METHOD1(DispatchPlatformMessage, bool(fml::RefPtr<PlatformMessage>));
};

Expand Down
25 changes: 4 additions & 21 deletions shell/common/shell.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,8 @@ bool Shell::Setup(std::unique_ptr<PlatformView> platform_view,

is_setup_ = true;

vm_->GetServiceProtocol()->AddHandler(this, GetServiceProtocolDescription());

PersistentCache::GetCacheForProcess()->AddWorkerTaskRunner(
task_runners_.GetIOTaskRunner());

Expand Down Expand Up @@ -1131,19 +1133,6 @@ void Shell::OnPreEngineRestart() {
latch.Wait();
}

// |Engine::Delegate|
void Shell::OnRootIsolateCreated() {
auto description = GetServiceProtocolDescription();
fml::TaskRunner::RunNowOrPostTask(
task_runners_.GetPlatformTaskRunner(),
[self = weak_factory_.GetWeakPtr(),
description = std::move(description)]() {
if (self) {
self->vm_->GetServiceProtocol()->AddHandler(self.get(), description);
}
});
}

// |Engine::Delegate|
void Shell::UpdateIsolateDescription(const std::string isolate_name,
int64_t isolate_port) {
Expand Down Expand Up @@ -1288,15 +1277,9 @@ bool Shell::HandleServiceProtocolMessage(
// |ServiceProtocol::Handler|
ServiceProtocol::Handler::Description Shell::GetServiceProtocolDescription()
const {
FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread());

if (!weak_engine_) {
return ServiceProtocol::Handler::Description();
}

return {
weak_engine_->GetUIIsolateMainPort(),
weak_engine_->GetUIIsolateName(),
engine_->GetUIIsolateMainPort(),
engine_->GetUIIsolateName(),
};
}

Expand Down
3 changes: 0 additions & 3 deletions shell/common/shell.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -513,9 +513,6 @@ class Shell final : public PlatformView::Delegate,
// |Engine::Delegate|
void OnPreEngineRestart() override;

// |Engine::Delegate|
void OnRootIsolateCreated() override;

// |Engine::Delegate|
void UpdateIsolateDescription(const std::string isolate_name,
int64_t isolate_port) override;
Expand Down

0 comments on commit 5585ed9

Please sign in to comment.