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.
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 @@
// 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 @@
// 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 @@
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 @@
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()
{
/*
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 @@
*/
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 @@
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 @@
// 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 @@
if (layer != 0) {
err = purgatorizeLayer_l(layer);
if (err == NO_ERROR) {
- layer->onRemoved();
setTransactionFlags(eTransactionNeeded);
}
}