Avoid separate allocation of SkImageCacherator

Embed directly in SkImage_Generator, and add a helper to handle param
validation.

R=reed@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2453473004

Review-Url: https://codereview.chromium.org/2453473004

3 files changed, 57 insertions, 43 deletions

diff --git a/src/core/SkImageCacherator.cpp b/src/core/SkImageCacherator.cpp
index 496ca74d9a..6858a4c09e 100644
--- a/src/core/SkImageCacherator.cpp
+++ b/src/core/SkImageCacherator.cpp
@@ -32,49 +32,55 @@
 // see skbug.com/ 4971, 5128, ...
 //#define SK_SUPPORT_COMPRESSED_TEXTURES_IN_CACHERATOR
 
-SkImageCacherator* SkImageCacherator::NewFromGenerator(SkImageGenerator* gen,
-                                                       const SkIRect* subset) {
+SkImageCacherator::Validator::Validator(SkImageGenerator* gen, const SkIRect* subset)
+    // We are required to take ownership of gen, regardless of whether we instantiate a cacherator
+    // or not.  On instantiation, the client is responsible for transferring ownership.
+    : fGenerator(gen) {
+
     if (!gen) {
-        return nullptr;
+        return;
     }
 
-    // We are required to take ownership of gen, regardless of if we return a cacherator or not
-    SkAutoTDelete<SkImageGenerator> genHolder(gen);
-
     const SkImageInfo& info = gen->getInfo();
     if (info.isEmpty()) {
-        return nullptr;
+        fGenerator.reset();
+        return;
     }
 
-    uint32_t uniqueID = gen->uniqueID();
+    fUniqueID = gen->uniqueID();
     const SkIRect bounds = SkIRect::MakeWH(info.width(), info.height());
     if (subset) {
         if (!bounds.contains(*subset)) {
-            return nullptr;
+            fGenerator.reset();
+            return;
         }
         if (*subset != bounds) {
             // we need a different uniqueID since we really are a subset of the raw generator
-            uniqueID = SkNextID::ImageID();
+            fUniqueID = SkNextID::ImageID();
         }
     } else {
         subset = &bounds;
     }
 
-    // Now that we know we can hand-off the generator (to be owned by the cacherator) we can
-    // release our holder. (we DONT want to delete it here anymore)
-    genHolder.release();
+    fInfo   = info.makeWH(subset->width(), subset->height());
+    fOrigin = SkIPoint::Make(subset->x(), subset->y());
+}
 
-    return new SkImageCacherator(gen, gen->getInfo().makeWH(subset->width(), subset->height()),
-                                 SkIPoint::Make(subset->x(), subset->y()), uniqueID);
+SkImageCacherator* SkImageCacherator::NewFromGenerator(SkImageGenerator* gen,
+                                                       const SkIRect* subset) {
+    Validator validator(gen, subset);
+
+    return validator ? new SkImageCacherator(&validator) : nullptr;
 }
 
-SkImageCacherator::SkImageCacherator(SkImageGenerator* gen, const SkImageInfo& info,
-                                     const SkIPoint& origin, uint32_t uniqueID)
-    : fNotThreadSafeGenerator(gen)
-    , fInfo(info)
-    , fOrigin(origin)
-    , fUniqueID(uniqueID)
-{}
+SkImageCacherator::SkImageCacherator(Validator* validator)
+    : fNotThreadSafeGenerator(validator->fGenerator.release()) // we take ownership
+    , fInfo(validator->fInfo)
+    , fOrigin(validator->fOrigin)
+    , fUniqueID(validator->fUniqueID)
+{
+    SkASSERT(fNotThreadSafeGenerator);
+}
 
 SkData* SkImageCacherator::refEncoded(GrContext* ctx) {
     ScopedGenerator generator(this);
diff --git a/src/core/SkImageCacherator.h b/src/core/SkImageCacherator.h
index 3be69a5785..2d9e94cc4d 100644
--- a/src/core/SkImageCacherator.h
+++ b/src/core/SkImageCacherator.h
@@ -68,7 +68,18 @@ public:
                               int srcX, int srcY);
 
 private:
-    SkImageCacherator(SkImageGenerator*, const SkImageInfo&, const SkIPoint&, uint32_t uniqueID);
+    struct Validator {
+        Validator(SkImageGenerator*, const SkIRect* subset);
+
+        operator bool() const { return fGenerator.get(); }
+
+        std::unique_ptr<SkImageGenerator> fGenerator;
+        SkImageInfo                       fInfo;
+        SkIPoint                          fOrigin;
+        uint32_t                          fUniqueID;
+    };
+
+    SkImageCacherator(Validator*);
 
     bool generateBitmap(SkBitmap*);
     bool tryLockAsBitmap(SkBitmap*, const SkImage*, SkImage::CachingHint);
@@ -100,6 +111,8 @@ private:
     const uint32_t      fUniqueID;
 
     friend class GrImageTextureMaker;
+    friend class SkImage;
+    friend class SkImage_Generator;
 };
 
 #endif
diff --git a/src/image/SkImage_Generator.cpp b/src/image/SkImage_Generator.cpp
index 412f573ba6..8fa022de7b 100644
--- a/src/image/SkImage_Generator.cpp
+++ b/src/image/SkImage_Generator.cpp
@@ -16,20 +16,20 @@
 
 class SkImage_Generator : public SkImage_Base {
 public:
-    SkImage_Generator(SkImageCacherator* cache)
-        : INHERITED(cache->info().width(), cache->info().height(), cache->uniqueID())
-        , fCache(cache) // take ownership
+    SkImage_Generator(SkImageCacherator::Validator* validator)
+        : INHERITED(validator->fInfo.width(), validator->fInfo.height(), validator->fUniqueID)
+        , fCache(validator)
     {}
 
     virtual SkImageInfo onImageInfo() const override {
-        return fCache->info();
+        return fCache.info();
     }
     SkAlphaType onAlphaType() const override {
-        return fCache->info().alphaType();
+        return fCache.info().alphaType();
     }
 
     bool onReadPixels(const SkImageInfo&, void*, size_t, int srcX, int srcY, CachingHint) const override;
-    SkImageCacherator* peekCacherator() const override { return fCache; }
+    SkImageCacherator* peekCacherator() const override { return &fCache; }
     SkData* onRefEncoded(GrContext*) const override;
     sk_sp<SkImage> onMakeSubset(const SkIRect&) const override;
     bool getROPixels(SkBitmap*, CachingHint) const override;
@@ -38,7 +38,7 @@ public:
     bool onIsLazyGenerated() const override { return true; }
 
 private:
-    SkAutoTDelete<SkImageCacherator> fCache;
+    mutable SkImageCacherator fCache;
 
     typedef SkImage_Base INHERITED;
 };
@@ -49,13 +49,13 @@ bool SkImage_Generator::onReadPixels(const SkImageInfo& dstInfo, void* dstPixels
                                      int srcX, int srcY, CachingHint chint) const {
     SkBitmap bm;
     if (kDisallow_CachingHint == chint) {
-        if (fCache->lockAsBitmapOnlyIfAlreadyCached(&bm)) {
+        if (fCache.lockAsBitmapOnlyIfAlreadyCached(&bm)) {
             return bm.readPixels(dstInfo, dstPixels, dstRB, srcX, srcY);
         } else {
             // Try passing the caller's buffer directly down to the generator. If this fails we
             // may still succeed in the general case, as the generator may prefer some other
             // config, which we could then convert via SkBitmap::readPixels.
-            if (fCache->directGeneratePixels(dstInfo, dstPixels, dstRB, srcX, srcY)) {
+            if (fCache.directGeneratePixels(dstInfo, dstPixels, dstRB, srcX, srcY)) {
                 return true;
             }
             // else fall through
@@ -69,16 +69,16 @@ bool SkImage_Generator::onReadPixels(const SkImageInfo& dstInfo, void* dstPixels
 }
 
 SkData* SkImage_Generator::onRefEncoded(GrContext* ctx) const {
-    return fCache->refEncoded(ctx);
+    return fCache.refEncoded(ctx);
 }
 
 bool SkImage_Generator::getROPixels(SkBitmap* bitmap, CachingHint chint) const {
-    return fCache->lockAsBitmap(bitmap, this, chint);
+    return fCache.lockAsBitmap(bitmap, this, chint);
 }
 
 GrTexture* SkImage_Generator::asTextureRef(GrContext* ctx, const GrTextureParams& params,
                                            SkSourceGammaTreatment gammaTreatment) const {
-    return fCache->lockAsTexture(ctx, params, gammaTreatment, this);
+    return fCache.lockAsTexture(ctx, params, gammaTreatment, this);
 }
 
 sk_sp<SkImage> SkImage_Generator::onMakeSubset(const SkIRect& subset) const {
@@ -98,12 +98,7 @@ sk_sp<SkImage> SkImage_Generator::onMakeSubset(const SkIRect& subset) const {
 }
 
 sk_sp<SkImage> SkImage::MakeFromGenerator(SkImageGenerator* generator, const SkIRect* subset) {
-    if (!generator) {
-        return nullptr;
-    }
-    SkImageCacherator* cache = SkImageCacherator::NewFromGenerator(generator, subset);
-    if (!cache) {
-        return nullptr;
-    }
-    return sk_make_sp<SkImage_Generator>(cache);
+    SkImageCacherator::Validator validator(generator, subset);
+
+    return validator ? sk_make_sp<SkImage_Generator>(&validator) : nullptr;
 }