Merge "Replace the use of native_handle_t by NativeHandle"

3 files changed, 22 insertions, 39 deletions

diff --git a/libs/ui/BufferHubBuffer.cpp b/libs/ui/BufferHubBuffer.cpp
index cce35fdf29..405bcb62e1 100644
--- a/libs/ui/BufferHubBuffer.cpp
+++ b/libs/ui/BufferHubBuffer.cpp
@@ -47,8 +47,8 @@ std::unique_ptr<BufferHubBuffer> BufferHubBuffer::create(uint32_t width, uint32_
     return buffer->isValid() ? std::move(buffer) : nullptr;
 }
 
-std::unique_ptr<BufferHubBuffer> BufferHubBuffer::import(const native_handle_t* token) {
-    if (token == nullptr) {
+std::unique_ptr<BufferHubBuffer> BufferHubBuffer::import(const sp<NativeHandle>& token) {
+    if (token == nullptr || token.get() == nullptr) {
         ALOGE("%s: token cannot be nullptr!", __FUNCTION__);
         return nullptr;
     }
@@ -105,7 +105,7 @@ BufferHubBuffer::BufferHubBuffer(uint32_t width, uint32_t height, uint32_t layer
     mBufferClient = std::move(client);
 }
 
-BufferHubBuffer::BufferHubBuffer(const native_handle_t* token) {
+BufferHubBuffer::BufferHubBuffer(const sp<NativeHandle>& token) {
     sp<IBufferHub> bufferhub = IBufferHub::getService();
     if (bufferhub.get() == nullptr) {
         ALOGE("%s: BufferHub service not found!", __FUNCTION__);
@@ -124,7 +124,7 @@ BufferHubBuffer::BufferHubBuffer(const native_handle_t* token) {
 
     // hidl_handle(native_handle_t*) simply creates a raw pointer reference withouth ownership
     // transfer.
-    if (!bufferhub->importBuffer(hidl_handle(token), import_cb).isOk()) {
+    if (!bufferhub->importBuffer(hidl_handle(token.get()->handle()), import_cb).isOk()) {
         ALOGE("%s: importBuffer transaction failed!", __FUNCTION__);
         return;
     } else if (ret != BufferHubStatus::NO_ERROR) {
@@ -328,7 +328,7 @@ bool BufferHubBuffer::isValid() const {
             mEventFd.get() >= 0 && mMetadata.isValid() && mBufferClient != nullptr;
 }
 
-native_handle_t* BufferHubBuffer::duplicate() {
+sp<NativeHandle> BufferHubBuffer::duplicate() {
     if (mBufferClient == nullptr) {
         ALOGE("%s: missing BufferClient!", __FUNCTION__);
         return nullptr;
@@ -352,7 +352,7 @@ native_handle_t* BufferHubBuffer::duplicate() {
         return nullptr;
     }
 
-    return native_handle_clone(token.getNativeHandle());
+    return NativeHandle::create(native_handle_clone(token.getNativeHandle()), /*ownsHandle=*/true);
 }
 
 } // namespace android
diff --git a/libs/ui/include/ui/BufferHubBuffer.h b/libs/ui/include/ui/BufferHubBuffer.h
index eac8c8437b..5c0903251a 100644
--- a/libs/ui/include/ui/BufferHubBuffer.h
+++ b/libs/ui/include/ui/BufferHubBuffer.h
@@ -23,6 +23,7 @@
 #include <ui/BufferHubDefs.h>
 #include <ui/BufferHubEventFd.h>
 #include <ui/BufferHubMetadata.h>
+#include <utils/NativeHandle.h>
 
 namespace android {
 
@@ -33,10 +34,8 @@ public:
                                                    uint32_t layerCount, uint32_t format,
                                                    uint64_t usage, size_t userMetadataSize);
 
-    // Imports the given token to a BufferHubBuffer. Not taking ownership of the token. Caller
-    // should close and destroy the token after calling this function regardless of output.
-    // TODO(b/122543147): use a movable wrapper for token
-    static std::unique_ptr<BufferHubBuffer> import(const native_handle_t* token);
+    // Imports the given token to a BufferHubBuffer. Not taking ownership of the token.
+    static std::unique_ptr<BufferHubBuffer> import(const sp<NativeHandle>& token);
 
     BufferHubBuffer(const BufferHubBuffer&) = delete;
     void operator=(const BufferHubBuffer&) = delete;
@@ -100,17 +99,15 @@ public:
 
     // Creates a token that stands for this BufferHubBuffer client and could be used for Import to
     // create another BufferHubBuffer. The new BufferHubBuffer will share the same underlying
-    // gralloc buffer and ashmem region for metadata. Note that the caller owns the token and
-    // should free it after use.
+    // gralloc buffer and ashmem region for metadata. Not taking ownership of the token.
     // Returns a valid token on success, nullptr on failure.
-    // TODO(b/122543147): use a movable wrapper for token
-    native_handle_t* duplicate();
+    sp<NativeHandle> duplicate();
 
 private:
     BufferHubBuffer(uint32_t width, uint32_t height, uint32_t layerCount, uint32_t format,
                     uint64_t usage, size_t userMetadataSize);
 
-    BufferHubBuffer(const native_handle_t* token);
+    BufferHubBuffer(const sp<NativeHandle>& token);
 
     int initWithBufferTraits(const frameworks::bufferhub::V1_0::BufferTraits& bufferTraits);
 
diff --git a/libs/ui/tests/BufferHubBuffer_test.cpp b/libs/ui/tests/BufferHubBuffer_test.cpp
index 3087a90787..634bce17fe 100644
--- a/libs/ui/tests/BufferHubBuffer_test.cpp
+++ b/libs/ui/tests/BufferHubBuffer_test.cpp
@@ -86,13 +86,10 @@ void BufferHubBufferStateTransitionTest::CreateTwoClientsOfABuffer() {
     b1ClientMask = b1->clientStateMask();
     ASSERT_NE(b1ClientMask, 0U);
 
-    native_handle_t* token = b1->duplicate();
+    sp<NativeHandle> token = b1->duplicate();
     ASSERT_THAT(token, NotNull());
 
-    // TODO(b/122543147): use a movalbe wrapper for token
     b2 = BufferHubBuffer::import(token);
-    native_handle_close(token);
-    native_handle_delete(token);
     ASSERT_THAT(b2, NotNull());
 
     b2ClientMask = b2->clientStateMask();
@@ -137,16 +134,14 @@ TEST_F(BufferHubBufferTest, DuplicateAndImportBuffer) {
     ASSERT_THAT(b1, NotNull());
     EXPECT_TRUE(b1->isValid());
 
-    native_handle_t* token = b1->duplicate();
-    EXPECT_TRUE(token);
+    sp<NativeHandle> token = b1->duplicate();
+    ASSERT_THAT(token, NotNull());
 
     // The detached buffer should still be valid.
     EXPECT_TRUE(b1->isConnected());
     EXPECT_TRUE(b1->isValid());
 
     std::unique_ptr<BufferHubBuffer> b2 = BufferHubBuffer::import(token);
-    native_handle_close(token);
-    native_handle_delete(token);
 
     ASSERT_THAT(b2, NotNull());
     EXPECT_TRUE(b2->isValid());
@@ -197,16 +192,13 @@ TEST_F(BufferHubBufferTest, ImportFreedBuffer) {
     ASSERT_THAT(b1, NotNull());
     EXPECT_TRUE(b1->isValid());
 
-    native_handle_t* token = b1->duplicate();
-    EXPECT_TRUE(token);
+    sp<NativeHandle> token = b1->duplicate();
+    ASSERT_THAT(token, NotNull());
 
     // Explicitly destroy b1. Backend buffer should be freed and token becomes invalid
     b1.reset();
 
-    // TODO(b/122543147): use a movalbe wrapper for token
     std::unique_ptr<BufferHubBuffer> b2 = BufferHubBuffer::import(token);
-    native_handle_close(token);
-    native_handle_delete(token);
 
     // Import should fail with INVALID_TOKEN
     EXPECT_THAT(b2, IsNull());
@@ -222,7 +214,7 @@ TEST_F(BufferHubBufferTest, ImportInvalidToken) {
     native_handle_t* token = native_handle_create(/*numFds=*/0, /*numInts=*/1);
     token->data[0] = 0;
 
-    auto b1 = BufferHubBuffer::import(token);
+    auto b1 = BufferHubBuffer::import(NativeHandle::create(token, /*ownHandle=*/true));
     native_handle_delete(token);
 
     EXPECT_THAT(b1, IsNull());
@@ -425,13 +417,10 @@ TEST_F(BufferHubBufferTest, createNewConsumerAfterGain) {
 
     // Create a consumer of the buffer and test if the consumer can acquire the
     // buffer if producer posts.
-    // TODO(b/122543147): use a movalbe wrapper for token
-    native_handle_t* token = b1->duplicate();
-    ASSERT_TRUE(token);
+    sp<NativeHandle> token = b1->duplicate();
+    ASSERT_THAT(token, NotNull());
 
     std::unique_ptr<BufferHubBuffer> b2 = BufferHubBuffer::import(token);
-    native_handle_close(token);
-    native_handle_delete(token);
 
     ASSERT_THAT(b2, NotNull());
     ASSERT_NE(b1->clientStateMask(), b2->clientStateMask());
@@ -450,13 +439,10 @@ TEST_F(BufferHubBufferTest, createNewConsumerAfterPost) {
 
     // Create a consumer of the buffer and test if the consumer can acquire the
     // buffer if producer posts.
-    // TODO(b/122543147): use a movalbe wrapper for token
-    native_handle_t* token = b1->duplicate();
-    ASSERT_TRUE(token);
+    sp<NativeHandle> token = b1->duplicate();
+    ASSERT_THAT(token, NotNull());
 
     std::unique_ptr<BufferHubBuffer> b2 = BufferHubBuffer::import(token);
-    native_handle_close(token);
-    native_handle_delete(token);
 
     ASSERT_THAT(b2, NotNull());
     ASSERT_NE(b1->clientStateMask(), b2->clientStateMask());