Properly fill in memory in sampled RLE BMPs

Bug: oss-fuzz:11039

Previously, RLE BMPs were not initialized when they were incomplete,
with the thinking that they were fully initialized at the start of
decoding. While this is true during full decodes (onGetPixels), it is
not true of scanline decodes where we may start with a failing skip.

Remove incorrect comment about creating a sampler in SkBmpRLECodec.
Instead create the sampler when appropriate. Rather than make it
implement its own version of SkSampler::fill, which would look like the
other implementations, add a new virtual method to determine the width
and leave the common implementation to the caller.

Simplify SkCodec::fillIncompleteImage, which always does basically the
same thing.

Change-Id: I885ebb7a0fe5add2a4f59bce57d07d98e4dc1bdb
Reviewed-on: https://skia-review.googlesource.com/c/163484
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>

7 files changed, 32 insertions, 63 deletions

diff --git a/src/codec/SkBmpRLECodec.cpp b/src/codec/SkBmpRLECodec.cpp
index 8fc0ea8045..669062942c 100644
--- a/src/codec/SkBmpRLECodec.cpp
+++ b/src/codec/SkBmpRLECodec.cpp
@@ -275,11 +275,10 @@ SkCodec::Result SkBmpRLECodec::onPrepareToDecode(const SkImageInfo& dstInfo,
  */
 int SkBmpRLECodec::decodeRows(const SkImageInfo& info, void* dst, size_t dstRowBytes,
         const Options& opts) {
-    const int width = this->dimensions().width();
     int height = info.height();
 
     // Account for sampling.
-    SkImageInfo dstInfo = info.makeWH(get_scaled_dimension(width, fSampleX), height);
+    SkImageInfo dstInfo = info.makeWH(this->fillWidth(), height);
 
     // Set the background as transparent.  Then, if the RLE code skips pixels,
     // the skipped pixels will be transparent.
@@ -538,6 +537,10 @@ public:
         SkASSERT(fCodec);
     }
 
+    int fillWidth() const override {
+        return fCodec->fillWidth();
+    }
+
 private:
     int onSetSampleX(int sampleX) override {
         return fCodec->setSampleX(sampleX);
@@ -547,20 +550,19 @@ private:
     SkBmpRLECodec* fCodec;
 };
 
-SkSampler* SkBmpRLECodec::getSampler(bool /*createIfNecessary*/) {
-    // We will always create an SkBmpRLESampler if one is requested.
-    // This allows clients to always use the SkBmpRLESampler's
-    // version of fill(), which does nothing since RLE decodes have
-    // already filled pixel memory.  This seems fine, since creating
-    // an SkBmpRLESampler is pretty inexpensive.
-    if (!fSampler) {
+SkSampler* SkBmpRLECodec::getSampler(bool createIfNecessary) {
+    if (!fSampler && createIfNecessary) {
         fSampler.reset(new SkBmpRLESampler(this));
     }
 
     return fSampler.get();
 }
 
-int SkBmpRLECodec::setSampleX(int sampleX){
+int SkBmpRLECodec::setSampleX(int sampleX) {
     fSampleX = sampleX;
-    return get_scaled_dimension(this->dimensions().width(), sampleX);
+    return this->fillWidth();
+}
+
+int SkBmpRLECodec::fillWidth() const {
+    return get_scaled_dimension(this->dimensions().width(), fSampleX);
 }
diff --git a/src/codec/SkBmpRLECodec.h b/src/codec/SkBmpRLECodec.h
index dc6f26d86e..4ed12d09cb 100644
--- a/src/codec/SkBmpRLECodec.h
+++ b/src/codec/SkBmpRLECodec.h
@@ -41,6 +41,8 @@ public:
 
     int setSampleX(int);
 
+    int fillWidth() const;
+
 protected:
 
     Result onGetPixels(const SkImageInfo& dstInfo, void* dst,
diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index f9c34dd245..dfd1e0a5e8 100644
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -559,41 +559,22 @@ int SkCodec::onOutputScanline(int inputScanline) const {
     }
 }
 
-static void fill_proc(const SkImageInfo& info, void* dst, size_t rowBytes,
-                      SkCodec::ZeroInitialized zeroInit, SkSampler* sampler) {
-    if (sampler) {
-        sampler->fill(info, dst, rowBytes, zeroInit);
-    } else {
-        SkSampler::Fill(info, dst, rowBytes, zeroInit);
-    }
-}
-
 void SkCodec::fillIncompleteImage(const SkImageInfo& info, void* dst, size_t rowBytes,
         ZeroInitialized zeroInit, int linesRequested, int linesDecoded) {
+    if (kYes_ZeroInitialized == zeroInit) {
+        return;
+    }
 
-    void* fillDst;
     const int linesRemaining = linesRequested - linesDecoded;
     SkSampler* sampler = this->getSampler(false);
 
-    int fillWidth = info.width();
-    if (fOptions.fSubset) {
-        fillWidth = fOptions.fSubset->width();
-    }
-
-    switch (this->getScanlineOrder()) {
-        case kTopDown_SkScanlineOrder: {
-            const SkImageInfo fillInfo = info.makeWH(fillWidth, linesRemaining);
-            fillDst = SkTAddOffset<void>(dst, linesDecoded * rowBytes);
-            fill_proc(fillInfo, fillDst, rowBytes, zeroInit, sampler);
-            break;
-        }
-        case kBottomUp_SkScanlineOrder: {
-            fillDst = dst;
-            const SkImageInfo fillInfo = info.makeWH(fillWidth, linesRemaining);
-            fill_proc(fillInfo, fillDst, rowBytes, zeroInit, sampler);
-            break;
-        }
-    }
+    const int fillWidth = sampler          ? sampler->fillWidth()      :
+                          fOptions.fSubset ? fOptions.fSubset->width() :
+                                             info.width()              ;
+    void* fillDst = this->getScanlineOrder() == kBottomUp_SkScanlineOrder ? dst :
+                        SkTAddOffset<void>(dst, linesDecoded * rowBytes);
+    const auto fillInfo = info.makeWH(fillWidth, linesRemaining);
+    SkSampler::Fill(fillInfo, fillDst, rowBytes, kNo_ZeroInitialized);
 }
 
 static inline bool select_xform_format(SkColorType colorType, bool forColorTable,
diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp
index 2e03b79a47..24d1720273 100644
--- a/src/codec/SkGifCodec.cpp
+++ b/src/codec/SkGifCodec.cpp
@@ -343,10 +343,8 @@ SkCodec::Result SkGifCodec::decodeFrame(bool firstAttempt, const Options& opts,
             //   draw anything, so we need to fill.
             if (frameContext->frameRect() != this->bounds()
                     || frameContext->interlaced() || !fCurrColorTableIsReal) {
-                // fill ignores the width (replaces it with the actual, scaled width).
-                // But we need to scale in Y.
-                auto fillInfo = dstInfo.makeWH(0, scaledHeight);
-                fSwizzler->fill(fillInfo, fDst, fDstRowBytes, opts.fZeroInitialized);
+                auto fillInfo = dstInfo.makeWH(fSwizzler->fillWidth(), scaledHeight);
+                SkSampler::Fill(fillInfo, fDst, fDstRowBytes, opts.fZeroInitialized);
                 filledBackground = true;
             }
         } else {
diff --git a/src/codec/SkMaskSwizzler.h b/src/codec/SkMaskSwizzler.h
index b8289f35c3..eff43ee025 100644
--- a/src/codec/SkMaskSwizzler.h
+++ b/src/codec/SkMaskSwizzler.h
@@ -35,13 +35,8 @@ public:
      */
     void swizzle(void* dst, const uint8_t* SK_RESTRICT src);
 
-    /**
-     * Implement fill using a custom width.
-     */
-    void fill(const SkImageInfo& info, void* dst, size_t rowBytes,
-              SkCodec::ZeroInitialized zeroInit) override {
-        const SkImageInfo fillInfo = info.makeWH(fDstWidth, info.height());
-        SkSampler::Fill(fillInfo, dst, rowBytes, zeroInit);
+    int fillWidth() const override {
+        return fDstWidth;
     }
 
     /**
diff --git a/src/codec/SkSampler.h b/src/codec/SkSampler.h
index 815153b3de..05043b981c 100644
--- a/src/codec/SkSampler.h
+++ b/src/codec/SkSampler.h
@@ -68,11 +68,7 @@ public:
     static void Fill(const SkImageInfo& info, void* dst, size_t rowBytes,
                      SkCodec::ZeroInitialized zeroInit);
 
-    /**
-     * Allow subclasses to implement unique versions of fill().
-     */
-    virtual void fill(const SkImageInfo& info, void* dst, size_t rowBytes,
-                      SkCodec::ZeroInitialized zeroInit) {} // FIXME: Can this be abstract?
+    virtual int fillWidth() const = 0;
 
     SkSampler()
         : fSampleY(1)
diff --git a/src/codec/SkSwizzler.h b/src/codec/SkSwizzler.h
index df2cf9f26b..b172e458cc 100644
--- a/src/codec/SkSwizzler.h
+++ b/src/codec/SkSwizzler.h
@@ -53,13 +53,8 @@ public:
      */
     void swizzle(void* dst, const uint8_t* SK_RESTRICT src);
 
-    /**
-     * Implement fill using a custom width.
-     */
-    void fill(const SkImageInfo& info, void* dst, size_t rowBytes,
-            SkCodec::ZeroInitialized zeroInit) override {
-        const SkImageInfo fillInfo = info.makeWH(fAllocatedWidth, info.height());
-        SkSampler::Fill(fillInfo, dst, rowBytes, zeroInit);
+    int fillWidth() const override {
+        return fAllocatedWidth;
     }
 
     /**