Move BufferHubMetadata off pdx::fileHandle

Use android::base::unique_fd to replace LocalHandle. For BorrowedHandle,
a const reference of the unique_fd is returned.

Updated the test cases to fit in current behavior.

Test: BufferHubMetadata_test, BufferHubBuffer_test, BufferNode_test,
buffer_hub-test (passed)
Fix: 118888072

Change-Id: I34de335ed9a10864ac226cd4d7d261ba0078045d

5 files changed, 46 insertions, 39 deletions

diff --git a/libs/ui/BufferHubBuffer.cpp b/libs/ui/BufferHubBuffer.cpp
index 8cc1a4e3b4..dd79775d01 100644
--- a/libs/ui/BufferHubBuffer.cpp
+++ b/libs/ui/BufferHubBuffer.cpp
@@ -36,10 +36,12 @@
 #include <private/dvr/bufferhub_rpc.h>
 #pragma clang diagnostic pop
 
-#include <ui/BufferHubBuffer.h>
-
 #include <poll.h>
 
+#include <android-base/unique_fd.h>
+#include <ui/BufferHubBuffer.h>
+
+using android::base::unique_fd;
 using android::dvr::BufferTraits;
 using android::dvr::DetachedBufferRPC;
 using android::dvr::NativeHandleWrapper;
@@ -132,7 +134,9 @@ int BufferHubBuffer::ImportGraphicBuffer() {
     const int bufferId = bufferTraits.id();
 
     // Import the metadata.
-    mMetadata = BufferHubMetadata::Import(bufferTraits.take_metadata_handle());
+    LocalHandle metadataHandle = bufferTraits.take_metadata_handle();
+    unique_fd metadataFd(metadataHandle.Release());
+    mMetadata = BufferHubMetadata::Import(std::move(metadataFd));
 
     if (!mMetadata.IsValid()) {
         ALOGE("BufferHubBuffer::ImportGraphicBuffer: invalid metadata.");
diff --git a/libs/ui/BufferHubMetadata.cpp b/libs/ui/BufferHubMetadata.cpp
index 1e08ed1388..18d9a2c963 100644
--- a/libs/ui/BufferHubMetadata.cpp
+++ b/libs/ui/BufferHubMetadata.cpp
@@ -48,47 +48,47 @@ BufferHubMetadata BufferHubMetadata::Create(size_t userMetadataSize) {
         return {};
     }
 
-    // Hand over the ownership of the fd to a pdx::LocalHandle immediately after the successful
-    // return of ashmem_create_region. The ashmemHandle is going to own the fd and to prevent fd
+    // Hand over the ownership of the fd to a unique_fd immediately after the successful
+    // return of ashmem_create_region. The ashmemFd is going to own the fd and to prevent fd
     // leaks during error handling.
-    pdx::LocalHandle ashmemHandle{fd};
+    unique_fd ashmemFd{fd};
 
-    if (ashmem_set_prot_region(ashmemHandle.Get(), kAshmemProt) != 0) {
+    if (ashmem_set_prot_region(ashmemFd.get(), kAshmemProt) != 0) {
         ALOGE("BufferHubMetadata::Create: failed to set protect region.");
         return {};
     }
 
-    return BufferHubMetadata::Import(std::move(ashmemHandle));
+    return BufferHubMetadata::Import(std::move(ashmemFd));
 }
 
 /* static */
-BufferHubMetadata BufferHubMetadata::Import(pdx::LocalHandle ashmemHandle) {
-    if (!ashmem_valid(ashmemHandle.Get())) {
+BufferHubMetadata BufferHubMetadata::Import(unique_fd ashmemFd) {
+    if (!ashmem_valid(ashmemFd.get())) {
         ALOGE("BufferHubMetadata::Import: invalid ashmem fd.");
         return {};
     }
 
-    size_t metadataSize = static_cast<size_t>(ashmem_get_size_region(ashmemHandle.Get()));
+    size_t metadataSize = static_cast<size_t>(ashmem_get_size_region(ashmemFd.get()));
     size_t userMetadataSize = metadataSize - kMetadataHeaderSize;
 
     // Note that here the buffer state is mapped from shared memory as an atomic object. The
     // std::atomic's constructor will not be called so that the original value stored in the memory
     // region can be preserved.
     auto metadataHeader = static_cast<MetadataHeader*>(mmap(nullptr, metadataSize, kAshmemProt,
-                                                            MAP_SHARED, ashmemHandle.Get(),
+                                                            MAP_SHARED, ashmemFd.get(),
                                                             /*offset=*/0));
     if (metadataHeader == nullptr) {
         ALOGE("BufferHubMetadata::Import: failed to map region.");
         return {};
     }
 
-    return BufferHubMetadata(userMetadataSize, std::move(ashmemHandle), metadataHeader);
+    return BufferHubMetadata(userMetadataSize, std::move(ashmemFd), metadataHeader);
 }
 
-BufferHubMetadata::BufferHubMetadata(size_t userMetadataSize, pdx::LocalHandle ashmemHandle,
+BufferHubMetadata::BufferHubMetadata(size_t userMetadataSize, unique_fd ashmemFd,
                                      MetadataHeader* metadataHeader)
       : mUserMetadataSize(userMetadataSize),
-        mAshmemHandle(std::move(ashmemHandle)),
+        mAshmemFd(std::move(ashmemFd)),
         mMetadataHeader(metadataHeader) {}
 
 BufferHubMetadata::~BufferHubMetadata() {
diff --git a/libs/ui/include/ui/BufferHubMetadata.h b/libs/ui/include/ui/BufferHubMetadata.h
index 94a9000a32..4261971f55 100644
--- a/libs/ui/include/ui/BufferHubMetadata.h
+++ b/libs/ui/include/ui/BufferHubMetadata.h
@@ -33,12 +33,17 @@
 #pragma clang diagnostic ignored "-Wundefined-func-template"
 #pragma clang diagnostic ignored "-Wunused-template"
 #pragma clang diagnostic ignored "-Wweak-vtables"
-#include <pdx/file_handle.h>
 #include <private/dvr/buffer_hub_defs.h>
 #pragma clang diagnostic pop
 
+#include <android-base/unique_fd.h>
+
 namespace android {
 
+namespace {
+using base::unique_fd;
+} // namespace
+
 class BufferHubMetadata {
 public:
     // Creates a new BufferHubMetadata backed by an ashmem region.
@@ -50,11 +55,8 @@ public:
 
     // Imports an existing BufferHubMetadata from an ashmem FD.
     //
-    // TODO(b/112338294): Refactor BufferHub to use Binder as its internal IPC backend instead of
-    // UDS.
-    //
-    // @param ashmemHandle Ashmem file handle representing an ashmem region.
-    static BufferHubMetadata Import(pdx::LocalHandle ashmemHandle);
+    // @param ashmemFd Ashmem file descriptor representing an ashmem region.
+    static BufferHubMetadata Import(unique_fd ashmemFd);
 
     BufferHubMetadata() = default;
 
@@ -67,7 +69,7 @@ public:
             mUserMetadataSize = other.mUserMetadataSize;
             other.mUserMetadataSize = 0;
 
-            mAshmemHandle = std::move(other.mAshmemHandle);
+            mAshmemFd = std::move(other.mAshmemFd);
 
             // The old raw mMetadataHeader pointer must be cleared, otherwise the destructor will
             // automatically mummap() the shared memory.
@@ -79,25 +81,25 @@ public:
 
     // Returns true if the metadata is valid, i.e. the metadata has a valid ashmem fd and the ashmem
     // has been mapped into virtual address space.
-    bool IsValid() const { return mAshmemHandle.IsValid() && mMetadataHeader != nullptr; }
+    bool IsValid() const { return mAshmemFd.get() != -1 && mMetadataHeader != nullptr; }
 
     size_t user_metadata_size() const { return mUserMetadataSize; }
     size_t metadata_size() const {
         return mUserMetadataSize + dvr::BufferHubDefs::kMetadataHeaderSize;
     }
 
-    const pdx::LocalHandle& ashmem_handle() const { return mAshmemHandle; }
+    const unique_fd& ashmem_fd() const { return mAshmemFd; }
     dvr::BufferHubDefs::MetadataHeader* metadata_header() { return mMetadataHeader; }
 
 private:
-    BufferHubMetadata(size_t userMetadataSize, pdx::LocalHandle ashmemHandle,
+    BufferHubMetadata(size_t userMetadataSize, unique_fd ashmemFd,
                       dvr::BufferHubDefs::MetadataHeader* metadataHeader);
 
     BufferHubMetadata(const BufferHubMetadata&) = delete;
     void operator=(const BufferHubMetadata&) = delete;
 
     size_t mUserMetadataSize = 0;
-    pdx::LocalHandle mAshmemHandle;
+    unique_fd mAshmemFd;
     dvr::BufferHubDefs::MetadataHeader* mMetadataHeader = nullptr;
 };
 
diff --git a/libs/ui/tests/BufferHubMetadata_test.cpp b/libs/ui/tests/BufferHubMetadata_test.cpp
index 4209392afa..70f86b3ef2 100644
--- a/libs/ui/tests/BufferHubMetadata_test.cpp
+++ b/libs/ui/tests/BufferHubMetadata_test.cpp
@@ -43,11 +43,11 @@ TEST_F(BufferHubMetadataTest, Import_Success) {
   EXPECT_TRUE(m1.IsValid());
   EXPECT_NE(m1.metadata_header(), nullptr);
 
-  pdx::LocalHandle h2 = m1.ashmem_handle().Duplicate();
-  EXPECT_TRUE(h2.IsValid());
+  unique_fd h2 = unique_fd(dup(m1.ashmem_fd().get()));
+  EXPECT_NE(h2.get(), -1);
 
   BufferHubMetadata m2 = BufferHubMetadata::Import(std::move(h2));
-  EXPECT_FALSE(h2.IsValid());
+  EXPECT_EQ(h2.get(), -1);
   EXPECT_TRUE(m1.IsValid());
   BufferHubDefs::MetadataHeader* mh1 = m1.metadata_header();
   EXPECT_NE(mh1, nullptr);
@@ -71,31 +71,29 @@ TEST_F(BufferHubMetadataTest, MoveMetadataInvalidatesOldOne) {
   BufferHubMetadata m1 = BufferHubMetadata::Create(sizeof(int));
   EXPECT_TRUE(m1.IsValid());
   EXPECT_NE(m1.metadata_header(), nullptr);
-  EXPECT_TRUE(m1.ashmem_handle().IsValid());
+  EXPECT_NE(m1.ashmem_fd().get(), -1);
   EXPECT_EQ(m1.user_metadata_size(), sizeof(int));
 
   BufferHubMetadata m2 = std::move(m1);
 
-  // After the move, the metadata header (a raw pointer) should be reset in the
-  // older buffer.
+  // After the move, the metadata header (a raw pointer) should be reset in the older buffer.
   EXPECT_EQ(m1.metadata_header(), nullptr);
   EXPECT_NE(m2.metadata_header(), nullptr);
 
-  EXPECT_FALSE(m1.ashmem_handle().IsValid());
-  EXPECT_TRUE(m2.ashmem_handle().IsValid());
+  EXPECT_EQ(m1.ashmem_fd().get(), -1);
+  EXPECT_NE(m2.ashmem_fd().get(), -1);
 
   EXPECT_EQ(m1.user_metadata_size(), 0U);
   EXPECT_EQ(m2.user_metadata_size(), sizeof(int));
 
   BufferHubMetadata m3{std::move(m2)};
 
-  // After the move, the metadata header (a raw pointer) should be reset in the
-  // older buffer.
+  // After the move, the metadata header (a raw pointer) should be reset in the older buffer.
   EXPECT_EQ(m2.metadata_header(), nullptr);
   EXPECT_NE(m3.metadata_header(), nullptr);
 
-  EXPECT_FALSE(m2.ashmem_handle().IsValid());
-  EXPECT_TRUE(m3.ashmem_handle().IsValid());
+  EXPECT_EQ(m2.ashmem_fd().get(), -1);
+  EXPECT_NE(m3.ashmem_fd().get(), -1);
 
   EXPECT_EQ(m2.user_metadata_size(), 0U);
   EXPECT_EQ(m3.user_metadata_size(), sizeof(int));
diff --git a/services/vr/bufferhubd/buffer_channel.cpp b/services/vr/bufferhubd/buffer_channel.cpp
index 589b31a285..6c64bcd279 100644
--- a/services/vr/bufferhubd/buffer_channel.cpp
+++ b/services/vr/bufferhubd/buffer_channel.cpp
@@ -83,10 +83,13 @@ Status<BufferTraits<BorrowedHandle>> BufferChannel::OnImport(
   ATRACE_NAME("BufferChannel::OnImport");
   ALOGD_IF(TRACE, "BufferChannel::OnImport: buffer=%d.", buffer_id());
 
+  BorrowedHandle ashmem_handle =
+      BorrowedHandle(buffer_node_->metadata().ashmem_fd().get());
+
   // TODO(b/112057680) Move away from the GraphicBuffer-based IonBuffer.
   return BufferTraits<BorrowedHandle>{
       /*buffer_handle=*/buffer_node_->buffer_handle(),
-      /*metadata_handle=*/buffer_node_->metadata().ashmem_handle().Borrow(),
+      /*metadata_handle=*/ashmem_handle,
       /*id=*/buffer_id(),
       /*client_state_mask=*/client_state_mask_,
       /*metadata_size=*/buffer_node_->metadata().metadata_size(),