Return an empty optional in HardwareBuffer::GetSystemUniqueID if the …

…underlying NDK API is unavailable (flutter#51839)
Jasguerrero · Apr 4, 2024 · 958384b · 958384b
1 parent 9755090
commit 958384b
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 1 deletion.
diff --git 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
@@ -108,7 +108,7 @@ std::optional<uint64_t> HardwareBuffer::GetSystemUniqueID() const {
 std::optional<uint64_t> 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
@@ -14,6 +14,11 @@ const ProcTable& GetProcTable() {
   return gProcTable;
 }
 
+// Only used by tests.
+ProcTable& GetMutableProcTable() {
+  return const_cast<ProcTable&>(GetProcTable());
+}
+
 template <class T>
 void ResolveAndroidProc(
     AndroidProc<T>& proc,

diff --git 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
@@ -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.";