Fix Browser ANR

The problem was that when attempting to forcefully destroy a texture
in TilesManager::cleanupLayersTextures(), the call to
LayerAndroid::removeTexture() may fail if the texture was busy being
painted -- the call to BackedDoubleBufferedTexture::release() would
return false, and the layer may thus still keep a pointer to the texture.
But the release() call, while indicating it failed, was only delaying
the release -- as soon as the texture was marked as not busy, it
could set its owner to nil.

We could thus have a situation where the layer did not reset its
texture pointers because the owner of the texture was not yet
changed, but the texture would then reset its owner to nil as soon
as it was not busy painting.

In TilesManager::cleanupLayersTexture() the next step before deleting
a texture is to check that the texture does not have an owner -- but
by then the texture could have been marked as not busy, and removed
its owner, letting TilesManager destroying it.

bug:3472320
Change-Id: I3bcf169b30dfacba1773d3b79a3c0d205bf3cbdb

diff --git a/WebCore/platform/graphics/android/BackedDoubleBufferedTexture.cpp b/WebCore/platform/graphics/android/BackedDoubleBufferedTexture.cpp
index 470ecf1..f68050f 100644 (file)
--- a/WebCore/platform/graphics/android/BackedDoubleBufferedTexture.cpp
+++ b/WebCore/platform/graphics/android/BackedDoubleBufferedTexture.cpp
@@ -228,16 +228,16 @@ bool BackedDoubleBufferedTexture::setOwner(TextureOwner* owner)
 bool BackedDoubleBufferedTexture::release(TextureOwner* owner)
 {
     android::Mutex::Autolock lock(m_busyLock);
-    if (m_owner == owner) {
-        if (!m_busy) {
-            m_owner = 0;
-            return true;
-        } else {
-            m_delayedRelease = true;
-            m_delayedReleaseOwner = owner;
-        }
+    if (m_owner != owner)
+        return false;
+
+    if (!m_busy) {
+        m_owner = 0;
+    } else {
+        m_delayedRelease = true;
+        m_delayedReleaseOwner = owner;
     }
-    return false;
+    return true;
 }
 
 void BackedDoubleBufferedTexture::setTile(TextureInfo* info, int x, int y,

diff --git a/WebCore/platform/graphics/android/BackedDoubleBufferedTexture.h b/WebCore/platform/graphics/android/BackedDoubleBufferedTexture.h
index 2d19806..f612114 100644 (file)
--- a/WebCore/platform/graphics/android/BackedDoubleBufferedTexture.h
+++ b/WebCore/platform/graphics/android/BackedDoubleBufferedTexture.h
@@ -96,6 +96,7 @@ public:
 
     // private member accessor functions
     TextureOwner* owner() { return m_owner; } // only used by the consumer thread
+    TextureOwner* delayedReleaseOwner() { return m_delayedReleaseOwner; }
     SkCanvas* canvas(); // only used by the producer thread
     SkBitmap* bitmap() { return m_bitmap; }
 

diff --git a/WebCore/platform/graphics/android/LayerAndroid.cpp b/WebCore/platform/graphics/android/LayerAndroid.cpp
index 89ce301..33d98ea 100644 (file)
--- a/WebCore/platform/graphics/android/LayerAndroid.cpp
+++ b/WebCore/platform/graphics/android/LayerAndroid.cpp
@@ -186,9 +186,13 @@ bool LayerAndroid::removeTexture(BackedDoubleBufferedTexture* aTexture)
             m_reservedTexture != m_drawingTexture)
             textureReleased &= m_reservedTexture->release(this);
     }
-    if (m_drawingTexture && m_drawingTexture->owner() != this)
+    if (m_drawingTexture &&
+        ((m_drawingTexture->owner() != this) ||
+         (m_drawingTexture->delayedReleaseOwner() == this)))
         m_drawingTexture = 0;
-    if (m_reservedTexture && m_reservedTexture->owner() != this)
+    if (m_reservedTexture &&
+        ((m_reservedTexture->owner() != this) ||
+         (m_reservedTexture->delayedReleaseOwner() == this)))
         m_reservedTexture = 0;
     return textureReleased;
 }