diff --git a/lib/io/dart_io.cc b/lib/io/dart_io.cc index 017054a5388fb..d9b6d59f3b04c 100644 --- a/lib/io/dart_io.cc +++ b/lib/io/dart_io.cc @@ -10,7 +10,7 @@ #include "third_party/tonic/converter/dart_converter.h" #include "third_party/tonic/logging/dart_error.h" -using tonic::LogIfError; +using tonic::CheckAndHandleError; using tonic::ToDart; namespace flutter { @@ -20,22 +20,22 @@ void DartIO::InitForIsolate(bool may_insecurely_connect_to_all_domains, Dart_Handle io_lib = Dart_LookupLibrary(ToDart("dart:io")); Dart_Handle result = Dart_SetNativeResolver(io_lib, dart::bin::LookupIONative, dart::bin::LookupIONativeSymbol); - FML_CHECK(!LogIfError(result)); + FML_CHECK(!CheckAndHandleError(result)); Dart_Handle embedder_config_type = Dart_GetNonNullableType(io_lib, ToDart("_EmbedderConfig"), 0, nullptr); - FML_CHECK(!LogIfError(embedder_config_type)); + FML_CHECK(!CheckAndHandleError(embedder_config_type)); Dart_Handle allow_insecure_connections_result = Dart_SetField( embedder_config_type, ToDart("_mayInsecurelyConnectToAllDomains"), ToDart(may_insecurely_connect_to_all_domains)); - FML_CHECK(!LogIfError(allow_insecure_connections_result)); + FML_CHECK(!CheckAndHandleError(allow_insecure_connections_result)); Dart_Handle dart_args[1]; dart_args[0] = ToDart(domain_network_policy); Dart_Handle set_domain_network_policy_result = Dart_Invoke( embedder_config_type, ToDart("_setDomainPolicies"), 1, dart_args); - FML_CHECK(!LogIfError(set_domain_network_policy_result)); + FML_CHECK(!CheckAndHandleError(set_domain_network_policy_result)); Dart_Handle ui_lib = Dart_LookupLibrary(ToDart("dart:ui")); Dart_Handle dart_validate_args[1]; @@ -43,12 +43,12 @@ void DartIO::InitForIsolate(bool may_insecurely_connect_to_all_domains, Dart_Handle http_connection_hook_closure = Dart_Invoke(ui_lib, ToDart("_getHttpConnectionHookClosure"), /*number_of_arguments=*/1, dart_validate_args); - FML_CHECK(!LogIfError(http_connection_hook_closure)); + FML_CHECK(!CheckAndHandleError(http_connection_hook_closure)); Dart_Handle http_lib = Dart_LookupLibrary(ToDart("dart:_http")); - FML_CHECK(!LogIfError(http_lib)); + FML_CHECK(!CheckAndHandleError(http_lib)); Dart_Handle set_http_connection_hook_result = Dart_SetField( http_lib, ToDart("_httpConnectionHook"), http_connection_hook_closure); - FML_CHECK(!LogIfError(set_http_connection_hook_result)); + FML_CHECK(!CheckAndHandleError(set_http_connection_hook_result)); } } // namespace flutter diff --git a/lib/ui/dart_runtime_hooks.cc b/lib/ui/dart_runtime_hooks.cc index 5cc82cb5832bd..60b7cf08d78b5 100644 --- a/lib/ui/dart_runtime_hooks.cc +++ b/lib/ui/dart_runtime_hooks.cc @@ -29,7 +29,6 @@ #include "third_party/tonic/scopes/dart_isolate_scope.h" using tonic::DartConverter; -using tonic::LogIfError; using tonic::ToDart; namespace flutter { diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index 3fd410d49a450..032e227b870c3 100644 --- a/lib/ui/fixtures/ui_test.dart +++ b/lib/ui/fixtures/ui_test.dart @@ -12,6 +12,33 @@ void main() {} /// validation. void _finish() native 'Finish'; +@pragma('vm:entry-point') +void customOnErrorTrue() { + PlatformDispatcher.instance.onError = (Object error, StackTrace? stack) { + _finish(); + return true; + }; + throw Exception('true'); +} + +@pragma('vm:entry-point') +void customOnErrorFalse() { + PlatformDispatcher.instance.onError = (Object error, StackTrace? stack) { + _finish(); + return false; + }; + throw Exception('false'); +} + +@pragma('vm:entry-point') +void customOnErrorThrow() { + PlatformDispatcher.instance.onError = (Object error, StackTrace? stack) { + _finish(); + throw Exception('throw2'); + }; + throw Exception('throw1'); +} + @pragma('vm:entry-point') void validateSceneBuilderAndScene() { final SceneBuilder builder = SceneBuilder(); @@ -311,9 +338,9 @@ void hooksTests() { } } - void expectIdentical(Zone originalZone, Zone callbackZone) { - if (!identical(callbackZone, originalZone)) { - throw 'Callback called in wrong zone.'; + void expectIdentical(Object a, Object b) { + if (!identical(a, b)) { + throw 'Expected $a to be identical to $b.'; } } @@ -368,6 +395,26 @@ void hooksTests() { } }); + test('onError preserves the callback zone', () { + late Zone originalZone; + late Zone callbackZone; + final Object error = Exception('foo'); + StackTrace? stackTrace; + + runZoned(() { + originalZone = Zone.current; + PlatformDispatcher.instance.onError = (Object exception, StackTrace? stackTrace) { + callbackZone = Zone.current; + expectIdentical(exception, error); + expectNotEquals(stackTrace, null); + return true; + }; + }); + + _callHook('_onError', 2, error, StackTrace.current); + expectIdentical(originalZone, callbackZone); + }); + test('updateUserSettings can handle an empty object', () { _callHook('_updateUserSettingsData', 1, '{}'); }); diff --git a/lib/ui/hooks.dart b/lib/ui/hooks.dart index 5ba01a6ca84e2..b2c3619272f71 100644 --- a/lib/ui/hooks.dart +++ b/lib/ui/hooks.dart @@ -115,28 +115,27 @@ void _drawFrame() { PlatformDispatcher.instance._drawFrame(); } +@pragma('vm:entry-point') +bool _onError(Object error, StackTrace? stackTrace) { + return PlatformDispatcher.instance._dispatchError(error, stackTrace ?? StackTrace.empty); +} + // ignore: always_declare_return_types, prefer_generic_function_type_aliases typedef _ListStringArgFunction(List args); @pragma('vm:entry-point') -void _runMainZoned(Function startMainIsolateFunction, - Function userMainFunction, - List args) { +void _runMain(Function startMainIsolateFunction, + Function userMainFunction, + List args) { startMainIsolateFunction(() { - runZonedGuarded(() { - if (userMainFunction is _ListStringArgFunction) { - userMainFunction(args); - } else { - userMainFunction(); - } - }, (Object error, StackTrace stackTrace) { - _reportUnhandledException(error.toString(), stackTrace.toString()); - }); + if (userMainFunction is _ListStringArgFunction) { + userMainFunction(args); + } else { + userMainFunction(); + } }, null); } -void _reportUnhandledException(String error, String stackTrace) native 'PlatformConfiguration_reportUnhandledException'; - /// Invokes [callback] inside the given [zone]. void _invoke(void Function()? callback, Zone zone) { if (callback == null) { diff --git a/lib/ui/painting/paint.cc b/lib/ui/painting/paint.cc index 137225f27eef6..0b32d62873646 100644 --- a/lib/ui/painting/paint.cc +++ b/lib/ui/painting/paint.cc @@ -323,10 +323,10 @@ flutter::Paint DartConverter::FromArguments( int index, Dart_Handle& exception) { Dart_Handle paint_objects = Dart_GetNativeArgument(args, index); - FML_DCHECK(!LogIfError(paint_objects)); + FML_DCHECK(!CheckAndHandleError(paint_objects)); Dart_Handle paint_data = Dart_GetNativeArgument(args, index + 1); - FML_DCHECK(!LogIfError(paint_data)); + FML_DCHECK(!CheckAndHandleError(paint_data)); return flutter::Paint(paint_objects, paint_data); } diff --git a/lib/ui/painting/rrect.cc b/lib/ui/painting/rrect.cc index 8bd9714f7635e..39889060e9cf3 100644 --- a/lib/ui/painting/rrect.cc +++ b/lib/ui/painting/rrect.cc @@ -40,7 +40,7 @@ RRect DartConverter::FromArguments(Dart_NativeArguments args, int index, Dart_Handle& exception) { Dart_Handle value = Dart_GetNativeArgument(args, index); - FML_DCHECK(!LogIfError(value)); + FML_DCHECK(!CheckAndHandleError(value)); return FromDart(value); } diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index b6de42b98fc34..0ae3d6ae49880 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -53,6 +53,16 @@ typedef _SetNeedsReportTimingsFunc = void Function(bool value); /// Signature for [PlatformDispatcher.onConfigurationChanged]. typedef PlatformConfigurationChangedCallback = void Function(PlatformConfiguration configuration); +/// Signature for [PlatformDispatcher.onError]. +/// +/// If this method returns false, the engine may use some fallback method to +/// provide information about the error. +/// +/// After calling this method, the process or the VM may terminate. Some severe +/// unhandled errors may not be able to call this method either, such as Dart +/// compilation errors or process terminating errors. +typedef ErrorCallback = bool Function(Object exception, StackTrace stackTrace); + // A gesture setting value that indicates it has not been set by the engine. const double _kUnsetGestureSetting = -1.0; @@ -1012,6 +1022,50 @@ class PlatformDispatcher { ); } + ErrorCallback? _onError; + Zone? _onErrorZone; + + /// A callback that is invoked when an unhandled error occurs in the root + /// isolate. + /// + /// This callback must return `true` if it has handled the error. Otherwise, + /// it must return `false` and a fallback mechanism such as printing to stderr + /// will be used, as configured by the specific platform embedding via + /// `Settings::unhandled_exception_callback`. + /// + /// The VM or the process may exit or become unresponsive after calling this + /// callback. The callback will not be called for exceptions that cause the VM + /// or process to terminate or become unresponsive before the callback can be + /// invoked. + /// + /// This callback is not directly invoked by errors in child isolates of the + /// root isolate. Programs that create new isolates must listen for errors on + /// those isolates and forward the errors to the root isolate. An example of + /// this can be found in the Flutter framework's `compute` function. + ErrorCallback? get onError => _onError; + set onError(ErrorCallback? callback) { + _onError = callback; + _onErrorZone = Zone.current; + } + + bool _dispatchError(Object error, StackTrace stackTrace) { + if (_onError == null) { + return false; + } + assert(_onErrorZone != null); + + if (identical(_onErrorZone, Zone.current)) { + return _onError!(error, stackTrace); + } else { + try { + return _onErrorZone!.runBinary(_onError!, error, stackTrace); + } catch (e, s) { + _onErrorZone!.handleUncaughtError(e, s); + return false; + } + } + } + /// The route or path that the embedder requested when the application was /// launched. /// diff --git a/lib/ui/ui_dart_state.cc b/lib/ui/ui_dart_state.cc index b9bd6a749fcbb..62befcea4c221 100644 --- a/lib/ui/ui_dart_state.cc +++ b/lib/ui/ui_dart_state.cc @@ -134,7 +134,7 @@ std::shared_ptr UIDartState::GetVolatilePathTracker() } void UIDartState::ScheduleMicrotask(Dart_Handle closure) { - if (tonic::LogIfError(closure) || !Dart_IsClosure(closure)) { + if (tonic::CheckAndHandleError(closure) || !Dart_IsClosure(closure)) { return; } @@ -186,19 +186,6 @@ tonic::DartErrorHandleType UIDartState::GetLastError() { return error; } -void UIDartState::ReportUnhandledException(const std::string& error, - const std::string& stack_trace) { - if (unhandled_exception_callback_ && - unhandled_exception_callback_(error, stack_trace)) { - return; - } - - // Either the exception handler was not set or it could not handle the error, - // just log the exception. - FML_LOG(ERROR) << "Unhandled Exception: " << error << std::endl - << stack_trace; -} - void UIDartState::LogMessage(const std::string& tag, const std::string& message) const { if (log_message_callback_) { diff --git a/lib/ui/ui_dart_state.h b/lib/ui/ui_dart_state.h index 8269ba34c9620..1c05304a8195a 100644 --- a/lib/ui/ui_dart_state.h +++ b/lib/ui/ui_dart_state.h @@ -128,9 +128,6 @@ class UIDartState : public tonic::DartState { tonic::DartErrorHandleType GetLastError(); - void ReportUnhandledException(const std::string& error, - const std::string& stack_trace); - // Logs `print` messages from the application via an embedder-specified // logging mechanism. // @@ -154,6 +151,10 @@ class UIDartState : public tonic::DartState { return {std::move(object), std::move(queue)}; }; + UnhandledExceptionCallback unhandled_exception_callback() const { + return unhandled_exception_callback_; + } + protected: UIDartState(TaskObserverAdd add_callback, TaskObserverRemove remove_callback, diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc index 3cebfb5d10e24..ba5febd18ee2d 100644 --- a/lib/ui/window/platform_configuration.cc +++ b/lib/ui/window/platform_configuration.cc @@ -82,29 +82,6 @@ void SetNeedsReportTimings(Dart_NativeArguments args) { ->SetNeedsReportTimings(value); } -void ReportUnhandledException(Dart_NativeArguments args) { - UIDartState::ThrowIfUIOperationsProhibited(); - - Dart_Handle exception = nullptr; - - auto error_name = - tonic::DartConverter::FromArguments(args, 0, exception); - if (exception) { - Dart_ThrowException(exception); - return; - } - - auto stack_trace = - tonic::DartConverter::FromArguments(args, 1, exception); - if (exception) { - Dart_ThrowException(exception); - return; - } - - UIDartState::Current()->ReportUnhandledException(std::move(error_name), - std::move(stack_trace)); -} - Dart_Handle SendPlatformMessage(Dart_Handle window, const std::string& name, Dart_Handle callback, @@ -197,6 +174,9 @@ PlatformConfiguration::~PlatformConfiguration() {} void PlatformConfiguration::DidCreateIsolate() { Dart_Handle library = Dart_LookupLibrary(tonic::ToDart("dart:ui")); + + on_error_.Set(tonic::DartState::Current(), + Dart_GetField(library, tonic::ToDart("_onError"))); update_locales_.Set(tonic::DartState::Current(), Dart_GetField(library, tonic::ToDart("_updateLocales"))); update_user_settings_data_.Set( @@ -237,7 +217,7 @@ void PlatformConfiguration::UpdateLocales( } tonic::DartState::Scope scope(dart_state); - tonic::LogIfError( + tonic::CheckAndHandleError( tonic::DartInvoke(update_locales_.Get(), { tonic::ToDart>(locales), @@ -252,10 +232,10 @@ void PlatformConfiguration::UpdateUserSettingsData(const std::string& data) { } tonic::DartState::Scope scope(dart_state); - tonic::LogIfError(tonic::DartInvoke(update_user_settings_data_.Get(), - { - tonic::StdStringToDart(data), - })); + tonic::CheckAndHandleError(tonic::DartInvoke(update_user_settings_data_.Get(), + { + tonic::StdStringToDart(data), + })); } void PlatformConfiguration::UpdateLifecycleState(const std::string& data) { @@ -265,10 +245,10 @@ void PlatformConfiguration::UpdateLifecycleState(const std::string& data) { return; } tonic::DartState::Scope scope(dart_state); - tonic::LogIfError(tonic::DartInvoke(update_lifecycle_state_.Get(), - { - tonic::StdStringToDart(data), - })); + tonic::CheckAndHandleError(tonic::DartInvoke(update_lifecycle_state_.Get(), + { + tonic::StdStringToDart(data), + })); } void PlatformConfiguration::UpdateSemanticsEnabled(bool enabled) { @@ -280,8 +260,8 @@ void PlatformConfiguration::UpdateSemanticsEnabled(bool enabled) { tonic::DartState::Scope scope(dart_state); UIDartState::ThrowIfUIOperationsProhibited(); - tonic::LogIfError(tonic::DartInvoke(update_semantics_enabled_.Get(), - {tonic::ToDart(enabled)})); + tonic::CheckAndHandleError(tonic::DartInvoke(update_semantics_enabled_.Get(), + {tonic::ToDart(enabled)})); } void PlatformConfiguration::UpdateAccessibilityFeatures(int32_t values) { @@ -292,8 +272,8 @@ void PlatformConfiguration::UpdateAccessibilityFeatures(int32_t values) { } tonic::DartState::Scope scope(dart_state); - tonic::LogIfError(tonic::DartInvoke(update_accessibility_features_.Get(), - {tonic::ToDart(values)})); + tonic::CheckAndHandleError(tonic::DartInvoke( + update_accessibility_features_.Get(), {tonic::ToDart(values)})); } void PlatformConfiguration::DispatchPlatformMessage( @@ -322,7 +302,7 @@ void PlatformConfiguration::DispatchPlatformMessage( pending_responses_[response_id] = response; } - tonic::LogIfError( + tonic::CheckAndHandleError( tonic::DartInvoke(dispatch_platform_message_.Get(), {tonic::ToDart(message->channel()), data_handle, tonic::ToDart(response_id)})); @@ -345,7 +325,7 @@ void PlatformConfiguration::DispatchSemanticsAction(int32_t id, return; } - tonic::LogIfError(tonic::DartInvoke( + tonic::CheckAndHandleError(tonic::DartInvoke( dispatch_semantics_action_.Get(), {tonic::ToDart(id), tonic::ToDart(static_cast(action)), args_handle})); @@ -362,7 +342,7 @@ void PlatformConfiguration::BeginFrame(fml::TimePoint frameTime, int64_t microseconds = (frameTime - fml::TimePoint()).ToMicroseconds(); - tonic::LogIfError( + tonic::CheckAndHandleError( tonic::DartInvoke(begin_frame_.Get(), { Dart_NewInteger(microseconds), Dart_NewInteger(frame_number), @@ -370,7 +350,7 @@ void PlatformConfiguration::BeginFrame(fml::TimePoint frameTime, UIDartState::Current()->FlushMicrotasksNow(); - tonic::LogIfError(tonic::DartInvokeVoid(draw_frame_.Get())); + tonic::CheckAndHandleError(tonic::DartInvokeVoid(draw_frame_.Get())); } void PlatformConfiguration::ReportTimings(std::vector timings) { @@ -394,9 +374,10 @@ void PlatformConfiguration::ReportTimings(std::vector timings) { memcpy(data, timings.data(), sizeof(int64_t) * timings.size()); FML_CHECK(Dart_TypedDataReleaseData(data_handle)); - tonic::LogIfError(tonic::DartInvoke(report_timings_.Get(), { - data_handle, - })); + tonic::CheckAndHandleError( + tonic::DartInvoke(report_timings_.Get(), { + data_handle, + })); } void PlatformConfiguration::CompletePlatformMessageEmptyResponse( @@ -462,8 +443,6 @@ void PlatformConfiguration::RegisterNatives( {"PlatformConfiguration_updateSemantics", UpdateSemantics, 2, true}, {"PlatformConfiguration_setIsolateDebugName", SetIsolateDebugName, 2, true}, - {"PlatformConfiguration_reportUnhandledException", - ReportUnhandledException, 2, true}, {"PlatformConfiguration_setNeedsReportTimings", SetNeedsReportTimings, 2, true}, {"PlatformConfiguration_getPersistentIsolateData", diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h index d112b2dcc002e..a8f3cf4ab3ac8 100644 --- a/lib/ui/window/platform_configuration.h +++ b/lib/ui/window/platform_configuration.h @@ -420,8 +420,11 @@ class PlatformConfiguration final { /// void CompletePlatformMessageEmptyResponse(int response_id); + Dart_Handle on_error() { return on_error_.Get(); } + private: PlatformConfigurationClient* client_; + tonic::DartPersistentValue on_error_; tonic::DartPersistentValue update_locales_; tonic::DartPersistentValue update_user_settings_data_; tonic::DartPersistentValue update_lifecycle_state_; diff --git a/lib/ui/window/platform_configuration_unittests.cc b/lib/ui/window/platform_configuration_unittests.cc index e18c9e466bd6c..530a092d79a26 100644 --- a/lib/ui/window/platform_configuration_unittests.cc +++ b/lib/ui/window/platform_configuration_unittests.cc @@ -115,5 +115,161 @@ TEST_F(ShellTest, PlatformConfigurationWindowMetricsUpdate) { DestroyShell(std::move(shell), std::move(task_runners)); } +TEST_F(ShellTest, PlatformConfigurationOnErrorHandlesError) { + auto message_latch = std::make_shared(); + bool did_throw = false; + + auto finish = [message_latch](Dart_NativeArguments args) { + message_latch->Signal(); + }; + AddNativeCallback("Finish", CREATE_NATIVE_ENTRY(finish)); + + Settings settings = CreateSettingsForFixture(); + settings.unhandled_exception_callback = + [&did_throw](const std::string& exception, + const std::string& stack_trace) -> bool { + did_throw = true; + return false; + }; + + TaskRunners task_runners("test", // label + GetCurrentTaskRunner(), // platform + CreateNewThread(), // raster + CreateNewThread(), // ui + CreateNewThread() // io + ); + + std::unique_ptr shell = + CreateShell(std::move(settings), std::move(task_runners)); + + ASSERT_TRUE(shell->IsSetup()); + auto run_configuration = RunConfiguration::InferFromSettings(settings); + run_configuration.SetEntrypoint("customOnErrorTrue"); + + shell->RunEngine(std::move(run_configuration), [&](auto result) { + ASSERT_EQ(result, Engine::RunStatus::Success); + }); + + message_latch->Wait(); + + // Flush the UI task runner to make sure errors that were triggered had a turn + // to propagate. + task_runners.GetUITaskRunner()->PostTask( + [&message_latch]() { message_latch->Signal(); }); + message_latch->Wait(); + + ASSERT_FALSE(did_throw); + DestroyShell(std::move(shell), std::move(task_runners)); +} + +TEST_F(ShellTest, PlatformConfigurationOnErrorDoesNotHandleError) { + auto message_latch = std::make_shared(); + std::string ex; + std::string st; + size_t throw_count = 0; + + auto finish = [message_latch](Dart_NativeArguments args) { + message_latch->Signal(); + }; + AddNativeCallback("Finish", CREATE_NATIVE_ENTRY(finish)); + + Settings settings = CreateSettingsForFixture(); + settings.unhandled_exception_callback = + [&ex, &st, &throw_count](const std::string& exception, + const std::string& stack_trace) -> bool { + throw_count += 1; + ex = std::move(exception); + st = std::move(stack_trace); + return true; + }; + + TaskRunners task_runners("test", // label + GetCurrentTaskRunner(), // platform + CreateNewThread(), // raster + CreateNewThread(), // ui + CreateNewThread() // io + ); + + std::unique_ptr shell = + CreateShell(std::move(settings), std::move(task_runners)); + + ASSERT_TRUE(shell->IsSetup()); + auto run_configuration = RunConfiguration::InferFromSettings(settings); + run_configuration.SetEntrypoint("customOnErrorFalse"); + + shell->RunEngine(std::move(run_configuration), [&](auto result) { + ASSERT_EQ(result, Engine::RunStatus::Success); + }); + + message_latch->Wait(); + + // Flush the UI task runner to make sure errors that were triggered had a turn + // to propagate. + task_runners.GetUITaskRunner()->PostTask( + [&message_latch]() { message_latch->Signal(); }); + message_latch->Wait(); + + ASSERT_EQ(throw_count, 1ul); + ASSERT_EQ(ex, "Exception: false") << ex; + ASSERT_EQ(st.rfind("#0 customOnErrorFalse", 0), 0ul) << st; + DestroyShell(std::move(shell), std::move(task_runners)); +} + +TEST_F(ShellTest, PlatformConfigurationOnErrorThrows) { + auto message_latch = std::make_shared(); + std::vector errors; + size_t throw_count = 0; + + auto finish = [message_latch](Dart_NativeArguments args) { + message_latch->Signal(); + }; + AddNativeCallback("Finish", CREATE_NATIVE_ENTRY(finish)); + + Settings settings = CreateSettingsForFixture(); + settings.unhandled_exception_callback = + [&errors, &throw_count](const std::string& exception, + const std::string& stack_trace) -> bool { + throw_count += 1; + errors.push_back(std::move(exception)); + errors.push_back(std::move(stack_trace)); + return true; + }; + + TaskRunners task_runners("test", // label + GetCurrentTaskRunner(), // platform + CreateNewThread(), // raster + CreateNewThread(), // ui + CreateNewThread() // io + ); + + std::unique_ptr shell = + CreateShell(std::move(settings), std::move(task_runners)); + + ASSERT_TRUE(shell->IsSetup()); + auto run_configuration = RunConfiguration::InferFromSettings(settings); + run_configuration.SetEntrypoint("customOnErrorThrow"); + + shell->RunEngine(std::move(run_configuration), [&](auto result) { + ASSERT_EQ(result, Engine::RunStatus::Success); + }); + + message_latch->Wait(); + + // Flush the UI task runner to make sure errors that were triggered had a turn + // to propagate. + task_runners.GetUITaskRunner()->PostTask( + [&message_latch]() { message_latch->Signal(); }); + message_latch->Wait(); + + ASSERT_EQ(throw_count, 2ul); + ASSERT_EQ(errors.size(), 4ul); + ASSERT_EQ(errors[0], "Exception: throw2") << errors[0]; + ASSERT_EQ(errors[1].rfind("#0 customOnErrorThrow"), 0ul) << errors[1]; + ASSERT_EQ(errors[2], "Exception: throw1") << errors[2]; + ASSERT_EQ(errors[3].rfind("#0 customOnErrorThrow"), 0ul) << errors[3]; + + DestroyShell(std::move(shell), std::move(task_runners)); +} + } // namespace testing } // namespace flutter diff --git a/lib/ui/window/window.cc b/lib/ui/window/window.cc index 01ff4eb146ca8..454608c251e92 100644 --- a/lib/ui/window/window.cc +++ b/lib/ui/window/window.cc @@ -32,7 +32,7 @@ void Window::DispatchPointerDataPacket(const PointerDataPacket& packet) { if (Dart_IsError(data_handle)) { return; } - tonic::LogIfError(tonic::DartInvokeField( + tonic::CheckAndHandleError(tonic::DartInvokeField( library_.value(), "_dispatchPointerDataPacket", {data_handle})); } @@ -44,7 +44,7 @@ void Window::UpdateWindowMetrics(const ViewportMetrics& metrics) { return; } tonic::DartState::Scope scope(dart_state); - tonic::LogIfError(tonic::DartInvokeField( + tonic::CheckAndHandleError(tonic::DartInvokeField( library_.value(), "_updateWindowMetrics", { tonic::ToDart(window_id_), diff --git a/lib/web_ui/lib/platform_dispatcher.dart b/lib/web_ui/lib/platform_dispatcher.dart index 61ed62a1e288b..7e72bee0c440f 100644 --- a/lib/web_ui/lib/platform_dispatcher.dart +++ b/lib/web_ui/lib/platform_dispatcher.dart @@ -14,6 +14,7 @@ typedef PlatformMessageResponseCallback = void Function(ByteData? data); typedef PlatformMessageCallback = void Function( String name, ByteData? data, PlatformMessageResponseCallback? callback); typedef PlatformConfigurationChangedCallback = void Function(PlatformConfiguration configuration); +typedef ErrorCallback = bool Function(Object exception, StackTrace? stackTrace); abstract class PlatformDispatcher { static PlatformDispatcher get instance => engine.EnginePlatformDispatcher.instance; @@ -104,6 +105,9 @@ abstract class PlatformDispatcher { SemanticsActionCallback? get onSemanticsAction; set onSemanticsAction(SemanticsActionCallback? callback); + ErrorCallback? get onError; + set onError(ErrorCallback? callback); + String get defaultRouteName; FrameData get frameData; diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/lib/web_ui/lib/src/engine/platform_dispatcher.dart index c4257dbd15c6f..5ad7a4dd23a8f 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -1024,6 +1024,19 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { _onSemanticsAction, _onSemanticsActionZone, id, action, args); } + // TODO(dnfield): make this work on web. + // https://github.com/flutter/flutter/issues/100277 + ui.ErrorCallback? _onError; + // ignore: unused_field + Zone? _onErrorZone; + @override + ui.ErrorCallback? get onError => _onError; + @override + set onError(ui.ErrorCallback? callback) { + _onError = callback; + _onErrorZone = Zone.current; + } + /// The route or path that the embedder requested when the application was /// launched. /// diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index e0ee729335d78..dd415f64c5f06 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -332,12 +332,13 @@ bool DartIsolate::Initialize(Dart_Isolate dart_isolate) { SetMessageHandlingTaskRunner(GetTaskRunners().GetUITaskRunner()); - if (tonic::LogIfError( + if (tonic::CheckAndHandleError( Dart_SetLibraryTagHandler(tonic::DartState::HandleLibraryTag))) { return false; } - if (tonic::LogIfError(Dart_SetDeferredLoadHandler(OnDartLoadLibrary))) { + if (tonic::CheckAndHandleError( + Dart_SetDeferredLoadHandler(OnDartLoadLibrary))) { return false; } @@ -366,7 +367,7 @@ bool DartIsolate::LoadLoadingUnit( Dart_Handle result = Dart_DeferredLoadComplete( loading_unit_id, dart_snapshot->GetDataMapping(), dart_snapshot->GetInstructionsMapping()); - if (tonic::LogIfError(result)) { + if (tonic::CheckAndHandleError(result)) { LoadLoadingUnitError(loading_unit_id, Dart_GetError(result), /*transient*/ true); return false; @@ -381,7 +382,7 @@ void DartIsolate::LoadLoadingUnitError(intptr_t loading_unit_id, tonic::DartState::Scope scope(this); Dart_Handle result = Dart_DeferredLoadCompleteError( loading_unit_id, error_message.c_str(), transient); - tonic::LogIfError(result); + tonic::CheckAndHandleError(result); } void DartIsolate::SetMessageHandlingTaskRunner( @@ -520,7 +521,7 @@ bool DartIsolate::LoadKernel(std::shared_ptr mapping, Dart_Handle library = Dart_LoadLibraryFromKernel(mapping->GetMapping(), mapping->GetSize()); - if (tonic::LogIfError(library)) { + if (tonic::CheckAndHandleError(library)) { return false; } @@ -530,7 +531,7 @@ bool DartIsolate::LoadKernel(std::shared_ptr mapping, } Dart_SetRootLibrary(library); - if (tonic::LogIfError(Dart_FinalizeLoading(false))) { + if (tonic::CheckAndHandleError(Dart_FinalizeLoading(false))) { return false; } return true; @@ -664,7 +665,7 @@ bool DartIsolate::MarkIsolateRunnable() { [[nodiscard]] static bool InvokeMainEntrypoint( Dart_Handle user_entrypoint_function, Dart_Handle args) { - if (tonic::LogIfError(user_entrypoint_function)) { + if (tonic::CheckAndHandleError(user_entrypoint_function)) { FML_LOG(ERROR) << "Could not resolve main entrypoint function."; return false; } @@ -673,13 +674,13 @@ bool DartIsolate::MarkIsolateRunnable() { tonic::DartInvokeField(Dart_LookupLibrary(tonic::ToDart("dart:isolate")), "_getStartMainIsolateFunction", {}); - if (tonic::LogIfError(start_main_isolate_function)) { + if (tonic::CheckAndHandleError(start_main_isolate_function)) { FML_LOG(ERROR) << "Could not resolve main entrypoint trampoline."; return false; } - if (tonic::LogIfError(tonic::DartInvokeField( - Dart_LookupLibrary(tonic::ToDart("dart:ui")), "_runMainZoned", + if (tonic::CheckAndHandleError(tonic::DartInvokeField( + Dart_LookupLibrary(tonic::ToDart("dart:ui")), "_runMain", {start_main_isolate_function, user_entrypoint_function, args}))) { FML_LOG(ERROR) << "Could not invoke the main entrypoint."; return false; diff --git a/runtime/dart_isolate_unittests.cc b/runtime/dart_isolate_unittests.cc index 0bf7dd0da1756..295b1a4c762b9 100644 --- a/runtime/dart_isolate_unittests.cc +++ b/runtime/dart_isolate_unittests.cc @@ -170,8 +170,8 @@ TEST_F(DartIsolateTest, CanRunDartCodeCodeSynchronously) { ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); ASSERT_TRUE(isolate->RunInIsolateScope([]() -> bool { - if (tonic::LogIfError(::Dart_Invoke(Dart_RootLibrary(), - tonic::ToDart("sayHi"), 0, nullptr))) { + if (tonic::CheckAndHandleError(::Dart_Invoke( + Dart_RootLibrary(), tonic::ToDart("sayHi"), 0, nullptr))) { return false; } return true; @@ -503,7 +503,7 @@ TEST_F(DartIsolateTest, DISABLED_ValidLoadingUnitSucceeds) { AddNativeCallback( "NotifySuccess", CREATE_NATIVE_ENTRY([this](Dart_NativeArguments args) { auto bool_handle = Dart_GetNativeArgument(args, 0); - ASSERT_FALSE(tonic::LogIfError(bool_handle)); + ASSERT_FALSE(tonic::CheckAndHandleError(bool_handle)); ASSERT_TRUE(tonic::DartConverter::FromDart(bool_handle)); Signal(); })); diff --git a/runtime/dart_plugin_registrant.cc b/runtime/dart_plugin_registrant.cc index 0f73b04c2a110..dfd2321b516e6 100644 --- a/runtime/dart_plugin_registrant.cc +++ b/runtime/dart_plugin_registrant.cc @@ -34,7 +34,8 @@ bool InvokeDartPluginRegistrantIfAvailable(Dart_Handle library_handle) { if (Dart_IsError(plugin_registrant)) { return false; } - tonic::LogIfError(tonic::DartInvokeField(plugin_registrant, "register", {})); + tonic::CheckAndHandleError( + tonic::DartInvokeField(plugin_registrant, "register", {})); return true; } diff --git a/runtime/dart_vm_initializer.cc b/runtime/dart_vm_initializer.cc index 5ee62aa2302e2..28148dcbbaf19 100644 --- a/runtime/dart_vm_initializer.cc +++ b/runtime/dart_vm_initializer.cc @@ -9,11 +9,74 @@ #include "flutter/fml/logging.h" #include "flutter/fml/synchronization/shared_mutex.h" #include "flutter/fml/trace_event.h" +#include "flutter/lib/ui/ui_dart_state.h" +#include "flutter/lib/ui/window/platform_configuration.h" +#include "third_party/tonic/converter/dart_converter.h" +#include "third_party/tonic/logging/dart_error.h" +namespace { // Tracks whether Dart has been initialized and if it is safe to call Dart // APIs. static std::atomic gDartInitialized; +void LogUnhandledException(Dart_Handle exception_handle, + Dart_Handle stack_trace_handle) { + const std::string error = + tonic::StdStringFromDart(Dart_ToString(exception_handle)); + const std::string stack_trace = + tonic::StdStringFromDart(Dart_ToString(stack_trace_handle)); + + auto state = flutter::UIDartState::Current(); + if (state && state->unhandled_exception_callback()) { + auto callback = state->unhandled_exception_callback(); + if (callback(error, stack_trace)) { + return; + } + } + + // Either the exception handler was not set or it could not handle the + // error, just log the exception. + FML_LOG(ERROR) << "Unhandled Exception: " << error << std::endl + << stack_trace; +} + +void ReportUnhandledException(Dart_Handle exception_handle, + Dart_Handle stack_trace_handle) { + // Hooks.dart will call the error handler on PlatformDispatcher if it is + // not null. If it is null, returns false, fall into the !handled branch + // below and log. + // If it is not null, defer to the return value of that closure + // to determine whether to report via logging. + bool handled = false; + auto state = flutter::UIDartState::Current(); + if (!state || !state->platform_configuration()) { + LogUnhandledException(exception_handle, stack_trace_handle); + return; + } + auto on_error = state->platform_configuration()->on_error(); + if (on_error) { + FML_DCHECK(!Dart_IsNull(on_error)); + Dart_Handle args[2]; + args[0] = exception_handle; + args[1] = stack_trace_handle; + Dart_Handle on_error_result = Dart_InvokeClosure(on_error, 2, args); + + // An exception was thrown by the exception handler. + if (Dart_IsError(on_error_result)) { + LogUnhandledException(Dart_ErrorGetException(on_error_result), + Dart_ErrorGetStackTrace(on_error_result)); + + handled = false; + } else { + handled = tonic::DartConverter::FromDart(on_error_result); + } + if (!handled) { + LogUnhandledException(exception_handle, stack_trace_handle); + } + } +} +} // namespace + void DartVMInitializer::Initialize(Dart_InitializeParams* params) { FML_DCHECK(!gDartInitialized); @@ -26,6 +89,7 @@ void DartVMInitializer::Initialize(Dart_InitializeParams* params) { } fml::tracing::TraceSetTimelineEventHandler(LogDartTimelineEvent); + tonic::SetUnhandledExceptionReporter(&ReportUnhandledException); } void DartVMInitializer::Cleanup() { diff --git a/runtime/type_conversions_unittests.cc b/runtime/type_conversions_unittests.cc index 650066ed2fd3d..345eb78ff9e5d 100644 --- a/runtime/type_conversions_unittests.cc +++ b/runtime/type_conversions_unittests.cc @@ -56,7 +56,7 @@ TEST_F(TypeConversionsTest, CanConvertEmptyList) { AddNativeCallback( "NotifySuccess", CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { auto bool_handle = Dart_GetNativeArgument(args, 0); - ASSERT_FALSE(tonic::LogIfError(bool_handle)); + ASSERT_FALSE(tonic::CheckAndHandleError(bool_handle)); ASSERT_TRUE(tonic::DartConverter::FromDart(bool_handle)); event.Signal(); })); @@ -64,7 +64,7 @@ TEST_F(TypeConversionsTest, CanConvertEmptyList) { "NotifyNative", CREATE_NATIVE_ENTRY([&](Dart_NativeArguments) { std::vector items; auto items_handle = tonic::ToDart(items); - ASSERT_FALSE(tonic::LogIfError(items_handle)); + ASSERT_FALSE(tonic::CheckAndHandleError(items_handle)); tonic::DartInvokeField(::Dart_RootLibrary(), "testCanConvertEmptyList", {items_handle}); })); @@ -77,7 +77,7 @@ TEST_F(TypeConversionsTest, CanConvertListOfStrings) { AddNativeCallback( "NotifySuccess", CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { auto bool_handle = Dart_GetNativeArgument(args, 0); - ASSERT_FALSE(tonic::LogIfError(bool_handle)); + ASSERT_FALSE(tonic::CheckAndHandleError(bool_handle)); ASSERT_TRUE(tonic::DartConverter::FromDart(bool_handle)); event.Signal(); })); @@ -89,7 +89,7 @@ TEST_F(TypeConversionsTest, CanConvertListOfStrings) { items.push_back("soldier"); items.push_back("sailor"); auto items_handle = tonic::ToDart(items); - ASSERT_FALSE(tonic::LogIfError(items_handle)); + ASSERT_FALSE(tonic::CheckAndHandleError(items_handle)); tonic::DartInvokeField(::Dart_RootLibrary(), "testCanConvertListOfStrings", {items_handle}); })); @@ -102,7 +102,7 @@ TEST_F(TypeConversionsTest, CanConvertListOfDoubles) { AddNativeCallback( "NotifySuccess", CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { auto bool_handle = Dart_GetNativeArgument(args, 0); - ASSERT_FALSE(tonic::LogIfError(bool_handle)); + ASSERT_FALSE(tonic::CheckAndHandleError(bool_handle)); ASSERT_TRUE(tonic::DartConverter::FromDart(bool_handle)); event.Signal(); })); @@ -114,7 +114,7 @@ TEST_F(TypeConversionsTest, CanConvertListOfDoubles) { items.push_back(3.0); items.push_back(4.0); auto items_handle = tonic::ToDart(items); - ASSERT_FALSE(tonic::LogIfError(items_handle)); + ASSERT_FALSE(tonic::CheckAndHandleError(items_handle)); tonic::DartInvokeField(::Dart_RootLibrary(), "testCanConvertListOfDoubles", {items_handle}); })); @@ -127,7 +127,7 @@ TEST_F(TypeConversionsTest, CanConvertListOfInts) { AddNativeCallback( "NotifySuccess", CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { auto bool_handle = Dart_GetNativeArgument(args, 0); - ASSERT_FALSE(tonic::LogIfError(bool_handle)); + ASSERT_FALSE(tonic::CheckAndHandleError(bool_handle)); ASSERT_TRUE(tonic::DartConverter::FromDart(bool_handle)); event.Signal(); })); @@ -139,7 +139,7 @@ TEST_F(TypeConversionsTest, CanConvertListOfInts) { items.push_back(3); items.push_back(4); auto items_handle = tonic::ToDart(items); - ASSERT_FALSE(tonic::LogIfError(items_handle)); + ASSERT_FALSE(tonic::CheckAndHandleError(items_handle)); tonic::DartInvokeField(::Dart_RootLibrary(), "testCanConvertListOfInts", {items_handle}); })); @@ -152,7 +152,7 @@ TEST_F(TypeConversionsTest, CanConvertListOfFloatsToListOfDartDoubles) { AddNativeCallback( "NotifySuccess", CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { auto bool_handle = Dart_GetNativeArgument(args, 0); - ASSERT_FALSE(tonic::LogIfError(bool_handle)); + ASSERT_FALSE(tonic::CheckAndHandleError(bool_handle)); ASSERT_TRUE(tonic::DartConverter::FromDart(bool_handle)); event.Signal(); })); @@ -164,7 +164,7 @@ TEST_F(TypeConversionsTest, CanConvertListOfFloatsToListOfDartDoubles) { items.push_back(3.0f); items.push_back(4.0f); auto items_handle = tonic::ToDart(items); - ASSERT_FALSE(tonic::LogIfError(items_handle)); + ASSERT_FALSE(tonic::CheckAndHandleError(items_handle)); // This will fail on type mismatch. tonic::DartInvokeField(::Dart_RootLibrary(), "testCanConvertListOfDoubles", {items_handle}); diff --git a/shell/common/fixtures/shell_test.dart b/shell/common/fixtures/shell_test.dart index e2e9c2f08a0fd..25cbe8a7b2b7b 100644 --- a/shell/common/fixtures/shell_test.dart +++ b/shell/common/fixtures/shell_test.dart @@ -13,6 +13,28 @@ void nativeReportTimingsCallback(List timings) native 'NativeReportTimingsC void nativeOnBeginFrame(int microseconds) native 'NativeOnBeginFrame'; void nativeOnPointerDataPacket(List sequences) native 'NativeOnPointerDataPacket'; +@pragma('vm:entry-point') +void onErrorA() { + PlatformDispatcher.instance.onError = (Object error, StackTrace? stack) { + notifyErrorA(error.toString()); + return true; + }; + Future.delayed(const Duration(seconds: 2)).then((_) { + throw Exception('I should be coming from A'); + }); +} + +@pragma('vm:entry-point') +void onErrorB() { + PlatformDispatcher.instance.onError = (Object error, StackTrace? stack) { + notifyErrorB(error.toString()); + return true; + }; + throw Exception('I should be coming from B'); +} + +void notifyErrorA(String message) native 'NotifyErrorA'; +void notifyErrorB(String message) native 'NotifyErrorB'; @pragma('vm:entry-point') void drawFrames() { diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 34669c28fad9e..51392683e9ea7 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -3721,6 +3721,74 @@ TEST_F(ShellTest, UsesPlatformMessageHandler) { DestroyShell(std::move(shell)); } +TEST_F(ShellTest, SpawnWorksWithOnError) { + auto settings = CreateSettingsForFixture(); + auto shell = CreateShell(settings); + ASSERT_TRUE(ValidateShell(shell.get())); + + auto configuration = RunConfiguration::InferFromSettings(settings); + ASSERT_TRUE(configuration.IsValid()); + configuration.SetEntrypoint("onErrorA"); + + auto second_configuration = RunConfiguration::InferFromSettings(settings); + ASSERT_TRUE(second_configuration.IsValid()); + second_configuration.SetEntrypoint("onErrorB"); + + fml::CountDownLatch latch(2); + + AddNativeCallback( + "NotifyErrorA", CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { + auto string_handle = Dart_GetNativeArgument(args, 0); + const char* c_str; + Dart_StringToCString(string_handle, &c_str); + EXPECT_STREQ(c_str, "Exception: I should be coming from A"); + latch.CountDown(); + })); + + AddNativeCallback( + "NotifyErrorB", CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { + auto string_handle = Dart_GetNativeArgument(args, 0); + const char* c_str; + Dart_StringToCString(string_handle, &c_str); + EXPECT_STREQ(c_str, "Exception: I should be coming from B"); + latch.CountDown(); + })); + + RunEngine(shell.get(), std::move(configuration)); + + ASSERT_TRUE(DartVMRef::IsInstanceRunning()); + + PostSync( + shell->GetTaskRunners().GetPlatformTaskRunner(), + [this, &spawner = shell, &second_configuration, &latch]() { + ::testing::NiceMock platform_view_delegate; + auto spawn = spawner->Spawn( + std::move(second_configuration), "", + [&platform_view_delegate](Shell& shell) { + auto result = + std::make_unique<::testing::NiceMock>( + platform_view_delegate, shell.GetTaskRunners()); + ON_CALL(*result, CreateRenderingSurface()) + .WillByDefault(::testing::Invoke([] { + return std::make_unique<::testing::NiceMock>(); + })); + return result; + }, + [](Shell& shell) { return std::make_unique(shell); }); + ASSERT_NE(nullptr, spawn.get()); + ASSERT_TRUE(ValidateShell(spawn.get())); + + // Before destroying the shell, wait for expectations of the spawned + // isolate to be met. + latch.Wait(); + + DestroyShell(std::move(spawn)); + }); + + DestroyShell(std::move(shell)); + ASSERT_FALSE(DartVMRef::IsInstanceRunning()); +} + } // namespace testing } // namespace flutter diff --git a/shell/platform/fuchsia/dart-pkg/fuchsia/sdk_ext/fuchsia.cc b/shell/platform/fuchsia/dart-pkg/fuchsia/sdk_ext/fuchsia.cc index 9ac85679c7b5d..2c80baa4be360 100644 --- a/shell/platform/fuchsia/dart-pkg/fuchsia/sdk_ext/fuchsia.cc +++ b/shell/platform/fuchsia/dart-pkg/fuchsia/sdk_ext/fuchsia.cc @@ -93,7 +93,7 @@ void SetReturnCode(Dart_NativeArguments arguments) { int64_t return_code; Dart_Handle status = Dart_GetNativeIntegerArgument(arguments, 0, &return_code); - if (!tonic::LogIfError(status)) { + if (!tonic::CheckAndHandleError(status)) { tonic::DartState::Current()->SetReturnCode(return_code); } } @@ -106,10 +106,10 @@ void Initialize(fidl::InterfaceHandle environment, zircon::dart::Initialize(); Dart_Handle library = Dart_LookupLibrary(ToDart("dart:fuchsia")); - FML_CHECK(!tonic::LogIfError(library)); + FML_CHECK(!tonic::CheckAndHandleError(library)); Dart_Handle result = Dart_SetNativeResolver( library, fuchsia::dart::NativeLookup, fuchsia::dart::NativeSymbol); - FML_CHECK(!tonic::LogIfError(result)); + FML_CHECK(!tonic::CheckAndHandleError(result)); auto dart_state = tonic::DartState::Current(); std::unique_ptr fuchsia_class_provider( @@ -122,21 +122,21 @@ void Initialize(fidl::InterfaceHandle environment, result = Dart_SetField( library, ToDart("_environment"), ToDart(zircon::dart::Handle::Create(environment.TakeChannel()))); - FML_CHECK(!tonic::LogIfError(result)); + FML_CHECK(!tonic::CheckAndHandleError(result)); } if (directory_request) { result = Dart_SetField( library, ToDart("_outgoingServices"), ToDart(zircon::dart::Handle::Create(std::move(directory_request)))); - FML_CHECK(!tonic::LogIfError(result)); + FML_CHECK(!tonic::CheckAndHandleError(result)); } if (view_ref) { result = Dart_SetField( library, ToDart("_viewRef"), ToDart(zircon::dart::Handle::Create((*view_ref).release()))); - FML_CHECK(!tonic::LogIfError(result)); + FML_CHECK(!tonic::CheckAndHandleError(result)); } } diff --git a/shell/platform/fuchsia/dart-pkg/zircon/sdk_ext/handle_waiter.cc b/shell/platform/fuchsia/dart-pkg/zircon/sdk_ext/handle_waiter.cc index 96d4d3180867c..4c502e4b70afe 100644 --- a/shell/platform/fuchsia/dart-pkg/zircon/sdk_ext/handle_waiter.cc +++ b/shell/platform/fuchsia/dart-pkg/zircon/sdk_ext/handle_waiter.cc @@ -91,30 +91,30 @@ void HandleWaiter::OnWaitComplete(async_dispatcher_t* dispatcher, // Put the closure invocation on the microtask queue. Dart_Handle zircon_lib = Dart_LookupLibrary(ToDart("dart:zircon")); - FML_DCHECK(!tonic::LogIfError(zircon_lib)); + FML_DCHECK(!tonic::CheckAndHandleError(zircon_lib)); Dart_Handle owc_type = Dart_GetClass(zircon_lib, ToDart("_OnWaitCompleteClosure")); - FML_DCHECK(!tonic::LogIfError(owc_type)); + FML_DCHECK(!tonic::CheckAndHandleError(owc_type)); FML_DCHECK(!callback_.is_empty()); std::vector owc_args{callback_.Release(), ToDart(status), ToDart(signal->observed)}; Dart_Handle owc = Dart_New(owc_type, Dart_Null(), owc_args.size(), owc_args.data()); - FML_DCHECK(!tonic::LogIfError(owc)); + FML_DCHECK(!tonic::CheckAndHandleError(owc)); Dart_Handle closure = Dart_GetField(owc, ToDart("_closure")); - FML_DCHECK(!tonic::LogIfError(closure)); + FML_DCHECK(!tonic::CheckAndHandleError(closure)); // TODO(issue#tbd): Use tonic::DartMicrotaskQueue::ScheduleMicrotask() // instead when tonic::DartState gets a microtask queue field. Dart_Handle async_lib = Dart_LookupLibrary(ToDart("dart:async")); - FML_DCHECK(!tonic::LogIfError(async_lib)); + FML_DCHECK(!tonic::CheckAndHandleError(async_lib)); std::vector sm_args{closure}; Dart_Handle sm_result = Dart_Invoke(async_lib, ToDart("scheduleMicrotask"), sm_args.size(), sm_args.data()); - FML_DCHECK(!tonic::LogIfError(sm_result)); + FML_DCHECK(!tonic::CheckAndHandleError(sm_result)); } } // namespace dart diff --git a/shell/platform/fuchsia/dart-pkg/zircon/sdk_ext/natives.cc b/shell/platform/fuchsia/dart-pkg/zircon/sdk_ext/natives.cc index 1d5a2ea814f9b..b3558ad882ec8 100644 --- a/shell/platform/fuchsia/dart-pkg/zircon/sdk_ext/natives.cc +++ b/shell/platform/fuchsia/dart-pkg/zircon/sdk_ext/natives.cc @@ -67,10 +67,10 @@ const uint8_t* NativeSymbol(Dart_NativeFunction native_function) { void Initialize() { Dart_Handle library = Dart_LookupLibrary(ToDart("dart:zircon")); - FML_CHECK(!tonic::LogIfError(library)); + FML_CHECK(!tonic::CheckAndHandleError(library)); Dart_Handle result = Dart_SetNativeResolver( library, zircon::dart::NativeLookup, zircon::dart::NativeSymbol); - FML_CHECK(!tonic::LogIfError(result)); + FML_CHECK(!tonic::CheckAndHandleError(result)); auto dart_state = tonic::DartState::Current(); std::unique_ptr zircon_class_provider( diff --git a/shell/platform/fuchsia/dart-pkg/zircon/sdk_ext/system.cc b/shell/platform/fuchsia/dart-pkg/zircon/sdk_ext/system.cc index 29a5d75a8e071..da4bf54e7985d 100644 --- a/shell/platform/fuchsia/dart-pkg/zircon/sdk_ext/system.cc +++ b/shell/platform/fuchsia/dart-pkg/zircon/sdk_ext/system.cc @@ -46,7 +46,7 @@ class ByteDataScope { explicit ByteDataScope(size_t size) { dart_handle_ = Dart_NewTypedData(Dart_TypedData_kByteData, size); - FML_DCHECK(!tonic::LogIfError(dart_handle_)); + FML_DCHECK(!tonic::CheckAndHandleError(dart_handle_)); Acquire(); FML_DCHECK(size == size_); } @@ -65,7 +65,7 @@ class ByteDataScope { void Release() { FML_DCHECK(is_valid_); Dart_Handle result = Dart_TypedDataReleaseData(dart_handle_); - tonic::LogIfError(result); + tonic::CheckAndHandleError(result); is_valid_ = false; data_ = nullptr; size_ = 0; @@ -81,8 +81,8 @@ class ByteDataScope { intptr_t size; Dart_Handle result = Dart_TypedDataAcquireData(dart_handle_, &type, &data_, &size); - is_valid_ = - !tonic::LogIfError(result) && type == Dart_TypedData_kByteData && data_; + is_valid_ = !tonic::CheckAndHandleError(result) && + type == Dart_TypedData_kByteData && data_; if (is_valid_) { size_ = size; } else { @@ -119,7 +119,7 @@ Dart_Handle ConstructDartObject(const char* class_name, Args&&... args) { tonic::DartState::Current()->class_library(); Dart_Handle type = Dart_HandleFromPersistent(class_library.GetClass("zircon", class_name)); - FML_DCHECK(!tonic::LogIfError(type)); + FML_DCHECK(!tonic::CheckAndHandleError(type)); const char* cstr; Dart_StringToCString(Dart_ToString(type), &cstr); @@ -128,7 +128,7 @@ Dart_Handle ConstructDartObject(const char* class_name, Args&&... args) { {std::forward(args)...}}; Dart_Handle object = Dart_New(type, Dart_EmptyString(), sizeof...(Args), args_array.data()); - FML_DCHECK(!tonic::LogIfError(object)); + FML_DCHECK(!tonic::CheckAndHandleError(object)); return object; } @@ -158,16 +158,16 @@ Dart_Handle MakeHandleInfoList( fdio_ns_t* GetNamespace() { // Grab the fdio_ns_t* out of the isolate. Dart_Handle zircon_lib = Dart_LookupLibrary(ToDart("dart:zircon")); - FML_DCHECK(!tonic::LogIfError(zircon_lib)); + FML_DCHECK(!tonic::CheckAndHandleError(zircon_lib)); Dart_Handle namespace_type = Dart_GetNonNullableType(zircon_lib, ToDart("_Namespace"), 0, nullptr); - FML_DCHECK(!tonic::LogIfError(namespace_type)); + FML_DCHECK(!tonic::CheckAndHandleError(namespace_type)); Dart_Handle namespace_field = Dart_GetField(namespace_type, ToDart("_namespace")); - FML_DCHECK(!tonic::LogIfError(namespace_field)); + FML_DCHECK(!tonic::CheckAndHandleError(namespace_field)); uint64_t fdio_ns_ptr; Dart_Handle result = Dart_IntegerToUint64(namespace_field, &fdio_ns_ptr); - FML_DCHECK(!tonic::LogIfError(result)); + FML_DCHECK(!tonic::CheckAndHandleError(result)); return reinterpret_cast(fdio_ns_ptr); } @@ -551,7 +551,7 @@ Dart_Handle System::VmoMap(fml::RefPtr vmo) { void* data = reinterpret_cast(mapped_addr); Dart_Handle object = Dart_NewExternalTypedData(Dart_TypedData_kUint8, data, static_cast(size)); - FML_DCHECK(!tonic::LogIfError(object)); + FML_DCHECK(!tonic::CheckAndHandleError(object)); SizedRegion* r = new SizedRegion(data, size); Dart_NewFinalizableHandle(object, reinterpret_cast(r), diff --git a/shell/platform/fuchsia/dart_runner/builtin_libraries.cc b/shell/platform/fuchsia/dart_runner/builtin_libraries.cc index 59cbd8bc1a08e..9fd6a9b203401 100644 --- a/shell/platform/fuchsia/dart_runner/builtin_libraries.cc +++ b/shell/platform/fuchsia/dart_runner/builtin_libraries.cc @@ -89,7 +89,7 @@ void Logger_PrintString(Dart_NativeArguments args) { void ScheduleMicrotask(Dart_NativeArguments args) { Dart_Handle closure = Dart_GetNativeArgument(args, 0); - if (tonic::LogIfError(closure) || !Dart_IsClosure(closure)) + if (tonic::CheckAndHandleError(closure) || !Dart_IsClosure(closure)) return; tonic::DartMicrotaskQueue::GetForCurrentThread()->ScheduleMicrotask(closure); } @@ -114,38 +114,38 @@ void InitBuiltinLibrariesForIsolate( // dart:fuchsia.builtin ------------------------------------------------------ Dart_Handle builtin_lib = Dart_LookupLibrary(ToDart("dart:fuchsia.builtin")); - FML_CHECK(!tonic::LogIfError(builtin_lib)); + FML_CHECK(!tonic::CheckAndHandleError(builtin_lib)); Dart_Handle result = Dart_SetNativeResolver(builtin_lib, BuiltinNativeLookup, BuiltinNativeSymbol); - FML_CHECK(!tonic::LogIfError(result)); + FML_CHECK(!tonic::CheckAndHandleError(result)); // dart:io ------------------------------------------------------------------- Dart_Handle io_lib = Dart_LookupLibrary(ToDart("dart:io")); - FML_CHECK(!tonic::LogIfError(io_lib)); + FML_CHECK(!tonic::CheckAndHandleError(io_lib)); result = Dart_SetNativeResolver(io_lib, dart::bin::IONativeLookup, dart::bin::IONativeSymbol); - FML_CHECK(!tonic::LogIfError(result)); + FML_CHECK(!tonic::CheckAndHandleError(result)); // dart:zircon --------------------------------------------------------------- Dart_Handle zircon_lib = Dart_LookupLibrary(ToDart("dart:zircon")); - FML_CHECK(!tonic::LogIfError(zircon_lib)); + FML_CHECK(!tonic::CheckAndHandleError(zircon_lib)); // NativeResolver already set by fuchsia::dart::Initialize(). // Core libraries ------------------------------------------------------------ Dart_Handle async_lib = Dart_LookupLibrary(ToDart("dart:async")); - FML_CHECK(!tonic::LogIfError(async_lib)); + FML_CHECK(!tonic::CheckAndHandleError(async_lib)); Dart_Handle core_lib = Dart_LookupLibrary(ToDart("dart:core")); - FML_CHECK(!tonic::LogIfError(core_lib)); + FML_CHECK(!tonic::CheckAndHandleError(core_lib)); Dart_Handle internal_lib = Dart_LookupLibrary(ToDart("dart:_internal")); - FML_CHECK(!tonic::LogIfError(internal_lib)); + FML_CHECK(!tonic::CheckAndHandleError(internal_lib)); Dart_Handle isolate_lib = Dart_LookupLibrary(ToDart("dart:isolate")); - FML_CHECK(!tonic::LogIfError(isolate_lib)); + FML_CHECK(!tonic::CheckAndHandleError(isolate_lib)); #if !defined(AOT_RUNTIME) // AOT: These steps already happened at compile time in gen_snapshot. @@ -153,16 +153,16 @@ void InitBuiltinLibrariesForIsolate( // We need to ensure that all the scripts loaded so far are finalized // as we are about to invoke some Dart code below to set up closures. result = Dart_FinalizeLoading(false); - FML_CHECK(!tonic::LogIfError(result)); + FML_CHECK(!tonic::CheckAndHandleError(result)); #endif // Setup the internal library's 'internalPrint' function. Dart_Handle print = Dart_Invoke(builtin_lib, ToDart("_getPrintClosure"), 0, nullptr); - FML_CHECK(!tonic::LogIfError(print)); + FML_CHECK(!tonic::CheckAndHandleError(print)); result = Dart_SetField(internal_lib, ToDart("_printClosure"), print); - FML_CHECK(!tonic::LogIfError(result)); + FML_CHECK(!tonic::CheckAndHandleError(result)); // Set up the 'scheduleImmediate' closure. Dart_Handle schedule_immediate_closure; @@ -175,33 +175,33 @@ void InitBuiltinLibrariesForIsolate( schedule_immediate_closure = Dart_Invoke( builtin_lib, ToDart("_getScheduleMicrotaskClosure"), 0, nullptr); } - FML_CHECK(!tonic::LogIfError(schedule_immediate_closure)); + FML_CHECK(!tonic::CheckAndHandleError(schedule_immediate_closure)); Dart_Handle schedule_args[1]; schedule_args[0] = schedule_immediate_closure; result = Dart_Invoke(async_lib, ToDart("_setScheduleImmediateClosure"), 1, schedule_args); - FML_CHECK(!tonic::LogIfError(result)); + FML_CHECK(!tonic::CheckAndHandleError(result)); // Set up the namespace in dart:io. Dart_Handle namespace_type = Dart_GetNonNullableType(io_lib, ToDart("_Namespace"), 0, nullptr); - FML_CHECK(!tonic::LogIfError(namespace_type)); + FML_CHECK(!tonic::CheckAndHandleError(namespace_type)); Dart_Handle namespace_args[1]; namespace_args[0] = ToDart(reinterpret_cast(namespc)); result = Dart_Invoke(namespace_type, ToDart("_setupNamespace"), 1, namespace_args); - FML_CHECK(!tonic::LogIfError(result)); + FML_CHECK(!tonic::CheckAndHandleError(result)); // Set up the namespace in dart:zircon. namespace_type = Dart_GetNonNullableType(zircon_lib, ToDart("_Namespace"), 0, nullptr); - FML_CHECK(!tonic::LogIfError(namespace_type)); + FML_CHECK(!tonic::CheckAndHandleError(namespace_type)); result = Dart_SetField(namespace_type, ToDart("_namespace"), ToDart(reinterpret_cast(namespc))); - FML_CHECK(!tonic::LogIfError(result)); + FML_CHECK(!tonic::CheckAndHandleError(result)); // Set up stdout and stderr. Dart_Handle stdio_args[3]; @@ -209,36 +209,36 @@ void InitBuiltinLibrariesForIsolate( stdio_args[1] = Dart_NewInteger(stdoutfd); stdio_args[2] = Dart_NewInteger(stderrfd); result = Dart_Invoke(io_lib, ToDart("_setStdioFDs"), 3, stdio_args); - FML_CHECK(!tonic::LogIfError(result)); + FML_CHECK(!tonic::CheckAndHandleError(result)); // Disable some dart:io operations. Dart_Handle embedder_config_type = Dart_GetNonNullableType(io_lib, ToDart("_EmbedderConfig"), 0, nullptr); - FML_CHECK(!tonic::LogIfError(embedder_config_type)); + FML_CHECK(!tonic::CheckAndHandleError(embedder_config_type)); result = Dart_SetField(embedder_config_type, ToDart("_mayExit"), Dart_False()); - FML_CHECK(!tonic::LogIfError(result)); + FML_CHECK(!tonic::CheckAndHandleError(result)); // Set the script location. result = Dart_SetField(builtin_lib, ToDart("_rawScript"), ToDart(script_uri)); - FML_CHECK(!tonic::LogIfError(result)); + FML_CHECK(!tonic::CheckAndHandleError(result)); // Setup the uriBase with the base uri of the fidl app. Dart_Handle uri_base = Dart_Invoke(io_lib, ToDart("_getUriBaseClosure"), 0, nullptr); - FML_CHECK(!tonic::LogIfError(uri_base)); + FML_CHECK(!tonic::CheckAndHandleError(uri_base)); result = Dart_SetField(core_lib, ToDart("_uriBaseClosure"), uri_base); - FML_CHECK(!tonic::LogIfError(result)); + FML_CHECK(!tonic::CheckAndHandleError(result)); Dart_Handle setup_hooks = ToDart("_setupHooks"); result = Dart_Invoke(builtin_lib, setup_hooks, 0, nullptr); - FML_CHECK(!tonic::LogIfError(result)); + FML_CHECK(!tonic::CheckAndHandleError(result)); result = Dart_Invoke(io_lib, setup_hooks, 0, nullptr); - FML_CHECK(!tonic::LogIfError(result)); + FML_CHECK(!tonic::CheckAndHandleError(result)); result = Dart_Invoke(isolate_lib, setup_hooks, 0, nullptr); - FML_CHECK(!tonic::LogIfError(result)); + FML_CHECK(!tonic::CheckAndHandleError(result)); } } // namespace dart_runner diff --git a/shell/platform/fuchsia/dart_runner/dart_component_controller.cc b/shell/platform/fuchsia/dart_runner/dart_component_controller.cc index b3d7a4ca1342c..7fbc755086a42 100644 --- a/shell/platform/fuchsia/dart_runner/dart_component_controller.cc +++ b/shell/platform/fuchsia/dart_runner/dart_component_controller.cc @@ -415,7 +415,7 @@ bool DartComponentController::Main() { return false; } for (size_t i = 0; i < arguments.size(); i++) { - tonic::LogIfError( + tonic::CheckAndHandleError( Dart_ListSetAt(dart_arguments, i, ToDart(arguments.at(i)))); } diff --git a/shell/platform/fuchsia/flutter/isolate_configurator.cc b/shell/platform/fuchsia/flutter/isolate_configurator.cc index d5bf4dc60abb2..030f4bf9bcfbb 100644 --- a/shell/platform/fuchsia/flutter/isolate_configurator.cc +++ b/shell/platform/fuchsia/flutter/isolate_configurator.cc @@ -51,44 +51,44 @@ void IsolateConfigurator::BindFuchsia() { void IsolateConfigurator::BindZircon() { // Tell dart:zircon about the FDIO namespace configured for this instance. Dart_Handle zircon_lib = Dart_LookupLibrary(tonic::ToDart("dart:zircon")); - FML_CHECK(!tonic::LogIfError(zircon_lib)); + FML_CHECK(!tonic::CheckAndHandleError(zircon_lib)); Dart_Handle namespace_type = Dart_GetNonNullableType( zircon_lib, tonic::ToDart("_Namespace"), 0, nullptr); - FML_CHECK(!tonic::LogIfError(namespace_type)); + FML_CHECK(!tonic::CheckAndHandleError(namespace_type)); Dart_Handle result = Dart_SetField(namespace_type, // tonic::ToDart("_namespace"), // tonic::ToDart(reinterpret_cast(fdio_ns_.get()))); - FML_CHECK(!tonic::LogIfError(result)); + FML_CHECK(!tonic::CheckAndHandleError(result)); } void IsolateConfigurator::BindDartIO() { // Grab the dart:io lib. Dart_Handle io_lib = Dart_LookupLibrary(tonic::ToDart("dart:io")); - FML_CHECK(!tonic::LogIfError(io_lib)); + FML_CHECK(!tonic::CheckAndHandleError(io_lib)); // Disable dart:io exit() Dart_Handle embedder_config_type = Dart_GetNonNullableType( io_lib, tonic::ToDart("_EmbedderConfig"), 0, nullptr); - FML_CHECK(!tonic::LogIfError(embedder_config_type)); + FML_CHECK(!tonic::CheckAndHandleError(embedder_config_type)); Dart_Handle result = Dart_SetField(embedder_config_type, tonic::ToDart("_mayExit"), Dart_False()); - FML_CHECK(!tonic::LogIfError(result)); + FML_CHECK(!tonic::CheckAndHandleError(result)); // Tell dart:io about the FDIO namespace configured for this instance. Dart_Handle namespace_type = Dart_GetNonNullableType(io_lib, tonic::ToDart("_Namespace"), 0, nullptr); - FML_CHECK(!tonic::LogIfError(namespace_type)); + FML_CHECK(!tonic::CheckAndHandleError(namespace_type)); Dart_Handle namespace_args[] = { Dart_NewInteger(reinterpret_cast(fdio_ns_.get())), // }; result = Dart_Invoke(namespace_type, tonic::ToDart("_setupNamespace"), 1, namespace_args); - FML_CHECK(!tonic::LogIfError(result)); + FML_CHECK(!tonic::CheckAndHandleError(result)); } } // namespace flutter_runner diff --git a/testing/test_dart_native_resolver.cc b/testing/test_dart_native_resolver.cc index f1123d18b751e..dc2cf3e303e34 100644 --- a/testing/test_dart_native_resolver.cc +++ b/testing/test_dart_native_resolver.cc @@ -101,11 +101,11 @@ void TestDartNativeResolver::SetNativeResolverForIsolate() { auto result = Dart_SetNativeResolver(Dart_RootLibrary(), DartNativeEntryResolverCallback, DartNativeEntrySymbolCallback); - FML_CHECK(!tonic::LogIfError(result)) + FML_CHECK(!tonic::CheckAndHandleError(result)) << "Could not set native resolver in test."; result = Dart_SetFfiNativeResolver(Dart_RootLibrary(), &FfiNativeResolver); - FML_CHECK(!tonic::LogIfError(result)) + FML_CHECK(!tonic::CheckAndHandleError(result)) << "Could not set FFI native resolver in test."; std::scoped_lock lock(gIsolateResolversMutex); diff --git a/third_party/tonic/converter/dart_converter.h b/third_party/tonic/converter/dart_converter.h index 0947241130a3b..be2d7818febde 100644 --- a/third_party/tonic/converter/dart_converter.h +++ b/third_party/tonic/converter/dart_converter.h @@ -397,15 +397,15 @@ inline Dart_Handle LookupNonNullableType(const std::string& library_name, const std::string& type_name) { auto library = Dart_LookupLibrary(DartConverter::ToDart(library_name)); - if (LogIfError(library)) { + if (CheckAndHandleError(library)) { return library; } auto type_string = DartConverter::ToDart(type_name); - if (LogIfError(type_string)) { + if (CheckAndHandleError(type_string)) { return type_string; } auto type = Dart_GetNonNullableType(library, type_string, 0, nullptr); - if (LogIfError(type)) { + if (CheckAndHandleError(type)) { return type; } return type; @@ -439,7 +439,7 @@ Dart_Handle CreateZeroInitializedDartObject( return ::Dart_NewDouble(0.0); } else { auto object = ::Dart_New(type_handle_or_null, ::Dart_Null(), 0, nullptr); - LogIfError(object); + CheckAndHandleError(object); return object; } return ::Dart_Null(); @@ -450,20 +450,20 @@ struct DartListFactory { static Dart_Handle NewList(Dart_Handle type_handle, intptr_t length) { bool is_nullable = false; auto is_nullable_handle = ::Dart_IsNullableType(type_handle, &is_nullable); - if (LogIfError(is_nullable_handle)) { + if (CheckAndHandleError(is_nullable_handle)) { return is_nullable_handle; } if (is_nullable) { auto list = ::Dart_NewListOfType(type_handle, length); - LogIfError(list); + CheckAndHandleError(list); return list; } else { auto sentinel = CreateZeroInitializedDartObject(type_handle); - if (LogIfError(sentinel)) { + if (CheckAndHandleError(sentinel)) { return sentinel; } auto list = ::Dart_NewListOfTypeFilled(type_handle, sentinel, length); - LogIfError(list); + CheckAndHandleError(list); return list; } return ::Dart_Null(); diff --git a/third_party/tonic/dart_args.h b/third_party/tonic/dart_args.h index 438ff7082bc81..2469e24f7b6ef 100644 --- a/third_party/tonic/dart_args.h +++ b/third_party/tonic/dart_args.h @@ -240,10 +240,10 @@ void DartCallConstructor(Sig func, Dart_NativeArguments args) { } Dart_Handle wrapper = Dart_GetNativeArgument(args, 0); - TONIC_CHECK(!LogIfError(wrapper)); + TONIC_CHECK(!CheckAndHandleError(wrapper)); intptr_t native_fields[DartWrappable::kNumberOfNativeFields]; - TONIC_CHECK(!LogIfError(Dart_GetNativeFieldsOfArgument( + TONIC_CHECK(!CheckAndHandleError(Dart_GetNativeFieldsOfArgument( args, 0, DartWrappable::kNumberOfNativeFields, native_fields))); TONIC_CHECK(!native_fields[DartWrappable::kPeerIndex]); diff --git a/third_party/tonic/dart_list.cc b/third_party/tonic/dart_list.cc index 095b45ade5364..302de94f49269 100644 --- a/third_party/tonic/dart_list.cc +++ b/third_party/tonic/dart_list.cc @@ -12,7 +12,7 @@ DartList::DartList(Dart_Handle dart_handle) : dart_handle_(dart_handle) { TONIC_DCHECK(Dart_IsList(dart_handle_)); intptr_t length; - is_valid_ = !LogIfError(Dart_ListLength(dart_handle_, &length)); + is_valid_ = !CheckAndHandleError(Dart_ListLength(dart_handle_, &length)); size_ = length; } @@ -32,14 +32,14 @@ DartList::DartList(DartList&& other) } void DartList::Set(size_t index, Dart_Handle value) { - LogIfError(Dart_ListSetAt(dart_handle_, index, value)); + CheckAndHandleError(Dart_ListSetAt(dart_handle_, index, value)); } DartList DartConverter::FromArguments(Dart_NativeArguments args, int index, Dart_Handle& exception) { Dart_Handle list = Dart_GetNativeArgument(args, index); - if (LogIfError(list) || !Dart_IsList(list)) { + if (CheckAndHandleError(list) || !Dart_IsList(list)) { exception = Dart_NewApiError("Invalid Argument"); return DartList(); } diff --git a/third_party/tonic/dart_message_handler.cc b/third_party/tonic/dart_message_handler.cc index 4119d54ec34c1..b1c501ea1f225 100644 --- a/third_party/tonic/dart_message_handler.cc +++ b/third_party/tonic/dart_message_handler.cc @@ -91,7 +91,7 @@ void DartMessageHandler::OnHandleMessage(DartState* dart_state) { Dart_SetPausedOnStart(false); // We've resumed, handle normal messages that are in the queue. result = Dart_HandleMessage(); - error = LogIfError(result); + error = CheckAndHandleError(result); dart_state->MessageEpilogue(result); if (!Dart_CurrentIsolate()) { isolate_exited_ = true; @@ -118,7 +118,7 @@ void DartMessageHandler::OnHandleMessage(DartState* dart_state) { Dart_IsFatalError(result)) { error = true; } else { - error = LogIfError(result); + error = CheckAndHandleError(result); } dart_state->MessageEpilogue(result); if (!Dart_CurrentIsolate()) { diff --git a/third_party/tonic/dart_microtask_queue.cc b/third_party/tonic/dart_microtask_queue.cc index 766151338040f..7aef037e836f4 100644 --- a/third_party/tonic/dart_microtask_queue.cc +++ b/third_party/tonic/dart_microtask_queue.cc @@ -80,7 +80,7 @@ void DartMicrotaskQueue::RunMicrotasks() { // log message. if (!dart_state->has_set_return_code() || !Dart_IsError(result) || !Dart_IsFatalError(result)) { - LogIfError(result); + CheckAndHandleError(result); } DartErrorHandleType error = GetErrorHandleType(result); if (error != kNoError) { diff --git a/third_party/tonic/dart_persistent_value.cc b/third_party/tonic/dart_persistent_value.cc index 652d2c9fc3a1e..0026d7206a420 100644 --- a/third_party/tonic/dart_persistent_value.cc +++ b/third_party/tonic/dart_persistent_value.cc @@ -40,6 +40,9 @@ void DartPersistentValue::Clear() { auto dart_state = dart_state_.lock(); if (!dart_state) { + // The Dart isolate was collected and the persistent value has been + // collected with it. value_ is a dangling reference. + value_ = nullptr; return; } diff --git a/third_party/tonic/dart_state.cc b/third_party/tonic/dart_state.cc index 3f37685c51be2..0b4771d08d034 100644 --- a/third_party/tonic/dart_state.cc +++ b/third_party/tonic/dart_state.cc @@ -56,7 +56,7 @@ DartState* DartState::From(Dart_Isolate isolate) { DartState* DartState::Current() { auto isolate_data = static_cast*>(Dart_CurrentIsolateData()); - return isolate_data->get(); + return isolate_data ? isolate_data->get() : nullptr; } std::weak_ptr DartState::GetWeakPtr() { diff --git a/third_party/tonic/dart_wrappable.cc b/third_party/tonic/dart_wrappable.cc index b746f9f8c7948..69cc7a533f6da 100644 --- a/third_party/tonic/dart_wrappable.cc +++ b/third_party/tonic/dart_wrappable.cc @@ -26,16 +26,16 @@ Dart_Handle DartWrappable::CreateDartWrapper(DartState* dart_state) { const DartWrapperInfo& info = GetDartWrapperInfo(); Dart_PersistentHandle type = dart_state->class_library().GetClass(info); - TONIC_DCHECK(!LogIfError(type)); + TONIC_DCHECK(!CheckAndHandleError(type)); Dart_Handle wrapper = Dart_New(type, dart_state->private_constructor_name(), 0, nullptr); - TONIC_DCHECK(!LogIfError(wrapper)); + TONIC_DCHECK(!CheckAndHandleError(wrapper)); Dart_Handle res = Dart_SetNativeInstanceField( wrapper, kPeerIndex, reinterpret_cast(this)); - TONIC_DCHECK(!LogIfError(res)); + TONIC_DCHECK(!CheckAndHandleError(res)); this->RetainDartWrappableReference(); // Balanced in FinalizeDartWrapper. dart_wrapper_.Set(dart_state, wrapper, this, GetAllocationSize(), @@ -51,9 +51,9 @@ void DartWrappable::AssociateWithDartWrapper(Dart_Handle wrapper) { dart_wrapper_.Clear(); } - TONIC_CHECK(!LogIfError(wrapper)); + TONIC_CHECK(!CheckAndHandleError(wrapper)); - TONIC_CHECK(!LogIfError(Dart_SetNativeInstanceField( + TONIC_CHECK(!CheckAndHandleError(Dart_SetNativeInstanceField( wrapper, kPeerIndex, reinterpret_cast(this)))); this->RetainDartWrappableReference(); // Balanced in FinalizeDartWrapper. @@ -66,7 +66,8 @@ void DartWrappable::AssociateWithDartWrapper(Dart_Handle wrapper) { void DartWrappable::ClearDartWrapper() { TONIC_DCHECK(!dart_wrapper_.is_empty()); Dart_Handle wrapper = dart_wrapper_.Get(); - TONIC_CHECK(!LogIfError(Dart_SetNativeInstanceField(wrapper, kPeerIndex, 0))); + TONIC_CHECK(!CheckAndHandleError( + Dart_SetNativeInstanceField(wrapper, kPeerIndex, 0))); dart_wrapper_.Clear(); this->ReleaseDartWrappableReference(); } diff --git a/third_party/tonic/logging/dart_error.cc b/third_party/tonic/logging/dart_error.cc index e1c4f59e8b45f..c10484ca6de44 100644 --- a/third_party/tonic/logging/dart_error.cc +++ b/third_party/tonic/logging/dart_error.cc @@ -4,6 +4,8 @@ #include "tonic/logging/dart_error.h" +#include + #include "tonic/common/macros.h" #include "tonic/converter/dart_converter.h" @@ -12,16 +14,30 @@ namespace DartError { const char kInvalidArgument[] = "Invalid argument."; } // namespace DartError -bool LogIfError(Dart_Handle handle) { +namespace { +void DefaultLogUnhandledException(Dart_Handle, Dart_Handle) {} +std::atomic log_unhandled_exception = + DefaultLogUnhandledException; + +void ReportUnhandledException(Dart_Handle exception_handle, + Dart_Handle stack_trace_handle) { + log_unhandled_exception.load()(exception_handle, stack_trace_handle); +} +} // namespace + +void SetUnhandledExceptionReporter( + DartError::UnhandledExceptionReporter reporter) { + log_unhandled_exception.store(reporter); +} + +bool CheckAndHandleError(Dart_Handle handle) { + // Specifically handle UnhandledExceptionErrors first. These exclude fatal + // errors that are shutting down the vm and compilation errors in source code. if (Dart_IsUnhandledExceptionError(handle)) { Dart_Handle exception_handle = Dart_ErrorGetException(handle); - const std::string exception = - tonic::StdStringFromDart(Dart_ToString(exception_handle)); Dart_Handle stack_trace_handle = Dart_ErrorGetStackTrace(handle); - const std::string stack_trace = - tonic::StdStringFromDart(Dart_ToString(stack_trace_handle)); - tonic::Log("Dart Unhandled Exception: %s, stack trace: %s", - exception.c_str(), stack_trace.c_str()); + + ReportUnhandledException(exception_handle, stack_trace_handle); return true; } else if (Dart_IsError(handle)) { tonic::Log("Dart Error: %s", Dart_GetError(handle)); diff --git a/third_party/tonic/logging/dart_error.h b/third_party/tonic/logging/dart_error.h index ede51f41da1c3..5cc5c0d5d81d0 100644 --- a/third_party/tonic/logging/dart_error.h +++ b/third_party/tonic/logging/dart_error.h @@ -7,13 +7,62 @@ #include "third_party/dart/runtime/include/dart_api.h" +#include "tonic/dart_persistent_value.h" + namespace tonic { namespace DartError { +using UnhandledExceptionReporter = void (*)(Dart_Handle, Dart_Handle); + extern const char kInvalidArgument[]; } // namespace DartError -bool LogIfError(Dart_Handle handle); +/// Check if a Dart_Handle is an error or exception. +/// +/// If it is an error or exception, this method will return true. +/// +/// If it is an unhandled error or exception, the closure in +/// |SetUnhandledExceptionReporter| is called. The DartVMInitializer provides +/// that closure, which checks with UIDartState::Current() if it is available +/// and falls back to simply printing the exception and stack to an error log if +/// the settings callback is not provided. +/// +/// If UIDartState::Current() is available, it can provide an onError callback +/// that forwards to `PlatformConfiguration.instance.onError`. If that callback +/// is not set, the callback from `Settings.unhandled_exception_callback` is +/// invoked. If that callback is not set, a simple error log is +/// printed. +/// +/// If the PlatformDispatcher callback throws an exception, the at least two +/// separate exceptions and stacktraces will be handled by either the +/// Settings.unhandled_exception_callback or the error printer: one for the +/// original exception, and one for the exception thrown in the callback. If the +/// callback returns false, the original exception and stacktrace are logged. If +/// it returns true, no additional logging is done. +/// +/// Leaving the PlatformDispatcher.instance.onError callback unset or returning +/// false from it matches the behavior of Flutter applications before the +/// introduction of PlatformDispatcher.onError, which is to print to the error +/// log. +/// +/// Dart has errors that are not considered unhandled exceptions, such as +/// Dart_* API usage errors. In these cases, `Dart_IsUnhandledException` returns +/// false but `Dart_IsError` returns true. Such errors are logged to stderr or +/// some similar mechanism provided by the platform such as logcat on Android. +/// Depending on which type of error occurs, the process may crash and the Dart +/// isolate may be unusable. Errors that fall into this category include +/// compilation errors, Dart API errors, and unwind errors that will terminate +/// the Dart VM. +/// +/// Historically known as LogIfError. +bool CheckAndHandleError(Dart_Handle handle); + +/// The fallback mechanism to log errors if the platform configuration error +/// handler returns false. +/// +/// Normally, UIDartState registers with this method in its constructor. +void SetUnhandledExceptionReporter( + DartError::UnhandledExceptionReporter reporter); enum DartErrorHandleType { kNoError, diff --git a/third_party/tonic/logging/dart_invoke.cc b/third_party/tonic/logging/dart_invoke.cc index 0b17c8349c764..abdcc82c29f33 100644 --- a/third_party/tonic/logging/dart_invoke.cc +++ b/third_party/tonic/logging/dart_invoke.cc @@ -22,13 +22,13 @@ Dart_Handle DartInvoke(Dart_Handle closure, int argc = args.size(); Dart_Handle* argv = const_cast(args.begin()); Dart_Handle handle = Dart_InvokeClosure(closure, argc, argv); - LogIfError(handle); + CheckAndHandleError(handle); return handle; } Dart_Handle DartInvokeVoid(Dart_Handle closure) { Dart_Handle handle = Dart_InvokeClosure(closure, 0, nullptr); - LogIfError(handle); + CheckAndHandleError(handle); return handle; } diff --git a/third_party/tonic/tests/dart_state_unittest.cc b/third_party/tonic/tests/dart_state_unittest.cc index 8ce1dfa15af7a..2e4a9c616115b 100644 --- a/third_party/tonic/tests/dart_state_unittest.cc +++ b/third_party/tonic/tests/dart_state_unittest.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "flutter/third_party/tonic/dart_state.h" #include "flutter/common/task_runners.h" #include "flutter/runtime/dart_vm_lifecycle.h" #include "flutter/runtime/isolate_configuration.h" @@ -12,6 +13,42 @@ namespace testing { using DartState = FixtureTest; +TEST_F(DartState, CurrentWithNullDataDoesNotSegfault) { + ASSERT_FALSE(DartVMRef::IsInstanceRunning()); + auto settings = CreateSettingsForFixture(); + auto vm_snapshot = DartSnapshot::VMSnapshotFromSettings(settings); + auto isolate_snapshot = DartSnapshot::IsolateSnapshotFromSettings(settings); + auto vm_ref = DartVMRef::Create(settings, vm_snapshot, isolate_snapshot); + ASSERT_TRUE(vm_ref); + auto vm_data = vm_ref.GetVMData(); + ASSERT_TRUE(vm_data); + TaskRunners task_runners(GetCurrentTestName(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner() // + ); + auto isolate_configuration = + IsolateConfiguration::InferFromSettings(settings); + Dart_IsolateFlags isolate_flags; + Dart_IsolateFlagsInitialize(&isolate_flags); + isolate_flags.null_safety = + isolate_configuration->IsNullSafetyEnabled(*isolate_snapshot); + isolate_flags.snapshot_is_dontneed_safe = isolate_snapshot->IsDontNeedSafe(); + char* error; + Dart_CreateIsolateGroup( + "main.dart", "main", vm_data->GetIsolateSnapshot()->GetDataMapping(), + vm_data->GetIsolateSnapshot()->GetInstructionsMapping(), &isolate_flags, + nullptr, nullptr, &error); + ASSERT_FALSE(error) << error; + ::free(error); + + ASSERT_FALSE(tonic::DartState::Current()); + + Dart_ShutdownIsolate(); + ASSERT_TRUE(Dart_CurrentIsolate() == nullptr); +} + TEST_F(DartState, IsShuttingDown) { ASSERT_FALSE(DartVMRef::IsInstanceRunning()); auto settings = CreateSettingsForFixture(); diff --git a/third_party/tonic/typed_data/dart_byte_data.cc b/third_party/tonic/typed_data/dart_byte_data.cc index cfb07399f22eb..cab038d14f85d 100644 --- a/third_party/tonic/typed_data/dart_byte_data.cc +++ b/third_party/tonic/typed_data/dart_byte_data.cc @@ -61,7 +61,7 @@ DartByteData::DartByteData(Dart_Handle list) Dart_TypedData_Type type; Dart_TypedDataAcquireData(list, &type, &data_, &length_in_bytes_); - TONIC_DCHECK(!LogIfError(list)); + TONIC_DCHECK(!CheckAndHandleError(list)); if (type != Dart_TypedData_kByteData) Dart_ThrowException(ToDart("Non-genuine ByteData passed to engine.")); } @@ -95,7 +95,7 @@ DartByteData DartConverter::FromArguments( int index, Dart_Handle& exception) { Dart_Handle data = Dart_GetNativeArgument(args, index); - TONIC_DCHECK(!LogIfError(data)); + TONIC_DCHECK(!CheckAndHandleError(data)); return DartByteData(data); } diff --git a/third_party/tonic/typed_data/typed_list.cc b/third_party/tonic/typed_data/typed_list.cc index ffe662591dea4..3336e0a8be395 100644 --- a/third_party/tonic/typed_data/typed_list.cc +++ b/third_party/tonic/typed_data/typed_list.cc @@ -23,7 +23,7 @@ TypedList::TypedList(Dart_Handle list) Dart_TypedData_Type type; Dart_TypedDataAcquireData(list, &type, reinterpret_cast(&data_), &num_elements_); - TONIC_DCHECK(!LogIfError(list)); + TONIC_DCHECK(!CheckAndHandleError(list)); if (type != kTypeName) Dart_ThrowException(ToDart("Non-genuine TypedData passed to engine.")); } @@ -61,7 +61,7 @@ DartConverter>::FromArguments( int index, Dart_Handle& exception) { Dart_Handle list = Dart_GetNativeArgument(args, index); - TONIC_DCHECK(!LogIfError(list)); + TONIC_DCHECK(!CheckAndHandleError(list)); return TypedList(list); } @@ -80,7 +80,7 @@ Dart_Handle DartConverter>::ToDart( unsigned int length) { const intptr_t buffer_length = static_cast(length); Dart_Handle array = Dart_NewTypedData(kTypeName, buffer_length); - TONIC_DCHECK(!LogIfError(array)); + TONIC_DCHECK(!CheckAndHandleError(array)); { Dart_TypedData_Type type; void* data = nullptr;