From cf7899dde476a2f02ef4826b10ca9ddf5541dc95 Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Fri, 27 Jan 2017 09:59:08 -0800 Subject: [PATCH] Return an exit code from sky_shell representing what kind of error occurred (flutter/engine#3368) This is intended to match the exit codes returned by the Dart command line tool --- DEPS | 2 +- engine/src/flutter/runtime/dart_controller.cc | 9 ++-- engine/src/flutter/runtime/dart_controller.h | 4 +- .../src/flutter/runtime/runtime_controller.cc | 12 ++++++ .../src/flutter/runtime/runtime_controller.h | 3 +- engine/src/flutter/shell/common/engine.cc | 14 ++++++- engine/src/flutter/shell/common/engine.h | 5 ++- .../src/flutter/shell/platform/linux/BUILD.gn | 1 + .../shell/platform/linux/main_linux.cc | 41 +++++++++++++++++-- 9 files changed, 78 insertions(+), 13 deletions(-) diff --git a/DEPS b/DEPS index a2c58bb344a..9c78db96922 100644 --- a/DEPS +++ b/DEPS @@ -58,7 +58,7 @@ deps = { Var('fuchsia_git') + '/ftl' + '@' + 'bb82d5f52ecca65650817a1c31c1f49eca54237a', 'src/lib/tonic': - Var('fuchsia_git') + '/tonic' + '@' + '4214b35e02a1286a5fb98895d0c480fa0da10f6d', + Var('fuchsia_git') + '/tonic' + '@' + '06659ecdf45787d5dde31b4dbc79a32dae626687', 'src/lib/zip': Var('fuchsia_git') + '/zip' + '@' + '92dc87ca645fe8e9f5151ef6dac86d8311a7222f', diff --git a/engine/src/flutter/runtime/dart_controller.cc b/engine/src/flutter/runtime/dart_controller.cc index 947b1db3e25..721bb9d214d 100644 --- a/engine/src/flutter/runtime/dart_controller.cc +++ b/engine/src/flutter/runtime/dart_controller.cc @@ -120,15 +120,18 @@ void DartController::RunFromSnapshot(const uint8_t* buffer, size_t size) { exit(1); } -void DartController::RunFromSource(const std::string& main, - const std::string& packages) { +tonic::DartErrorHandleType DartController::RunFromSource( + const std::string& main, const std::string& packages) { tonic::DartState::Scope scope(dart_state()); tonic::FileLoader& loader = dart_state()->file_loader(); if (!packages.empty() && !loader.LoadPackagesMap(ResolvePath(packages))) FTL_LOG(WARNING) << "Failed to load package map: " << packages; - LogIfError(loader.LoadScript(main)); + Dart_Handle result = loader.LoadScript(main); + LogIfError(result); + tonic::DartErrorHandleType error = tonic::GetErrorHandleType(result); if (SendStartMessage(Dart_RootLibrary())) exit(1); + return error; } void DartController::CreateIsolateFor(const std::string& script_uri, diff --git a/engine/src/flutter/runtime/dart_controller.h b/engine/src/flutter/runtime/dart_controller.h index a9c560882a7..bdf2afd111c 100644 --- a/engine/src/flutter/runtime/dart_controller.h +++ b/engine/src/flutter/runtime/dart_controller.h @@ -9,6 +9,7 @@ #include "dart/runtime/include/dart_api.h" #include "lib/ftl/macros.h" +#include "lib/tonic/logging/dart_error.h" namespace blink { class UIDartState; @@ -20,7 +21,8 @@ class DartController { void RunFromPrecompiledSnapshot(); void RunFromSnapshot(const uint8_t* buffer, size_t size); - void RunFromSource(const std::string& main, const std::string& packages); + tonic::DartErrorHandleType RunFromSource(const std::string& main, + const std::string& packages); void CreateIsolateFor(const std::string& script_uri, std::unique_ptr ui_dart_state); diff --git a/engine/src/flutter/runtime/runtime_controller.cc b/engine/src/flutter/runtime/runtime_controller.cc index 0d7d7ce15d9..3e143cd6bcf 100644 --- a/engine/src/flutter/runtime/runtime_controller.cc +++ b/engine/src/flutter/runtime/runtime_controller.cc @@ -10,6 +10,7 @@ #include "flutter/lib/ui/window/window.h" #include "flutter/runtime/dart_controller.h" #include "flutter/runtime/runtime_delegate.h" +#include "lib/tonic/dart_message_handler.h" using tonic::DartState; @@ -149,4 +150,15 @@ bool RuntimeController::HasLivePorts() { return Dart_HasLivePorts(); } +tonic::DartErrorHandleType RuntimeController::GetLastError() { + if (!dart_controller_) { + return tonic::kNoError; + } + UIDartState* dart_state = dart_controller_->dart_state(); + if (!dart_state) { + return tonic::kNoError; + } + return dart_state->message_handler().isolate_last_error(); +} + } // namespace blink diff --git a/engine/src/flutter/runtime/runtime_controller.h b/engine/src/flutter/runtime/runtime_controller.h index 074336d7131..941673c683c 100644 --- a/engine/src/flutter/runtime/runtime_controller.h +++ b/engine/src/flutter/runtime/runtime_controller.h @@ -41,10 +41,9 @@ class RuntimeController : public WindowClient, public IsolateClient { void DispatchSemanticsAction(int32_t id, SemanticsAction action); Dart_Port GetMainPort(); - std::string GetIsolateName(); - bool HasLivePorts(); + tonic::DartErrorHandleType GetLastError(); private: explicit RuntimeController(RuntimeDelegate* client); diff --git a/engine/src/flutter/shell/common/engine.cc b/engine/src/flutter/shell/common/engine.cc index ea5dab7315c..5154d61f202 100644 --- a/engine/src/flutter/shell/common/engine.cc +++ b/engine/src/flutter/shell/common/engine.cc @@ -66,6 +66,7 @@ Engine::Engine(PlatformView* platform_view) platform_view->rasterizer().GetWeakRasterizerPtr(), platform_view->GetVsyncWaiter(), this)), + load_script_error_(tonic::kNoError), activity_running_(false), have_surface_(false), weak_factory_(this) {} @@ -127,7 +128,8 @@ void Engine::RunBundleAndSource(const std::string& bundle_path, if (!bundle_path.empty()) ConfigureAssetBundle(bundle_path); ConfigureRuntime(GetScriptUriFromPath(main)); - runtime_->dart_controller()->RunFromSource(main, packages_path); + load_script_error_ = + runtime_->dart_controller()->RunFromSource(main, packages_path); } void Engine::BeginFrame(ftl::TimePoint frame_time) { @@ -161,6 +163,16 @@ bool Engine::UIIsolateHasLivePorts() { return runtime_->HasLivePorts(); } +tonic::DartErrorHandleType Engine::GetUIIsolateLastError() { + if (!runtime_) + return tonic::kNoError; + return runtime_->GetLastError(); +} + +tonic::DartErrorHandleType Engine::GetLoadScriptError() { + return load_script_error_; +} + void Engine::OnOutputSurfaceCreated(const ftl::Closure& gpu_continuation) { blink::Threads::Gpu()->PostTask(gpu_continuation); have_surface_ = true; diff --git a/engine/src/flutter/shell/common/engine.h b/engine/src/flutter/shell/common/engine.h index 3fb35c8a0f0..832b684a198 100644 --- a/engine/src/flutter/shell/common/engine.h +++ b/engine/src/flutter/shell/common/engine.h @@ -56,10 +56,10 @@ class Engine : public blink::RuntimeDelegate { const std::string& bundle); Dart_Port GetUIIsolateMainPort(); - std::string GetUIIsolateName(); - bool UIIsolateHasLivePorts(); + tonic::DartErrorHandleType GetUIIsolateLastError(); + tonic::DartErrorHandleType GetLoadScriptError(); void OnOutputSurfaceCreated(const ftl::Closure& gpu_continuation); void OnOutputSurfaceDestroyed(const ftl::Closure& gpu_continuation); @@ -97,6 +97,7 @@ class Engine : public blink::RuntimeDelegate { ftl::WeakPtr platform_view_; std::unique_ptr animator_; std::unique_ptr runtime_; + tonic::DartErrorHandleType load_script_error_; ftl::RefPtr pending_push_route_message_; blink::ViewportMetrics viewport_metrics_; std::string language_code_; diff --git a/engine/src/flutter/shell/platform/linux/BUILD.gn b/engine/src/flutter/shell/platform/linux/BUILD.gn index bbbed5b582d..e974ca945f6 100644 --- a/engine/src/flutter/shell/platform/linux/BUILD.gn +++ b/engine/src/flutter/shell/platform/linux/BUILD.gn @@ -20,6 +20,7 @@ executable("linux") { "//flutter/shell/gpu", "//flutter/shell/testing", "//lib/ftl", + "//lib/tonic", # Required by FontCacheLinux. Not Skia. Skia uses a custom font manager # that delegates to us. diff --git a/engine/src/flutter/shell/platform/linux/main_linux.cc b/engine/src/flutter/shell/platform/linux/main_linux.cc index 4756b9c04ca..9a951ac336b 100644 --- a/engine/src/flutter/shell/platform/linux/main_linux.cc +++ b/engine/src/flutter/shell/platform/linux/main_linux.cc @@ -17,32 +17,60 @@ #include "flutter/shell/testing/test_runner.h" #include "flutter/shell/testing/testing.h" #include "flutter/sky/engine/public/web/Sky.h" +#include "lib/tonic/dart_microtask_queue.h" namespace { +// Exit codes used by the Dart command line tool. +const int kApiErrorExitCode = 253; +const int kCompilationErrorExitCode = 254; +const int kErrorExitCode = 255; + // Checks whether the engine's main Dart isolate has no pending work. If so, // then exit the given message loop. class ScriptCompletionTaskObserver : public base::MessageLoop::TaskObserver { public: ScriptCompletionTaskObserver(base::MessageLoop& main_message_loop) - : main_message_loop_(main_message_loop), prev_live_(false) {} + : main_message_loop_(main_message_loop), + prev_live_(false), + last_error_(tonic::kNoError) {} void WillProcessTask(const base::PendingTask& pending_task) override {} void DidProcessTask(const base::PendingTask& pending_task) override { shell::TestRunner& test_runner = shell::TestRunner::Shared(); bool live = test_runner.platform_view().engine().UIIsolateHasLivePorts(); - if (prev_live_ && !live) + if (prev_live_ && !live) { + last_error_ = test_runner.platform_view().engine().GetUIIsolateLastError(); main_message_loop_.PostTask(FROM_HERE, main_message_loop_.QuitWhenIdleClosure()); + } prev_live_ = live; } + tonic::DartErrorHandleType last_error() { + return last_error_; + } + private: base::MessageLoop& main_message_loop_; bool prev_live_; + tonic::DartErrorHandleType last_error_; }; +int ConvertErrorTypeToExitCode(tonic::DartErrorHandleType error) { + switch (error) { + case tonic::kCompilationErrorType: + return kCompilationErrorExitCode; + case tonic::kApiErrorType: + return kApiErrorExitCode; + case tonic::kUnknownErrorType: + return kErrorExitCode; + default: + return 0; + } +} + void RunNonInteractive(bool run_forever) { base::MessageLoop message_loop; @@ -64,9 +92,16 @@ void RunNonInteractive(bool run_forever) { message_loop.Run(); + shell::TestRunner& test_runner = shell::TestRunner::Shared(); + tonic::DartErrorHandleType error = test_runner.platform_view().engine().GetLoadScriptError(); + if (error == tonic::kNoError) + error = task_observer.last_error(); + if (error == tonic::kNoError) + error = tonic::DartMicrotaskQueue::GetLastError(); + // The script has completed and the engine may not be in a clean state, // so just stop the process. - exit(0); + exit(ConvertErrorTypeToExitCode(error)); } static bool IsDartFile(const std::string& path) {