From 68ea91a49311562347311448450199a957590c1c Mon Sep 17 00:00:00 2001 From: "commit-bot@chromium.org" Date: Thu, 17 Apr 2014 21:02:29 +0000 Subject: [PATCH] Revert of Fix memory leak in SkGradientShader. (https://codereview.chromium.org/240303003/) Reason for revert: This and https://codereview.chromium.org/207683004 are causing memory leaks. Original issue's description: > Fix memory leak in SkGradientShader. > > Make sure pointer to gradient shader cache is unreffed in SkGradientShaderBase::getGradientTableBitmap. > Rename methods returning a "pre-reffed" object to indicate obligations. > > BUG=skia:1976 > > Committed: http://code.google.com/p/skia/source/detail?r=14223 R=reed@google.com, scroggo@google.com, dominikg@chromium.org TBR=dominikg@chromium.org, reed@google.com, scroggo@google.com NOTREECHECKS=true NOTRY=true BUG=skia:1976 Author: bungeman@google.com Review URL: https://codereview.chromium.org/241603002 git-svn-id: http://skia.googlecode.com/svn/trunk@14245 2bbb7eff-a529-9590-31e7-b0007b416f81 --- src/core/SkPictureShader.cpp | 19 ++++++++++++------- src/core/SkPictureShader.h | 2 +- src/effects/gradients/SkGradientShader.cpp | 6 +++--- src/effects/gradients/SkGradientShaderPriv.h | 2 +- 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/core/SkPictureShader.cpp b/src/core/SkPictureShader.cpp index 7110a5ba2f..1e9507190c 100644 --- a/src/core/SkPictureShader.cpp +++ b/src/core/SkPictureShader.cpp @@ -55,7 +55,7 @@ void SkPictureShader::flatten(SkWriteBuffer& buffer) const { } } -SkShader* SkPictureShader::refBitmapShader(const SkMatrix& matrix) const { +SkShader* SkPictureShader::buildBitmapShader(const SkMatrix& matrix) const { if (!fPicture || (0 == fPicture->width() && 0 == fPicture->height())) { return NULL; } @@ -121,12 +121,17 @@ SkShader* SkPictureShader::validInternal(const SkBitmap& device, const SkPaint& return NULL; } - SkAutoTUnref bitmapShader(this->refBitmapShader(matrix)); - if (!bitmapShader || !bitmapShader->validContext(device, paint, matrix)) { + SkShader* bitmapShader = this->buildBitmapShader(matrix); + if (!bitmapShader) { return NULL; } - return bitmapShader.detach(); + if (!bitmapShader->validContext(device, paint, matrix)) { + bitmapShader->unref(); + return NULL; + } + + return bitmapShader; } bool SkPictureShader::validContext(const SkBitmap& device, const SkPaint& paint, @@ -137,13 +142,13 @@ bool SkPictureShader::validContext(const SkBitmap& device, const SkPaint& paint, SkShader::Context* SkPictureShader::createContext(const SkBitmap& device, const SkPaint& paint, const SkMatrix& matrix, void* storage) const { - SkAutoTUnref bitmapShader(this->validInternal(device, paint, matrix, NULL)); + SkShader* bitmapShader = this->validInternal(device, paint, matrix, NULL); if (!bitmapShader) { return NULL; } return SkNEW_PLACEMENT_ARGS(storage, PictureShaderContext, - (*this, device, paint, matrix, bitmapShader.detach())); + (*this, device, paint, matrix, bitmapShader)); } size_t SkPictureShader::contextSize() const { @@ -204,7 +209,7 @@ void SkPictureShader::toString(SkString* str) const { #if SK_SUPPORT_GPU GrEffectRef* SkPictureShader::asNewEffect(GrContext* context, const SkPaint& paint) const { - SkAutoTUnref bitmapShader(this->refBitmapShader(context->getMatrix())); + SkAutoTUnref bitmapShader(this->buildBitmapShader(context->getMatrix())); if (!bitmapShader) { return NULL; } diff --git a/src/core/SkPictureShader.h b/src/core/SkPictureShader.h index d1be059182..a0a377d0cd 100644 --- a/src/core/SkPictureShader.h +++ b/src/core/SkPictureShader.h @@ -69,7 +69,7 @@ private: SkShader* validInternal(const SkBitmap& device, const SkPaint& paint, const SkMatrix& matrix, SkMatrix* totalInverse) const; - SkShader* refBitmapShader(const SkMatrix&) const; + SkShader* buildBitmapShader(const SkMatrix&) const; SkPicture* fPicture; TileMode fTmx, fTmy; diff --git a/src/effects/gradients/SkGradientShader.cpp b/src/effects/gradients/SkGradientShader.cpp index 3547fbec94..666204e0ae 100644 --- a/src/effects/gradients/SkGradientShader.cpp +++ b/src/effects/gradients/SkGradientShader.cpp @@ -211,7 +211,7 @@ SkGradientShaderBase::GradientShaderBaseContext::GradientShaderBaseContext( const SkGradientShaderBase& shader, const SkBitmap& device, const SkPaint& paint, const SkMatrix& matrix) : INHERITED(shader, device, paint, matrix) - , fCache(shader.refCache(getPaintAlpha())) + , fCache(shader.getCache(getPaintAlpha())) { const SkMatrix& inverse = this->getTotalInverse(); @@ -558,7 +558,7 @@ void SkGradientShaderBase::GradientShaderCache::initCache32(GradientShaderCache* * The gradient holds a cache for the most recent value of alpha. Successive * callers with the same alpha value will share the same cache. */ -SkGradientShaderBase::GradientShaderCache* SkGradientShaderBase::refCache(U8CPU alpha) const { +SkGradientShaderBase::GradientShaderCache* SkGradientShaderBase::getCache(U8CPU alpha) const { SkAutoMutexAcquire ama(fCacheMutex); if (!fCache || fCache->getAlpha() != alpha) { fCache.reset(SkNEW_ARGS(GradientShaderCache, (alpha, *this))); @@ -581,7 +581,7 @@ SkGradientShaderBase::GradientShaderCache* SkGradientShaderBase::refCache(U8CPU void SkGradientShaderBase::getGradientTableBitmap(SkBitmap* bitmap) const { // our caller assumes no external alpha, so we ensure that our cache is // built with 0xFF - SkAutoTUnref cache(this->refCache(0xFF)); + GradientShaderCache* cache = this->getCache(0xFF); // don't have a way to put the mapper into our cache-key yet if (fMapper) { diff --git a/src/effects/gradients/SkGradientShaderPriv.h b/src/effects/gradients/SkGradientShaderPriv.h index 1d0f008917..e01609462b 100644 --- a/src/effects/gradients/SkGradientShaderPriv.h +++ b/src/effects/gradients/SkGradientShaderPriv.h @@ -215,7 +215,7 @@ private: SkColor* fOrigColors; // original colors, before modulation by paint in context. bool fColorsAreOpaque; - GradientShaderCache* refCache(U8CPU alpha) const; + GradientShaderCache* getCache(U8CPU alpha) const; mutable SkMutex fCacheMutex; mutable SkAutoTUnref fCache;