Skip to content

Commit

Permalink
Engine::Run returns enum: success, failure, or isolate already running (
Browse files Browse the repository at this point in the history
flutter#6324)

* If isolate is already running, return true

* Use shell::Engine::RunStatus as result of Engine::Run
  • Loading branch information
dnfield authored Sep 24, 2018
1 parent 83ec05d commit 89516aa
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 19 deletions.
38 changes: 26 additions & 12 deletions shell/common/engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -103,18 +103,23 @@ bool Engine::Restart(RunConfiguration configuration) {
delegate_.OnPreEngineRestart();
runtime_controller_ = runtime_controller_->Clone();
UpdateAssetManager(nullptr);
return Run(std::move(configuration));
return Run(std::move(configuration)) == Engine::RunStatus::Success;
}

bool Engine::Run(RunConfiguration configuration) {
Engine::RunStatus Engine::Run(RunConfiguration configuration) {
if (!configuration.IsValid()) {
FML_LOG(ERROR) << "Engine run configuration was invalid.";
return false;
return RunStatus::Failure;
}

if (!PrepareAndLaunchIsolate(std::move(configuration))) {
auto isolate_launch_status =
PrepareAndLaunchIsolate(std::move(configuration));
if (isolate_launch_status == Engine::RunStatus::Failure) {
FML_LOG(ERROR) << "Engine not prepare and launch isolate.";
return false;
return isolate_launch_status;
} else if (isolate_launch_status ==
Engine::RunStatus::FailureAlreadyRunning) {
return isolate_launch_status;
}

std::shared_ptr<blink::DartIsolate> isolate =
Expand All @@ -136,10 +141,12 @@ bool Engine::Run(RunConfiguration configuration) {
}
}

return isolate_running;
return isolate_running ? Engine::RunStatus::Success
: Engine::RunStatus::Failure;
}

bool Engine::PrepareAndLaunchIsolate(RunConfiguration configuration) {
shell::Engine::RunStatus Engine::PrepareAndLaunchIsolate(
RunConfiguration configuration) {
TRACE_EVENT0("flutter", "Engine::PrepareAndLaunchIsolate");

UpdateAssetManager(configuration.GetAssetManager());
Expand All @@ -150,28 +157,35 @@ bool Engine::PrepareAndLaunchIsolate(RunConfiguration configuration) {
runtime_controller_->GetRootIsolate().lock();

if (!isolate) {
return false;
return RunStatus::Failure;
}

// This can happen on iOS after a plugin shows a native window and returns to
// the Flutter ViewController.
if (isolate->GetPhase() == blink::DartIsolate::Phase::Running) {
FML_DLOG(WARNING) << "Isolate was already running!";
return RunStatus::FailureAlreadyRunning;
}

if (!isolate_configuration->PrepareIsolate(*isolate)) {
FML_LOG(ERROR) << "Could not prepare to run the isolate.";
return false;
return RunStatus::Failure;
}

if (configuration.GetEntrypointLibrary().empty()) {
if (!isolate->Run(configuration.GetEntrypoint())) {
FML_LOG(ERROR) << "Could not run the isolate.";
return false;
return RunStatus::Failure;
}
} else {
if (!isolate->RunFromLibrary(configuration.GetEntrypointLibrary(),
configuration.GetEntrypoint())) {
FML_LOG(ERROR) << "Could not run the isolate.";
return false;
return RunStatus::Failure;
}
}

return true;
return RunStatus::Success;
}

void Engine::BeginFrame(fml::TimePoint frame_time) {
Expand Down
11 changes: 9 additions & 2 deletions shell/common/engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@ namespace shell {

class Engine final : public blink::RuntimeDelegate {
public:
// Used by Engine::Run
enum class RunStatus {
Success, // Successful call to Run()
FailureAlreadyRunning, // Isolate was already running; may not be considered a failure by callers
Failure, // Isolate could not be started or other unspecified failure
};

class Delegate {
public:
virtual void OnEngineUpdateSemantics(
Expand Down Expand Up @@ -56,7 +63,7 @@ class Engine final : public blink::RuntimeDelegate {
fml::WeakPtr<Engine> GetWeakPtr() const;

FML_WARN_UNUSED_RESULT
bool Run(RunConfiguration configuration);
RunStatus Run(RunConfiguration configuration);

// Used to "cold reload" a running application where the shell (along with the
// platform view and its rasterizer bindings) remains the same but the root
Expand Down Expand Up @@ -149,7 +156,7 @@ class Engine final : public blink::RuntimeDelegate {

bool GetAssetAsBuffer(const std::string& name, std::vector<uint8_t>* data);

bool PrepareAndLaunchIsolate(RunConfiguration configuration);
RunStatus PrepareAndLaunchIsolate(RunConfiguration configuration);

FML_DISALLOW_COPY_AND_ASSIGN(Engine);
};
Expand Down
2 changes: 1 addition & 1 deletion shell/platform/android/android_shell_holder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ void AndroidShellHolder::Launch(RunConfiguration config) {
config = std::move(config) //
]() mutable {
FML_LOG(INFO) << "Attempting to launch engine configuration...";
if (!engine || !engine->Run(std::move(config))) {
if (!engine || engine->Run(std::move(config)) == shell::Engine::RunStatus::Failure) {
FML_LOG(ERROR) << "Could not launch engine in configuration.";
} else {
FML_LOG(INFO) << "Isolate for engine configuration successfully "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ - (void)runWithEntrypointAndLibraryUri:(NSString*)entrypoint libraryUri:(NSStrin
fml::MakeCopyable([engine = _shell->GetEngine(), config = std::move(config)]() mutable {
BOOL success = NO;
FML_LOG(INFO) << "Attempting to launch background engine configuration...";
if (!engine || !engine->Run(std::move(config))) {
if (!engine || engine->Run(std::move(config)) == shell::Engine::RunStatus::Failure) {
FML_LOG(ERROR) << "Could not launch engine with configuration.";
} else {
FML_LOG(INFO) << "Background Isolate successfully started and run.";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ - (void)viewWillAppear:(BOOL)animated {
]() mutable {
if (engine) {
auto result = engine->Run(std::move(config));
if (!result) {
if (result == shell::Engine::RunStatus::Failure) {
FML_LOG(ERROR) << "Could not launch engine with configuration.";
}
}
Expand Down
2 changes: 1 addition & 1 deletion shell/platform/embedder/embedder_engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ bool EmbedderEngine::Run(RunConfiguration run_configuration) {
]() mutable {
if (engine) {
auto result = engine->Run(std::move(config));
if (!result) {
if (result == shell::Engine::RunStatus::Failure) {
FML_LOG(ERROR) << "Could not launch the engine with configuration.";
}
}
Expand Down
2 changes: 1 addition & 1 deletion shell/testing/tester_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ int RunTester(const blink::Settings& settings, bool run_forever) {
fml::MessageLoop::GetCurrent().AddTaskObserver(
reinterpret_cast<intptr_t>(&completion_observer),
[&completion_observer]() { completion_observer.DidProcessTask(); });
if (engine->Run(std::move(config))) {
if (engine->Run(std::move(config)) != shell::Engine::RunStatus::Failure) {
engine_did_run = true;

blink::ViewportMetrics metrics;
Expand Down

0 comments on commit 89516aa

Please sign in to comment.