Implement importBuffer for BufferHubService (467e08fb) · Commits · e / os / platform_frameworks_native

libs/ui/tests/Android.bp

+1 −0

Original line number	Original line	Diff line number	Diff line
	@@ -53,6 +53,7 @@ cc_test {
	],		],
	shared_libs: [		shared_libs: [
	"android.frameworks.bufferhub@1.0",		"android.frameworks.bufferhub@1.0",
			"libcutils",
	"libhidlbase",		"libhidlbase",
	"libhwbinder",		"libhwbinder",
	"libpdx_default_transport",		"libpdx_default_transport",

libs/ui/tests/BufferHubBuffer_test.cpp

+55 −2

Original line number	Original line	Diff line number	Diff line
	@@ -19,6 +19,7 @@
	#include <android/frameworks/bufferhub/1.0/IBufferClient.h>		#include <android/frameworks/bufferhub/1.0/IBufferClient.h>
	#include <android/frameworks/bufferhub/1.0/IBufferHub.h>		#include <android/frameworks/bufferhub/1.0/IBufferHub.h>
	#include <android/hardware_buffer.h>		#include <android/hardware_buffer.h>
			#include <cutils/native_handle.h>
	#include <gtest/gtest.h>		#include <gtest/gtest.h>
	#include <hidl/ServiceManagement.h>		#include <hidl/ServiceManagement.h>
	#include <hwbinder/IPCThreadState.h>		#include <hwbinder/IPCThreadState.h>
	@@ -144,7 +145,7 @@ TEST_F(BufferHubBufferTest, AllocateBuffer) {
	EXPECT_TRUE(bufferHub->allocateBuffer(desc, kUserMetadataSize, callback).isOk());		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.		// TODO(b/116681016): directly test on BufferHubBuffer instead of the service.
	sp<IBufferHub> bufferhub = IBufferHub::getService();		sp<IBufferHub> bufferhub = IBufferHub::getService();
	ASSERT_NE(nullptr, bufferhub.get());		ASSERT_NE(nullptr, bufferhub.get());
	@@ -170,11 +171,63 @@ TEST_F(BufferHubBufferTest, DuplicateBuffer) {
	token = outToken;		token = outToken;
	ret = status;		ret = status;
	};		};
	EXPECT_TRUE(client->duplicate(dup_cb).isOk());		ASSERT_TRUE(client->duplicate(dup_cb).isOk());
	EXPECT_EQ(ret, BufferHubStatus::NO_ERROR);		EXPECT_EQ(ret, BufferHubStatus::NO_ERROR);
	ASSERT_NE(token.getNativeHandle(), nullptr);		ASSERT_NE(token.getNativeHandle(), nullptr);
	EXPECT_EQ(token->numInts, 1);		EXPECT_EQ(token->numInts, 1);
	EXPECT_EQ(token->numFds, 0);		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		} // namespace

services/bufferhub/BufferHubService.cpp

+37 −3

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

	Return<void> BufferHubService::importBuffer(const hidl_handle& /nativeHandle/,		Return<void> BufferHubService::importBuffer(const hidl_handle& tokenHandle,
	importBuffer_cb _hidl_cb) {		importBuffer_cb _hidl_cb) {
	// TODO(b/118614157): implement buffer import		if (!tokenHandle.getNativeHandle() \|\| tokenHandle->numFds != 0 \|\| tokenHandle->numInts != 1) {
	_hidl_cb(/bufferClient=/nullptr, /status=/BufferHubStatus::NO_ERROR);		// 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();		return Void();
	}		}

services/bufferhub/include/bufferhub/BufferClient.h

+6 −2

Original line number	Original line	Diff line number	Diff line
	@@ -37,15 +37,19 @@ class BufferHubService;
	class BufferClient : public IBufferClient {		class BufferClient : public IBufferClient {
	public:		public:
	// Creates a server-side buffer client from an existing BufferNode. Note that		// 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.		// Returns a raw pointer to the BufferClient on success, nullptr on failure.
	static BufferClient* create(BufferHubService* service, const std::shared_ptr<BufferNode>& node);		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;		Return<void> duplicate(duplicate_cb _hidl_cb) override;

	private:		private:
	BufferClient(wp<BufferHubService> service, const std::shared_ptr<BufferNode>& node)		BufferClient(wp<BufferHubService> service, const std::shared_ptr<BufferNode>& node)
	: mService(service), mBufferNode(node){};		: mService(service), mBufferNode(node) {}

	wp<BufferHubService> mService;		wp<BufferHubService> mService;
	std::shared_ptr<BufferNode> mBufferNode;		std::shared_ptr<BufferNode> mBufferNode;

services/bufferhub/include/bufferhub/BufferHubService.h

+1 −1

Original line number	Original line	Diff line number	Diff line
	@@ -42,7 +42,7 @@ public:
	Return<void> allocateBuffer(const HardwareBufferDescription& description,		Return<void> allocateBuffer(const HardwareBufferDescription& description,
	const uint32_t userMetadataSize,		const uint32_t userMetadataSize,
	allocateBuffer_cb _hidl_cb) override;		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		// Non-binder functions
	// Internal help function for IBufferClient::duplicate.		// Internal help function for IBufferClient::duplicate.