diff options
author | Nicolas Roard <nicolasroard@google.com> | 2011-09-28 19:08:00 -0700 |
---|---|---|
committer | Nicolas Roard <nicolasroard@google.com> | 2011-09-29 13:13:17 -0700 |
commit | 74523e723bc5677780989d6a5834adc5c72c3683 (patch) | |
tree | f88f6888f48a88348ca4e49c47eda904c944d581 /Source/WebCore/platform/graphics/android | |
parent | 852641461c4dedc4682153d1299b2a9b6fde6ea2 (diff) | |
download | external_webkit-74523e723bc5677780989d6a5834adc5c72c3683.zip external_webkit-74523e723bc5677780989d6a5834adc5c72c3683.tar.gz external_webkit-74523e723bc5677780989d6a5834adc5c72c3683.tar.bz2 |
Fix a memory leak with LayerAndroid
(we had a cycle between LayerAndroid and PaintedSurface)
bug:5366807
Change-Id: I1b7341984b3cc6fd998b9ab54205bb963eb9223d
Diffstat (limited to 'Source/WebCore/platform/graphics/android')
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(); |