Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8a7e765b authored by Tianyu Jiang's avatar Tianyu Jiang
Browse files

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
parent 8381914f

libs/ui/BufferHubBuffer.cpp

+6 −6
Original line number Diff line number Diff line
@@ -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

libs/ui/include/ui/BufferHubBuffer.h

+6 −9
Original line number Diff line number Diff line
@@ -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);

libs/ui/tests/BufferHubBuffer_test.cpp

+10 −24
Original line number Diff line number Diff line
@@ -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());