From cc3d8b1aa20539f66307b1ef187f4b601051a136 Mon Sep 17 00:00:00 2001 From: Nicolas Roard Date: Thu, 17 Mar 2011 10:31:57 -0700 Subject: 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 --- .../graphics/android/BackedDoubleBufferedTexture.cpp | 18 +++++++++--------- .../graphics/android/BackedDoubleBufferedTexture.h | 1 + WebCore/platform/graphics/android/LayerAndroid.cpp | 8 ++++++-- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/WebCore/platform/graphics/android/BackedDoubleBufferedTexture.cpp b/WebCore/platform/graphics/android/BackedDoubleBufferedTexture.cpp index 470ecf1..f68050f 100644 --- 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 --- 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 --- 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; } -- cgit v1.1