Revert of Extract most of the mutable state of SkShader into a separate Context object. (https://codereview.chromium.org/207683004/)

Reason for revert:
Causing memory leaks in Chromium.

Original issue's description:
> Extract most of the mutable state of SkShader into a separate Context object.
>
> SkShader currently stores some state during draw calls via setContext(...).
> Move that mutable state into a separate SkShader::Context class that is
> constructed on demand for the duration of the draw.
>
> Calls to setContext() are replaced with createContext() which returns a context
> corresponding to the shader object or NULL if the parameters to createContext
> are invalid.
>
> TEST=out/Debug/dm
> BUG=skia:1976
>
> Committed: http://code.google.com/p/skia/source/detail?r=14216

R=scroggo@google.com, skyostil@chromium.org, tomhudson@chromium.org, senorblanco@chromium.org, reed@google.com, dominikg@chromium.org
TBR=dominikg@chromium.org, reed@google.com, scroggo@google.com, senorblanco@chromium.org, skyostil@chromium.org, tomhudson@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=skia:1976

Author: bungeman@google.com

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

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

1 files changed, 46 insertions, 79 deletions

diff --git a/src/core/SkPictureShader.cpp b/src/core/SkPictureShader.cpp
index 1e9507190c..15df3a37a5 100644
--- a/src/core/SkPictureShader.cpp
+++ b/src/core/SkPictureShader.cpp
@@ -55,9 +55,9 @@ void SkPictureShader::flatten(SkWriteBuffer& buffer) const {
     }
 }
 
-SkShader* SkPictureShader::buildBitmapShader(const SkMatrix& matrix) const {
+bool SkPictureShader::buildBitmapShader(const SkMatrix& matrix) const {
     if (!fPicture || (0 == fPicture->width() && 0 == fPicture->height())) {
-        return NULL;
+        return false;
     }
 
     SkMatrix m;
@@ -78,20 +78,17 @@ SkShader* SkPictureShader::buildBitmapShader(const SkMatrix& matrix) const {
 
     SkISize tileSize = scaledSize.toRound();
     if (tileSize.isEmpty()) {
-        return NULL;
+        return false;
     }
 
     // The actual scale, compensating for rounding.
     SkSize tileScale = SkSize::Make(SkIntToScalar(tileSize.width()) / fPicture->width(),
                                     SkIntToScalar(tileSize.height()) / fPicture->height());
 
-    SkAutoMutexAcquire ama(fCachedBitmapShaderMutex);
-
-    if (!fCachedBitmapShader || tileScale != fCachedTileScale ||
-        this->getLocalMatrix() != fCachedLocalMatrix) {
+    if (!fCachedShader || tileScale != fCachedTileScale) {
         SkBitmap bm;
         if (!bm.allocN32Pixels(tileSize.width(), tileSize.height())) {
-            return NULL;
+            return false;
         }
         bm.eraseColor(SK_ColorTRANSPARENT);
 
@@ -99,96 +96,66 @@ SkShader* SkPictureShader::buildBitmapShader(const SkMatrix& matrix) const {
         canvas.scale(tileScale.width(), tileScale.height());
         canvas.drawPicture(*fPicture);
 
-        fCachedBitmapShader.reset(CreateBitmapShader(bm, fTmx, fTmy));
+        fCachedShader.reset(CreateBitmapShader(bm, fTmx, fTmy));
         fCachedTileScale = tileScale;
-        fCachedLocalMatrix = this->getLocalMatrix();
-
-        SkMatrix shaderMatrix = this->getLocalMatrix();
-        shaderMatrix.preScale(1 / tileScale.width(), 1 / tileScale.height());
-        fCachedBitmapShader->setLocalMatrix(shaderMatrix);
     }
 
-    // Increment the ref counter inside the mutex to ensure the returned pointer is still valid.
-    // Otherwise, the pointer may have been overwritten on a different thread before the object's
-    // ref count was incremented.
-    fCachedBitmapShader.get()->ref();
-    return fCachedBitmapShader;
-}
+    SkMatrix shaderMatrix = this->getLocalMatrix();
+    shaderMatrix.preScale(1 / tileScale.width(), 1 / tileScale.height());
+    fCachedShader->setLocalMatrix(shaderMatrix);
 
-SkShader* SkPictureShader::validInternal(const SkBitmap& device, const SkPaint& paint,
-                                         const SkMatrix& matrix, SkMatrix* totalInverse) const {
-    if (!this->INHERITED::validContext(device, paint, matrix, totalInverse)) {
-        return NULL;
-    }
+    return true;
+}
 
-    SkShader* bitmapShader = this->buildBitmapShader(matrix);
-    if (!bitmapShader) {
-        return NULL;
+bool SkPictureShader::setContext(const SkBitmap& device,
+                                 const SkPaint& paint,
+                                 const SkMatrix& matrix) {
+    if (!this->buildBitmapShader(matrix)) {
+        return false;
     }
 
-    if (!bitmapShader->validContext(device, paint, matrix)) {
-        bitmapShader->unref();
-        return NULL;
+    if (!this->INHERITED::setContext(device, paint, matrix)) {
+        return false;
     }
 
-    return bitmapShader;
-}
-
-bool SkPictureShader::validContext(const SkBitmap& device, const SkPaint& paint,
-                                   const SkMatrix& matrix, SkMatrix* totalInverse) const {
-    SkAutoTUnref<SkShader> shader(this->validInternal(device, paint, matrix, totalInverse));
-    return shader != NULL;
-}
-
-SkShader::Context* SkPictureShader::createContext(const SkBitmap& device, const SkPaint& paint,
-                                                  const SkMatrix& matrix, void* storage) const {
-    SkShader* bitmapShader = this->validInternal(device, paint, matrix, NULL);
-    if (!bitmapShader) {
-        return NULL;
+    SkASSERT(fCachedShader);
+    if (!fCachedShader->setContext(device, paint, matrix)) {
+        this->INHERITED::endContext();
+        return false;
     }
 
-    return SkNEW_PLACEMENT_ARGS(storage, PictureShaderContext,
-                                (*this, device, paint, matrix, bitmapShader));
+    return true;
 }
 
-size_t SkPictureShader::contextSize() const {
-    return sizeof(PictureShaderContext);
-}
-
-SkPictureShader::PictureShaderContext::PictureShaderContext(
-        const SkPictureShader& shader, const SkBitmap& device,
-        const SkPaint& paint, const SkMatrix& matrix, SkShader* bitmapShader)
-    : INHERITED(shader, device, paint, matrix)
-    , fBitmapShader(bitmapShader)
-{
-    SkASSERT(fBitmapShader);
-    fBitmapShaderContextStorage = sk_malloc_throw(fBitmapShader->contextSize());
-    fBitmapShaderContext = fBitmapShader->createContext(
-            device, paint, matrix, fBitmapShaderContextStorage);
-    SkASSERT(fBitmapShaderContext);
-}
+void SkPictureShader::endContext() {
+    SkASSERT(fCachedShader);
+    fCachedShader->endContext();
 
-SkPictureShader::PictureShaderContext::~PictureShaderContext() {
-    fBitmapShaderContext->SkShader::Context::~Context();
-    sk_free(fBitmapShaderContextStorage);
+    this->INHERITED::endContext();
 }
 
-uint32_t SkPictureShader::PictureShaderContext::getFlags() const {
-    return fBitmapShaderContext->getFlags();
+uint32_t SkPictureShader::getFlags() {
+    if (NULL != fCachedShader) {
+        return fCachedShader->getFlags();
+    }
+    return 0;
 }
 
-SkShader::Context::ShadeProc SkPictureShader::PictureShaderContext::asAShadeProc(void** ctx) {
-    return fBitmapShaderContext->asAShadeProc(ctx);
+SkShader::ShadeProc SkPictureShader::asAShadeProc(void** ctx) {
+    if (fCachedShader) {
+        return fCachedShader->asAShadeProc(ctx);
+    }
+    return NULL;
 }
 
-void SkPictureShader::PictureShaderContext::shadeSpan(int x, int y, SkPMColor dstC[], int count) {
-    SkASSERT(fBitmapShaderContext);
-    fBitmapShaderContext->shadeSpan(x, y, dstC, count);
+void SkPictureShader::shadeSpan(int x, int y, SkPMColor dstC[], int count) {
+    SkASSERT(fCachedShader);
+    fCachedShader->shadeSpan(x, y, dstC, count);
 }
 
-void SkPictureShader::PictureShaderContext::shadeSpan16(int x, int y, uint16_t dstC[], int count) {
-    SkASSERT(fBitmapShaderContext);
-    fBitmapShaderContext->shadeSpan16(x, y, dstC, count);
+void SkPictureShader::shadeSpan16(int x, int y, uint16_t dstC[], int count) {
+    SkASSERT(fCachedShader);
+    fCachedShader->shadeSpan16(x, y, dstC, count);
 }
 
 #ifndef SK_IGNORE_TO_STRING
@@ -209,10 +176,10 @@ 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()));
-    if (!bitmapShader) {
+    if (!this->buildBitmapShader(context->getMatrix())) {
         return NULL;
     }
-    return bitmapShader->asNewEffect(context, paint);
+    SkASSERT(fCachedShader);
+    return fCachedShader->asNewEffect(context, paint);
 }
 #endif