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

Commit 4a5ede75 authored by Fan Xu's avatar Fan Xu Committed by Android (Google) Code Review
Browse files

Merge "Implement importBuffer for BufferHubService"

parents daedc2a6 467e08fb

libs/ui/tests/Android.bp

+1 −0
Original line number Diff line number Diff line
@@ -53,6 +53,7 @@ cc_test { 
    ],

    shared_libs: [

        "android.frameworks.bufferhub@1.0",

        "libcutils",

        "libhidlbase",

        "libhwbinder",

        "libpdx_default_transport",

libs/ui/tests/BufferHubBuffer_test.cpp

+55 −2
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ 
#include <android/frameworks/bufferhub/1.0/IBufferClient.h>

#include <android/frameworks/bufferhub/1.0/IBufferHub.h>

#include <android/hardware_buffer.h>

#include <cutils/native_handle.h>

#include <gtest/gtest.h>

#include <hidl/ServiceManagement.h>

#include <hwbinder/IPCThreadState.h>
@@ -144,7 +145,7 @@ TEST_F(BufferHubBufferTest, AllocateBuffer) { 
    EXPECT_TRUE(bufferHub->allocateBuffer(desc, kUserMetadataSize, callback).isOk());

}



TEST_F(BufferHubBufferTest, DuplicateBuffer) {

TEST_F(BufferHubBufferTest, DuplicateAndImportBuffer) {

    // TODO(b/116681016): directly test on BufferHubBuffer instead of the service.

    sp<IBufferHub> bufferhub = IBufferHub::getService();

    ASSERT_NE(nullptr, bufferhub.get());
@@ -170,11 +171,63 @@ TEST_F(BufferHubBufferTest, DuplicateBuffer) { 
        token = outToken;

        ret = status;

    };

    EXPECT_TRUE(client->duplicate(dup_cb).isOk());

    ASSERT_TRUE(client->duplicate(dup_cb).isOk());

    EXPECT_EQ(ret, BufferHubStatus::NO_ERROR);

    ASSERT_NE(token.getNativeHandle(), nullptr);

    EXPECT_EQ(token->numInts, 1);

    EXPECT_EQ(token->numFds, 0);



    sp<IBufferClient> client2;

    IBufferHub::importBuffer_cb import_cb = [&](const auto& outClient, const auto& status) {

        ret = status;

        client2 = outClient;

    };

    ASSERT_TRUE(bufferhub->importBuffer(token, import_cb).isOk());

    EXPECT_EQ(ret, BufferHubStatus::NO_ERROR);

    EXPECT_NE(nullptr, client2.get());

    // TODO(b/116681016): once BufferNode.id() is exposed via BufferHubBuffer, check origin.id =

    // improted.id here.

}



// nullptr must not crash the service

TEST_F(BufferHubBufferTest, ImportNullToken) {

    // TODO(b/116681016): directly test on BufferHubBuffer instead of the service.

    sp<IBufferHub> bufferhub = IBufferHub::getService();

    ASSERT_NE(nullptr, bufferhub.get());



    hidl_handle nullToken;

    sp<IBufferClient> client;

    BufferHubStatus ret;

    IBufferHub::importBuffer_cb import_cb = [&](const auto& outClient, const auto& status) {

        client = outClient;

        ret = status;

    };

    ASSERT_TRUE(bufferhub->importBuffer(nullToken, import_cb).isOk());

    EXPECT_EQ(ret, BufferHubStatus::INVALID_TOKEN);

    EXPECT_EQ(nullptr, client.get());

}



// This test has a very little chance to fail (number of existing tokens / 2 ^ 32)

TEST_F(BufferHubBufferTest, ImportInvalidToken) {

    // TODO(b/116681016): directly test on BufferHubBuffer instead of the service.

    sp<IBufferHub> bufferhub = IBufferHub::getService();

    ASSERT_NE(nullptr, bufferhub.get());



    native_handle_t* tokenHandle = native_handle_create(/*numFds=*/0, /*numInts=*/1);

    tokenHandle->data[0] = 0;



    hidl_handle invalidToken(tokenHandle);

    sp<IBufferClient> client;

    BufferHubStatus ret;

    IBufferHub::importBuffer_cb import_cb = [&](const auto& outClient, const auto& status) {

        client = outClient;

        ret = status;

    };

    ASSERT_TRUE(bufferhub->importBuffer(invalidToken, import_cb).isOk());

    EXPECT_EQ(ret, BufferHubStatus::INVALID_TOKEN);

    EXPECT_EQ(nullptr, client.get());



    native_handle_delete(tokenHandle);

}



} // namespace

services/bufferhub/BufferHubService.cpp

+37 −3
Original line number Diff line number Diff line
@@ -51,10 +51,44 @@ Return<void> BufferHubService::allocateBuffer(const HardwareBufferDescription& d 
    return Void();

}



Return<void> BufferHubService::importBuffer(const hidl_handle& /*nativeHandle*/,

Return<void> BufferHubService::importBuffer(const hidl_handle& tokenHandle,

                                            importBuffer_cb _hidl_cb) {

    // TODO(b/118614157): implement buffer import

    _hidl_cb(/*bufferClient=*/nullptr, /*status=*/BufferHubStatus::NO_ERROR);

    if (!tokenHandle.getNativeHandle() || tokenHandle->numFds != 0 || tokenHandle->numInts != 1) {

        // nullptr handle or wrong format

        _hidl_cb(/*bufferClient=*/nullptr, /*status=*/BufferHubStatus::INVALID_TOKEN);

        return Void();

    }



    uint32_t token = tokenHandle->data[0];



    wp<BufferClient> originClientWp;

    {

        std::lock_guard<std::mutex> lock(mTokenMapMutex);

        auto iter = mTokenMap.find(token);

        if (iter == mTokenMap.end()) {

            // Invalid token

            _hidl_cb(/*bufferClient=*/nullptr, /*status=*/BufferHubStatus::INVALID_TOKEN);

            return Void();

        }



        originClientWp = iter->second;

        mTokenMap.erase(iter);

    }



    // Check if original client is dead

    sp<BufferClient> originClient = originClientWp.promote();

    if (!originClient) {

        // Should not happen since token should be removed if already gone

        ALOGE("%s: original client %p gone!", __FUNCTION__, originClientWp.unsafe_get());

        _hidl_cb(/*bufferClient=*/nullptr, /*status=*/BufferHubStatus::BUFFER_FREED);

        return Void();

    }



    sp<BufferClient> client = new BufferClient(*originClient);



    std::lock_guard<std::mutex> lock(mClientListMutex);

    mClientList.push_back(client);

    _hidl_cb(/*bufferClient=*/client, /*status=*/BufferHubStatus::NO_ERROR);

    return Void();

}

services/bufferhub/include/bufferhub/BufferClient.h

+6 −2
Original line number Diff line number Diff line
@@ -37,15 +37,19 @@ class BufferHubService; 
class BufferClient : public IBufferClient {

public:

    // Creates a server-side buffer client from an existing BufferNode. Note that

    // this funciton takes ownership of the shared_ptr.

    // this function takes ownership of the shared_ptr.

    // Returns a raw pointer to the BufferClient on success, nullptr on failure.

    static BufferClient* create(BufferHubService* service, const std::shared_ptr<BufferNode>& node);



    // Creates a BufferClient from an existing BufferClient. Will share the same BufferNode.

    explicit BufferClient(const BufferClient& other)

          : mService(other.mService), mBufferNode(other.mBufferNode) {}



    Return<void> duplicate(duplicate_cb _hidl_cb) override;



private:

    BufferClient(wp<BufferHubService> service, const std::shared_ptr<BufferNode>& node)

          : mService(service), mBufferNode(node){};

          : mService(service), mBufferNode(node) {}



    wp<BufferHubService> mService;

    std::shared_ptr<BufferNode> mBufferNode;

services/bufferhub/include/bufferhub/BufferHubService.h

+1 −1
Original line number Diff line number Diff line
@@ -42,7 +42,7 @@ public: 
    Return<void> allocateBuffer(const HardwareBufferDescription& description,

                                const uint32_t userMetadataSize,

                                allocateBuffer_cb _hidl_cb) override;

    Return<void> importBuffer(const hidl_handle& nativeHandle, importBuffer_cb _hidl_cb) override;

    Return<void> importBuffer(const hidl_handle& tokenHandle, importBuffer_cb _hidl_cb) override;



    // Non-binder functions

    // Internal help function for IBufferClient::duplicate.