From f1348100a76dad9b60254b5e7793131af52252cf Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Tue, 9 Jul 2024 14:40:42 -0700 Subject: [PATCH 1/2] [Impeller] Validation errors in tests cause GTest failures. Earlier, this used to take down the entire process. Now, the entire set of failures will be listed instead. --- impeller/base/validation.cc | 9 +++++++++ impeller/base/validation.h | 16 ++++++++++++++++ impeller/playground/playground_test.cc | 23 +++++++++++++++++++---- 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/impeller/base/validation.cc b/impeller/base/validation.cc index ebba11852489c..9768a84e6dd66 100644 --- a/impeller/base/validation.cc +++ b/impeller/base/validation.cc @@ -12,11 +12,16 @@ namespace impeller { static std::atomic_int32_t sValidationLogsDisabledCount = 0; static std::atomic_int32_t sValidationLogsAreFatal = 0; +static ValidationFailureCallback sValidationFailureCallback; void ImpellerValidationErrorsSetFatal(bool fatal) { sValidationLogsAreFatal = fatal; } +void ImpellerValidationErrorsSetCallback(ValidationFailureCallback callback) { + sValidationFailureCallback = callback; +} + ScopedValidationDisable::ScopedValidationDisable() { sValidationLogsDisabledCount++; } @@ -47,6 +52,10 @@ std::ostream& ValidationLog::GetStream() { } void ImpellerValidationBreak(const char* message, const char* file, int line) { + if (sValidationFailureCallback && + sValidationFailureCallback(message, file, line)) { + return; + } const auto severity = ImpellerValidationErrorsAreFatal() ? fml::LOG_FATAL : fml::LOG_ERROR; auto fml_log = fml::LogMessage{severity, file, line, nullptr}; diff --git a/impeller/base/validation.h b/impeller/base/validation.h index 47ef7c20fc559..789ea8c004aa8 100644 --- a/impeller/base/validation.h +++ b/impeller/base/validation.h @@ -5,6 +5,7 @@ #ifndef FLUTTER_IMPELLER_BASE_VALIDATION_H_ #define FLUTTER_IMPELLER_BASE_VALIDATION_H_ +#include #include namespace impeller { @@ -37,6 +38,21 @@ void ImpellerValidationErrorsSetFatal(bool fatal); bool ImpellerValidationErrorsAreFatal(); +using ValidationFailureCallback = + std::function; + +//------------------------------------------------------------------------------ +/// @brief Sets a callback that callers (usually tests) can set to +/// intercept validation failures. +/// +/// Returning true from the callback indicates that Impeller can +/// continue and avoid any default behavior on tripping validation +/// (which could include process termination). +/// +/// @param[in] callback The callback +/// +void ImpellerValidationErrorsSetCallback(ValidationFailureCallback callback); + struct ScopedValidationDisable { ScopedValidationDisable(); diff --git a/impeller/playground/playground_test.cc b/impeller/playground/playground_test.cc index ce9cf9241b05d..2e5136336a04a 100644 --- a/impeller/playground/playground_test.cc +++ b/impeller/playground/playground_test.cc @@ -11,9 +11,26 @@ namespace impeller { PlaygroundTest::PlaygroundTest() - : Playground(PlaygroundSwitches{flutter::testing::GetArgsForProcess()}) {} + : Playground(PlaygroundSwitches{flutter::testing::GetArgsForProcess()}) { + ImpellerValidationErrorsSetCallback( + [](const char* message, const char* file, int line) -> bool { + // GTEST_MESSAGE_AT_ can only be used in a function that returns void. + // Hence the goofy lambda. The failure message and location will still + // be correct however. + // + // https://google.github.io/googletest/advanced.html#assertion-placement + [message, file, line]() -> void { + GTEST_MESSAGE_AT_(file, line, "Impeller Validation Error", + ::testing::TestPartResult::kFatalFailure) + << message; + }(); + return true; + }); +} -PlaygroundTest::~PlaygroundTest() = default; +PlaygroundTest::~PlaygroundTest() { + ImpellerValidationErrorsSetCallback(nullptr); +} namespace { bool DoesSupportWideGamutTests() { @@ -36,8 +53,6 @@ void PlaygroundTest::SetUp() { return; } - ImpellerValidationErrorsSetFatal(true); - // Test names that end with "WideGamut" will render with wide gamut support. std::string test_name = flutter::testing::GetCurrentTestName(); PlaygroundSwitches switches = switches_; From c82bb74b50560e2170fbb4a69804700d25b7b90c Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Tue, 9 Jul 2024 15:59:22 -0700 Subject: [PATCH 2/2] Tidy. --- impeller/base/validation.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/base/validation.cc b/impeller/base/validation.cc index 9768a84e6dd66..987ad4b4e6aa4 100644 --- a/impeller/base/validation.cc +++ b/impeller/base/validation.cc @@ -19,7 +19,7 @@ void ImpellerValidationErrorsSetFatal(bool fatal) { } void ImpellerValidationErrorsSetCallback(ValidationFailureCallback callback) { - sValidationFailureCallback = callback; + sValidationFailureCallback = std::move(callback); } ScopedValidationDisable::ScopedValidationDisable() {