Post to all existing and non-existing clients of the buffer in BufferHubBuffer

Fix: 122483970
Test: AHardwareBufferTest BufferHub_test BufferHubServer_test VtsHalBufferHubV1_0TargetTest

Change-Id: Ia0d3119c34fb68573747948f38ecc3477c65fcd5

2 files changed, 54 insertions, 10 deletions

diff --git a/libs/ui/BufferHubBuffer.cpp b/libs/ui/BufferHubBuffer.cpp
index 0582e1afe7..11849608ba 100644
--- a/libs/ui/BufferHubBuffer.cpp
+++ b/libs/ui/BufferHubBuffer.cpp
@@ -64,7 +64,6 @@ static constexpr char kBufferHubClientPath[] = "system/buffer_hub/client";
 
 using BufferHubDefs::AnyClientAcquired;
 using BufferHubDefs::AnyClientGained;
-using BufferHubDefs::AnyClientPosted;
 using BufferHubDefs::IsClientAcquired;
 using BufferHubDefs::IsClientGained;
 using BufferHubDefs::IsClientPosted;
@@ -226,8 +225,7 @@ int BufferHubBuffer::Gain() {
 
 int BufferHubBuffer::Post() {
     uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire);
-    uint32_t current_active_clients_bit_mask = 0U;
-    uint32_t updated_buffer_state = 0U;
+    uint32_t updated_buffer_state = (~mClientStateMask) & kHighBitsMask;
     do {
         if (!IsClientGained(current_buffer_state, mClientStateMask)) {
             ALOGE("%s: Cannot post a buffer that is not gained by this client. buffer_id=%d "
@@ -236,9 +234,7 @@ int BufferHubBuffer::Post() {
             return -EBUSY;
         }
         // Set the producer client buffer state to released, other clients' buffer state to posted.
-        current_active_clients_bit_mask = active_clients_bit_mask_->load(std::memory_order_acquire);
-        updated_buffer_state =
-                current_active_clients_bit_mask & (~mClientStateMask) & kHighBitsMask;
+        // Post to all existing and non-existing clients.
     } while (!buffer_state_->compare_exchange_weak(current_buffer_state, updated_buffer_state,
                                                    std::memory_order_acq_rel,
                                                    std::memory_order_acquire));
diff --git a/libs/ui/tests/BufferHubBuffer_test.cpp b/libs/ui/tests/BufferHubBuffer_test.cpp
index 1b339a06d9..69b9590d40 100644
--- a/libs/ui/tests/BufferHubBuffer_test.cpp
+++ b/libs/ui/tests/BufferHubBuffer_test.cpp
@@ -67,9 +67,9 @@ protected:
     }
 
     std::unique_ptr<BufferHubBuffer> b1;
-    uint64_t b1ClientMask = 0U;
+    uint32_t b1ClientMask = 0U;
     std::unique_ptr<BufferHubBuffer> b2;
-    uint64_t b2ClientMask = 0U;
+    uint32_t b2ClientMask = 0U;
 
 private:
     // Creates b1 and b2 as the clients of the same buffer for testing.
@@ -125,7 +125,7 @@ TEST_F(BufferHubBufferTest, DuplicateBufferHubBuffer) {
     auto b1 = BufferHubBuffer::Create(kWidth, kHeight, kLayerCount, kFormat, kUsage,
                                       kUserMetadataSize);
     int id1 = b1->id();
-    uint64_t bufferStateMask1 = b1->client_state_mask();
+    uint32_t bufferStateMask1 = b1->client_state_mask();
     EXPECT_NE(bufferStateMask1, 0U);
     EXPECT_TRUE(b1->IsValid());
     EXPECT_EQ(b1->user_metadata_size(), kUserMetadataSize);
@@ -148,7 +148,7 @@ TEST_F(BufferHubBufferTest, DuplicateBufferHubBuffer) {
     EXPECT_EQ(b2->user_metadata_size(), kUserMetadataSize);
 
     int id2 = b2->id();
-    uint64_t bufferStateMask2 = b2->client_state_mask();
+    uint32_t bufferStateMask2 = b2->client_state_mask();
     EXPECT_NE(bufferStateMask2, 0U);
 
     // These two buffer instances are based on the same physical buffer under the
@@ -340,5 +340,53 @@ TEST_F(BufferHubBufferStateTransitionTest, ReleaseBuffer_fromSelfInAcquiredState
     EXPECT_EQ(b2->Release(), 0);
 }
 
+TEST_F(BufferHubBufferStateTransitionTest, BasicUsage) {
+    // 1 producer buffer and 1 consumer buffer initialised in testcase setup.
+    // Test if this set of basic operation succeed:
+    // Producer post three times to the consumer, and released by consumer.
+    for (int i = 0; i < 3; ++i) {
+        ASSERT_EQ(b1->Gain(), 0);
+        ASSERT_EQ(b1->Post(), 0);
+        ASSERT_EQ(b2->Acquire(), 0);
+        ASSERT_EQ(b2->Release(), 0);
+    }
+}
+
+TEST_F(BufferHubBufferTest, createNewConsumerAfterGain) {
+    // Create a poducer buffer and gain.
+    std::unique_ptr<BufferHubBuffer> b1 =
+            BufferHubBuffer::Create(kWidth, kHeight, kLayerCount, kFormat, kUsage,
+                                    kUserMetadataSize);
+    ASSERT_EQ(b1->Gain(), 0);
+
+    // Create a consumer of the buffer and test if the consumer can acquire the
+    // buffer if producer posts.
+    auto statusOrHandle = b1->Duplicate();
+    ASSERT_TRUE(statusOrHandle);
+    std::unique_ptr<BufferHubBuffer> b2 = BufferHubBuffer::Import(std::move(statusOrHandle.take()));
+    ASSERT_NE(b1->client_state_mask(), b2->client_state_mask());
+
+    ASSERT_EQ(b1->Post(), 0);
+    EXPECT_EQ(b2->Acquire(), 0);
+}
+
+TEST_F(BufferHubBufferTest, createNewConsumerAfterPost) {
+    // Create a poducer buffer and post.
+    std::unique_ptr<BufferHubBuffer> b1 =
+            BufferHubBuffer::Create(kWidth, kHeight, kLayerCount, kFormat, kUsage,
+                                    kUserMetadataSize);
+    ASSERT_EQ(b1->Gain(), 0);
+    ASSERT_EQ(b1->Post(), 0);
+
+    // Create a consumer of the buffer and test if the consumer can acquire the
+    // buffer if producer posts.
+    auto statusOrHandle = b1->Duplicate();
+    ASSERT_TRUE(statusOrHandle);
+    std::unique_ptr<BufferHubBuffer> b2 = BufferHubBuffer::Import(std::move(statusOrHandle.take()));
+    ASSERT_NE(b1->client_state_mask(), b2->client_state_mask());
+
+    EXPECT_EQ(b2->Acquire(), 0);
+}
+
 } // namespace
 } // namespace android