Fix a memory leak with LayerAndroid

(we had a cycle between LayerAndroid and PaintedSurface)

bug:5366807
Change-Id: I1b7341984b3cc6fd998b9ab54205bb963eb9223d

7 files changed, 78 insertions, 24 deletions

diff --git a/Source/WebCore/platform/graphics/android/GLWebViewState.cpp b/Source/WebCore/platform/graphics/android/GLWebViewState.cpp
index cd93f36..2f3613b 100644
--- a/Source/WebCore/platform/graphics/android/GLWebViewState.cpp
+++ b/Source/WebCore/platform/graphics/android/GLWebViewState.cpp
@@ -116,6 +116,10 @@ GLWebViewState::GLWebViewState(android::Mutex* buttonMutex)
 
 GLWebViewState::~GLWebViewState()
 {
+    // Unref the existing tree/PaintedSurfaces
+    if (m_previouslyUsedRoot)
+        TilesManager::instance()->swapLayersTextures(m_previouslyUsedRoot, 0);
+
     // Take care of the transfer queue such that Tex Gen thread will not stuck
     TilesManager::instance()->unregisterGLWebViewState(this);
 
diff --git a/Source/WebCore/platform/graphics/android/LayerAndroid.cpp b/Source/WebCore/platform/graphics/android/LayerAndroid.cpp
index 81f0ce7..b8db80b 100644
--- a/Source/WebCore/platform/graphics/android/LayerAndroid.cpp
+++ b/Source/WebCore/platform/graphics/android/LayerAndroid.cpp
@@ -170,12 +170,8 @@ LayerAndroid::LayerAndroid(SkPicture* picture) : Layer(),
 
 LayerAndroid::~LayerAndroid()
 {
-    if (m_texture)
-        m_texture->removeLayer(this);
-    SkSafeUnref(m_texture);
     if (m_imageTexture)
         TilesManager::instance()->removeImage(m_imageTexture->imageRef());
-    removeChildren();
     delete m_extra;
     SkSafeUnref(m_recordingPicture);
     m_animations.clear();
@@ -666,6 +662,12 @@ bool LayerAndroid::needsTexture()
         && m_recordingPicture->width() && m_recordingPicture->height());
 }
 
+void LayerAndroid::removeTexture(PaintedSurface* texture)
+{
+    if (texture == m_texture)
+        m_texture = 0;
+}
+
 IntRect LayerAndroid::clippedRect() const
 {
     IntRect r(0, 0, getWidth(), getHeight());
@@ -739,7 +741,7 @@ void LayerAndroid::assignTextureTo(LayerAndroid* newTree)
         if (newLayer && m_texture) {
             m_texture->replaceLayer(newLayer);
             newLayer->m_texture = m_texture;
-            SkSafeRef(newLayer->m_texture);
+            m_texture = 0;
         }
         if (!newLayer && m_texture) {
             m_texture->removeLayer(this);
@@ -801,6 +803,8 @@ void LayerAndroid::clearDirtyRegion()
 
 void LayerAndroid::prepare(GLWebViewState* glWebViewState)
 {
+    m_state = glWebViewState;
+
     int count = this->countChildren();
     if (count > 0) {
         Vector <LayerAndroid*> sublayers;
diff --git a/Source/WebCore/platform/graphics/android/LayerAndroid.h b/Source/WebCore/platform/graphics/android/LayerAndroid.h
index 6c17bba..d6bb497 100644
--- a/Source/WebCore/platform/graphics/android/LayerAndroid.h
+++ b/Source/WebCore/platform/graphics/android/LayerAndroid.h
@@ -106,7 +106,7 @@ public:
     virtual ~LayerAndroid();
 
     virtual TiledPage* page() { return 0; }
-    virtual GLWebViewState* state() { return 0; }
+    virtual GLWebViewState* state() { return m_state; }
 
     void setBackfaceVisibility(bool value) { m_backfaceVisibility = value; }
     void setTransform(const TransformationMatrix& matrix) { m_transform = matrix; }
@@ -120,6 +120,7 @@ public:
     bool outsideViewport();
 
     virtual bool needsTexture();
+    void removeTexture(PaintedSurface*);
 
     // Debug helper methods
     int nbLayers();
@@ -377,6 +378,8 @@ private:
 
     RenderLayer* m_owningLayer;
 
+    GLWebViewState* m_state;
+
     typedef Layer INHERITED;
 };
 
diff --git a/Source/WebCore/platform/graphics/android/PaintedSurface.cpp b/Source/WebCore/platform/graphics/android/PaintedSurface.cpp
index 306eeb5..8a4c072 100644
--- a/Source/WebCore/platform/graphics/android/PaintedSurface.cpp
+++ b/Source/WebCore/platform/graphics/android/PaintedSurface.cpp
@@ -49,12 +49,32 @@
 
 namespace WebCore {
 
+PaintedSurface::~PaintedSurface()
+{
+    XLOG("dtor of %x m_layer: %x", this, m_layer);
+    android::Mutex::Autolock lock(m_layerLock);
+    SkSafeUnref(m_layer);
+#ifdef DEBUG_COUNT
+    ClassTracker::instance()->decrement("PaintedSurface");
+#endif
+    delete m_tiledTexture;
+}
+
 bool PaintedSurface::busy()
 {
     android::Mutex::Autolock lock(m_layerLock);
     return m_busy;
 }
 
+void PaintedSurface::removeLayer()
+{
+    android::Mutex::Autolock lock(m_layerLock);
+    if (m_layer)
+        m_layer->removeTexture(this);
+    SkSafeUnref(m_layer);
+    m_layer = 0;
+}
+
 void PaintedSurface::removeLayer(LayerAndroid* layer)
 {
     android::Mutex::Autolock lock(m_layerLock);
diff --git a/Source/WebCore/platform/graphics/android/PaintedSurface.h b/Source/WebCore/platform/graphics/android/PaintedSurface.h
index ffb609e..88a9303 100644
--- a/Source/WebCore/platform/graphics/android/PaintedSurface.h
+++ b/Source/WebCore/platform/graphics/android/PaintedSurface.h
@@ -58,14 +58,7 @@ public:
 #endif
         m_tiledTexture = new TiledTexture(this);
     }
-    virtual ~PaintedSurface()
-    {
-        SkSafeUnref(m_layer);
-#ifdef DEBUG_COUNT
-        ClassTracker::instance()->decrement("PaintedSurface");
-#endif
-        delete m_tiledTexture;
-    };
+    virtual ~PaintedSurface();
 
     // PaintedSurface methods
 
@@ -74,6 +67,7 @@ public:
     bool draw();
     void markAsDirty(const SkRegion& dirtyArea);
     bool paint(SkCanvas*);
+    void removeLayer();
     void removeLayer(LayerAndroid* layer);
     void replaceLayer(LayerAndroid* layer);
     bool busy();
diff --git a/Source/WebCore/platform/graphics/android/TilesManager.cpp b/Source/WebCore/platform/graphics/android/TilesManager.cpp
index 8724fbc..c7e6c5a 100644
--- a/Source/WebCore/platform/graphics/android/TilesManager.cpp
+++ b/Source/WebCore/platform/graphics/android/TilesManager.cpp
@@ -215,16 +215,11 @@ void TilesManager::swapLayersTextures(LayerAndroid* oldTree, LayerAndroid* newTr
     if (newTree)
         newTree->createTexture();
 
-    WTF::Vector<PaintedSurface*> collect;
-    for (unsigned int i = 0; i < m_paintedSurfaces.size(); i++) {
-        PaintedSurface* surface = m_paintedSurfaces[i];
-        if (!surface->layer())
-            collect.append(surface);
-    }
-    for (unsigned int i = 0; i < collect.size(); i++) {
-        m_paintedSurfaces.remove(m_paintedSurfaces.find(collect[i]));
-        SkSafeUnref(collect[i]);
-    }
+    GLWebViewState* oldState = 0;
+    if (oldTree && !newTree)
+        oldState = oldTree->state();
+
+    paintedSurfacesCleanup(oldState);
 }
 
 void TilesManager::addPaintedSurface(PaintedSurface* surface)
@@ -383,6 +378,39 @@ float TilesManager::layerTileHeight()
     return LAYER_TILE_HEIGHT;
 }
 
+void TilesManager::paintedSurfacesCleanup(GLWebViewState* state)
+{
+    // PaintedSurfaces are created by LayerAndroid with a refcount of 1,
+    // and just transferred to new (corresponding) layers when a new layer tree
+    // is received.
+    // PaintedSurface also keep a reference on the Layer it currently has, so
+    // when we unref the tree of layer, those layers with a PaintedSurface will
+    // still be around if we do nothing.
+    // Here, if the surface does not have any associated layer, it means that we
+    // received a new layer tree without a corresponding layer (i.e. a layer
+    // using a texture has been removed by webkit).
+    // In that case, we remove the PaintedSurface from our list, and unref it.
+    // If the surface does have a layer, but the GLWebViewState associated to
+    // that layer is different from the one passed in parameter, it means we can
+    // also remove the surface (and we also remove/unref any layer that surface
+    // has). We do this when we deallocate GLWebViewState (i.e. the webview has
+    // been destroyed) and also when we switch to a page without
+    // composited layers.
+
+    WTF::Vector<PaintedSurface*> collect;
+    for (unsigned int i = 0; i < m_paintedSurfaces.size(); i++) {
+        PaintedSurface* surface = m_paintedSurfaces[i];
+        if (!surface->layer() || (state && surface->layer()->state() == state))
+            collect.append(surface);
+    }
+    for (unsigned int i = 0; i < collect.size(); i++) {
+        PaintedSurface* surface = collect[i];
+        m_paintedSurfaces.remove(m_paintedSurfaces.find(surface));
+        surface->removeLayer();
+        SkSafeUnref(surface);
+    }
+}
+
 void TilesManager::unregisterGLWebViewState(GLWebViewState* state)
 {
     // Discard the whole queue b/c we lost GL context already.
diff --git a/Source/WebCore/platform/graphics/android/TilesManager.h b/Source/WebCore/platform/graphics/android/TilesManager.h
index c582065..071aa88 100644
--- a/Source/WebCore/platform/graphics/android/TilesManager.h
+++ b/Source/WebCore/platform/graphics/android/TilesManager.h
@@ -108,6 +108,7 @@ public:
     static float tileHeight();
     static float layerTileWidth();
     static float layerTileHeight();
+    void paintedSurfacesCleanup(GLWebViewState* state);
     void unregisterGLWebViewState(GLWebViewState* state);
 
     void allocateTiles();