From 89eeaf461ea136195337452728c5f34525009124 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 24 Sep 2018 18:01:22 -0400 Subject: [PATCH] Engine::Run returns enum: success, failure, or isolate already running (flutter/engine#6324) * If isolate is already running, return true * Use shell::Engine::RunStatus as result of Engine::Run --- engine/src/flutter/shell/common/engine.cc | 38 +++++++++++++------ engine/src/flutter/shell/common/engine.h | 11 +++++- .../platform/android/android_shell_holder.cc | 2 +- .../Source/FlutterHeadlessDartRunner.mm | 2 +- .../framework/Source/FlutterViewController.mm | 2 +- .../platform/embedder/embedder_engine.cc | 2 +- .../src/flutter/shell/testing/tester_main.cc | 2 +- 7 files changed, 40 insertions(+), 19 deletions(-) diff --git a/engine/src/flutter/shell/common/engine.cc b/engine/src/flutter/shell/common/engine.cc index b79dbfae33..2a9ee3f2c0 100644 --- a/engine/src/flutter/shell/common/engine.cc +++ b/engine/src/flutter/shell/common/engine.cc @@ -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 isolate = @@ -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()); @@ -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) { diff --git a/engine/src/flutter/shell/common/engine.h b/engine/src/flutter/shell/common/engine.h index f35b9ba421..e5561d1c8c 100644 --- a/engine/src/flutter/shell/common/engine.h +++ b/engine/src/flutter/shell/common/engine.h @@ -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( @@ -56,7 +63,7 @@ class Engine final : public blink::RuntimeDelegate { fml::WeakPtr 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 @@ -149,7 +156,7 @@ class Engine final : public blink::RuntimeDelegate { bool GetAssetAsBuffer(const std::string& name, std::vector* data); - bool PrepareAndLaunchIsolate(RunConfiguration configuration); + RunStatus PrepareAndLaunchIsolate(RunConfiguration configuration); FML_DISALLOW_COPY_AND_ASSIGN(Engine); }; diff --git a/engine/src/flutter/shell/platform/android/android_shell_holder.cc b/engine/src/flutter/shell/platform/android/android_shell_holder.cc index 83d4953471..fac31e2297 100644 --- a/engine/src/flutter/shell/platform/android/android_shell_holder.cc +++ b/engine/src/flutter/shell/platform/android/android_shell_holder.cc @@ -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 " diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterHeadlessDartRunner.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterHeadlessDartRunner.mm index e35089e03d..5d0f42af6c 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterHeadlessDartRunner.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterHeadlessDartRunner.mm @@ -101,7 +101,7 @@ static std::string CreateShellLabel() { 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."; diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index 891877220c..efda071b08 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -458,7 +458,7 @@ ]() 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."; } } diff --git a/engine/src/flutter/shell/platform/embedder/embedder_engine.cc b/engine/src/flutter/shell/platform/embedder/embedder_engine.cc index ed33aa84c2..1f4dc3402b 100644 --- a/engine/src/flutter/shell/platform/embedder/embedder_engine.cc +++ b/engine/src/flutter/shell/platform/embedder/embedder_engine.cc @@ -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."; } } diff --git a/engine/src/flutter/shell/testing/tester_main.cc b/engine/src/flutter/shell/testing/tester_main.cc index 4e10dab16f..48d8bdf28e 100644 --- a/engine/src/flutter/shell/testing/tester_main.cc +++ b/engine/src/flutter/shell/testing/tester_main.cc @@ -171,7 +171,7 @@ int RunTester(const blink::Settings& settings, bool run_forever) { fml::MessageLoop::GetCurrent().AddTaskObserver( reinterpret_cast(&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;