From 958384b6231384a2b51eb7229e54e56338c8766a Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Tue, 2 Apr 2024 15:33:51 -0700 Subject: [PATCH] Return an empty optional in HardwareBuffer::GetSystemUniqueID if the underlying NDK API is unavailable (#51839) --- impeller/toolkit/android/BUILD.gn | 2 ++ impeller/toolkit/android/hardware_buffer.cc | 2 +- impeller/toolkit/android/proc_table.cc | 5 +++++ impeller/toolkit/android/proc_table.h | 4 ++++ .../android/toolkit_android_unittests.cc | 17 +++++++++++++++++ 5 files changed, 29 insertions(+), 1 deletion(-) diff --git a/impeller/toolkit/android/BUILD.gn b/impeller/toolkit/android/BUILD.gn index e0f83d652b573..758a5a9e9911f 100644 --- a/impeller/toolkit/android/BUILD.gn +++ b/impeller/toolkit/android/BUILD.gn @@ -50,6 +50,8 @@ source_set("unittests_lib") { ":unittests_fixtures", "//flutter/testing", ] + + defines = [ "TESTING" ] } executable("unittests") { diff --git a/impeller/toolkit/android/hardware_buffer.cc b/impeller/toolkit/android/hardware_buffer.cc index b3e6e5afc485d..a9ec935e95900 100644 --- a/impeller/toolkit/android/hardware_buffer.cc +++ b/impeller/toolkit/android/hardware_buffer.cc @@ -108,7 +108,7 @@ std::optional HardwareBuffer::GetSystemUniqueID() const { std::optional HardwareBuffer::GetSystemUniqueID( AHardwareBuffer* buffer) { if (!GetProcTable().AHardwareBuffer_getId) { - return false; + return std::nullopt; } uint64_t out_id = 0u; if (GetProcTable().AHardwareBuffer_getId(buffer, &out_id) != 0) { diff --git a/impeller/toolkit/android/proc_table.cc b/impeller/toolkit/android/proc_table.cc index 71e6ce5c4d14d..24f9f99008a3d 100644 --- a/impeller/toolkit/android/proc_table.cc +++ b/impeller/toolkit/android/proc_table.cc @@ -14,6 +14,11 @@ const ProcTable& GetProcTable() { return gProcTable; } +// Only used by tests. +ProcTable& GetMutableProcTable() { + return const_cast(GetProcTable()); +} + template void ResolveAndroidProc( AndroidProc& proc, diff --git a/impeller/toolkit/android/proc_table.h b/impeller/toolkit/android/proc_table.h index 83b38a1293c79..af398fd396bf9 100644 --- a/impeller/toolkit/android/proc_table.h +++ b/impeller/toolkit/android/proc_table.h @@ -124,6 +124,10 @@ struct ProcTable { const ProcTable& GetProcTable(); +#ifdef TESTING +ProcTable& GetMutableProcTable(); +#endif + } // namespace impeller::android #endif // FLUTTER_IMPELLER_TOOLKIT_ANDROID_PROC_TABLE_H_ diff --git a/impeller/toolkit/android/toolkit_android_unittests.cc b/impeller/toolkit/android/toolkit_android_unittests.cc index 4d26b0bfb2a1e..c0086d4d264eb 100644 --- a/impeller/toolkit/android/toolkit_android_unittests.cc +++ b/impeller/toolkit/android/toolkit_android_unittests.cc @@ -12,6 +12,18 @@ namespace impeller::android::testing { +#define DISABLE_ANDROID_PROC(name) \ + struct Disable##name { \ + Disable##name() { \ + real_proc = GetMutableProcTable().name.proc; \ + GetMutableProcTable().name.proc = nullptr; \ + } \ + ~Disable##name() { \ + GetMutableProcTable().name.proc = real_proc; \ + } \ + decltype(name)* real_proc; \ + } disable##name; + TEST(ToolkitAndroidTest, CanCreateProcTable) { ProcTable proc_table; ASSERT_TRUE(proc_table.IsValid()); @@ -49,6 +61,11 @@ TEST(ToolkitAndroidTest, CanGetHardwareBufferIDs) { ASSERT_TRUE(buffer.GetSystemUniqueID().has_value()); } +TEST(ToolkitAndroidTest, HardwareBufferNullIDIfAPIUnavailable) { + DISABLE_ANDROID_PROC(AHardwareBuffer_getId); + ASSERT_FALSE(HardwareBuffer::GetSystemUniqueID(nullptr).has_value()); +} + TEST(ToolkitAndroidTest, CanDescribeHardwareBufferHandles) { if (!HardwareBuffer::IsAvailableOnPlatform()) { GTEST_SKIP() << "Hardware buffers are not supported on this platform.";