Merge "fix [2873058] Surface::dequeueBuffer blocks on last buffer, i.e. cannot dequeue all allocated buffers at once." into gingerbread

3 files changed, 78 insertions, 22 deletions

diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 758da4e..629d993 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -294,19 +294,9 @@ sp<GraphicBuffer> Layer::requestBuffer(int index,
      * This is called from the client's Surface::dequeue(). This can happen
      * at any time, especially while we're in the middle of using the
      * buffer 'index' as our front buffer.
-     *
-     * Make sure the buffer we're resizing is not the front buffer and has been
-     * dequeued. Once this condition is asserted, we are guaranteed that this
-     * buffer cannot become the front buffer under our feet, since we're called
-     * from Surface::dequeue()
      */
-    status_t err = lcblk->assertReallocate(index);
-    LOGE_IF(err, "assertReallocate(%d) failed (%s)", index, strerror(-err));
-    if (err != NO_ERROR) {
-        // the surface may have died
-        return buffer;
-    }
 
+    status_t err = NO_ERROR;
     uint32_t w, h, f;
     { // scope for the lock
         Mutex::Autolock _l(mLock);
@@ -319,23 +309,17 @@ sp<GraphicBuffer> Layer::requestBuffer(int index,
         w = reqWidth  ? reqWidth  : mWidth;
         h = reqHeight ? reqHeight : mHeight;
         f = reqFormat ? reqFormat : mFormat;
-        buffer = mBufferManager.detachBuffer(index);
         if (fixedSizeChanged || formatChanged) {
             lcblk->reallocateAllExcept(index);
         }
     }
 
+    // here we have to reallocate a new buffer because the buffer could be
+    // used as the front buffer, or by a client in our process
+    // (eg: status bar), and we can't release the handle under its feet.
     const uint32_t effectiveUsage = getEffectiveUsage(usage);
-    if (buffer!=0 && buffer->getStrongCount() == 1) {
-        err = buffer->reallocate(w, h, f, effectiveUsage);
-    } else {
-        // here we have to reallocate a new buffer because we could have a
-        // client in our process with a reference to it (eg: status bar),
-        // and we can't release the handle under its feet.
-        buffer.clear();
-        buffer = new GraphicBuffer(w, h, f, effectiveUsage);
-        err = buffer->initCheck();
-    }
+    buffer = new GraphicBuffer(w, h, f, effectiveUsage);
+    err = buffer->initCheck();
 
     if (err || buffer->handle == 0) {
         LOGE_IF(err || buffer->handle == 0,
diff --git a/services/surfaceflinger/tests/surface/Android.mk b/services/surfaceflinger/tests/surface/Android.mk
new file mode 100644
index 0000000..ce0e807
--- /dev/null
+++ b/services/surfaceflinger/tests/surface/Android.mk
@@ -0,0 +1,18 @@
+LOCAL_PATH:= $(call my-dir)
+include $(CLEAR_VARS)
+
+LOCAL_SRC_FILES:= \
+	surface.cpp
+
+LOCAL_SHARED_LIBRARIES := \
+	libcutils \
+	libutils \
+	libbinder \
+    libui \
+    libsurfaceflinger_client
+
+LOCAL_MODULE:= test-surface
+
+LOCAL_MODULE_TAGS := tests
+
+include $(BUILD_EXECUTABLE)
diff --git a/services/surfaceflinger/tests/surface/surface.cpp b/services/surfaceflinger/tests/surface/surface.cpp
new file mode 100644
index 0000000..b4de4b4
--- /dev/null
+++ b/services/surfaceflinger/tests/surface/surface.cpp
@@ -0,0 +1,54 @@
+#include <cutils/memory.h>
+
+#include <utils/Log.h>
+
+#include <binder/IPCThreadState.h>
+#include <binder/ProcessState.h>
+#include <binder/IServiceManager.h>
+
+#include <surfaceflinger/Surface.h>
+#include <surfaceflinger/ISurface.h>
+#include <surfaceflinger/SurfaceComposerClient.h>
+
+#include <ui/Overlay.h>
+
+using namespace android;
+
+int main(int argc, char** argv)
+{
+    // set up the thread-pool
+    sp<ProcessState> proc(ProcessState::self());
+    ProcessState::self()->startThreadPool();
+
+    // create a client to surfaceflinger
+    sp<SurfaceComposerClient> client = new SurfaceComposerClient();
+    
+    // create pushbuffer surface
+    sp<SurfaceControl> surfaceControl = client->createSurface(
+            getpid(), 0, 160, 240, PIXEL_FORMAT_RGB_565);
+    client->openTransaction();
+    surfaceControl->setLayer(100000);
+    client->closeTransaction();
+
+    // pretend it went cross-process
+    Parcel parcel;
+    SurfaceControl::writeSurfaceToParcel(surfaceControl, &parcel);
+    parcel.setDataPosition(0);
+    sp<Surface> surface = Surface::readFromParcel(parcel);
+    ANativeWindow* window = surface.get();
+
+    printf("window=%p\n", window);
+
+    int err = native_window_set_buffer_count(window, 8);
+    android_native_buffer_t* buffer;
+
+    for (int i=0 ; i<8 ; i++) {
+        window->dequeueBuffer(window, &buffer);
+        printf("buffer %d: %p\n", i, buffer);
+    }
+
+    printf("test complete. CTRL+C to finish.\n");
+
+    IPCThreadState::self()->joinThreadPool();
+    return 0;
+}