Fix a potential sync problem for TransferQueue

bug:5044597

Change-Id: Id8cd326774a174a537f2b104d934539269395151

4 files changed, 65 insertions, 24 deletions

diff --git a/Source/WebCore/platform/graphics/android/TexturesGenerator.cpp b/Source/WebCore/platform/graphics/android/TexturesGenerator.cpp
index 704b7d0..9780336 100644
--- a/Source/WebCore/platform/graphics/android/TexturesGenerator.cpp
+++ b/Source/WebCore/platform/graphics/android/TexturesGenerator.cpp
@@ -98,8 +98,19 @@ void TexturesGenerator::removeOperationsForFilter(OperationFilter* filter, bool
 
     if (waitForRunning && m_currentOperation) {
         QueuedOperation* operation = m_currentOperation;
-        if (operation && filter->check(operation))
+
+        if (operation && filter->check(operation)) {
             m_waitForCompletion = true;
+            // The reason we are signaling the transferQueue is :
+            // TransferQueue may be waiting a slot to work on, but now UI
+            // thread is waiting for Tex Gen thread to finish first before the
+            // UI thread can free a slot for the transferQueue.
+            // Therefore, it could be a deadlock.
+            // The solution is use this as a flag to tell Tex Gen thread that
+            // UI thread is waiting now, Tex Gen thread should not wait for the
+            // queue any more.
+            TilesManager::instance()->transferQueue()->interruptTransferQueue(true);
+        }
 
         delete filter;
 
@@ -185,6 +196,7 @@ bool TexturesGenerator::threadLoop()
             stop = true;
         if (m_waitForCompletion) {
             m_waitForCompletion = false;
+            TilesManager::instance()->transferQueue()->interruptTransferQueue(false);
             mRequestedOperationsCond.signal();
         }
         mRequestedOperationsLock.unlock();
diff --git a/Source/WebCore/platform/graphics/android/TilesManager.h b/Source/WebCore/platform/graphics/android/TilesManager.h
index e884fc0..3541422 100644
--- a/Source/WebCore/platform/graphics/android/TilesManager.h
+++ b/Source/WebCore/platform/graphics/android/TilesManager.h
@@ -120,39 +120,48 @@ public:
 
     void allocateTiles();
 
-    void setExpandedTileBounds(bool enabled) {
+    void setExpandedTileBounds(bool enabled)
+    {
         m_expandedTileBounds = enabled;
     }
 
-    bool getShowVisualIndicator() {
+    bool getShowVisualIndicator()
+    {
         return m_showVisualIndicator;
     }
 
-    void setShowVisualIndicator(bool showVisualIndicator) {
+    void setShowVisualIndicator(bool showVisualIndicator)
+    {
         m_showVisualIndicator = showVisualIndicator;
     }
 
-    SharedTextureMode getSharedTextureMode() {
+    SharedTextureMode getSharedTextureMode()
+    {
         return SurfaceTextureMode;
     }
 
-    TilesProfiler* getProfiler() {
+    TilesProfiler* getProfiler()
+    {
         return &m_profiler;
     }
 
-    TilesTracker* getTilesTracker() {
+    TilesTracker* getTilesTracker()
+    {
         return &m_tilesTracker;
     }
 
-    bool invertedScreen() {
+    bool invertedScreen()
+    {
         return m_invertedScreen;
     }
 
-    void setInvertedScreen(bool invert) {
+    void setInvertedScreen(bool invert)
+    {
         m_invertedScreen = invert;
     }
 
-    void setInvertedScreenContrast(float contrast) {
+    void setInvertedScreenContrast(float contrast)
+    {
         m_shader.setContrast(contrast);
     }
 
diff --git a/Source/WebCore/platform/graphics/android/TransferQueue.cpp b/Source/WebCore/platform/graphics/android/TransferQueue.cpp
index ae94b2e..a4fd594 100644
--- a/Source/WebCore/platform/graphics/android/TransferQueue.cpp
+++ b/Source/WebCore/platform/graphics/android/TransferQueue.cpp
@@ -158,21 +158,38 @@ void TransferQueue::blitTileFromQueue(GLuint fboID, BaseTileTexture* destTex, GL
     glBindFramebuffer(GL_FRAMEBUFFER, 0); // rebind the standard FBO
 
     // Add a sync point here to WAR a driver bug.
-    glViewport(0,0,0,0);
+    glViewport(0, 0, 0, 0);
     TilesManager::instance()->shader()->drawQuad(rect, destTex->m_ownTextureId,
                                                  1.0, GL_TEXTURE_2D);
 
     GLUtils::checkGlError("copy the surface texture into the normal one");
 }
 
+void TransferQueue::interruptTransferQueue(bool interrupt)
+{
+    m_transferQueueItemLocks.lock();
+    m_interruptedByRemovingOp = interrupt;
+    if (m_interruptedByRemovingOp)
+        m_transferQueueItemCond.signal();
+    m_transferQueueItemLocks.unlock();
+}
+
+// This function must be called inside the m_transferQueueItemLocks, for the
+// wait, m_interruptedByRemovingOp and getHasGLContext().
+// Only called by updateQueueWithBitmap() for now.
 bool TransferQueue::readyForUpdate()
 {
     if (!getHasGLContext())
         return false;
     // Don't use a while loop since when the WebView tear down, the emptyCount
     // will still be 0, and we bailed out b/c of GL context lost.
-    if (!m_emptyItemCount)
+    if (!m_emptyItemCount) {
+        if (m_interruptedByRemovingOp)
+            return false;
         m_transferQueueItemCond.wait(m_transferQueueItemLocks);
+        if (m_interruptedByRemovingOp)
+            return false;
+    }
 
     if (!getHasGLContext())
         return false;
diff --git a/Source/WebCore/platform/graphics/android/TransferQueue.h b/Source/WebCore/platform/graphics/android/TransferQueue.h
index 8d9df68..550623e 100644
--- a/Source/WebCore/platform/graphics/android/TransferQueue.h
+++ b/Source/WebCore/platform/graphics/android/TransferQueue.h
@@ -62,21 +62,11 @@ public:
     // The lock will be done when returning true.
     bool readyForUpdate();
 
+    void interruptTransferQueue(bool);
     // This queue can be accessed from UI and TexGen thread, therefore, we need
     // a lock to protect its access
     TileTransferData* m_transferQueue;
 
-    // We are using wait/signal to handle our own queue sync.
-    // First of all, if we don't have our own lock, then while WebView is
-    // destroyed, the UI thread will wait for the Tex Gen to get out from
-    // dequeue operation, which will not succeed. B/c at this moment, we
-    // already lost the GL Context.
-    // Now we maintain a counter, which is m_emptyItemCount. When this reach
-    // 0, then we need the Tex Gen thread to wait. UI thread can signal this
-    // wait after calling updateTexImage at the draw call , or after WebView
-    // is destroyed.
-    android::Mutex m_transferQueueItemLocks;
-
     sp<ANativeWindow> m_ANW;
 
 private:
@@ -114,8 +104,21 @@ private:
     GLState m_GLStateBeforeBlit;
     sp<android::SurfaceTexture> m_sharedSurfaceTexture;
 
-    android::Condition m_transferQueueItemCond;
     int m_emptyItemCount;
+
+    bool m_interruptedByRemovingOp;
+
+    // We are using wait/signal to handle our own queue sync.
+    // First of all, if we don't have our own lock, then while WebView is
+    // destroyed, the UI thread will wait for the Tex Gen to get out from
+    // dequeue operation, which will not succeed. B/c at this moment, we
+    // already lost the GL Context.
+    // Now we maintain a counter, which is m_emptyItemCount. When this reach
+    // 0, then we need the Tex Gen thread to wait. UI thread can signal this
+    // wait after calling updateTexImage at the draw call , or after WebView
+    // is destroyed.
+    android::Mutex m_transferQueueItemLocks;
+    android::Condition m_transferQueueItemCond;
 };
 
 } // namespace WebCore