Revert of Make SkAndroidCodec support ico (patchset #7 id:130002 of https://codereview.chromium.org/1472933002/ )

Reason for revert:
Crashing: https://uberchromegw.corp.google.com/i/client.skia.android/builders/Test-Android-GCC-NexusPlayer-CPU-SSE4-x86-Release/builds/1499/steps/dm/logs/stdio

Also, related ASAN failures:
https://uberchromegw.corp.google.com/i/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-ASAN/builds/3676/steps/dm/logs/stdio

Original issue's description:
> Make SkAndroidCodec support ico
>
> BUG=skia:
>
> Committed: https://skia.googlesource.com/skia/+/1603e9310f62cf0dd543cdb09dea06aa78373f13

TBR=djsollen@google.com,msarett@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

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

7 files changed, 74 insertions, 238 deletions

diff --git a/src/codec/SkAndroidCodec.cpp b/src/codec/SkAndroidCodec.cpp
index c26b3eff9b..cf6e253d97 100644
--- a/src/codec/SkAndroidCodec.cpp
+++ b/src/codec/SkAndroidCodec.cpp
@@ -34,9 +34,11 @@ SkAndroidCodec* SkAndroidCodec::NewFromStream(SkStream* stream, SkPngChunkReader
         case kWBMP_SkEncodedFormat:
         case kBMP_SkEncodedFormat:
         case kGIF_SkEncodedFormat:
-        case kICO_SkEncodedFormat:
             return new SkSampledCodec(codec.detach());
         default:
+            // FIXME: SkSampledCodec is temporarily disabled for other formats
+            // while focusing on the formats that are supported by
+            // BitmapRegionDecoder.
             return nullptr;
     }
 }
diff --git a/src/codec/SkBmpCodec.cpp b/src/codec/SkBmpCodec.cpp
index aff54021a7..191c2ad800 100644
--- a/src/codec/SkBmpCodec.cpp
+++ b/src/codec/SkBmpCodec.cpp
@@ -481,8 +481,6 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco, SkCodec** codecOut) {
         // Return the codec
         switch (inputFormat) {
             case kStandard_BmpInputFormat:
-                // We require streams to have a memory base for Bmp-in-Ico decodes.
-                SkASSERT(!inIco || nullptr != stream->getMemoryBase());
                 *codecOut = new SkBmpStandardCodec(imageInfo, stream, bitsPerPixel, numColors,
                         bytesPerColor, offset - bytesRead, rowOrder, inIco);
                 return true;
diff --git a/src/codec/SkBmpStandardCodec.cpp b/src/codec/SkBmpStandardCodec.cpp
index 9e90f3c630..fd4d6d18bc 100644
--- a/src/codec/SkBmpStandardCodec.cpp
+++ b/src/codec/SkBmpStandardCodec.cpp
@@ -27,7 +27,6 @@ SkBmpStandardCodec::SkBmpStandardCodec(const SkImageInfo& info, SkStream* stream
     , fSrcRowBytes(SkAlign4(compute_row_bytes(this->getInfo().width(), this->bitsPerPixel())))
     , fSrcBuffer(new uint8_t [fSrcRowBytes])
     , fInIco(inIco)
-    , fAndMaskRowBytes(fInIco ? SkAlign4(compute_row_bytes(this->getInfo().width(), 1)) : 0)
 {}
 
 /*
@@ -61,6 +60,9 @@ SkCodec::Result SkBmpStandardCodec::onGetPixels(const SkImageInfo& dstInfo,
         *rowsDecoded = rows;
         return kIncompleteInput;
     }
+    if (fInIco) {
+        return this->decodeIcoMask(dstInfo, dst, dstRowBytes);
+    }
     return kSuccess;
 }
 
@@ -225,8 +227,9 @@ SkCodec::Result SkBmpStandardCodec::prepareToDecode(const SkImageInfo& dstInfo,
 /*
  * Performs the bitmap decoding for standard input format
  */
-int SkBmpStandardCodec::decodeRows(const SkImageInfo& dstInfo, void* dst, size_t dstRowBytes,
-        const Options& opts) {
+int SkBmpStandardCodec::decodeRows(const SkImageInfo& dstInfo,
+                                               void* dst, size_t dstRowBytes,
+                                               const Options& opts) {
     // Iterate over rows of the image
     const int height = dstInfo.height();
     for (int y = 0; y < height; y++) {
@@ -243,75 +246,29 @@ int SkBmpStandardCodec::decodeRows(const SkImageInfo& dstInfo, void* dst, size_t
         fSwizzler->swizzle(dstRow, fSrcBuffer.get());
     }
 
-    if (fInIco) {
-        const int startScanline = this->currScanline();
-        if (startScanline < 0) {
-            // We are not performing a scanline decode.
-            // Just decode the entire ICO mask and return.
-            decodeIcoMask(this->stream(), dstInfo, dst, dstRowBytes);
-            return height;
-        }
-
-        // In order to perform a scanline ICO decode, we must be able
-        // to skip ahead in the stream in order to apply the AND mask
-        // to the requested scanlines.
-        // We will do this by taking advantage of the fact that
-        // SkIcoCodec always uses a SkMemoryStream as its underlying
-        // representation of the stream.
-        const void* memoryBase = this->stream()->getMemoryBase();
-        SkASSERT(nullptr != memoryBase);
-        SkASSERT(this->stream()->hasLength());
-        SkASSERT(this->stream()->hasPosition());
-
-        const size_t length = this->stream()->getLength();
-        const size_t currPosition = this->stream()->getPosition();
-
-        // Calculate how many bytes we must skip to reach the AND mask.
-        const int remainingScanlines = this->getInfo().height() - startScanline - height;
-        const size_t bytesToSkip = remainingScanlines * fSrcRowBytes +
-                startScanline * fAndMaskRowBytes;
-        const size_t subStreamStartPosition = currPosition + bytesToSkip;
-        if (subStreamStartPosition >= length) {
-            // FIXME: How can we indicate that this decode was actually incomplete?
-            return height;
-        }
-
-        // Create a subStream to pass to decodeIcoMask().  It is useful to encapsulate
-        // the memory base into a stream in order to safely handle incomplete images
-        // without reading out of bounds memory.
-        const void* subStreamMemoryBase = SkTAddOffset<const void>(memoryBase,
-                subStreamStartPosition);
-        const size_t subStreamLength = length - subStreamStartPosition;
-        // This call does not transfer ownership of the subStreamMemoryBase.
-        SkMemoryStream subStream(subStreamMemoryBase, subStreamLength, false);
-
-        // FIXME: If decodeIcoMask does not succeed, is there a way that we can
-        //        indicate the decode was incomplete?
-        decodeIcoMask(&subStream, dstInfo, dst, dstRowBytes);
-    }
-
+    // Finished decoding the entire image
     return height;
 }
 
-void SkBmpStandardCodec::decodeIcoMask(SkStream* stream, const SkImageInfo& dstInfo,
+// TODO (msarett): This function will need to be modified in order to perform row by row decodes
+//                 when the Ico scanline decoder is implemented.
+SkCodec::Result SkBmpStandardCodec::decodeIcoMask(const SkImageInfo& dstInfo,
         void* dst, size_t dstRowBytes) {
     // BMP in ICO have transparency, so this cannot be 565, and this mask
     // prevents us from using kIndex8. The below code depends on the output
     // being an SkPMColor.
     SkASSERT(dstInfo.colorType() == kN32_SkColorType);
 
-    // If we are sampling, make sure that we only mask the sampled pixels.
-    // We do not need to worry about sampling in the y-dimension because that
-    // should be handled by SkSampledCodec.
-    int sampleX = fSwizzler->sampleX();
-    int startX = get_start_coord(sampleX);
+    // The AND mask is always 1 bit per pixel
+    const int width = this->getInfo().width();
+    const size_t rowBytes = SkAlign4(compute_row_bytes(width, 1));
 
     SkPMColor* dstPtr = (SkPMColor*) dst;
     for (int y = 0; y < dstInfo.height(); y++) {
         // The srcBuffer will at least be large enough
-        if (stream->read(fSrcBuffer.get(), fAndMaskRowBytes) != fAndMaskRowBytes) {
+        if (stream()->read(fSrcBuffer.get(), rowBytes) != rowBytes) {
             SkCodecPrintf("Warning: incomplete AND mask for bmp-in-ico.\n");
-            return;
+            return kIncompleteInput;
         }
 
         int row = this->getDstRow(y, dstInfo.height());
@@ -319,15 +276,17 @@ void SkBmpStandardCodec::decodeIcoMask(SkStream* stream, const SkImageInfo& dstI
         SkPMColor* dstRow =
                 SkTAddOffset<SkPMColor>(dstPtr, row * dstRowBytes);
 
-        for (int x = startX; x < this->getInfo().width(); x += sampleX) {
+        for (int x = 0; x < width; x++) {
             int quotient;
             int modulus;
             SkTDivMod(x, 8, &quotient, &modulus);
             uint32_t shift = 7 - modulus;
-            uint32_t alphaBit = (fSrcBuffer.get()[quotient] >> shift) & 0x1;
-            dstRow[get_dst_coord(x, sampleX)] &= alphaBit - 1;
+            uint32_t alphaBit =
+                    (fSrcBuffer.get()[quotient] >> shift) & 0x1;
+            dstRow[x] &= alphaBit - 1;
         }
     }
+    return kSuccess;
 }
 
 uint32_t SkBmpStandardCodec::onGetFillValue(SkColorType colorType, SkAlphaType alphaType) const {
diff --git a/src/codec/SkBmpStandardCodec.h b/src/codec/SkBmpStandardCodec.h
index 7f23616a60..d687eaad28 100644
--- a/src/codec/SkBmpStandardCodec.h
+++ b/src/codec/SkBmpStandardCodec.h
@@ -75,12 +75,7 @@ private:
     int decodeRows(const SkImageInfo& dstInfo, void* dst, size_t dstRowBytes,
             const Options& opts) override;
 
-    /*
-     * @param stream This may be a pointer to the stream owned by the parent SkCodec
-     *               or a sub-stream of the stream owned by the parent SkCodec.
-     *               Either way, this stream is unowned.
-     */
-    void decodeIcoMask(SkStream* stream, const SkImageInfo& dstInfo, void* dst, size_t dstRowBytes);
+    Result decodeIcoMask(const SkImageInfo& dstInfo, void* dst, size_t dstRowBytes);
 
     SkAutoTUnref<SkColorTable>          fColorTable;     // owned
     const uint32_t                      fNumColors;
@@ -90,7 +85,6 @@ private:
     const size_t                        fSrcRowBytes;
     SkAutoTDeleteArray<uint8_t>         fSrcBuffer;
     const bool                          fInIco;
-    const size_t                        fAndMaskRowBytes; // only used for fInIco decodes
 
     typedef SkBmpCodec INHERITED;
 };
diff --git a/src/codec/SkCodec_libico.cpp b/src/codec/SkCodec_libico.cpp
index e1314d7121..8c5a1b3412 100644
--- a/src/codec/SkCodec_libico.cpp
+++ b/src/codec/SkCodec_libico.cpp
@@ -15,43 +15,6 @@
 #include "SkTDArray.h"
 #include "SkTSort.h"
 
-static bool ico_conversion_possible(const SkImageInfo& dstInfo) {
-    // We only support kN32_SkColorType.
-    // This makes sense for BMP-in-ICO.  The presence of an AND
-    // mask (which changes colors and adds transparency) means that
-    // we cannot use k565 or kIndex8.
-    // FIXME: For PNG-in-ICO, we could technically support whichever
-    //        color types that the png supports.
-    if (kN32_SkColorType != dstInfo.colorType()) {
-        return false;
-    }
-
-    // We only support transparent alpha types.  This is necessary for
-    // BMP-in-ICOs since there will be an AND mask.
-    // FIXME: For opaque PNG-in-ICOs, we should be able to support kOpaque.
-    return kPremul_SkAlphaType == dstInfo.alphaType() ||
-            kUnpremul_SkAlphaType == dstInfo.alphaType();
-}
-
-static SkImageInfo fix_embedded_alpha(const SkImageInfo& dstInfo, SkAlphaType embeddedAlpha) {
-    // FIXME (msarett): ICO is considered non-opaque, even if the embedded BMP
-    // incorrectly claims it has no alpha.
-    switch (embeddedAlpha) {
-        case kPremul_SkAlphaType:
-        case kUnpremul_SkAlphaType:
-            // Use the requested alpha type if the embedded codec supports alpha.
-            embeddedAlpha = dstInfo.alphaType();
-            break;
-        case kOpaque_SkAlphaType:
-            // If the embedded codec claims it is opaque, decode as if it is opaque.
-            break;
-        default:
-            SkASSERT(false);
-            break;
-    }
-    return dstInfo.makeAlphaType(embeddedAlpha);
-}
-
 /*
  * Checks the start of the stream to see if the image is an Ico or Cur
  */
@@ -234,7 +197,6 @@ SkIcoCodec::SkIcoCodec(const SkImageInfo& info,
                        SkTArray<SkAutoTDelete<SkCodec>, true>* codecs)
     : INHERITED(info, nullptr)
     , fEmbeddedCodecs(codecs)
-    , fCurrScanlineCodec(nullptr)
 {}
 
 /*
@@ -264,21 +226,15 @@ SkISize SkIcoCodec::onGetScaledDimensions(float desiredScale) const {
     return fEmbeddedCodecs->operator[](minIndex)->getInfo().dimensions();
 }
 
-int SkIcoCodec::chooseCodec(const SkISize& requestedSize, int startIndex) {
-    SkASSERT(startIndex >= 0);
-
+bool SkIcoCodec::onDimensionsSupported(const SkISize& dim) {
     // FIXME: Cache the index from onGetScaledDimensions?
-    for (int i = startIndex; i < fEmbeddedCodecs->count(); i++) {
-        if (fEmbeddedCodecs->operator[](i)->getInfo().dimensions() == requestedSize) {
-            return i;
+    for (int32_t i = 0; i < fEmbeddedCodecs->count(); i++) {
+        if (fEmbeddedCodecs->operator[](i)->getInfo().dimensions() == dim) {
+            return true;
         }
     }
 
-    return -1;
-}
-
-bool SkIcoCodec::onDimensionsSupported(const SkISize& dim) {
-    return this->chooseCodec(dim, 0) >= 0;
+    return false;
 }
 
 /*
@@ -293,89 +249,54 @@ SkCodec::Result SkIcoCodec::onGetPixels(const SkImageInfo& dstInfo,
         return kUnimplemented;
     }
 
-    if (!ico_conversion_possible(dstInfo)) {
+    if (!valid_alpha(dstInfo.alphaType(), this->getInfo().alphaType())) {
         return kInvalidConversion;
     }
 
-    int index = 0;
-    SkCodec::Result result = kInvalidScale;
-    while (true) {
-        index = this->chooseCodec(dstInfo.dimensions(), index);
-        if (index < 0) {
-            break;
-        }
-
-        SkCodec* embeddedCodec = fEmbeddedCodecs->operator[](index);
-        SkImageInfo decodeInfo = fix_embedded_alpha(dstInfo, embeddedCodec->getInfo().alphaType());
-        SkASSERT(decodeInfo.colorType() == kN32_SkColorType);
-        result = embeddedCodec->getPixels(decodeInfo, dst, dstRowBytes, &opts, colorTable,
-                colorCount);
-
-        switch (result) {
-            case kSuccess:
-            case kIncompleteInput:
-                // The embedded codec will handle filling incomplete images, so we will indicate
-                // that all of the rows are initialized.
-                *rowsDecoded = decodeInfo.height();
-                return result;
-            default:
-                // Continue trying to find a valid embedded codec on a failed decode.
-                break;
-        }
-
-        index++;
-    }
-
-    SkCodecPrintf("Error: No matching candidate image in ico.\n");
-    return result;
-}
-
-SkCodec::Result SkIcoCodec::onStartScanlineDecode(const SkImageInfo& dstInfo,
-        const SkCodec::Options& options, SkPMColor colorTable[], int* colorCount) {
-    if (!ico_conversion_possible(dstInfo)) {
-        return kInvalidConversion;
-    }
-
-    int index = 0;
-    SkCodec::Result result = kInvalidScale;
-    while (true) {
-        index = this->chooseCodec(dstInfo.dimensions(), index);
-        if (index < 0) {
-            break;
-        }
-
-        SkCodec* embeddedCodec = fEmbeddedCodecs->operator[](index);
-        SkImageInfo decodeInfo = fix_embedded_alpha(dstInfo, embeddedCodec->getInfo().alphaType());
-        result = embeddedCodec->startScanlineDecode(decodeInfo, &options, colorTable, colorCount);
-        if (kSuccess == result) {
-            fCurrScanlineCodec = embeddedCodec;
+    // We return invalid scale if there is no candidate image with matching
+    // dimensions.
+    Result result = kInvalidScale;
+    for (int32_t i = 0; i < fEmbeddedCodecs->count(); i++) {
+        SkCodec* embeddedCodec = fEmbeddedCodecs->operator[](i);
+        // If the dimensions match, try to decode
+        if (dstInfo.dimensions() == embeddedCodec->getInfo().dimensions()) {
+
+            // Perform the decode
+            // FIXME (msarett): ICO is considered non-opaque, even if the embedded BMP
+            // incorrectly claims it has no alpha.
+            SkAlphaType embeddedAlpha = embeddedCodec->getInfo().alphaType();
+            switch (embeddedAlpha) {
+                case kPremul_SkAlphaType:
+                case kUnpremul_SkAlphaType:
+                    // Use the requested alpha type if the embedded codec supports alpha.
+                    embeddedAlpha = dstInfo.alphaType();
+                    break;
+                case kOpaque_SkAlphaType:
+                    // If the embedded codec claims it is opaque, decode as if it is opaque.
+                    break;
+                default:
+                    SkASSERT(false);
+                    break;
+            }
+            SkImageInfo info = dstInfo.makeAlphaType(embeddedAlpha);
+            result = embeddedCodec->getPixels(info, dst, dstRowBytes, &opts, colorTable,
+                    colorCount);
+            // The embedded codec will handle filling incomplete images, so we will indicate
+            // that all of the rows are initialized.
+            *rowsDecoded = info.height();
+
+            // On a fatal error, keep trying to find an image to decode
+            if (kInvalidConversion == result || kInvalidInput == result ||
+                    kInvalidScale == result) {
+                SkCodecPrintf("Warning: Attempt to decode candidate ico failed.\n");
+                continue;
+            }
+
+            // On success or partial success, return the result
             return result;
         }
-
-        index++;
     }
 
     SkCodecPrintf("Error: No matching candidate image in ico.\n");
     return result;
 }
-
-int SkIcoCodec::onGetScanlines(void* dst, int count, size_t rowBytes) {
-    SkASSERT(fCurrScanlineCodec);
-    return fCurrScanlineCodec->getScanlines(dst, count, rowBytes);
-}
-
-bool SkIcoCodec::onSkipScanlines(int count) {
-    SkASSERT(fCurrScanlineCodec);
-    return fCurrScanlineCodec->skipScanlines(count);
-}
-
-SkCodec::SkScanlineOrder SkIcoCodec::onGetScanlineOrder() const {
-    // FIXME: This function will possibly return the wrong value if it is called
-    //        before startScanlineDecode().
-    return fCurrScanlineCodec ? fCurrScanlineCodec->getScanlineOrder() :
-            INHERITED::onGetScanlineOrder();
-}
-
-SkSampler* SkIcoCodec::getSampler(bool createIfNecessary) {
-    return fCurrScanlineCodec ? fCurrScanlineCodec->getSampler(createIfNecessary) : nullptr;
-}
diff --git a/src/codec/SkCodec_libico.h b/src/codec/SkCodec_libico.h
index a1d99b884d..92675f4d74 100644
--- a/src/codec/SkCodec_libico.h
+++ b/src/codec/SkCodec_libico.h
@@ -47,45 +47,17 @@ protected:
         return kICO_SkEncodedFormat;
     }
 
-    SkScanlineOrder onGetScanlineOrder() const override;
-
 private:
 
-    Result onStartScanlineDecode(const SkImageInfo& dstInfo, const SkCodec::Options& options,
-            SkPMColor inputColorPtr[], int* inputColorCount) override;
-
-    int onGetScanlines(void* dst, int count, size_t rowBytes) override;
-
-    bool onSkipScanlines(int count) override;
-
-    SkSampler* getSampler(bool createIfNecessary) override;
-
-    /*
-     * Searches fEmbeddedCodecs for a codec that matches requestedSize.
-     * The search starts at startIndex and ends when an appropriate codec
-     * is found, or we have reached the end of the array.
-     *
-     * @return the index of the matching codec or -1 if there is no
-     *         matching codec between startIndex and the end of
-     *         the array.
-     */
-    int chooseCodec(const SkISize& requestedSize, int startIndex);
-
     /*
      * Constructor called by NewFromStream
      * @param embeddedCodecs codecs for the embedded images, takes ownership
      */
-    SkIcoCodec(const SkImageInfo& srcInfo, SkTArray<SkAutoTDelete<SkCodec>, true>* embeddedCodecs);
-
-    SkAutoTDelete<SkTArray<SkAutoTDelete<SkCodec>, true>> fEmbeddedCodecs; // owned
+    SkIcoCodec(const SkImageInfo& srcInfo,
+               SkTArray<SkAutoTDelete<SkCodec>, true>* embeddedCodecs);
 
-    // Only used by the scanline decoder.  onStartScanlineDecode() will set
-    // fCurrScanlineCodec to one of the fEmbeddedCodecs, if it can find a
-    // codec of the appropriate size.  We will use fCurrScanlineCodec for
-    // subsequent calls to onGetScanlines() or onSkipScanlines().
-    // fCurrScanlineCodec is owned by this class, but should not be an
-    // SkAutoTDelete.  It will be deleted by the destructor of fEmbeddedCodecs.
-    SkCodec* fCurrScanlineCodec;
+    SkAutoTDelete<SkTArray<SkAutoTDelete<SkCodec>, true>>
+            fEmbeddedCodecs; // owned
 
     typedef SkCodec INHERITED;
 };
diff --git a/src/codec/SkSwizzler.h b/src/codec/SkSwizzler.h
index bdb10a1416..7f5bbc6209 100644
--- a/src/codec/SkSwizzler.h
+++ b/src/codec/SkSwizzler.h
@@ -163,16 +163,6 @@ public:
         SkSampler::Fill(fillInfo, dst, rowBytes, colorOrIndex, zeroInit);
     }
 
-    /**
-     *  If fSampleX > 1, the swizzler is sampling every fSampleX'th pixel and
-     *  discarding the rest.
-     *
-     *  This getter is currently used by SkBmpStandardCodec for Bmp-in-Ico decodes.
-     *  Ideally, the subclasses of SkCodec would have no knowledge of sampling, but
-     *  this allows us to apply a transparency mask to pixels after swizzling.
-     */
-    int sampleX() const { return fSampleX; }
-
 private:
 
     /**