Fix random crashes when HW acceleration is turned on.

Bug:3107362

Change-Id: I354a07369056e696deed7458a4f4e14d54b7f6c8

9 files changed, 44 insertions, 31 deletions

diff --git a/WebCore/platform/graphics/android/BaseLayerAndroid.cpp b/WebCore/platform/graphics/android/BaseLayerAndroid.cpp
index d5119b1..023740e 100644
--- a/WebCore/platform/graphics/android/BaseLayerAndroid.cpp
+++ b/WebCore/platform/graphics/android/BaseLayerAndroid.cpp
@@ -83,16 +83,23 @@ BaseLayerAndroid::~BaseLayerAndroid()
 void BaseLayerAndroid::setContent(const PictureSet& src)
 {
 #if USE(ACCELERATED_COMPOSITING)
-    if (m_glWebViewState) {
-        m_glWebViewState->baseLayerLock();
-        m_content.set(src);
-        m_glWebViewState->baseLayerUnlock();
-    } else {
-        m_content.set(src);
-    }
-#else
+    // FIXME: We lock here because we do not want
+    // to paint and change the m_content concurrently.
+    // We should instead refactor PictureSet to use
+    // an atomic refcounting scheme and use atomic operations
+    // to swap PictureSets.
+    android::Mutex::Autolock lock(m_drawLock);
+#endif
     m_content.set(src);
+}
+
+void BaseLayerAndroid::draw(SkCanvas* canvas)
+{
+#if USE(ACCELERATED_COMPOSITING)
+    android::Mutex::Autolock lock(m_drawLock);
 #endif
+    if (!m_content.isEmpty())
+        m_content.draw(canvas);
 }
 
 #if USE(ACCELERATED_COMPOSITING)
diff --git a/WebCore/platform/graphics/android/BaseLayerAndroid.h b/WebCore/platform/graphics/android/BaseLayerAndroid.h
index 408549a..0a7fe29 100644
--- a/WebCore/platform/graphics/android/BaseLayerAndroid.h
+++ b/WebCore/platform/graphics/android/BaseLayerAndroid.h
@@ -47,6 +47,11 @@ public:
 #endif
     void setContent(const android::PictureSet& src);
     android::PictureSet* content() { return &m_content; }
+    // This method will paint using the current PictureSet onto
+    // the passed canvas. We used it to paint the GL tiles as well as
+    // WebView::copyBaseContentToPicture(), so a lock is necessary as
+    // we are running in different threads.
+    void draw(SkCanvas* canvas);
 
     bool drawGL(IntRect& rect, SkRect& viewport,
                 float scale, SkColor color = SK_ColorWHITE);
@@ -56,6 +61,7 @@ private:
     bool drawBasePictureInGL(SkRect& viewport, float scale);
 
     GLWebViewState* m_glWebViewState;
+    android::Mutex m_drawLock;
 #endif
     android::PictureSet m_content;
     SkRect m_previousVisible;
diff --git a/WebCore/platform/graphics/android/BaseTile.cpp b/WebCore/platform/graphics/android/BaseTile.cpp
index 47a154c..d64db78 100644
--- a/WebCore/platform/graphics/android/BaseTile.cpp
+++ b/WebCore/platform/graphics/android/BaseTile.cpp
@@ -140,7 +140,7 @@ bool BaseTile::isBitmapReady()
 }
 
 // Called from the texture generation thread
-bool BaseTile::paintBitmap()
+void BaseTile::paintBitmap()
 {
     XLOG("paintBitmap(%x) %d, %d with page %x", this, m_x, m_y, m_page);
     // the mutex ensures you are reading the most current value
@@ -153,7 +153,7 @@ bool BaseTile::paintBitmap()
     m_varLock.unlock();
 
     if (!texture)
-        return false;
+        return;
 
     TextureInfo* textureInfo = texture->producerLock();
 
@@ -162,13 +162,13 @@ bool BaseTile::paintBitmap()
     // under us)
     if (texture->owner() != this || texture->usedLevel() > 1) {
         texture->producerRelease();
-        return false;
+        return;
     }
 
     PaintingInfo info(x, y, tiledPage->glWebViewState());
     if (texture->consumerTextureUpToDate(info)) {
         texture->producerRelease();
-        return true;
+        return;
     }
 
     float tileWidth = textureInfo->m_width;
@@ -185,7 +185,7 @@ bool BaseTile::paintBitmap()
     canvas->scale(scale, scale);
     canvas->translate(-x * w, -y * h);
 
-    bool didPaint = tiledPage->paintBaseLayerContent(canvas);
+    tiledPage->paintBaseLayerContent(canvas);
 
     canvas->restore();
 
@@ -202,8 +202,6 @@ bool BaseTile::paintBitmap()
 #endif
 
     texture->producerUpdate(this, textureInfo, info);
-
-    return didPaint;
 }
 
 } // namespace WebCore
diff --git a/WebCore/platform/graphics/android/BaseTile.h b/WebCore/platform/graphics/android/BaseTile.h
index 31c306e..b7e63d6 100644
--- a/WebCore/platform/graphics/android/BaseTile.h
+++ b/WebCore/platform/graphics/android/BaseTile.h
@@ -73,7 +73,7 @@ public:
     void draw(float transparency, SkRect& rect);
 
     // the only thread-safe function called by the background thread
-    bool paintBitmap();
+    void paintBitmap();
 
     float scale() const { return m_scale; }
     void setScale(float scale) { m_scale = scale; }
diff --git a/WebCore/platform/graphics/android/GLWebViewState.cpp b/WebCore/platform/graphics/android/GLWebViewState.cpp
index bbaed78..1a9b509 100644
--- a/WebCore/platform/graphics/android/GLWebViewState.cpp
+++ b/WebCore/platform/graphics/android/GLWebViewState.cpp
@@ -132,17 +132,14 @@ void GLWebViewState::resetExtra(bool repaint)
     m_navLayer = 0;
 }
 
-bool GLWebViewState::paintBaseLayerContent(SkCanvas* canvas)
+void GLWebViewState::paintBaseLayerContent(SkCanvas* canvas)
 {
     android::Mutex::Autolock lock(m_baseLayerLock);
-    if (m_baseLayer && m_baseLayer->content()
-        && !m_baseLayer->content()->isEmpty()) {
-        m_baseLayer->content()->draw(canvas);
+    if (m_baseLayer) {
+        m_baseLayer->draw(canvas);
         if (m_extra && m_navLayer)
             m_extra->draw(canvas, m_navLayer);
-        return true;
     }
-    return false;
 }
 
 void GLWebViewState::scheduleUpdate(const double& currentTime, float scale)
diff --git a/WebCore/platform/graphics/android/GLWebViewState.h b/WebCore/platform/graphics/android/GLWebViewState.h
index 1fc62df..95bc07d 100644
--- a/WebCore/platform/graphics/android/GLWebViewState.h
+++ b/WebCore/platform/graphics/android/GLWebViewState.h
@@ -155,7 +155,7 @@ public:
     int originalTilesPosY() const { return m_originalTilesPosY; }
     void setOriginalTilesPosY(int pos) { m_originalTilesPosY = pos; }
 
-    bool paintBaseLayerContent(SkCanvas* canvas);
+    void paintBaseLayerContent(SkCanvas* canvas);
     void setBaseLayer(BaseLayerAndroid* layer, IntRect& rect);
     void setExtra(android::DrawExtra* extra, LayerAndroid* navLayer);
     void resetExtra(bool repaint);
@@ -178,9 +178,6 @@ public:
     unsigned int currentPictureCounter() const { return m_currentPictureCounter; }
     SkRect& invalidatedRect() { return m_invalidatedRect; }
 
-    void baseLayerLock() { m_baseLayerLock.lock(); }
-    void baseLayerUnlock() { m_baseLayerLock.unlock(); }
-
 private:
 
     // Delay between scheduling a new page when the scale
diff --git a/WebCore/platform/graphics/android/TiledPage.cpp b/WebCore/platform/graphics/android/TiledPage.cpp
index fc33377..6430b02 100644
--- a/WebCore/platform/graphics/android/TiledPage.cpp
+++ b/WebCore/platform/graphics/android/TiledPage.cpp
@@ -260,11 +260,10 @@ void TiledPage::draw(float transparency, SkRect& viewport, int firstTileX, int f
 #endif // DEBUG
 }
 
-bool TiledPage::paintBaseLayerContent(SkCanvas* canvas)
+void TiledPage::paintBaseLayerContent(SkCanvas* canvas)
 {
     if (m_glWebViewState)
-        return m_glWebViewState->paintBaseLayerContent(canvas);
-    return false;
+        m_glWebViewState->paintBaseLayerContent(canvas);
 }
 
 TiledPage* TiledPage::sibling()
diff --git a/WebCore/platform/graphics/android/TiledPage.h b/WebCore/platform/graphics/android/TiledPage.h
index 2daf226..8be2361 100644
--- a/WebCore/platform/graphics/android/TiledPage.h
+++ b/WebCore/platform/graphics/android/TiledPage.h
@@ -68,7 +68,7 @@ public:
     void draw(float transparency, SkRect& viewport, int firstTileX, int firstTileY);
 
     // used by individual tiles to generate the bitmap for their tile
-    bool paintBaseLayerContent(SkCanvas*);
+    void paintBaseLayerContent(SkCanvas*);
     // used by individual tiles to get the information about the current picture
     GLWebViewState* glWebViewState() { return m_glWebViewState; }
 
diff --git a/WebKit/android/nav/WebView.cpp b/WebKit/android/nav/WebView.cpp
index cd5b4ae..0de13a8 100644
--- a/WebKit/android/nav/WebView.cpp
+++ b/WebKit/android/nav/WebView.cpp
@@ -188,6 +188,15 @@ WebView(JNIEnv* env, jobject javaWebView, int viewImpl) :
         env->DeleteWeakGlobalRef(m_javaGlue.m_obj);
         m_javaGlue.m_obj = 0;
     }
+#if USE(ACCELERATED_COMPOSITING)
+    // We remove the base layer from glWebViewState
+    // as we are about to destroy it, while the
+    // glWebViewState destructor will be called just after.
+    // If we do not remove it here, we risk having BaseTiles
+    // trying to paint using a deallocated base layer.
+    IntRect rect;
+    m_glWebViewState.setBaseLayer(0, rect);
+#endif
     delete m_frameCacheUI;
     delete m_navPictureUI;
     delete m_baseLayer;
@@ -1306,7 +1315,7 @@ void copyBaseContentToPicture(SkPicture* picture)
     if (!m_baseLayer)
         return;
     PictureSet* content = m_baseLayer->content();
-    content->draw(picture->beginRecording(content->width(), content->height(),
+    m_baseLayer->draw(picture->beginRecording(content->width(), content->height(),
             SkPicture::kUsePathBoundsForClip_RecordingFlag));
     picture->endRecording();
 }