Merge "Fix ANR in the browser Sometimes we were not releasing textures as they were busy; they could still be deleted when swapping the layers trees, and as they were also still present in the LayerTexture Hashmap this was causing an ANR (at best -- the texture was already deallocated, the LayerTexture dtor was then trying to release() them...)"

9 files changed, 87 insertions, 25 deletions

diff --git a/WebCore/platform/graphics/android/BackedDoubleBufferedTexture.cpp b/WebCore/platform/graphics/android/BackedDoubleBufferedTexture.cpp
index 77f673a..d16b53e 100644
--- a/WebCore/platform/graphics/android/BackedDoubleBufferedTexture.cpp
+++ b/WebCore/platform/graphics/android/BackedDoubleBufferedTexture.cpp
@@ -47,6 +47,8 @@ BackedDoubleBufferedTexture::BackedDoubleBufferedTexture(uint32_t w, uint32_t h,
     , m_usedLevel(-1)
     , m_config(config)
     , m_owner(0)
+    , m_delayedReleaseOwner(0)
+    , m_delayedRelease(false)
     , m_busy(false)
 {
     m_size.set(w, h);
@@ -117,15 +119,27 @@ TextureInfo* BackedDoubleBufferedTexture::producerLock()
 void BackedDoubleBufferedTexture::producerRelease()
 {
     DoubleBufferedTexture::producerRelease();
-    android::Mutex::Autolock lock(m_busyLock);
-    m_busy = false;
+    setNotBusy();
 }
 
 void BackedDoubleBufferedTexture::producerReleaseAndSwap()
 {
     DoubleBufferedTexture::producerReleaseAndSwap();
+    setNotBusy();
+}
+
+void BackedDoubleBufferedTexture::setNotBusy()
+{
     android::Mutex::Autolock lock(m_busyLock);
     m_busy = false;
+    if (m_delayedRelease) {
+        if (m_owner == m_delayedReleaseOwner) {
+            m_owner->removeOwned(this);
+            m_owner = 0;
+        }
+        m_delayedRelease = false;
+        m_delayedReleaseOwner = 0;
+    }
 }
 
 bool BackedDoubleBufferedTexture::busy()
@@ -175,24 +189,45 @@ bool BackedDoubleBufferedTexture::setOwner(TextureOwner* owner)
 {
     // if the writable texture is busy (i.e. currently being written to) then we
     // can't change the owner out from underneath that texture
-    android::Mutex::Autolock lock(m_busyLock);
-    if (!m_busy) {
+    m_busyLock.lock();
+    bool busy = m_busy;
+    m_busyLock.unlock();
+    if (!busy) {
+        // if we are not busy we can try to remove the texture from the layer;
+        // LayerAndroid::removeTexture() is protected by the same lock as
+        // LayerAndroid::paintBitmapGL(), so either we execute removeTexture()
+        // first and paintBitmapGL() will bail out, or we execute it after,
+        // and paintBitmapGL() will mark the texture as busy before
+        // relinquishing the lock. LayerAndroid::removeTexture() will call
+        // BackedDoubleBufferedTexture::release(), which will then do nothing
+        // if the texture is busy and we then don't return true.
+        bool proceed = true;
         if (m_owner && m_owner != owner)
-            m_owner->removeTexture(this);
-        m_owner = owner;
-        owner->addOwned(this);
-        return true;
+            proceed = m_owner->removeTexture(this);
+
+        if (proceed) {
+            m_owner = owner;
+            owner->addOwned(this);
+            return true;
+        }
     }
     return false;
 }
 
-void BackedDoubleBufferedTexture::release(TextureOwner* owner)
+bool BackedDoubleBufferedTexture::release(TextureOwner* owner)
 {
+    android::Mutex::Autolock lock(m_busyLock);
     if (m_owner == owner) {
-        m_owner->removeOwned(this);
-        m_owner = 0;
+        if (!m_busy) {
+            m_owner->removeOwned(this);
+            m_owner = 0;
+            return true;
+        } else {
+            m_delayedRelease = true;
+            m_delayedReleaseOwner = owner;
+        }
     }
-
+    return false;
 }
 
 } // namespace WebCore
diff --git a/WebCore/platform/graphics/android/BackedDoubleBufferedTexture.h b/WebCore/platform/graphics/android/BackedDoubleBufferedTexture.h
index 1c50d87..b1f170b 100644
--- a/WebCore/platform/graphics/android/BackedDoubleBufferedTexture.h
+++ b/WebCore/platform/graphics/android/BackedDoubleBufferedTexture.h
@@ -68,7 +68,7 @@ public:
     // allows consumer thread to assign ownership of the texture to the tile. It
     // returns false if ownership cannot be transferred because the tile is busy
     bool acquire(TextureOwner* owner);
-    void release(TextureOwner* owner);
+    bool release(TextureOwner* owner);
 
     // set the texture owner if not busy. Return false if busy, true otherwise.
     bool setOwner(TextureOwner* owner);
@@ -78,6 +78,7 @@ public:
     SkCanvas* canvas(); // only used by the producer thread
 
     bool busy();
+    void setNotBusy();
 
     const SkSize& getSize() const { return m_size; }
 
@@ -98,6 +99,12 @@ private:
     SkBitmap::Config m_config;
     TextureOwner* m_owner;
 
+    // When trying to release a texture, we may delay this if the texture is
+    // currently used (busy being painted). We use the following two variables
+    // to do so in setNotBusy()
+    TextureOwner* m_delayedReleaseOwner;
+    bool m_delayedRelease;
+
     // This values signals that the texture is currently in use by the consumer.
     // This allows us to prevent the owner of the texture from changing while the
     // consumer is holding a lock on the texture.
diff --git a/WebCore/platform/graphics/android/BaseTile.cpp b/WebCore/platform/graphics/android/BaseTile.cpp
index 960d073..e5275c6 100644
--- a/WebCore/platform/graphics/android/BaseTile.cpp
+++ b/WebCore/platform/graphics/android/BaseTile.cpp
@@ -107,13 +107,14 @@ void BaseTile::reserveTexture()
     m_texture = texture;
 }
 
-void BaseTile::removeTexture(BackedDoubleBufferedTexture* texture)
+bool BaseTile::removeTexture(BackedDoubleBufferedTexture* texture)
 {
     XLOG("%x removeTexture res: %x... page %x", this, m_texture, m_page);
     // We update atomically, so paintBitmap() can see the correct value
     android::AutoMutex lock(m_atomicSync);
     if (m_texture == texture)
         m_texture = 0;
+    return true;
 }
 
 void BaseTile::setScale(float scale)
diff --git a/WebCore/platform/graphics/android/BaseTile.h b/WebCore/platform/graphics/android/BaseTile.h
index 0527fcd..af7df3a 100644
--- a/WebCore/platform/graphics/android/BaseTile.h
+++ b/WebCore/platform/graphics/android/BaseTile.h
@@ -90,7 +90,7 @@ public:
     BackedDoubleBufferedTexture* texture() { return m_texture; }
 
     // TextureOwner implementation
-    virtual void removeTexture(BackedDoubleBufferedTexture* texture);
+    virtual bool removeTexture(BackedDoubleBufferedTexture* texture);
     virtual TiledPage* page() { return m_page; }
 
 private:
diff --git a/WebCore/platform/graphics/android/GLWebViewState.cpp b/WebCore/platform/graphics/android/GLWebViewState.cpp
index 38b747d..5ba094b 100644
--- a/WebCore/platform/graphics/android/GLWebViewState.cpp
+++ b/WebCore/platform/graphics/android/GLWebViewState.cpp
@@ -108,9 +108,9 @@ void GLWebViewState::setBaseLayer(BaseLayerAndroid* layer, const IntRect& rect)
     // We only update the layers if we are not currently
     // waiting for a tiledPage to be painted
     if (m_baseLayerUpdate) {
+        layer->safeRef();
         m_currentBaseLayer->safeUnref();
         m_currentBaseLayer = layer;
-        m_currentBaseLayer->safeRef();
     }
     inval(rect);
 }
@@ -118,9 +118,9 @@ void GLWebViewState::setBaseLayer(BaseLayerAndroid* layer, const IntRect& rect)
 void GLWebViewState::unlockBaseLayerUpdate() {
     m_baseLayerUpdate = true;
     android::Mutex::Autolock lock(m_baseLayerLock);
+    m_baseLayer->safeRef();
     m_currentBaseLayer->safeUnref();
     m_currentBaseLayer = m_baseLayer;
-    m_currentBaseLayer->safeRef();
     inval(m_invalidateRect);
     IntRect empty;
     m_invalidateRect = empty;
@@ -295,6 +295,19 @@ void GLWebViewState::setViewport(SkRect& viewport, float scale)
     m_tiledPageB->updateBaseTileSize();
 }
 
+bool GLWebViewState::drawGL(IntRect& rect, SkRect& viewport, float scale, SkColor color)
+{
+    m_baseLayerLock.lock();
+    BaseLayerAndroid* baseLayer = m_currentBaseLayer;
+    baseLayer->safeRef();
+    m_baseLayerLock.unlock();
+    if (!baseLayer)
+        return false;
+    bool ret = baseLayer->drawGL(rect, viewport, scale, color);
+    baseLayer->safeUnref();
+    return ret;
+}
+
 } // namespace WebCore
 
 #endif // USE(ACCELERATED_COMPOSITING)
diff --git a/WebCore/platform/graphics/android/GLWebViewState.h b/WebCore/platform/graphics/android/GLWebViewState.h
index 7fa5e39..2082f2c 100644
--- a/WebCore/platform/graphics/android/GLWebViewState.h
+++ b/WebCore/platform/graphics/android/GLWebViewState.h
@@ -205,6 +205,9 @@ public:
         return false;
     }
 
+    bool drawGL(IntRect& rect, SkRect& viewport,
+                float scale, SkColor color = SK_ColorWHITE);
+
 private:
     void inval(const IntRect& rect); // caller must hold m_baseLayerLock
 
diff --git a/WebCore/platform/graphics/android/LayerAndroid.cpp b/WebCore/platform/graphics/android/LayerAndroid.cpp
index 42104dc..8771d3b 100644
--- a/WebCore/platform/graphics/android/LayerAndroid.cpp
+++ b/WebCore/platform/graphics/android/LayerAndroid.cpp
@@ -157,28 +157,31 @@ LayerAndroid::LayerAndroid(SkPicture* picture) : SkLayer(),
 #endif
 }
 
-void LayerAndroid::removeTexture(BackedDoubleBufferedTexture* aTexture)
+bool LayerAndroid::removeTexture(BackedDoubleBufferedTexture* aTexture)
 {
     LayerTexture* texture = static_cast<LayerTexture*>(aTexture);
     android::AutoMutex lock(m_atomicSync);
+
+    bool textureReleased = true;
     if (!texture) { // remove ourself from both textures
         if (m_drawingTexture)
-            m_drawingTexture->release(this);
+            textureReleased &= m_drawingTexture->release(this);
         if (m_reservedTexture &&
             m_reservedTexture != m_drawingTexture)
-            m_reservedTexture->release(this);
+            textureReleased &= m_reservedTexture->release(this);
     } else {
         if (m_drawingTexture && m_drawingTexture == texture)
-            m_drawingTexture->release(this);
+            textureReleased &= m_drawingTexture->release(this);
         if (m_reservedTexture &&
             m_reservedTexture == texture &&
             m_reservedTexture != m_drawingTexture)
-            m_reservedTexture->release(this);
+            textureReleased &= m_reservedTexture->release(this);
     }
     if (m_drawingTexture && m_drawingTexture->owner() != this)
         m_drawingTexture = 0;
     if (m_reservedTexture && m_reservedTexture->owner() != this)
         m_reservedTexture = 0;
+    return textureReleased;
 }
 
 LayerAndroid::~LayerAndroid()
@@ -926,7 +929,7 @@ void LayerAndroid::paintBitmapGL()
     // If LayerAndroid::removeTexture() is called before us, we'd have bailed
     // out early as texture would have been null; if it is called after us, we'd
     // have marked the texture has being busy, and the texture will not be
-    // destroy immediately.
+    // destroyed immediately.
     texture->producerAcquireContext();
     TextureInfo* textureInfo = texture->producerLock();
     m_atomicSync.unlock();
diff --git a/WebCore/platform/graphics/android/LayerAndroid.h b/WebCore/platform/graphics/android/LayerAndroid.h
index 53e513b..2cb56c1 100644
--- a/WebCore/platform/graphics/android/LayerAndroid.h
+++ b/WebCore/platform/graphics/android/LayerAndroid.h
@@ -94,7 +94,7 @@ public:
     virtual ~LayerAndroid();
 
     // TextureOwner methods
-    virtual void removeTexture(BackedDoubleBufferedTexture* texture);
+    virtual bool removeTexture(BackedDoubleBufferedTexture* texture);
 
     LayerTexture* texture() { return m_reservedTexture; }
     virtual TiledPage* page() { return 0; }
diff --git a/WebCore/platform/graphics/android/TextureOwner.h b/WebCore/platform/graphics/android/TextureOwner.h
index c421e8a..684efac 100644
--- a/WebCore/platform/graphics/android/TextureOwner.h
+++ b/WebCore/platform/graphics/android/TextureOwner.h
@@ -36,7 +36,7 @@ class BackedDoubleBufferedTexture;
 class TextureOwner {
 public:
     virtual ~TextureOwner();
-    virtual void removeTexture(BackedDoubleBufferedTexture* texture) = 0;
+    virtual bool removeTexture(BackedDoubleBufferedTexture* texture) = 0;
     virtual TiledPage* page() = 0;
 
     void addOwned(BackedDoubleBufferedTexture*);