Attempt to fix [2152536] ANR in browser

The ANR is caused by SurfaceFlinger waiting for buffers of a removed surface to become availlable.
When it is removed from the current list, a Surface is marked as NO_INIT, which causes SF to return
immediately in the above case. For some reason, the surface here wasn't marked as NO_INIT.

This change makes the code more robust by always (irregadless or errors) setting the NO_INIT status
in all code paths where a surface is removed from the list.

Additionaly added more information in the logs, should this happen again.

10 files changed, 31 insertions, 39 deletions

diff --git a/libs/surfaceflinger/Buffer.cpp b/libs/surfaceflinger/Buffer.cpp
index 65650aa..6190cd8 100644
--- a/libs/surfaceflinger/Buffer.cpp
+++ b/libs/surfaceflinger/Buffer.cpp
@@ -20,17 +20,12 @@
 
 #include <utils/Errors.h>
 #include <utils/Log.h>
-#include <binder/MemoryBase.h>
-#include <binder/IMemory.h>
 
 #include <ui/PixelFormat.h>
-#include <ui/Surface.h>
 #include <pixelflinger/pixelflinger.h>
 
 #include "Buffer.h"
 #include "BufferAllocator.h"
-#include "SurfaceFlinger.h"
-
 
 namespace android {
 
diff --git a/libs/surfaceflinger/Buffer.h b/libs/surfaceflinger/Buffer.h
index 79f4eeb..203da3b 100644
--- a/libs/surfaceflinger/Buffer.h
+++ b/libs/surfaceflinger/Buffer.h
@@ -20,20 +20,11 @@
 #include <stdint.h>
 #include <sys/types.h>
 
-#include <hardware/gralloc.h>
-
-#include <utils/Atomic.h>
-
 #include <ui/PixelFormat.h>
 #include <ui/Rect.h>
-#include <ui/Surface.h>
-
 #include <pixelflinger/pixelflinger.h>
-
-#include <private/ui/SharedBufferStack.h>
 #include <private/ui/SurfaceBuffer.h>
 
-class copybit_image_t;
 struct android_native_buffer_t;
 
 namespace android {
@@ -42,8 +33,6 @@ namespace android {
 // Buffer
 // ===========================================================================
 
-class NativeBuffer;
-
 class Buffer : public SurfaceBuffer
 {
 public:
diff --git a/libs/surfaceflinger/BufferAllocator.cpp b/libs/surfaceflinger/BufferAllocator.cpp
index caf9bec..3e37bc3 100644
--- a/libs/surfaceflinger/BufferAllocator.cpp
+++ b/libs/surfaceflinger/BufferAllocator.cpp
@@ -15,8 +15,6 @@
 ** limitations under the License.
 */
 
-#include <sys/mman.h>
-#include <cutils/ashmem.h>
 #include <cutils/log.h>
 
 #include <utils/Singleton.h>
diff --git a/libs/surfaceflinger/Layer.cpp b/libs/surfaceflinger/Layer.cpp
index 07222ec..13201db 100644
--- a/libs/surfaceflinger/Layer.cpp
+++ b/libs/surfaceflinger/Layer.cpp
@@ -65,14 +65,6 @@ Layer::~Layer()
     // the actual buffers will be destroyed here
 }
 
-// called with SurfaceFlinger::mStateLock as soon as the layer is entered
-// in the purgatory list
-void Layer::onRemoved()
-{
-    // wake up the condition
-    lcblk->setStatus(NO_INIT);
-}
-
 void Layer::destroy()
 {
     for (size_t i=0 ; i<NUM_BUFFERS ; i++) {
diff --git a/libs/surfaceflinger/Layer.h b/libs/surfaceflinger/Layer.h
index 6c7b27b..f111840 100644
--- a/libs/surfaceflinger/Layer.h
+++ b/libs/surfaceflinger/Layer.h
@@ -85,8 +85,6 @@ private:
         return mBuffers[mFrontBufferIndex];
     }
  
-    virtual void onRemoved();
-
     void reloadTexture(const Region& dirty);
 
     sp<SurfaceBuffer> requestBuffer(int index, int usage);
diff --git a/libs/surfaceflinger/LayerBase.cpp b/libs/surfaceflinger/LayerBase.cpp
index d83c842..83814cc 100644
--- a/libs/surfaceflinger/LayerBase.cpp
+++ b/libs/surfaceflinger/LayerBase.cpp
@@ -690,6 +690,14 @@ sp<LayerBaseClient::Surface> LayerBaseClient::createSurface() const
             const_cast<LayerBaseClient *>(this));
 }
 
+// called with SurfaceFlinger::mStateLock as soon as the layer is entered
+// in the purgatory list
+void LayerBaseClient::onRemoved()
+{
+    // wake up the condition
+    lcblk->setStatus(NO_INIT);
+}
+
 // ---------------------------------------------------------------------------
 
 LayerBaseClient::Surface::Surface(
@@ -700,7 +708,6 @@ LayerBaseClient::Surface::Surface(
 {
 }
 
-
 LayerBaseClient::Surface::~Surface() 
 {
     /*
diff --git a/libs/surfaceflinger/LayerBase.h b/libs/surfaceflinger/LayerBase.h
index 16ee542..0dfa4fe 100644
--- a/libs/surfaceflinger/LayerBase.h
+++ b/libs/surfaceflinger/LayerBase.h
@@ -205,10 +205,13 @@ public:
      */
     virtual bool isSecure() const       { return false; }
 
-    /** signal this layer that it's not needed any longer. called from the 
-     * main thread */
+    /** Called from the main thread, when the surface is removed from the
+     * draw list */
     virtual status_t ditch() { return NO_ERROR; }
 
+    /** called with the state lock when the surface is removed from the
+     *  current list */
+    virtual void onRemoved() { };
     
     
     enum { // flags for doTransaction()
@@ -318,7 +321,7 @@ public:
             sp<Surface> getSurface();
     virtual sp<Surface> createSurface() const;
     
-    virtual void onRemoved() { }
+    virtual void onRemoved();
 
     class Surface : public BnSurface 
     {
diff --git a/libs/surfaceflinger/SurfaceFlinger.cpp b/libs/surfaceflinger/SurfaceFlinger.cpp
index eb0983a..f2b918f 100644
--- a/libs/surfaceflinger/SurfaceFlinger.cpp
+++ b/libs/surfaceflinger/SurfaceFlinger.cpp
@@ -1073,6 +1073,8 @@ status_t SurfaceFlinger::purgatorizeLayer_l(const sp<LayerBase>& layerBase)
     // remove the layer from the main list (through a transaction).
     ssize_t err = removeLayer_l(layerBase);
 
+    layerBase->onRemoved();
+
     // 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 BClient::destroySurface(),
@@ -1321,7 +1323,6 @@ status_t SurfaceFlinger::removeSurface(SurfaceID index)
     if (layer != 0) {
         err = purgatorizeLayer_l(layer);
         if (err == NO_ERROR) {
-            layer->onRemoved();
             setTransactionFlags(eTransactionNeeded);
         }
     }
diff --git a/libs/ui/SharedBufferStack.cpp b/libs/ui/SharedBufferStack.cpp
index 9ad4349..47c596c 100644
--- a/libs/ui/SharedBufferStack.cpp
+++ b/libs/ui/SharedBufferStack.cpp
@@ -114,6 +114,12 @@ uint32_t SharedBufferBase::getIdentity()
     return stack.identity;
 }
 
+status_t SharedBufferBase::getStatus() const
+{
+    SharedBufferStack& stack( *mSharedStack );
+    return stack.status;
+}
+
 size_t SharedBufferBase::getFrontBuffer() const
 {
     SharedBufferStack& stack( *mSharedStack );
@@ -135,7 +141,6 @@ String8 SharedBufferBase::dump(char const* prefix) const
     return result;
 }
 
-
 // ============================================================================
 // conditions and updates
 // ============================================================================
@@ -375,8 +380,10 @@ status_t SharedBufferServer::unlock(int buffer)
 
 void SharedBufferServer::setStatus(status_t status)
 {
-    StatusUpdate update(this, status);
-    updateCondition( update );
+    if (status < NO_ERROR) {
+        StatusUpdate update(this, status);
+        updateCondition( update );
+    }
 }
 
 status_t SharedBufferServer::reallocate()
diff --git a/libs/ui/Surface.cpp b/libs/ui/Surface.cpp
index 64522fb..285edb4 100644
--- a/libs/ui/Surface.cpp
+++ b/libs/ui/Surface.cpp
@@ -733,9 +733,11 @@ status_t Surface::getBufferLocked(int index, int usage)
             index, usage);
     if (buffer != 0) { // this should never happen by construction
         LOGE_IF(buffer->handle == NULL, 
-                "requestBuffer(%d, %08x) returned a buffer with a null handle",
-                index, usage);
-        if (buffer->handle != NULL) { 
+                "Surface (identity=%d) requestBuffer(%d, %08x) returned"
+                "a buffer with a null handle", mIdentity, index, usage);
+        err = mSharedBufferClient->getStatus();
+        LOGE_IF(err,  "Surface (identity=%d) state = %d", mIdentity, err);
+        if (!err && buffer->handle != NULL) {
             err = getBufferMapper().registerBuffer(buffer->handle);
             LOGW_IF(err, "registerBuffer(...) failed %d (%s)",
                     err, strerror(-err));