remove unneeded SkShader::validContext

src/core/SkPictureShader.cpp

Index: src/core/SkPictureShader.cpp
diff --git a/src/core/SkPictureShader.cpp b/src/core/SkPictureShader.cpp
index 81f9242854576442f221cce7814261b10bb41817..b8f377b783b7bef19838992942fcc3ddc3bbbe2a 100644
--- a/src/core/SkPictureShader.cpp
+++ b/src/core/SkPictureShader.cpp
@@ -110,58 +110,51 @@ SkShader* SkPictureShader::refBitmapShader(const SkMatrix& matrix) const {
     return fCachedBitmapShader;
 }
 
-SkShader* SkPictureShader::validInternal(const ContextRec& rec, SkMatrix* totalInverse) const {
-    if (!this->INHERITED::validContext(rec, totalInverse)) {
-        return NULL;
-    }
+size_t SkPictureShader::contextSize() const {
+    return sizeof(PictureShaderContext);
+}
 
+SkShader::Context* SkPictureShader::onCreateContext(const ContextRec& rec, void* storage) const {
     SkAutoTUnref<SkShader> bitmapShader(this->refBitmapShader(*rec.fMatrix));
-    if (!bitmapShader || !bitmapShader->validContext(rec)) {
+    if (NULL == bitmapShader.get()) {
         return NULL;
     }
 
-    return bitmapShader.detach();
-}
-
-bool SkPictureShader::validContext(const ContextRec& rec, SkMatrix* totalInverse) const {
-    SkAutoTUnref<SkShader> shader(this->validInternal(rec, totalInverse));
-    return shader != NULL;
-}
-
-SkShader::Context* SkPictureShader::createContext(const ContextRec& rec, void* storage) const {
-    SkAutoTUnref<SkShader> bitmapShader(this->validInternal(rec, NULL));
-    if (!bitmapShader) {
-        return NULL;
+    PictureShaderContext* ctx = SkNEW_PLACEMENT_ARGS(storage, PictureShaderContext,
+                                                     (*this, rec, bitmapShader.detach()));
+    if (!ctx->isValid()) {

[scroggo, 2014/05/05 14:01:19]

Don't we have a general rule against creating an object that might be invalid
and then querying it?

This could be simplified by creating the BitmapShaderContext outside of
PictureShaderContext and passing it in.

[reed1, 2014/05/05 15:10:01]

Done.

+        ctx->~PictureShaderContext();
+        ctx = NULL;
     }
-
-    return SkNEW_PLACEMENT_ARGS(storage, PictureShaderContext, (*this, rec, bitmapShader.detach()));
+    return ctx;
 }
 
-size_t SkPictureShader::contextSize() const {
-    return sizeof(PictureShaderContext);
-}
+/////////////////////////////////////////////////////////////////////////////////////////
 
 SkPictureShader::PictureShaderContext::PictureShaderContext(
         const SkPictureShader& shader, const ContextRec& rec, SkShader* bitmapShader)
     : INHERITED(shader, rec)
-    , fBitmapShader(bitmapShader)
+    , fBitmapShader(SkRef(bitmapShader))
 {
-    SkASSERT(fBitmapShader);
-    fBitmapShaderContextStorage = sk_malloc_throw(fBitmapShader->contextSize());
-    fBitmapShaderContext = fBitmapShader->createContext(rec, fBitmapShaderContextStorage);
-    SkASSERT(fBitmapShaderContext);
+    fBitmapShaderContextStorage = sk_malloc_throw(bitmapShader->contextSize());
+    fBitmapShaderContext = bitmapShader->createContext(rec, fBitmapShaderContextStorage);
+    //if fBitmapShaderContext is null, we are invalid
 }
 
 SkPictureShader::PictureShaderContext::~PictureShaderContext() {
-    fBitmapShaderContext->~Context();
+    if (fBitmapShaderContext) {
+        fBitmapShaderContext->~Context();
+    }
     sk_free(fBitmapShaderContextStorage);
 }
 
 uint32_t SkPictureShader::PictureShaderContext::getFlags() const {
+    SkASSERT(fBitmapShaderContext);
     return fBitmapShaderContext->getFlags();
 }
 
 SkShader::Context::ShadeProc SkPictureShader::PictureShaderContext::asAShadeProc(void** ctx) {
+    SkASSERT(fBitmapShaderContext);
     return fBitmapShaderContext->asAShadeProc(ctx);
 }