Replace the use of native_handle_t by NativeHandle
so that user do not need to explicitly destroy and close the native
handle after importing a BufferHubBuffer using the token obtained from
duplicate method.
Change-Id: Id4f878e8881db7495444b1a43a33b70eabfcb7d7
Fix: 122543147
Test: BufferHub_test
diff --git a/libs/ui/BufferHubBuffer.cpp b/libs/ui/BufferHubBuffer.cpp
index cce35fd..405bcb6 100644
--- a/libs/ui/BufferHubBuffer.cpp
+++ b/libs/ui/BufferHubBuffer.cpp
@@ -47,8 +47,8 @@
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 @@
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 @@
// 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 @@
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 @@
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 eac8c84..5c09032 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 @@
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 @@
// 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 3087a90..634bce1 100644
--- a/libs/ui/tests/BufferHubBuffer_test.cpp
+++ b/libs/ui/tests/BufferHubBuffer_test.cpp
@@ -86,13 +86,10 @@
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 @@
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 @@
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 @@
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 @@
// 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 @@
// 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());