From 593c9b3ffe45ea2b4c313758f8698b7ea1d30031 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Tue, 9 Jul 2019 15:34:16 -0700 Subject: [PATCH 01/10] Make platform_view_android_jni delegate to platform_view_android --- shell/platform/android/platform_view_android_jni.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/shell/platform/android/platform_view_android_jni.cc b/shell/platform/android/platform_view_android_jni.cc index eaeb349d5ef8a..f25ddefb9becc 100644 --- a/shell/platform/android/platform_view_android_jni.cc +++ b/shell/platform/android/platform_view_android_jni.cc @@ -276,7 +276,7 @@ static void SetViewportMetrics(JNIEnv* env, static_cast(physicalViewInsetLeft), }; - ANDROID_SHELL_HOLDER->SetViewportMetrics(metrics); + ANDROID_SHELL_HOLDER->GetPlatformView()->SetViewportMetrics(metrics); } static jobject GetBitmap(JNIEnv* env, jobject jcaller, jlong shell_holder) { @@ -387,7 +387,8 @@ static void DispatchPointerDataPacket(JNIEnv* env, jint position) { uint8_t* data = static_cast(env->GetDirectBufferAddress(buffer)); auto packet = std::make_unique(data, position); - ANDROID_SHELL_HOLDER->DispatchPointerDataPacket(std::move(packet)); + ANDROID_SHELL_HOLDER->GetPlatformView()->DispatchPointerDataPacket( + std::move(packet)); } static void DispatchSemanticsAction(JNIEnv* env, From d3b153803b6c2625131dcecfe4ef56b193871ba1 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Tue, 9 Jul 2019 15:40:41 -0700 Subject: [PATCH 02/10] just SetViewportMetrics --- shell/platform/android/android_shell_holder.cc | 6 +++--- shell/platform/android/platform_view_android_jni.cc | 5 ++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index e9b114fc42252..a86f014de7c7b 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -179,9 +179,9 @@ void AndroidShellHolder::SetViewportMetrics( } shell_->GetTaskRunners().GetUITaskRunner()->PostTask( - [engine = shell_->GetEngine(), metrics]() { - if (engine) { - engine->SetViewportMetrics(metrics); + [platform_view = platform_view_, metrics]() { + if (platform_view) { + platform_view->SetViewportMetrics(metrics); } }); } diff --git a/shell/platform/android/platform_view_android_jni.cc b/shell/platform/android/platform_view_android_jni.cc index f25ddefb9becc..eaeb349d5ef8a 100644 --- a/shell/platform/android/platform_view_android_jni.cc +++ b/shell/platform/android/platform_view_android_jni.cc @@ -276,7 +276,7 @@ static void SetViewportMetrics(JNIEnv* env, static_cast(physicalViewInsetLeft), }; - ANDROID_SHELL_HOLDER->GetPlatformView()->SetViewportMetrics(metrics); + ANDROID_SHELL_HOLDER->SetViewportMetrics(metrics); } static jobject GetBitmap(JNIEnv* env, jobject jcaller, jlong shell_holder) { @@ -387,8 +387,7 @@ static void DispatchPointerDataPacket(JNIEnv* env, jint position) { uint8_t* data = static_cast(env->GetDirectBufferAddress(buffer)); auto packet = std::make_unique(data, position); - ANDROID_SHELL_HOLDER->GetPlatformView()->DispatchPointerDataPacket( - std::move(packet)); + ANDROID_SHELL_HOLDER->DispatchPointerDataPacket(std::move(packet)); } static void DispatchSemanticsAction(JNIEnv* env, From d6e773d41d6a741a1275b269f8ffbf9e3c0484ec Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 10 Jul 2019 12:23:11 -0700 Subject: [PATCH 03/10] Don't give shell holders access to the engine --- runtime/runtime_controller.cc | 14 ---- runtime/runtime_controller.h | 4 - shell/common/engine.cc | 8 -- shell/common/engine.h | 2 - shell/common/shell.cc | 18 ++-- shell/common/shell.h | 18 ++-- shell/common/shell_test.cc | 17 ++-- .../platform/android/android_shell_holder.cc | 44 ++-------- shell/platform/android/android_shell_holder.h | 5 -- .../android/platform_view_android_jni.cc | 5 +- .../ios/framework/Source/FlutterEngine.mm | 25 ++---- shell/platform/embedder/embedder_engine.cc | 84 ++++++++----------- shell/platform/embedder/embedder_engine.h | 1 - shell/testing/tester_main.cc | 19 ++--- 14 files changed, 92 insertions(+), 172 deletions(-) diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index e457f5986e1e7..78fcff6acd459 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -348,20 +348,6 @@ std::string RuntimeController::GetIsolateName() { return root_isolate ? root_isolate->debug_name() : ""; } -bool RuntimeController::HasLivePorts() { - std::shared_ptr root_isolate = root_isolate_.lock(); - if (!root_isolate) { - return false; - } - tonic::DartState::Scope scope(root_isolate); - return Dart_HasLivePorts(); -} - -tonic::DartErrorHandleType RuntimeController::GetLastError() { - std::shared_ptr root_isolate = root_isolate_.lock(); - return root_isolate ? root_isolate->GetLastError() : tonic::kNoError; -} - std::weak_ptr RuntimeController::GetRootIsolate() { return root_isolate_; } diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index c3b3e35fa064c..aed7d89e38720 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -78,10 +78,6 @@ class RuntimeController final : public WindowClient { std::string GetIsolateName(); - bool HasLivePorts(); - - tonic::DartErrorHandleType GetLastError(); - std::weak_ptr GetRootIsolate(); std::pair GetRootIsolateReturnCode(); diff --git a/shell/common/engine.cc b/shell/common/engine.cc index d2c752a4747df..04558cfc1498f 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -237,14 +237,6 @@ std::string Engine::GetUIIsolateName() { return runtime_controller_->GetIsolateName(); } -bool Engine::UIIsolateHasLivePorts() { - return runtime_controller_->HasLivePorts(); -} - -tonic::DartErrorHandleType Engine::GetUIIsolateLastError() { - return runtime_controller_->GetLastError(); -} - void Engine::OnOutputSurfaceCreated() { have_surface_ = true; StartAnimatorIfPossible(); diff --git a/shell/common/engine.h b/shell/common/engine.h index 95739c9b2bf61..6ec2845f2ca5d 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -100,8 +100,6 @@ class Engine final : public RuntimeDelegate { bool UIIsolateHasLivePorts(); - tonic::DartErrorHandleType GetUIIsolateLastError(); - std::pair GetUIIsolateReturnCode(); void OnOutputSurfaceCreated(); diff --git a/shell/common/shell.cc b/shell/common/shell.cc index a50ef70afb0ab..d6c1bfb0b72ee 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -380,6 +380,17 @@ void Shell::NotifyLowMemoryWarning() const { // to purge them. } +Engine::RunStatus Shell::RunEngine(RunConfiguration run_configuration) { + FML_DCHECK(is_setup_); + FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread()); + + if (!weak_engine_) { + return Engine::RunStatus::Failure; + } + + return weak_engine_->Run(std::move(run_configuration)); +} + bool Shell::IsSetup() const { return is_setup_; } @@ -433,11 +444,6 @@ fml::WeakPtr Shell::GetRasterizer() { return weak_rasterizer_; } -fml::WeakPtr Shell::GetEngine() { - FML_DCHECK(is_setup_); - return weak_engine_; -} - fml::WeakPtr Shell::GetPlatformView() { FML_DCHECK(is_setup_); return weak_platform_view_; @@ -915,7 +921,7 @@ void Shell::ReportTimings() { auto timings = std::move(unreported_timings_); unreported_timings_ = {}; - task_runners_.GetUITaskRunner()->PostTask([timings, engine = GetEngine()] { + task_runners_.GetUITaskRunner()->PostTask([timings, engine = weak_engine_] { if (engine) { engine->ReportTimings(std::move(timings)); } diff --git a/shell/common/shell.h b/shell/common/shell.h index db188aab63c0d..0a05a2c1822a5 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -164,6 +164,17 @@ class Shell final : public PlatformView::Delegate, /// ~Shell(); + //---------------------------------------------------------------------------- + /// @brief Starts an isolate for the given RunConfiguration. + /// + /// @return The status of the run operation. Re-entrant calls will return + /// Engine::RunStatus::FailureAlreadyRunning. Otherwise, calls + /// return Engine::RunStatus::Success on success or + /// Engine::RunStatus::Failure on failure. + /// + FML_WARN_UNUSED_RESULT + Engine::RunStatus RunEngine(RunConfiguration run_configuration); + //------------------------------------------------------------------------------ /// @return The settings used to launch this shell. /// @@ -190,13 +201,6 @@ class Shell final : public PlatformView::Delegate, /// fml::WeakPtr GetRasterizer(); - //------------------------------------------------------------------------------ - /// @brief Engines may only be accessed on the UI thread. - /// - /// @return A weak pointer to the engine. - /// - fml::WeakPtr GetEngine(); - //---------------------------------------------------------------------------- /// @brief Platform views may only be accessed on the platform task /// runner. diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc index 9afd64ad165f3..fee072c5570d0 100644 --- a/shell/common/shell_test.cc +++ b/shell/common/shell_test.cc @@ -75,12 +75,13 @@ void ShellTest::RunEngine(Shell* shell, RunConfiguration configuration) { fml::AutoResetWaitableEvent latch; fml::TaskRunner::RunNowOrPostTask( shell->GetTaskRunners().GetUITaskRunner(), - fml::MakeCopyable([&latch, config = std::move(configuration), - engine = shell->GetEngine()]() mutable { - ASSERT_TRUE(engine); - ASSERT_EQ(engine->Run(std::move(config)), Engine::RunStatus::Success); - latch.Signal(); - })); + fml::MakeCopyable( + [&latch, config = std::move(configuration), &shell]() mutable { + ASSERT_TRUE(shell); + ASSERT_EQ(shell->RunEngine(std::move(config)), + Engine::RunStatus::Success); + latch.Signal(); + })); latch.Wait(); } @@ -90,7 +91,7 @@ void ShellTest::PumpOneFrame(Shell* shell) { // won't be rasterized. fml::AutoResetWaitableEvent latch; shell->GetTaskRunners().GetUITaskRunner()->PostTask( - [&latch, engine = shell->GetEngine()]() { + [&latch, engine = shell->weak_engine_]() { engine->SetViewportMetrics({1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0}); engine->animator_->BeginFrame(fml::TimePoint::Now(), fml::TimePoint::Now()); @@ -100,7 +101,7 @@ void ShellTest::PumpOneFrame(Shell* shell) { latch.Reset(); // Call |Render| to rasterize a layer tree and trigger |OnFrameRasterized| - fml::WeakPtr runtime_delegate = shell->GetEngine(); + fml::WeakPtr runtime_delegate = shell->weak_engine_; shell->GetTaskRunners().GetUITaskRunner()->PostTask( [&latch, runtime_delegate]() { auto layer_tree = std::make_unique(); diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index a86f014de7c7b..c00df378138fe 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -156,14 +156,14 @@ void AndroidShellHolder::Launch(RunConfiguration config) { if (!IsValid()) { return; } - + const auto* shell = shell_; shell_->GetTaskRunners().GetUITaskRunner()->PostTask( - fml::MakeCopyable([engine = shell_->GetEngine(), // - config = std::move(config) // + fml::MakeCopyable([&shell, // + config = std::move(config) // ]() mutable { FML_LOG(INFO) << "Attempting to launch engine configuration..."; - if (!engine || - engine->Run(std::move(config)) == Engine::RunStatus::Failure) { + if (!shell || + shell->RunEngine(std::move(config)) == Engine::RunStatus::Failure) { FML_LOG(ERROR) << "Could not launch engine in configuration."; } else { FML_LOG(INFO) << "Isolate for engine configuration successfully " @@ -172,40 +172,6 @@ void AndroidShellHolder::Launch(RunConfiguration config) { })); } -void AndroidShellHolder::SetViewportMetrics( - const flutter::ViewportMetrics& metrics) { - if (!IsValid()) { - return; - } - - shell_->GetTaskRunners().GetUITaskRunner()->PostTask( - [platform_view = platform_view_, metrics]() { - if (platform_view) { - platform_view->SetViewportMetrics(metrics); - } - }); -} - -void AndroidShellHolder::DispatchPointerDataPacket( - std::unique_ptr packet) { - if (!IsValid()) { - return; - } - - TRACE_EVENT0("flutter", "AndroidShellHolder::DispatchPointerDataPacket"); - TRACE_FLOW_BEGIN("flutter", "PointerEvent", next_pointer_flow_id_); - - shell_->GetTaskRunners().GetUITaskRunner()->PostTask(fml::MakeCopyable( - [engine = shell_->GetEngine(), packet = std::move(packet), - flow_id = next_pointer_flow_id_] { - if (engine) { - engine->DispatchPointerDataPacket(*packet, flow_id); - } - })); - - next_pointer_flow_id_++; -} - Rasterizer::Screenshot AndroidShellHolder::Screenshot( Rasterizer::ScreenshotType type, bool base64_encode) { diff --git a/shell/platform/android/android_shell_holder.h b/shell/platform/android/android_shell_holder.h index ce966ebb6da66..83d92b77886b3 100644 --- a/shell/platform/android/android_shell_holder.h +++ b/shell/platform/android/android_shell_holder.h @@ -30,11 +30,6 @@ class AndroidShellHolder { void Launch(RunConfiguration configuration); - void SetViewportMetrics(const flutter::ViewportMetrics& metrics); - - void DispatchPointerDataPacket( - std::unique_ptr packet); - const flutter::Settings& GetSettings() const; fml::WeakPtr GetPlatformView(); diff --git a/shell/platform/android/platform_view_android_jni.cc b/shell/platform/android/platform_view_android_jni.cc index eaeb349d5ef8a..f25ddefb9becc 100644 --- a/shell/platform/android/platform_view_android_jni.cc +++ b/shell/platform/android/platform_view_android_jni.cc @@ -276,7 +276,7 @@ static void SetViewportMetrics(JNIEnv* env, static_cast(physicalViewInsetLeft), }; - ANDROID_SHELL_HOLDER->SetViewportMetrics(metrics); + ANDROID_SHELL_HOLDER->GetPlatformView()->SetViewportMetrics(metrics); } static jobject GetBitmap(JNIEnv* env, jobject jcaller, jlong shell_holder) { @@ -387,7 +387,8 @@ static void DispatchPointerDataPacket(JNIEnv* env, jint position) { uint8_t* data = static_cast(env->GetDirectBufferAddress(buffer)); auto packet = std::make_unique(data, position); - ANDROID_SHELL_HOLDER->DispatchPointerDataPacket(std::move(packet)); + ANDROID_SHELL_HOLDER->GetPlatformView()->DispatchPointerDataPacket( + std::move(packet)); } static void DispatchSemanticsAction(JNIEnv* env, diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index b4b0a54a0479b..cd8ba9c0194cb 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -63,8 +63,6 @@ @implementation FlutterEngine { int64_t _nextTextureId; - uint64_t _nextPointerFlowId; - BOOL _allowHeadlessExecution; FlutterBinaryMessengerRelay* _binaryMessenger; } @@ -126,24 +124,17 @@ - (void)dealloc { } - (void)updateViewportMetrics:(flutter::ViewportMetrics)viewportMetrics { - self.shell.GetTaskRunners().GetUITaskRunner()->PostTask( - [engine = self.shell.GetEngine(), metrics = viewportMetrics]() { - if (engine) { - engine->SetViewportMetrics(std::move(metrics)); - } - }); + if (!self.platformView) { + return; + } + self.platformView->updateViewportMetrics(std::move(viewportMetrics)); } - (void)dispatchPointerDataPacket:(std::unique_ptr)packet { - TRACE_EVENT0("flutter", "dispatchPointerDataPacket"); - TRACE_FLOW_BEGIN("flutter", "PointerEvent", _nextPointerFlowId); - self.shell.GetTaskRunners().GetUITaskRunner()->PostTask(fml::MakeCopyable( - [engine = self.shell.GetEngine(), packet = std::move(packet), flow_id = _nextPointerFlowId] { - if (engine) { - engine->DispatchPointerDataPacket(*packet, flow_id); - } - })); - _nextPointerFlowId++; + if (!self.platformView) { + return; + } + self.platformView->DispatchPointerDataPacket(*packet, flow_id); } - (fml::WeakPtr)platformView { diff --git a/shell/platform/embedder/embedder_engine.cc b/shell/platform/embedder/embedder_engine.cc index 1dd109bfd53eb..fca99efee9fad 100644 --- a/shell/platform/embedder/embedder_engine.cc +++ b/shell/platform/embedder/embedder_engine.cc @@ -63,13 +63,13 @@ bool EmbedderEngine::Run(RunConfiguration run_configuration) { if (!IsValid() || !run_configuration.IsValid()) { return false; } - + const auto& shell = shell_; shell_->GetTaskRunners().GetUITaskRunner()->PostTask( - fml::MakeCopyable([engine = shell_->GetEngine(), // engine + fml::MakeCopyable([&shell, // shell config = std::move(run_configuration) // config ]() mutable { - if (engine) { - auto result = engine->Run(std::move(config)); + if (shell) { + auto result = shell->RunEngine(std::move(config)); if (result == Engine::RunStatus::Failure) { FML_LOG(ERROR) << "Could not launch the engine with configuration."; } @@ -84,12 +84,11 @@ bool EmbedderEngine::SetViewportMetrics(flutter::ViewportMetrics metrics) { return false; } - shell_->GetTaskRunners().GetUITaskRunner()->PostTask( - [engine = shell_->GetEngine(), metrics = std::move(metrics)]() { - if (engine) { - engine->SetViewportMetrics(std::move(metrics)); - } - }); + auto platform_view = shell_->GetPlatformView(); + if (!platform_view) { + return false; + } + platform_view->SetViewportMetrics(std::move(metrics)); return true; } @@ -99,18 +98,12 @@ bool EmbedderEngine::DispatchPointerDataPacket( return false; } - TRACE_EVENT0("flutter", "EmbedderEngine::DispatchPointerDataPacket"); - TRACE_FLOW_BEGIN("flutter", "PointerEvent", next_pointer_flow_id_); - - shell_->GetTaskRunners().GetUITaskRunner()->PostTask(fml::MakeCopyable( - [engine = shell_->GetEngine(), packet = std::move(packet), - flow_id = next_pointer_flow_id_] { - if (engine) { - engine->DispatchPointerDataPacket(*packet, flow_id); - } - })); - next_pointer_flow_id_++; + auto platform_view = shell_->GetPlatformView(); + if (!platform_view) { + return false; + } + platform_view->DispatchPointerDataPacket(std::move(packet)); return true; } @@ -120,13 +113,12 @@ bool EmbedderEngine::SendPlatformMessage( return false; } - shell_->GetTaskRunners().GetUITaskRunner()->PostTask( - [engine = shell_->GetEngine(), message] { - if (engine) { - engine->DispatchPlatformMessage(message); - } - }); + auto platform_view = shell_->GetPlatformView(); + if (!platform_view) { + return false; + } + platform_view->DispatchPlatformMessage(message); return true; } @@ -160,12 +152,12 @@ bool EmbedderEngine::SetSemanticsEnabled(bool enabled) { if (!IsValid()) { return false; } - shell_->GetTaskRunners().GetUITaskRunner()->PostTask( - [engine = shell_->GetEngine(), enabled] { - if (engine) { - engine->SetSemanticsEnabled(enabled); - } - }); + + auto platform_view = shell_->GetPlatformView(); + if (!platform_view) { + return false; + } + platform_view->SetSemanticsEnabled(enabled); return true; } @@ -173,12 +165,11 @@ bool EmbedderEngine::SetAccessibilityFeatures(int32_t flags) { if (!IsValid()) { return false; } - shell_->GetTaskRunners().GetUITaskRunner()->PostTask( - [engine = shell_->GetEngine(), flags] { - if (engine) { - engine->SetAccessibilityFeatures(flags); - } - }); + auto platform_view = shell_->GetPlatformView(); + if (!platform_view) { + return false; + } + platform_view->SetAccessibilityFeatures(flags); return true; } @@ -188,16 +179,11 @@ bool EmbedderEngine::DispatchSemanticsAction(int id, if (!IsValid()) { return false; } - shell_->GetTaskRunners().GetUITaskRunner()->PostTask( - fml::MakeCopyable([engine = shell_->GetEngine(), // engine - id, // id - action, // action - args = std::move(args) // args - ]() mutable { - if (engine) { - engine->DispatchSemanticsAction(id, action, std::move(args)); - } - })); + auto platform_view = shell_->GetPlatformView(); + if (!platform_view) { + return false; + } + platform_view->DispatchSemanticsAction(id, action, std::move(args)); return true; } diff --git a/shell/platform/embedder/embedder_engine.h b/shell/platform/embedder/embedder_engine.h index dfae111ba6e81..8ef64502f98cc 100644 --- a/shell/platform/embedder/embedder_engine.h +++ b/shell/platform/embedder/embedder_engine.h @@ -78,7 +78,6 @@ class EmbedderEngine { const EmbedderExternalTextureGL::ExternalTextureCallback external_texture_callback_; bool is_valid_ = false; - uint64_t next_pointer_flow_id_ = 0; FML_DISALLOW_COPY_AND_ASSIGN(EmbedderEngine); }; diff --git a/shell/testing/tester_main.cc b/shell/testing/tester_main.cc index e36ba8252ab47..72e1aecd562d1 100644 --- a/shell/testing/tester_main.cc +++ b/shell/testing/tester_main.cc @@ -30,8 +30,7 @@ class ScriptCompletionTaskObserver { ScriptCompletionTaskObserver(Shell& shell, fml::RefPtr main_task_runner, bool run_forever) - : engine_(shell.GetEngine()), - main_task_runner_(std::move(main_task_runner)), + : main_task_runner_(std::move(main_task_runner)), run_forever_(run_forever) {} int GetExitCodeForLastError() const { @@ -52,9 +51,11 @@ class ScriptCompletionTaskObserver { } void DidProcessTask() { - if (engine_) { - last_error_ = engine_->GetUIIsolateLastError(); - if (engine_->UIIsolateHasLivePorts()) { + auto dart_state = UIDartState::Current(); + if (!dart_state) { + last_error_ = dart_state->GetLastError(); + tonic::DartState::Scope scope(dart_state); + if (Dart_HasLivePorts()) { // The UI isolate still has live ports and is running. Nothing to do // just yet. return; @@ -76,7 +77,6 @@ class ScriptCompletionTaskObserver { } private: - fml::WeakPtr engine_; fml::RefPtr main_task_runner_; bool run_forever_ = false; tonic::DartErrorHandleType last_error_ = tonic::kUnknownErrorType; @@ -178,14 +178,13 @@ int RunTester(const flutter::Settings& settings, bool run_forever) { fml::AutoResetWaitableEvent sync_run_latch; fml::TaskRunner::RunNowOrPostTask( shell->GetTaskRunners().GetUITaskRunner(), - fml::MakeCopyable([&sync_run_latch, &completion_observer, - engine = shell->GetEngine(), + fml::MakeCopyable([&sync_run_latch, &completion_observer, &shell, config = std::move(run_configuration), &engine_did_run]() mutable { fml::MessageLoop::GetCurrent().AddTaskObserver( reinterpret_cast(&completion_observer), [&completion_observer]() { completion_observer.DidProcessTask(); }); - if (engine->Run(std::move(config)) != + if (shell->RunEngine(std::move(config)) != flutter::Engine::RunStatus::Failure) { engine_did_run = true; @@ -193,7 +192,7 @@ int RunTester(const flutter::Settings& settings, bool run_forever) { metrics.device_pixel_ratio = 3.0; metrics.physical_width = 2400; // 800 at 3x resolution metrics.physical_height = 1800; // 600 at 3x resolution - engine->SetViewportMetrics(metrics); + shell->GetPlatformView()->SetViewportMetrics(metrics); } else { FML_DLOG(ERROR) << "Could not launch the engine with configuration."; From 650a3092d93996cf1b7d74a1d874fc5ab6d1a6ba Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 10 Jul 2019 14:51:40 -0700 Subject: [PATCH 04/10] make sure android and ios compile, move logging ot shell, run on platform thread --- shell/common/shell.cc | 35 ++++++++++++++++--- shell/common/shell.h | 14 ++++---- shell/common/shell_test.cc | 15 ++++---- .../platform/android/android_shell_holder.cc | 16 ++------- .../ios/framework/Source/FlutterEngine.mm | 18 +++------- shell/platform/embedder/embedder_engine.cc | 14 +------- shell/testing/tester_main.cc | 18 +++++----- 7 files changed, 59 insertions(+), 71 deletions(-) diff --git a/shell/common/shell.cc b/shell/common/shell.cc index d6c1bfb0b72ee..f672f6388f071 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -380,15 +380,40 @@ void Shell::NotifyLowMemoryWarning() const { // to purge them. } -Engine::RunStatus Shell::RunEngine(RunConfiguration run_configuration) { +void Shell::RunEngine(RunConfiguration run_configuration) { + RunEngine(std::move(run_configuration), nullptr); +} + +void Shell::RunEngine(RunConfiguration run_configuration, + std::function result_callback) { FML_DCHECK(is_setup_); - FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread()); + FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); if (!weak_engine_) { - return Engine::RunStatus::Failure; + result_callback(Engine::RunStatus::Failure); } - - return weak_engine_->Run(std::move(run_configuration)); + fml::TaskRunner::RunNowOrPostTask( + task_runners_.GetUITaskRunner(), + fml::MakeCopyable([run_configuration = std::move(run_configuration), + weak_engine = weak_engine_, + result_callback = + std::move(result_callback)]() mutable { + if (!weak_engine) { + if (result_callback) { + FML_LOG(ERROR) + << "Could not launch engine with configuration - no engine."; + result_callback(Engine::RunStatus::Failure); + } + return; + } + auto result = weak_engine->Run(std::move(run_configuration)); + if (result == flutter::Engine::RunStatus::Failure) { + FML_LOG(ERROR) << "Could not launch engine with configuration."; + } + if (result_callback) { + result_callback(result); + } + })); } bool Shell::IsSetup() const { diff --git a/shell/common/shell.h b/shell/common/shell.h index 0a05a2c1822a5..b706f76d80226 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -167,13 +167,15 @@ class Shell final : public PlatformView::Delegate, //---------------------------------------------------------------------------- /// @brief Starts an isolate for the given RunConfiguration. /// - /// @return The status of the run operation. Re-entrant calls will return - /// Engine::RunStatus::FailureAlreadyRunning. Otherwise, calls - /// return Engine::RunStatus::Success on success or - /// Engine::RunStatus::Failure on failure. + void RunEngine(RunConfiguration run_configuration); + + //---------------------------------------------------------------------------- + /// @brief Starts an isolate for the given RunConfiguration. The + /// result_callback will be called with the status of the + /// operation. /// - FML_WARN_UNUSED_RESULT - Engine::RunStatus RunEngine(RunConfiguration run_configuration); + void RunEngine(RunConfiguration run_configuration, + std::function result_callback); //------------------------------------------------------------------------------ /// @return The settings used to launch this shell. diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc index fee072c5570d0..006f6ec1b2cf4 100644 --- a/shell/common/shell_test.cc +++ b/shell/common/shell_test.cc @@ -73,15 +73,12 @@ void ShellTest::PlatformViewNotifyCreated(Shell* shell) { void ShellTest::RunEngine(Shell* shell, RunConfiguration configuration) { fml::AutoResetWaitableEvent latch; - fml::TaskRunner::RunNowOrPostTask( - shell->GetTaskRunners().GetUITaskRunner(), - fml::MakeCopyable( - [&latch, config = std::move(configuration), &shell]() mutable { - ASSERT_TRUE(shell); - ASSERT_EQ(shell->RunEngine(std::move(config)), - Engine::RunStatus::Success); - latch.Signal(); - })); + + shell->RunEngine(std::move(configuration), + [&latch](Engine::RunStatus run_status) { + ASSERT_EQ(run_status, Engine::RunStatus::Success); + latch.Signal(); + }); latch.Wait(); } diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index c00df378138fe..452e069bd5276 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -156,20 +156,8 @@ void AndroidShellHolder::Launch(RunConfiguration config) { if (!IsValid()) { return; } - const auto* shell = shell_; - shell_->GetTaskRunners().GetUITaskRunner()->PostTask( - fml::MakeCopyable([&shell, // - config = std::move(config) // - ]() mutable { - FML_LOG(INFO) << "Attempting to launch engine configuration..."; - if (!shell || - shell->RunEngine(std::move(config)) == Engine::RunStatus::Failure) { - FML_LOG(ERROR) << "Could not launch engine in configuration."; - } else { - FML_LOG(INFO) << "Isolate for engine configuration successfully " - "started and run."; - } - })); + + shell_->RunEngine(std::move(config)); } Rasterizer::Screenshot AndroidShellHolder::Screenshot( diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index cd8ba9c0194cb..cd57d30e1f8c2 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -127,14 +127,14 @@ - (void)updateViewportMetrics:(flutter::ViewportMetrics)viewportMetrics { if (!self.platformView) { return; } - self.platformView->updateViewportMetrics(std::move(viewportMetrics)); + self.platformView->SetViewportMetrics(std::move(viewportMetrics)); } - (void)dispatchPointerDataPacket:(std::unique_ptr)packet { if (!self.platformView) { return; } - self.platformView->DispatchPointerDataPacket(*packet, flow_id); + self.platformView->DispatchPointerDataPacket(std::move(packet)); } - (fml::WeakPtr)platformView { @@ -303,18 +303,8 @@ - (void)maybeSetupPlatformViewChannels { - (void)launchEngine:(NSString*)entrypoint libraryURI:(NSString*)libraryOrNil { // Launch the Dart application with the inferred run configuration. - self.shell.GetTaskRunners().GetUITaskRunner()->PostTask(fml::MakeCopyable( - [engine = _shell->GetEngine(), - config = [_dartProject.get() runConfigurationForEntrypoint:entrypoint - libraryOrNil:libraryOrNil] // - ]() mutable { - if (engine) { - auto result = engine->Run(std::move(config)); - if (result == flutter::Engine::RunStatus::Failure) { - FML_LOG(ERROR) << "Could not launch engine with configuration."; - } - } - })); + self.shell.RunEngine([_dartProject.get() runConfigurationForEntrypoint:entrypoint + libraryOrNil:libraryOrNil]); } - (BOOL)createShell:(NSString*)entrypoint libraryURI:(NSString*)libraryURI { diff --git a/shell/platform/embedder/embedder_engine.cc b/shell/platform/embedder/embedder_engine.cc index fca99efee9fad..6f675e785dabb 100644 --- a/shell/platform/embedder/embedder_engine.cc +++ b/shell/platform/embedder/embedder_engine.cc @@ -63,19 +63,7 @@ bool EmbedderEngine::Run(RunConfiguration run_configuration) { if (!IsValid() || !run_configuration.IsValid()) { return false; } - const auto& shell = shell_; - shell_->GetTaskRunners().GetUITaskRunner()->PostTask( - fml::MakeCopyable([&shell, // shell - config = std::move(run_configuration) // config - ]() mutable { - if (shell) { - auto result = shell->RunEngine(std::move(config)); - if (result == Engine::RunStatus::Failure) { - FML_LOG(ERROR) << "Could not launch the engine with configuration."; - } - } - })); - + shell_->RunEngine(std::move(run_configuration)); return true; } diff --git a/shell/testing/tester_main.cc b/shell/testing/tester_main.cc index 72e1aecd562d1..c3553d721cf5a 100644 --- a/shell/testing/tester_main.cc +++ b/shell/testing/tester_main.cc @@ -176,26 +176,24 @@ int RunTester(const flutter::Settings& settings, bool run_forever) { bool engine_did_run = false; fml::AutoResetWaitableEvent sync_run_latch; - fml::TaskRunner::RunNowOrPostTask( - shell->GetTaskRunners().GetUITaskRunner(), - fml::MakeCopyable([&sync_run_latch, &completion_observer, &shell, + shell->RunEngine( + std::move(run_configuration), + fml::MakeCopyable([&sync_run_latch, &completion_observer, + platform_view = shell->GetPlatformView(), config = std::move(run_configuration), - &engine_did_run]() mutable { + &engine_did_run]( + Engine::RunStatus run_status) mutable { fml::MessageLoop::GetCurrent().AddTaskObserver( reinterpret_cast(&completion_observer), [&completion_observer]() { completion_observer.DidProcessTask(); }); - if (shell->RunEngine(std::move(config)) != - flutter::Engine::RunStatus::Failure) { + if (run_status != flutter::Engine::RunStatus::Failure) { engine_did_run = true; flutter::ViewportMetrics metrics; metrics.device_pixel_ratio = 3.0; metrics.physical_width = 2400; // 800 at 3x resolution metrics.physical_height = 1800; // 600 at 3x resolution - shell->GetPlatformView()->SetViewportMetrics(metrics); - - } else { - FML_DLOG(ERROR) << "Could not launch the engine with configuration."; + platform_view->SetViewportMetrics(metrics); } sync_run_latch.Signal(); })); From 15d8eb630788ac497696ece820abf3a91eb81621 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 24 Jul 2019 10:17:12 -0700 Subject: [PATCH 05/10] Fix tester, fix test, add back ui isolate methods and expose through shell --- runtime/runtime_controller.cc | 14 +++++++++++++ runtime/runtime_controller.h | 4 ++++ shell/common/engine.cc | 8 ++++++++ shell/common/shell.cc | 30 ++++++++++++++++++++++++++- shell/common/shell.h | 26 ++++++++++++++++++++++++ shell/common/shell_test.cc | 15 ++++++++------ shell/testing/tester_main.cc | 38 ++++++++++------------------------- 7 files changed, 101 insertions(+), 34 deletions(-) diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index 92a9360237403..7cc771ff8dd23 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -342,6 +342,20 @@ std::string RuntimeController::GetIsolateName() { return root_isolate ? root_isolate->debug_name() : ""; } +bool RuntimeController::HasLivePorts() { + std::shared_ptr root_isolate = root_isolate_.lock(); + if (!root_isolate) { + return false; + } + tonic::DartState::Scope scope(root_isolate); + return Dart_HasLivePorts(); +} + +tonic::DartErrorHandleType RuntimeController::GetLastError() { + std::shared_ptr root_isolate = root_isolate_.lock(); + return root_isolate ? root_isolate->GetLastError() : tonic::kNoError; +} + std::weak_ptr RuntimeController::GetRootIsolate() { return root_isolate_; } diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index a3c48d21b5c94..845bc45ef3b77 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -77,6 +77,10 @@ class RuntimeController final : public WindowClient { std::string GetIsolateName(); + bool HasLivePorts(); + + tonic::DartErrorHandleType GetLastError(); + std::weak_ptr GetRootIsolate(); std::pair GetRootIsolateReturnCode(); diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 8a85b8c47ee23..ad8237862c0c3 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -235,6 +235,14 @@ std::string Engine::GetUIIsolateName() { return runtime_controller_->GetIsolateName(); } +bool Engine::UIIsolateHasLivePorts() { + return runtime_controller_->HasLivePorts(); +} + +tonic::DartErrorHandleType Engine::GetUIIsolateLastError() { + return runtime_controller_->GetLastError(); +} + void Engine::OnOutputSurfaceCreated() { have_surface_ = true; StartAnimatorIfPossible(); diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 0c253ebcc8b39..f15f1d1a739b8 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -3,7 +3,6 @@ // found in the LICENSE file. #define RAPIDJSON_HAS_STDSTRING 1 - #include "flutter/shell/common/shell.h" #include @@ -408,6 +407,35 @@ void Shell::RunEngine(RunConfiguration run_configuration, })); } +std::optional Shell::GetUIIsolateLastError() const { + FML_DCHECK(is_setup_); + FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); + + if (!weak_engine_) { + return std::nullopt; + } + switch (weak_engine_->GetUIIsolateLastError()) { + case tonic::kCompilationErrorType: + return kCompilationErrorExitCode; + case tonic::kApiErrorType: + return kApiErrorExitCode; + case tonic::kUnknownErrorType: + return kErrorExitCode; + default: + return 0; + } +} + +bool Shell::EngineHasLivePorts() const { + FML_DCHECK(is_setup_); + FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); + + if (!weak_engine_) { + return false; + } + return weak_engine_->UIIsolateHasLivePorts(); +} + bool Shell::IsSetup() const { return is_setup_; } diff --git a/shell/common/shell.h b/shell/common/shell.h index 845efee24673f..77bc81e8c0cf7 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -78,6 +78,13 @@ class Shell final : public PlatformView::Delegate, public Rasterizer::Delegate, public ServiceProtocol::Handler { public: + /// The Dart error code for an API error. + const int kApiErrorExitCode = 253; + /// The Dart error code for a compilation error. + const int kCompilationErrorExitCode = 254; + /// The Dart error code for an unkonwn error. + const int kErrorExitCode = 255; + template using CreateCallback = std::function(Shell&)>; @@ -259,6 +266,25 @@ class Shell final : public PlatformView::Delegate, /// fml::Status WaitForFirstFrame(fml::TimeDelta timeout); + //---------------------------------------------------------------------------- + /// @brief Used by embedders to get the last error from the Dart UI + /// Isolate, if one exists. + /// + /// @return Returns the last error code from the UI Isolate. + /// + std::optional GetUIIsolateLastError() const; + + //---------------------------------------------------------------------------- + /// @brief Used by embedders to check if the Engine is running and has + /// any live ports remaining. For example, the Flutter tester uses + /// this method to check whether it should continue to wait for + /// a running test or not. + /// + /// @return Returns if the shell has an engine and the engine has any live + /// Dart ports. + /// + bool EngineHasLivePorts() const; + private: using ServiceProtocolHandler = std::functionRunEngine(std::move(configuration), - [&latch](Engine::RunStatus run_status) { - ASSERT_EQ(run_status, Engine::RunStatus::Success); - latch.Signal(); - }); + fml::TaskRunner::RunNowOrPostTask( + shell->GetTaskRunners().GetPlatformTaskRunner(), + [shell, &latch, &configuration]() { + shell->RunEngine(std::move(configuration), + [&latch](Engine::RunStatus run_status) { + ASSERT_EQ(run_status, Engine::RunStatus::Success); + latch.Signal(); + }); + }); latch.Wait(); } diff --git a/shell/testing/tester_main.cc b/shell/testing/tester_main.cc index 8b7f748d0e5fb..f03aca8ecb4e8 100644 --- a/shell/testing/tester_main.cc +++ b/shell/testing/tester_main.cc @@ -20,6 +20,7 @@ #include "flutter/shell/common/switches.h" #include "flutter/shell/common/thread_host.h" #include "third_party/dart/runtime/include/bin/dart_io_api.h" +#include "third_party/dart/runtime/include/dart_api.h" namespace flutter { @@ -30,36 +31,18 @@ class ScriptCompletionTaskObserver { ScriptCompletionTaskObserver(Shell& shell, fml::RefPtr main_task_runner, bool run_forever) - : main_task_runner_(std::move(main_task_runner)), + : shell_(shell), + main_task_runner_(std::move(main_task_runner)), run_forever_(run_forever) {} - int GetExitCodeForLastError() const { - // Exit codes used by the Dart command line tool. - const int kApiErrorExitCode = 253; - const int kCompilationErrorExitCode = 254; - const int kErrorExitCode = 255; - switch (last_error_) { - case tonic::kCompilationErrorType: - return kCompilationErrorExitCode; - case tonic::kApiErrorType: - return kApiErrorExitCode; - case tonic::kUnknownErrorType: - return kErrorExitCode; - default: - return 0; - } - } + int GetExitCodeForLastError() const { return last_error_.value_or(0); } void DidProcessTask() { - auto dart_state = UIDartState::Current(); - if (!dart_state) { - last_error_ = dart_state->GetLastError(); - tonic::DartState::Scope scope(dart_state); - if (Dart_HasLivePorts()) { - // The UI isolate still has live ports and is running. Nothing to do - // just yet. - return; - } + last_error_ = shell_.GetUIIsolateLastError(); + if (shell_.EngineHasLivePorts()) { + // The UI isolate still has live ports and is running. Nothing to do + // just yet. + return; } if (run_forever_) { @@ -77,9 +60,10 @@ class ScriptCompletionTaskObserver { } private: + Shell& shell_; fml::RefPtr main_task_runner_; bool run_forever_ = false; - tonic::DartErrorHandleType last_error_ = tonic::kUnknownErrorType; + std::optional last_error_; bool has_terminated = false; FML_DISALLOW_COPY_AND_ASSIGN(ScriptCompletionTaskObserver); From df66441945862cff0025d2a746718acc7235dd7a Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 24 Jul 2019 10:22:07 -0700 Subject: [PATCH 06/10] add it back and deprecate --- shell/common/shell.cc | 5 +++++ shell/common/shell.h | 11 +++++++++++ 2 files changed, 16 insertions(+) diff --git a/shell/common/shell.cc b/shell/common/shell.cc index f15f1d1a739b8..0457989c57351 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -489,6 +489,11 @@ fml::WeakPtr Shell::GetRasterizer() { return weak_rasterizer_; } +fml::WeakPtr Shell::GetEngine() { + FML_DCHECK(is_setup_); + return weak_engine_; +} + fml::WeakPtr Shell::GetPlatformView() { FML_DCHECK(is_setup_); return weak_platform_view_; diff --git a/shell/common/shell.h b/shell/common/shell.h index 77bc81e8c0cf7..78fed37e23fc1 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -211,6 +211,17 @@ class Shell final : public PlatformView::Delegate, /// fml::WeakPtr GetRasterizer(); + //------------------------------------------------------------------------------ + /// @brief Engines may only be accessed on the UI thread. This method is + /// deprecated, and implementers should instead use other API + /// available on the Shell or the PlatformView. + /// + /// @return A weak pointer to the engine. + /// + [[deprecated( + "This will be removed - API expecting to call this should use " + "functionality exposed through the Shell instead.")]] fml::WeakPtr + GetEngine(); //---------------------------------------------------------------------------- /// @brief Platform views may only be accessed on the platform task /// runner. From f54322394db55a56f76c83b59ff04e0365023443 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 24 Jul 2019 10:41:47 -0700 Subject: [PATCH 07/10] undeprecate, guard for fuchsia --- shell/common/shell.cc | 4 ++++ shell/common/shell.h | 10 ++++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 0457989c57351..e938ee13aea96 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -489,10 +489,14 @@ fml::WeakPtr Shell::GetRasterizer() { return weak_rasterizer_; } +// TODO(dnfield): Remove this when either Topaz is up to date or flutter_runner +// is built out of this repo. +#ifdef OS_FUCHSIA fml::WeakPtr Shell::GetEngine() { FML_DCHECK(is_setup_); return weak_engine_; } +#endif // OS_FUCHSIA fml::WeakPtr Shell::GetPlatformView() { FML_DCHECK(is_setup_); diff --git a/shell/common/shell.h b/shell/common/shell.h index 78fed37e23fc1..71c5193750e0a 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -211,6 +211,9 @@ class Shell final : public PlatformView::Delegate, /// fml::WeakPtr GetRasterizer(); +// TODO(dnfield): Remove this when either Topaz is up to date or flutter_runner +// is built out of this repo. +#ifdef OS_FUCHSIA //------------------------------------------------------------------------------ /// @brief Engines may only be accessed on the UI thread. This method is /// deprecated, and implementers should instead use other API @@ -218,10 +221,9 @@ class Shell final : public PlatformView::Delegate, /// /// @return A weak pointer to the engine. /// - [[deprecated( - "This will be removed - API expecting to call this should use " - "functionality exposed through the Shell instead.")]] fml::WeakPtr - GetEngine(); + fml::WeakPtr GetEngine(); +#endif // OS_FUCHSIA + //---------------------------------------------------------------------------- /// @brief Platform views may only be accessed on the platform task /// runner. From e510c32c1c6b899bd0a0f3dbbde05f4b65d02701 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Tue, 30 Jul 2019 13:14:41 -0700 Subject: [PATCH 08/10] review --- shell/common/shell.cc | 70 ++++++++++++++++++++---------------- shell/common/shell.h | 21 ++++++----- shell/testing/tester_main.cc | 6 ++-- 3 files changed, 56 insertions(+), 41 deletions(-) diff --git a/shell/common/shell.cc b/shell/common/shell.cc index e938ee13aea96..7bba32b761a83 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -377,63 +377,71 @@ void Shell::RunEngine(RunConfiguration run_configuration) { void Shell::RunEngine(RunConfiguration run_configuration, std::function result_callback) { + auto result = [platform_runner = task_runners_.GetPlatformTaskRunner(), + result_callback](Engine::RunStatus run_result) { + if (!result_callback) { + return; + } + platform_runner->PostTask( + [result_callback, run_result]() { result_callback(run_result); }); + }; FML_DCHECK(is_setup_); FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); if (!weak_engine_) { - result_callback(Engine::RunStatus::Failure); + result(Engine::RunStatus::Failure); } fml::TaskRunner::RunNowOrPostTask( task_runners_.GetUITaskRunner(), - fml::MakeCopyable([run_configuration = std::move(run_configuration), - weak_engine = weak_engine_, - result_callback = - std::move(result_callback)]() mutable { - if (!weak_engine) { - if (result_callback) { - FML_LOG(ERROR) - << "Could not launch engine with configuration - no engine."; - result_callback(Engine::RunStatus::Failure); - } - return; - } - auto result = weak_engine->Run(std::move(run_configuration)); - if (result == flutter::Engine::RunStatus::Failure) { - FML_LOG(ERROR) << "Could not launch engine with configuration."; - } - if (result_callback) { - result_callback(result); - } - })); + fml::MakeCopyable( + [run_configuration = std::move(run_configuration), + weak_engine = weak_engine_, result]() mutable { + if (!weak_engine) { + FML_LOG(ERROR) + << "Could not launch engine with configuration - no engine."; + result(Engine::RunStatus::Failure); + return; + } + auto run_result = weak_engine->Run(std::move(run_configuration)); + if (run_result == flutter::Engine::RunStatus::Failure) { + FML_LOG(ERROR) << "Could not launch engine with configuration."; + } + result(run_result); + })); } -std::optional Shell::GetUIIsolateLastError() const { +std::optional Shell::GetUIIsolateLastError() const { FML_DCHECK(is_setup_); FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); - if (!weak_engine_) { + // We're using the unique_ptr here because we're sure we're on the Platform + // Thread and callers expect this to be synchronous. + if (!engine_) { return std::nullopt; } - switch (weak_engine_->GetUIIsolateLastError()) { + switch (engine_->GetUIIsolateLastError()) { case tonic::kCompilationErrorType: - return kCompilationErrorExitCode; + return DartErrorCode::CompilationError; case tonic::kApiErrorType: - return kApiErrorExitCode; + return DartErrorCode::ApiError; case tonic::kUnknownErrorType: - return kErrorExitCode; - default: - return 0; + return DartErrorCode::UnknownError; + case tonic::kNoError: + return DartErrorCode::NoError; } + return DartErrorCode::UnknownError; } bool Shell::EngineHasLivePorts() const { FML_DCHECK(is_setup_); FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); - if (!weak_engine_) { + // We're using the unique_ptr here because we're sure we're on the Platform + // Thread and callers expect this to be synchronous. + if (!engine_) { return false; } - return weak_engine_->UIIsolateHasLivePorts(); + return engine_->UIIsolateHasLivePorts(); } bool Shell::IsSetup() const { diff --git a/shell/common/shell.h b/shell/common/shell.h index 71c5193750e0a..4f635e3618ad8 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -35,6 +35,18 @@ namespace flutter { +/// Error exit codes for the Dart isolate. +enum class DartErrorCode { + /// No error has occurred. + NoError = 0, + /// The Dart error code for an API error. + ApiError = 253, + /// The Dart error code for a compilation error. + CompilationError = 254, + /// The Dart error code for an unkonwn error. + UnknownError = 255 +}; + //------------------------------------------------------------------------------ /// Perhaps the single most important class in the Flutter engine repository. /// When embedders create a Flutter application, they are referring to the @@ -78,13 +90,6 @@ class Shell final : public PlatformView::Delegate, public Rasterizer::Delegate, public ServiceProtocol::Handler { public: - /// The Dart error code for an API error. - const int kApiErrorExitCode = 253; - /// The Dart error code for a compilation error. - const int kCompilationErrorExitCode = 254; - /// The Dart error code for an unkonwn error. - const int kErrorExitCode = 255; - template using CreateCallback = std::function(Shell&)>; @@ -285,7 +290,7 @@ class Shell final : public PlatformView::Delegate, /// /// @return Returns the last error code from the UI Isolate. /// - std::optional GetUIIsolateLastError() const; + std::optional GetUIIsolateLastError() const; //---------------------------------------------------------------------------- /// @brief Used by embedders to check if the Engine is running and has diff --git a/shell/testing/tester_main.cc b/shell/testing/tester_main.cc index f03aca8ecb4e8..9eb36c8e15a43 100644 --- a/shell/testing/tester_main.cc +++ b/shell/testing/tester_main.cc @@ -35,7 +35,9 @@ class ScriptCompletionTaskObserver { main_task_runner_(std::move(main_task_runner)), run_forever_(run_forever) {} - int GetExitCodeForLastError() const { return last_error_.value_or(0); } + int GetExitCodeForLastError() const { + return static_cast(last_error_.value_or(DartErrorCode::NoError)); + } void DidProcessTask() { last_error_ = shell_.GetUIIsolateLastError(); @@ -63,7 +65,7 @@ class ScriptCompletionTaskObserver { Shell& shell_; fml::RefPtr main_task_runner_; bool run_forever_ = false; - std::optional last_error_; + std::optional last_error_; bool has_terminated = false; FML_DISALLOW_COPY_AND_ASSIGN(ScriptCompletionTaskObserver); From 90367504c87d932211d658ae2321d694af1f42e3 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Tue, 30 Jul 2019 16:48:03 -0700 Subject: [PATCH 09/10] fix deadlock in tester_main --- shell/testing/tester_main.cc | 39 +++++++++++++++--------------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/shell/testing/tester_main.cc b/shell/testing/tester_main.cc index 9eb36c8e15a43..0b7b290b09680 100644 --- a/shell/testing/tester_main.cc +++ b/shell/testing/tester_main.cc @@ -161,29 +161,22 @@ int RunTester(const flutter::Settings& settings, bool run_forever) { bool engine_did_run = false; - fml::AutoResetWaitableEvent sync_run_latch; - shell->RunEngine( - std::move(run_configuration), - fml::MakeCopyable([&sync_run_latch, &completion_observer, - platform_view = shell->GetPlatformView(), - config = std::move(run_configuration), - &engine_did_run]( - Engine::RunStatus run_status) mutable { - fml::MessageLoop::GetCurrent().AddTaskObserver( - reinterpret_cast(&completion_observer), - [&completion_observer]() { completion_observer.DidProcessTask(); }); - if (run_status != flutter::Engine::RunStatus::Failure) { - engine_did_run = true; - - flutter::ViewportMetrics metrics; - metrics.device_pixel_ratio = 3.0; - metrics.physical_width = 2400; // 800 at 3x resolution - metrics.physical_height = 1800; // 600 at 3x resolution - platform_view->SetViewportMetrics(metrics); - } - sync_run_latch.Signal(); - })); - sync_run_latch.Wait(); + fml::MessageLoop::GetCurrent().AddTaskObserver( + reinterpret_cast(&completion_observer), + [&completion_observer]() { completion_observer.DidProcessTask(); }); + + shell->RunEngine(std::move(run_configuration), + [&engine_did_run](Engine::RunStatus run_status) mutable { + if (run_status != flutter::Engine::RunStatus::Failure) { + engine_did_run = true; + } + }); + + flutter::ViewportMetrics metrics; + metrics.device_pixel_ratio = 3.0; + metrics.physical_width = 2400; // 800 at 3x resolution + metrics.physical_height = 1800; // 600 at 3x resolution + shell->GetPlatformView()->SetViewportMetrics(metrics); // Run the message loop and wait for the script to do its thing. fml::MessageLoop::GetCurrent().Run(); From d8d508ceeb9122c08e9a4e4389fe7ee42a8725b1 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Tue, 30 Jul 2019 16:49:49 -0700 Subject: [PATCH 10/10] One miss --- shell/testing/tester_main.cc | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/shell/testing/tester_main.cc b/shell/testing/tester_main.cc index 0b7b290b09680..52f6b15590f6c 100644 --- a/shell/testing/tester_main.cc +++ b/shell/testing/tester_main.cc @@ -183,15 +183,8 @@ int RunTester(const flutter::Settings& settings, bool run_forever) { // Cleanup the completion observer synchronously as it is living on the // stack. - fml::AutoResetWaitableEvent latch; - fml::TaskRunner::RunNowOrPostTask( - shell->GetTaskRunners().GetUITaskRunner(), - [&latch, &completion_observer] { - fml::MessageLoop::GetCurrent().RemoveTaskObserver( - reinterpret_cast(&completion_observer)); - latch.Signal(); - }); - latch.Wait(); + fml::MessageLoop::GetCurrent().RemoveTaskObserver( + reinterpret_cast(&completion_observer)); if (!engine_did_run) { // If the engine itself didn't have a chance to run, there is no point in