get rid of purgatory and fix QueuesToWindowComposer query

the purgatory list wasn't needed anymore; in fact it had no effect as
buffer life-time management is now handled by the BufferQueue.

For QueuesToWindowComposer we keep a list of wp<> on the IBinder
for IGraphicBufferProducers we hand over to clients so we can
easily check if an IGraphicBufferProducer is ours. We clean-up the
list when our IGraphicBufferProducer are destroyed.

Bug: 8349142
Change-Id: I1aa06652ade8c72d0004a3f5e6c3d6e8a82fc2ae

5 files changed, 51 insertions, 118 deletions

diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 5996c90..1677c76 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -108,7 +108,7 @@ Layer::Layer(SurfaceFlinger* flinger, const sp<Client>& client,
 void Layer::onFirstRef()
 {
     // Creates a custom BufferQueue for SurfaceFlingerConsumer to use
-    sp<BufferQueue> bq = new SurfaceTextureLayer();
+    sp<BufferQueue> bq = new SurfaceTextureLayer(mFlinger);
     mSurfaceFlingerConsumer = new SurfaceFlingerConsumer(mTextureName, true,
             GL_TEXTURE_EXTERNAL_OES, false, bq);
 
@@ -153,8 +153,9 @@ void Layer::onFrameAvailable() {
     mFlinger->signalLayerUpdate();
 }
 
-// called with SurfaceFlinger::mStateLock as soon as the layer is entered
-// in the purgatory list
+// called with SurfaceFlinger::mStateLock from the drawing thread after
+// the layer has been remove from the current state list (and just before
+// it's removed from the drawing state list)
 void Layer::onRemoved() {
     mSurfaceFlingerConsumer->abandon();
 }
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 98f6ded..fba6bba 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -574,39 +574,7 @@ bool SurfaceFlinger::authenticateSurfaceTexture(
         const sp<IGraphicBufferProducer>& bufferProducer) const {
     Mutex::Autolock _l(mStateLock);
     sp<IBinder> surfaceTextureBinder(bufferProducer->asBinder());
-
-    // We want to determine whether the IGraphicBufferProducer was created by
-    // SurfaceFlinger.  Check to see if we can find it in the layer list.
-    const LayerVector& currentLayers = mCurrentState.layersSortedByZ;
-    size_t count = currentLayers.size();
-    for (size_t i=0 ; i<count ; i++) {
-        const sp<Layer>& layer(currentLayers[i]);
-        // Get the consumer interface of SurfaceFlingerConsumer's
-        // BufferQueue.  If it's the same Binder object as the graphic
-        // buffer producer interface, return success.
-        sp<IBinder> lbcBinder = layer->getBufferQueue()->asBinder();
-        if (lbcBinder == surfaceTextureBinder) {
-            return true;
-        }
-    }
-
-    // Check the layers in the purgatory.  This check is here so that if a
-    // GLConsumer gets destroyed before all the clients are done using it,
-    // the error will not be reported as "surface XYZ is not authenticated", but
-    // will instead fail later on when the client tries to use the surface,
-    // which should be reported as "surface XYZ returned an -ENODEV".  The
-    // purgatorized layers are no less authentic than the visible ones, so this
-    // should not cause any harm.
-    size_t purgatorySize =  mLayerPurgatory.size();
-    for (size_t i=0 ; i<purgatorySize ; i++) {
-        const sp<Layer>& layer(mLayerPurgatory.itemAt(i));
-        sp<IBinder> lbcBinder = layer->getBufferQueue()->asBinder();
-        if (lbcBinder == surfaceTextureBinder) {
-            return true;
-        }
-    }
-
-    return false;
+    return mGraphicBufferProducerList.indexOf(surfaceTextureBinder) >= 0;
 }
 
 status_t SurfaceFlinger::getDisplayInfo(const sp<IBinder>& display, DisplayInfo* info) {
@@ -1676,6 +1644,7 @@ void SurfaceFlinger::drawWormhole(const sp<const DisplayDevice>& hw,
 
 void SurfaceFlinger::addClientLayer(const sp<Client>& client,
         const sp<IBinder>& handle,
+        const sp<IGraphicBufferProducer>& gbc,
         const sp<Layer>& lbc)
 {
     // attach this layer to the client
@@ -1684,45 +1653,22 @@ void SurfaceFlinger::addClientLayer(const sp<Client>& client,
     // add this layer to the current state list
     Mutex::Autolock _l(mStateLock);
     mCurrentState.layersSortedByZ.add(lbc);
+    mGraphicBufferProducerList.add(gbc->asBinder());
 }
 
 status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer)
 {
     Mutex::Autolock _l(mStateLock);
-    status_t err = purgatorizeLayer_l(layer);
-    if (err == NO_ERROR)
-        setTransactionFlags(eTransactionNeeded);
-    return err;
-}
-
-status_t SurfaceFlinger::removeLayer_l(const sp<Layer>& layer)
-{
     ssize_t index = mCurrentState.layersSortedByZ.remove(layer);
     if (index >= 0) {
+        mLayersPendingRemoval.push(layer);
         mLayersRemoved = true;
+        setTransactionFlags(eTransactionNeeded);
         return NO_ERROR;
     }
     return status_t(index);
 }
 
-status_t SurfaceFlinger::purgatorizeLayer_l(const sp<Layer>& layer)
-{
-    // First add the layer to the purgatory list, which makes sure it won't
-    // go away, then remove it from the main list (through a transaction).
-    ssize_t err = removeLayer_l(layer);
-    if (err >= 0) {
-        mLayerPurgatory.add(layer);
-    }
-
-    mLayersPendingRemoval.push(layer);
-
-    // it's possible that we don't find a layer, because it might
-    // have been destroyed already -- this is not technically an error
-    // from the user because there is a race between Client::destroySurface(),
-    // ~Client() and ~LayerCleaner().
-    return (err == NAME_NOT_FOUND) ? status_t(NO_ERROR) : err;
-}
-
 uint32_t SurfaceFlinger::peekTransactionFlags(uint32_t flags)
 {
     return android_atomic_release_load(&mTransactionFlags);
@@ -1957,7 +1903,7 @@ status_t SurfaceFlinger::createLayer(
     }
 
     if (result == NO_ERROR) {
-        addClientLayer(client, *handle, layer);
+        addClientLayer(client, *handle, *gbp, layer);
         setTransactionFlags(eTransactionNeeded);
     }
     return result;
@@ -2010,44 +1956,25 @@ status_t SurfaceFlinger::createDimLayer(const sp<Client>& client,
 
 status_t SurfaceFlinger::onLayerRemoved(const sp<Client>& client, const sp<IBinder>& handle)
 {
-    /*
-     * called by the window manager, when a surface should be marked for
-     * destruction.
-     *
-     * The surface is removed from the current and drawing lists, but placed
-     * in the purgatory queue, so it's not destroyed right-away (we need
-     * to wait for all client's references to go away first).
-     */
-
-    status_t err = NAME_NOT_FOUND;
-    Mutex::Autolock _l(mStateLock);
-    sp<Layer> layer = client->getLayerUser(handle);
-
-    if (layer != 0) {
-        err = purgatorizeLayer_l(layer);
-        if (err == NO_ERROR) {
-            setTransactionFlags(eTransactionNeeded);
-        }
+    // called by the window manager when it wants to remove a Layer
+    status_t err = NO_ERROR;
+    sp<Layer> l(client->getLayerUser(handle));
+    if (l != NULL) {
+        err = removeLayer(l);
+        ALOGE_IF(err<0 && err != NAME_NOT_FOUND,
+                "error removing layer=%p (%s)", l.get(), strerror(-err));
     }
     return err;
 }
 
 status_t SurfaceFlinger::onLayerDestroyed(const wp<Layer>& layer)
 {
-    // called by ~LayerCleaner() when all references are gone
+    // called by ~LayerCleaner() when all references to the IBinder (handle)
+    // are gone
     status_t err = NO_ERROR;
     sp<Layer> l(layer.promote());
     if (l != NULL) {
-        Mutex::Autolock _l(mStateLock);
-        err = removeLayer_l(l);
-        if (err == NAME_NOT_FOUND) {
-            // The surface wasn't in the current list, which means it was
-            // removed already, which means it is in the purgatory,
-            // and need to be removed from there.
-            ssize_t idx = mLayerPurgatory.remove(l);
-            ALOGE_IF(idx < 0,
-                    "layer=%p is not in the purgatory list", l.get());
-        }
+        err = removeLayer(l);
         ALOGE_IF(err<0 && err != NAME_NOT_FOUND,
                 "error removing layer=%p (%s)", l.get(), strerror(-err));
     }
@@ -2359,18 +2286,6 @@ void SurfaceFlinger::dumpAllLocked(
     }
 
     /*
-     * Dump the layers in the purgatory
-     */
-
-    const size_t purgatorySize = mLayerPurgatory.size();
-    snprintf(buffer, SIZE, "Purgatory state (%d entries)\n", purgatorySize);
-    result.append(buffer);
-    for (size_t i=0 ; i<purgatorySize ; i++) {
-        const sp<Layer>& layer(mLayerPurgatory.itemAt(i));
-        layer->shortDump(result, buffer, SIZE);
-    }
-
-    /*
      * Dump Display state
      */
 
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index f124347..e6734d2 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -128,6 +128,7 @@ private:
     friend class Client;
     friend class DisplayEventConnection;
     friend class Layer;
+    friend class SurfaceTextureLayer;
 
     // We're reference counted, never destroy SurfaceFlinger directly
     virtual ~SurfaceFlinger();
@@ -272,8 +273,6 @@ private:
 
     // called in response to the window-manager calling
     // ISurfaceComposerClient::destroySurface()
-    // The specified layer is first placed in a purgatory list
-    // until all references from the client are released.
     status_t onLayerRemoved(const sp<Client>& client, const sp<IBinder>& handle);
 
     // called when all clients have released all their references to
@@ -285,11 +284,10 @@ private:
     status_t removeLayer(const sp<Layer>& layer);
 
     // add a layer to SurfaceFlinger
-    void addClientLayer(const sp<Client>& client, const sp<IBinder>& handle,
-        const sp<Layer>& lbc);
-
-    status_t removeLayer_l(const sp<Layer>& layer);
-    status_t purgatorizeLayer_l(const sp<Layer>& layer);
+    void addClientLayer(const sp<Client>& client,
+            const sp<IBinder>& handle,
+            const sp<IGraphicBufferProducer>& gbc,
+            const sp<Layer>& lbc);
 
     /* ------------------------------------------------------------------------
      * Boot animation, on/off animations and screen capture
@@ -403,10 +401,10 @@ private:
     State mCurrentState;
     volatile int32_t mTransactionFlags;
     Condition mTransactionCV;
-    SortedVector< sp<Layer> > mLayerPurgatory;
     bool mTransactionPending;
     bool mAnimTransactionPending;
     Vector< sp<Layer> > mLayersPendingRemoval;
+    SortedVector< wp<IBinder> > mGraphicBufferProducerList;
 
     // protected by mStateLock (but we could use another lock)
     bool mLayersRemoved;
diff --git a/services/surfaceflinger/SurfaceTextureLayer.cpp b/services/surfaceflinger/SurfaceTextureLayer.cpp
index 395c8c8..d0f0dae 100644
--- a/services/surfaceflinger/SurfaceTextureLayer.cpp
+++ b/services/surfaceflinger/SurfaceTextureLayer.cpp
@@ -20,17 +20,35 @@
 
 #include <utils/Errors.h>
 
+#include "SurfaceFlinger.h"
 #include "SurfaceTextureLayer.h"
 
 namespace android {
 // ---------------------------------------------------------------------------
 
 
-SurfaceTextureLayer::SurfaceTextureLayer()
-    : BufferQueue(true) {
+SurfaceTextureLayer::SurfaceTextureLayer(const sp<SurfaceFlinger>& flinger)
+    : BufferQueue(true), flinger(flinger) {
 }
 
 SurfaceTextureLayer::~SurfaceTextureLayer() {
+    // remove ourselves from SurfaceFlinger's list. We do this asynchronously
+    // because we don't know where this dtor is called from, it could be
+    // called with the mStateLock held, leading to a dead-lock (it actually
+    // happens).
+    class MessageCleanUpList : public MessageBase {
+        sp<SurfaceFlinger> flinger;
+        wp<IBinder> gbp;
+    public:
+        MessageCleanUpList(const sp<SurfaceFlinger>& flinger, const wp<IBinder>& gbp)
+            : flinger(flinger), gbp(gbp) { }
+        virtual bool handler() {
+            Mutex::Autolock _l(flinger->mStateLock);
+            flinger->mGraphicBufferProducerList.remove(gbp);
+            return true;
+        }
+    };
+    flinger->postMessageAsync( new MessageCleanUpList(flinger, this) );
 }
 
 status_t SurfaceTextureLayer::connect(int api, QueueBufferOutput* output) {
diff --git a/services/surfaceflinger/SurfaceTextureLayer.h b/services/surfaceflinger/SurfaceTextureLayer.h
index a75ccf4..13cff2f 100644
--- a/services/surfaceflinger/SurfaceTextureLayer.h
+++ b/services/surfaceflinger/SurfaceTextureLayer.h
@@ -28,15 +28,16 @@ namespace android {
 // ---------------------------------------------------------------------------
 
 class Layer;
+class SurfaceFlinger;
 
 /*
  * This is a thin wrapper around BufferQueue, used by the Layer class.
  */
-class SurfaceTextureLayer : public BufferQueue
-{
+class SurfaceTextureLayer : public BufferQueue {
+    sp<SurfaceFlinger> flinger;
 public:
-    SurfaceTextureLayer();
-    ~SurfaceTextureLayer();
+    SurfaceTextureLayer(const sp<SurfaceFlinger>& flinger);
+    virtual ~SurfaceTextureLayer();
 
     // After calling the superclass connect(), set or clear synchronous
     // mode appropriately for the specified API.