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
R=reed@google.com, scroggo@google.com

Author: dominikg@chromium.org

Review URL: https://codereview.chromium.org/240303003

git-svn-id: http://skia.googlecode.com/svn/trunk@14223 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/src/core/SkPictureShader.cpp b/src/core/SkPictureShader.cpp
index 1e9507190c60a4ac4e8b16785c9dc06948f13e55..7110a5ba2f0cbe229e89de3e0f8e4354382dc042 100644 (file)
--- a/src/core/SkPictureShader.cpp
+++ b/src/core/SkPictureShader.cpp
@@ -55,7 +55,7 @@ void SkPictureShader::flatten(SkWriteBuffer& buffer) const {
     }
 }
 
-SkShader* SkPictureShader::buildBitmapShader(const SkMatrix& matrix) const {
+SkShader* SkPictureShader::refBitmapShader(const SkMatrix& matrix) const {
     if (!fPicture || (0 == fPicture->width() && 0 == fPicture->height())) {
         return NULL;
     }
@@ -121,17 +121,12 @@ SkShader* SkPictureShader::validInternal(const SkBitmap& device, const SkPaint&
         return NULL;
     }
 
-    SkShader* bitmapShader = this->buildBitmapShader(matrix);
-    if (!bitmapShader) {
-        return NULL;
-    }
-
-    if (!bitmapShader->validContext(device, paint, matrix)) {
-        bitmapShader->unref();
+    SkAutoTUnref<SkShader> bitmapShader(this->refBitmapShader(matrix));
+    if (!bitmapShader || !bitmapShader->validContext(device, paint, matrix)) {
         return NULL;
     }
 
-    return bitmapShader;
+    return bitmapShader.detach();
 }
 
 bool SkPictureShader::validContext(const SkBitmap& device, const SkPaint& paint,
@@ -142,13 +137,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 {
-    SkShader* bitmapShader = this->validInternal(device, paint, matrix, NULL);
+    SkAutoTUnref<SkShader> bitmapShader(this->validInternal(device, paint, matrix, NULL));
     if (!bitmapShader) {
         return NULL;
     }
 
     return SkNEW_PLACEMENT_ARGS(storage, PictureShaderContext,
-                                (*this, device, paint, matrix, bitmapShader));
+                                (*this, device, paint, matrix, bitmapShader.detach()));
 }
 
 size_t SkPictureShader::contextSize() const {
@@ -209,7 +204,7 @@ void SkPictureShader::toString(SkString* str) const {
 
 #if SK_SUPPORT_GPU
 GrEffectRef* SkPictureShader::asNewEffect(GrContext* context, const SkPaint& paint) const {
-    SkAutoTUnref<SkShader> bitmapShader(this->buildBitmapShader(context->getMatrix()));
+    SkAutoTUnref<SkShader> bitmapShader(this->refBitmapShader(context->getMatrix()));
     if (!bitmapShader) {
         return NULL;
     }

diff --git a/src/core/SkPictureShader.h b/src/core/SkPictureShader.h
index a0a377d0cdada654f7f0f863533d780f781365d7..d1be0591824e27e1f28962b665c7f33ba40995af 100644 (file)
--- 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* buildBitmapShader(const SkMatrix&) const;
+    SkShader* refBitmapShader(const SkMatrix&) const;
 
     SkPicture*  fPicture;
     TileMode    fTmx, fTmy;

diff --git a/src/effects/gradients/SkGradientShader.cpp b/src/effects/gradients/SkGradientShader.cpp
index 666204e0aed451ad0f236c4cb72c26a7fa7945be..3547fbec94f4e372c5ee3dc70e7c0bd4a03effdd 100644 (file)
--- 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.getCache(getPaintAlpha()))
+    , fCache(shader.refCache(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::getCache(U8CPU alpha) const {
+SkGradientShaderBase::GradientShaderCache* SkGradientShaderBase::refCache(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::getCache(U8CPU
 void SkGradientShaderBase::getGradientTableBitmap(SkBitmap* bitmap) const {
     // our caller assumes no external alpha, so we ensure that our cache is
     // built with 0xFF
-    GradientShaderCache* cache = this->getCache(0xFF);
+    SkAutoTUnref<GradientShaderCache> cache(this->refCache(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 e01609462b6ea22038f24562a8d982383a03c362..1d0f008917d42d974c393d3fb4c31dbd580d8572 100644 (file)
--- 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* getCache(U8CPU alpha) const;
+    GradientShaderCache* refCache(U8CPU alpha) const;
     mutable SkMutex                           fCacheMutex;
     mutable SkAutoTUnref<GradientShaderCache> fCache;