Prevent race condition in tile texture discard

bug:5461107

Tiles were being destroyed, and subsequently dereferenced in TransferQueue

Change-Id: I4fea289e5fda03a69f07554f57120c4c5bf7b016

diff --git a/Source/WebCore/platform/graphics/android/BaseTile.cpp b/Source/WebCore/platform/graphics/android/BaseTile.cpp
index 350001a..05b3a3d 100644 (file)
--- a/Source/WebCore/platform/graphics/android/BaseTile.cpp
+++ b/Source/WebCore/platform/graphics/android/BaseTile.cpp
@@ -504,6 +504,14 @@ void BaseTile::discardTextures() {
     m_state = Unpainted;
 }
 
+void BaseTile::discardBackTexture() {
+    android::AutoMutex lock(m_atomicSync);
+    if (m_backTexture) {
+        m_backTexture->release(this);
+        m_backTexture = 0;
+    }
+}
+
 bool BaseTile::swapTexturesIfNeeded() {
     android::AutoMutex lock(m_atomicSync);
     if (m_state == ReadyToSwap) {

diff --git a/Source/WebCore/platform/graphics/android/BaseTile.h b/Source/WebCore/platform/graphics/android/BaseTile.h
index cabf5f1..685ca43 100644 (file)
--- a/Source/WebCore/platform/graphics/android/BaseTile.h
+++ b/Source/WebCore/platform/graphics/android/BaseTile.h
@@ -127,6 +127,7 @@ public:
     // only used for prioritization - the higher, the more relevant the tile is
     unsigned long long drawCount() { return m_drawCount; }
     void discardTextures();
+    void discardBackTexture();
     bool swapTexturesIfNeeded();
     void backTextureTransfer();
     void backTextureTransferFail();

diff --git a/Source/WebCore/platform/graphics/android/BaseTileTexture.cpp b/Source/WebCore/platform/graphics/android/BaseTileTexture.cpp
index 5b7acdb..caaf116 100644 (file)
--- a/Source/WebCore/platform/graphics/android/BaseTileTexture.cpp
+++ b/Source/WebCore/platform/graphics/android/BaseTileTexture.cpp
@@ -91,7 +91,11 @@ void BaseTileTexture::discardGLTexture()
     if (m_ownTextureId)
         GLUtils::deleteTexture(&m_ownTextureId);
 
-    releaseAndRemoveFromTile();
+    if (m_owner) {
+        // clear both Tile->Texture and Texture->Tile links
+        m_owner->removeTexture(this);
+        release(m_owner);
+    }
 }
 
 void BaseTileTexture::destroyTextures(SharedTexture** textures)
@@ -209,16 +213,6 @@ bool BaseTileTexture::release(TextureOwner* owner)
     return true;
 }
 
-void BaseTileTexture::releaseAndRemoveFromTile()
-{
-    // NOTE: only call on UI thread, so m_owner won't be changing
-    if (m_owner) {
-        // clear both Tile->Texture and Texture->Tile links
-        m_owner->removeTexture(this);
-        release(m_owner);
-    }
-}
-
 void BaseTileTexture::setTile(TextureInfo* info, int x, int y,
                                           float scale, TilePainter* painter,
                                           unsigned int pictureCount)

diff --git a/Source/WebCore/platform/graphics/android/TransferQueue.cpp b/Source/WebCore/platform/graphics/android/TransferQueue.cpp
index 3fc1b93..4e29870 100644 (file)
--- a/Source/WebCore/platform/graphics/android/TransferQueue.cpp
+++ b/Source/WebCore/platform/graphics/android/TransferQueue.cpp
@@ -440,6 +440,7 @@ void TransferQueue::addItemInTransferQueue(const TileRenderInfo* renderInfo,
         XLOG("ERROR update a tile which is dirty already @ index %d", index);
     }
 
+    m_transferQueue[index].savedBaseTileTexturePtr = renderInfo->baseTile->backTexture();
     m_transferQueue[index].savedBaseTilePtr = renderInfo->baseTile;
     m_transferQueue[index].status = pendingBlit;
     m_transferQueue[index].uploadType = type;
@@ -493,14 +494,16 @@ void TransferQueue::cleanupTransportQueue()
             // since tiles in the queue may be from another webview, remove
             // their textures so that they will be repainted / retransferred
             BaseTile* tile = m_transferQueue[index].savedBaseTilePtr;
-            if (tile) {
-                BaseTileTexture* texture = tile->backTexture();
-                if (texture)
-                    texture->releaseAndRemoveFromTile();
+            BaseTileTexture* texture = m_transferQueue[index].savedBaseTileTexturePtr;
+            if (tile && texture && texture->owner() == tile) {
+                // since tile destruction removes textures on the UI thread, the
+                // texture->owner ptr guarantees the tile is valid
+                tile->discardBackTexture();
+                XLOG("transfer queue discarded tile %p, removed texture", tile);
             }
-            XLOG("transfer queue discarded tile %p, removed texture", tile);
 
             m_transferQueue[index].savedBaseTilePtr = 0;
+            m_transferQueue[index].savedBaseTileTexturePtr = 0;
             m_transferQueue[index].status = emptyItem;
         }
         index = (index + 1) % ST_BUFFER_NUMBER;

diff --git a/Source/WebCore/platform/graphics/android/TransferQueue.h b/Source/WebCore/platform/graphics/android/TransferQueue.h
index bddc85d..63455de 100644 (file)
--- a/Source/WebCore/platform/graphics/android/TransferQueue.h
+++ b/Source/WebCore/platform/graphics/android/TransferQueue.h
@@ -70,6 +70,7 @@ public:
     TileTransferData()
     : status(emptyItem)
     , savedBaseTilePtr(0)
+    , savedBaseTileTexturePtr(0)
     , uploadType(DEFAULT_UPLOAD_TYPE)
     , bitmap(0)
     , m_syncKHR(EGL_NO_SYNC_KHR)
@@ -84,6 +85,7 @@ public:
 
     TransferItemStatus status;
     BaseTile* savedBaseTilePtr;
+    BaseTileTexture* savedBaseTileTexturePtr;
     TextureTileInfo tileInfo;
     TextureUploadType uploadType;
     // This is only useful in Cpu upload code path, so it will be dynamically