Surface: add unit test for GetAndFlushRemovedBuffers

Also fix the removed list flush behavior to match spec.

Test: the new test pass, smoke test GCA
Bug: 36869090
Change-Id: I8f7bdd8b168424f4e79980d21a7388aa9e35597e

diff --git a/libs/gui/Surface.cpp b/libs/gui/Surface.cpp
index a6d9e66..3ed7dcb 100644 (file)
--- a/libs/gui/Surface.cpp
+++ b/libs/gui/Surface.cpp
@@ -476,6 +476,9 @@ int Surface::dequeueBuffer(android_native_buffer_t** buffer, int* fenceFd) {
 
     {
         Mutex::Autolock lock(mMutex);
+        if (mReportRemovedBuffers) {
+            mRemovedBuffers.clear();
+        }
 
         reqWidth = mReqWidth ? mReqWidth : mUserWidth;
         reqHeight = mReqHeight ? mReqHeight : mUserHeight;
@@ -530,7 +533,6 @@ int Surface::dequeueBuffer(android_native_buffer_t** buffer, int* fenceFd) {
 
     if ((result & IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION) || gbuf == nullptr) {
         if (mReportRemovedBuffers && (gbuf != nullptr)) {
-            mRemovedBuffers.clear();
             mRemovedBuffers.push_back(gbuf);
         }
         result = mGraphicBufferProducer->requestBuffer(buf, &gbuf);
@@ -1202,6 +1204,9 @@ int Surface::detachNextBuffer(sp<GraphicBuffer>* outBuffer,
     }
 
     Mutex::Autolock lock(mMutex);
+    if (mReportRemovedBuffers) {
+        mRemovedBuffers.clear();
+    }
 
     sp<GraphicBuffer> buffer(NULL);
     sp<Fence> fence(NULL);
@@ -1218,10 +1223,6 @@ int Surface::detachNextBuffer(sp<GraphicBuffer>* outBuffer,
         *outFence = Fence::NO_FENCE;
     }
 
-    if (mReportRemovedBuffers) {
-        mRemovedBuffers.clear();
-    }
-
     for (int i = 0; i < NUM_BUFFER_SLOTS; i++) {
         if (mSlots[i].buffer != NULL &&
                 mSlots[i].buffer->handle == buffer->handle) {
@@ -1241,6 +1242,9 @@ int Surface::attachBuffer(ANativeWindowBuffer* buffer)
     ALOGV("Surface::attachBuffer");
 
     Mutex::Autolock lock(mMutex);
+    if (mReportRemovedBuffers) {
+        mRemovedBuffers.clear();
+    }
 
     sp<GraphicBuffer> graphicBuffer(static_cast<GraphicBuffer*>(buffer));
     uint32_t priorGeneration = graphicBuffer->mGenerationNumber;
@@ -1254,7 +1258,6 @@ int Surface::attachBuffer(ANativeWindowBuffer* buffer)
         return result;
     }
     if (mReportRemovedBuffers && (mSlots[attachedSlot].buffer != nullptr)) {
-        mRemovedBuffers.clear();
         mRemovedBuffers.push_back(mSlots[attachedSlot].buffer);
     }
     mSlots[attachedSlot].buffer = graphicBuffer;

diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp
index 08d6715..fcaa23a 100644 (file)
--- a/libs/gui/tests/Surface_test.cpp
+++ b/libs/gui/tests/Surface_test.cpp
@@ -24,6 +24,7 @@
 #include <cutils/properties.h>
 #include <gui/BufferItemConsumer.h>
 #include <gui/IDisplayEventConnection.h>
+#include <gui/IProducerListener.h>
 #include <gui/ISurfaceComposer.h>
 #include <gui/Surface.h>
 #include <gui/SurfaceComposerClient.h>
@@ -320,6 +321,77 @@ TEST_F(SurfaceTest, DynamicSetBufferCount) {
     ASSERT_EQ(NO_ERROR, window->queueBuffer(window.get(), buffer, fence));
 }
 
+TEST_F(SurfaceTest, GetAndFlushRemovedBuffers) {
+    sp<IGraphicBufferProducer> producer;
+    sp<IGraphicBufferConsumer> consumer;
+    BufferQueue::createBufferQueue(&producer, &consumer);
+
+    sp<DummyConsumer> dummyConsumer(new DummyConsumer);
+    consumer->consumerConnect(dummyConsumer, false);
+    consumer->setConsumerName(String8("TestConsumer"));
+
+    sp<Surface> surface = new Surface(producer);
+    sp<ANativeWindow> window(surface);
+    sp<DummyProducerListener> listener = new DummyProducerListener();
+    ASSERT_EQ(OK, surface->connect(
+            NATIVE_WINDOW_API_CPU,
+            /*listener*/listener,
+            /*reportBufferRemoval*/true));
+    const int BUFFER_COUNT = 4;
+    ASSERT_EQ(NO_ERROR, native_window_set_buffer_count(window.get(), BUFFER_COUNT));
+
+    sp<GraphicBuffer> detachedBuffer;
+    sp<Fence> outFence;
+    int fences[BUFFER_COUNT];
+    ANativeWindowBuffer* buffers[BUFFER_COUNT];
+    // Allocate buffers because detachNextBuffer requires allocated buffers
+    for (int i = 0; i < BUFFER_COUNT; i++) {
+        ASSERT_EQ(NO_ERROR, window->dequeueBuffer(window.get(), &buffers[i], &fences[i]));
+    }
+    for (int i = 0; i < BUFFER_COUNT; i++) {
+        ASSERT_EQ(NO_ERROR, window->cancelBuffer(window.get(), buffers[i], fences[i]));
+    }
+
+    // Test detached buffer is correctly reported
+    ASSERT_EQ(NO_ERROR, surface->detachNextBuffer(&detachedBuffer, &outFence));
+    std::vector<sp<GraphicBuffer>> removedBuffers;
+    ASSERT_EQ(OK, surface->getAndFlushRemovedBuffers(&removedBuffers));
+    ASSERT_EQ(1u, removedBuffers.size());
+    ASSERT_EQ(detachedBuffer->handle, removedBuffers.at(0)->handle);
+    // Test the list is flushed one getAndFlushRemovedBuffers returns
+    ASSERT_EQ(OK, surface->getAndFlushRemovedBuffers(&removedBuffers));
+    ASSERT_EQ(0u, removedBuffers.size());
+
+
+    // Test removed buffer list is cleanup after next dequeueBuffer call
+    ASSERT_EQ(NO_ERROR, surface->detachNextBuffer(&detachedBuffer, &outFence));
+    ASSERT_EQ(NO_ERROR, window->dequeueBuffer(window.get(), &buffers[0], &fences[0]));
+    ASSERT_EQ(OK, surface->getAndFlushRemovedBuffers(&removedBuffers));
+    ASSERT_EQ(0u, removedBuffers.size());
+    ASSERT_EQ(NO_ERROR, window->cancelBuffer(window.get(), buffers[0], fences[0]));
+
+    // Test removed buffer list is cleanup after next detachNextBuffer call
+    ASSERT_EQ(NO_ERROR, surface->detachNextBuffer(&detachedBuffer, &outFence));
+    ASSERT_EQ(NO_ERROR, surface->detachNextBuffer(&detachedBuffer, &outFence));
+    ASSERT_EQ(OK, surface->getAndFlushRemovedBuffers(&removedBuffers));
+    ASSERT_EQ(1u, removedBuffers.size());
+    ASSERT_EQ(detachedBuffer->handle, removedBuffers.at(0)->handle);
+
+    // Re-allocate buffers since all buffers are detached up to now
+    for (int i = 0; i < BUFFER_COUNT; i++) {
+        ASSERT_EQ(NO_ERROR, window->dequeueBuffer(window.get(), &buffers[i], &fences[i]));
+    }
+    for (int i = 0; i < BUFFER_COUNT; i++) {
+        ASSERT_EQ(NO_ERROR, window->cancelBuffer(window.get(), buffers[i], fences[i]));
+    }
+
+    ASSERT_EQ(NO_ERROR, surface->detachNextBuffer(&detachedBuffer, &outFence));
+    ASSERT_EQ(NO_ERROR, surface->attachBuffer(detachedBuffer.get()));
+    ASSERT_EQ(OK, surface->getAndFlushRemovedBuffers(&removedBuffers));
+    // Depends on which slot GraphicBufferProducer impl pick, the attach call might
+    // get 0 or 1 buffer removed.
+    ASSERT_LE(removedBuffers.size(), 1u);
+}
 
 class FakeConsumer : public BnConsumerListener {
 public: