Revert of Make SkPngCodec decode progressively. (patchset #26 id:520001 of https://codereview.chromium.org/1997703003/ )

Reason for revert:
Still causing problems in Google3, e.g.

https://test.corp.google.com/ui#cl=124138817&flags=CAMQBQ==&id=OCL:124138817:BASE:124139560:1465227435491:219ffbdb&t=//third_party/skia/HEAD:dm

Original issue's description:
> Make SkPngCodec decode progressively.
>
> This is a step towards using SkCodec in Chromium, where progressive
> decoding is necessary.
>
> Switch from using png_read_row (which expects all the data to be
> available) to png_process_data, which uses callbacks when rows are
> available.
>
> Create a new API for SkCodec, which supports progressive decoding and
> scanline decoding. Future changes will switch the other clients off of
> startScanlineDecode and get/skip-Scanlines to the new API.
>
> Remove SkCodec::kNone_ScanlineOrder, which was only used for interlaced
> PNG images. In the new API, interlaced PNG fits kTopDown. Also remove
> updateCurrScanline(), which was only used by the old implementation for
> interlaced PNG.
>
> DMSrcSink:
> - In CodecSrc::kScanline_Mode, use the new method for scanline decoding
> for the supported formats (just PNG and PNG-in-ICO for now).
>
> fuzz.cpp:
> - Remove reference to kNone_ScanlineOrder
>
> SkCodec:
> - Add new APIs:
>     - startIncrementalDecode
>     - incrementalDecode
> - Remove kNone_SkScanlineOrder and updateCurrScanline()
>
> SkPngCodec:
> - Implement new APIs
> - Switch from sk_read_fn/png_read_row etc to png_process_data
> - Expand AutoCleanPng's role to decode the header and create the
>   SkPngCodec
> - Make the interlaced PNG decoder report how many lines were
>   initialized during an incomplete decode
> - Make initializeSwizzler return a bool instead of an SkCodec::Result
>   (It only returned kSuccess or kInvalidInput anyway)
>
> SkIcoCodec:
> - Implement the new APIs; supported for PNG in ICO
>
> SkSampledCodec:
> - Call the new method for decoding scanlines, and fall back to the old
>   method if the new version is unimplemented
> - Remove references to kNone_SkScanlineOrder
>
> tests/CodecPartial:
> - Add a test which decodes part of an image, then finishes the decode,
>   and compares it to the straightforward method
>
> tests/CodecTest:
> - Add a test which decodes all scanlines using the new method
> - Repurpose the Codec_stripes test to decode using the new method in
>   sections rather than all at once
> - In the method check(), add a parameter for whether the image supports
>   the new method of scanline decoding, and be explicit about whether an
>   image supports incomplete
> - Test incomplete PNG decodes. We should have been doing it anyway for
>   non-interlaced (except for an image that is too small - one row), but
>   the new method supports interlaced incomplete as well
> - Make test_invalid_parameters test the new method
> - Add a test to ensure that it's safe to fall back to scanline decoding without
>   rewinding
>
> BUG=skia:4211
>
> The new version was generally faster than the old version (but not significantly so).
>
> Some raw performance differences can be found at https://docs.google.com/a/google.com/spreadsheets/d/1Gis3aRCEa72qBNDRMgGDg3jD-pMgO-FXldlNF9ejo4o/
>
> Design doc can be found at https://docs.google.com/a/google.com/document/d/11Mn8-ePDKwVEMCjs3nWwSjxcSpJ_Cu8DF57KNtUmgLM/
>
> GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1997703003
>
> Committed: https://skia.googlesource.com/skia/+/a4b09a117d4d1ba5dda372e6a2323e653766539e
>
> Committed: https://skia.googlesource.com/skia/+/30e78c9737ff4861dc4e3fa1e4cd010680ed6965
>
> Committed: https://skia.googlesource.com/skia/+/6fb2391b2cc83ee2160b4e994faa8128975acc1f

TBR=reed@google.com,msarett@google.com,scroggo@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=skia:4211
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2044573002

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

4 files changed, 145 insertions, 405 deletions

diff --git a/tests/CodecPartial.cpp b/tests/CodecPartial.cpp
deleted file mode 100644
index 8775e87c63..0000000000
--- a/tests/CodecPartial.cpp
+++ /dev/null
@@ -1,155 +0,0 @@
-/*
- * Copyright 2016 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#include "SkBitmap.h"
-#include "SkCodec.h"
-#include "SkData.h"
-#include "SkImageInfo.h"
-#include "SkRWBuffer.h"
-#include "SkString.h"
-
-#include "Resources.h"
-#include "Test.h"
-
-static sk_sp<SkData> make_from_resource(const char* name) {
-    SkString fullPath = GetResourcePath(name);
-    return SkData::MakeFromFileName(fullPath.c_str());
-}
-
-static SkImageInfo standardize_info(SkCodec* codec) {
-    SkImageInfo defaultInfo = codec->getInfo();
-    // Note: This drops the SkColorSpace, allowing the equality check between two
-    // different codecs created from the same file to have the same SkImageInfo.
-    return SkImageInfo::MakeN32Premul(defaultInfo.width(), defaultInfo.height());
-}
-
-static bool create_truth(sk_sp<SkData> data, SkBitmap* dst) {
-    SkAutoTDelete<SkCodec> codec(SkCodec::NewFromData(data.get()));
-    if (!codec) {
-        return false;
-    }
-
-    const SkImageInfo info = standardize_info(codec);
-    dst->allocPixels(info);
-    return SkCodec::kSuccess == codec->getPixels(info, dst->getPixels(), dst->rowBytes());
-}
-
-/*
- *  Represents a stream without all of its data.
- */
-class HaltingStream : public SkStream {
-public:
-    HaltingStream(sk_sp<SkData> data)
-        : fTotalSize(data->size())
-        , fLimit(fTotalSize / 2)
-        , fStream(std::move(data))
-    {}
-
-    void addNewData() {
-        // Arbitrary size, but deliberately different from
-        // the buffer size used by SkPngCodec.
-        fLimit = SkTMin(fTotalSize, fLimit + 1000);
-    }
-
-    size_t read(void* buffer, size_t size) override {
-        if (fStream.getPosition() + size > fLimit) {
-            size = fLimit - fStream.getPosition();
-        }
-
-        return fStream.read(buffer, size);
-    }
-
-    bool isAtEnd() const override {
-        return fStream.isAtEnd();
-    }
-
-    bool hasPosition() const override { return true; }
-    size_t getPosition() const override { return fStream.getPosition(); }
-    bool rewind() override { return fStream.rewind(); }
-    bool move(long offset) override { return fStream.move(offset); }
-
-private:
-    const size_t    fTotalSize;
-    size_t          fLimit;
-    SkMemoryStream  fStream;
-};
-
-static void test_partial(skiatest::Reporter* r, const char* name) {
-    sk_sp<SkData> file = make_from_resource(name);
-    if (!file) {
-        SkDebugf("missing resource %s\n", name);
-        return;
-    }
-
-    SkBitmap truth;
-    if (!create_truth(file, &truth)) {
-        ERRORF(r, "Failed to decode %s\n", name);
-        return;
-    }
-
-    const size_t fileSize = file->size();
-
-    // Now decode part of the file
-    HaltingStream* stream = new HaltingStream(file);
-
-    // Note that we cheat and hold on to a pointer to stream, though it is owned by
-    // partialCodec.
-    SkAutoTDelete<SkCodec> partialCodec(SkCodec::NewFromStream(stream));
-    if (!partialCodec) {
-        // Technically, this could be a small file where half the file is not
-        // enough.
-        ERRORF(r, "Failed to create codec for %s", name);
-        return;
-    }
-
-    const SkImageInfo info = standardize_info(partialCodec);
-    SkASSERT(info == truth.info());
-    SkBitmap incremental;
-    incremental.allocPixels(info);
-
-    const SkCodec::Result startResult = partialCodec->startIncrementalDecode(info,
-            incremental.getPixels(), incremental.rowBytes());
-    if (startResult != SkCodec::kSuccess) {
-        ERRORF(r, "Failed to start incremental decode\n");
-        return;
-    }
-
-    while (true) {
-        const SkCodec::Result result = partialCodec->incrementalDecode();
-
-        if (stream->getPosition() == fileSize) {
-            REPORTER_ASSERT(r, result == SkCodec::kSuccess);
-            break;
-        }
-
-        SkASSERT(stream->getPosition() < fileSize);
-
-        REPORTER_ASSERT(r, result == SkCodec::kIncompleteInput);
-
-        // Append an arbitrary amount of data.
-        stream->addNewData();
-    }
-
-    // compare to original
-    for (int i = 0; i < info.height(); i++) {
-        REPORTER_ASSERT(r, !memcmp(truth.getAddr(0, 0), incremental.getAddr(0, 0),
-                                   info.minRowBytes()));
-    }
-}
-
-DEF_TEST(Codec_partial, r) {
-    test_partial(r, "plane.png");
-    test_partial(r, "plane_interlaced.png");
-    test_partial(r, "yellow_rose.png");
-    test_partial(r, "index8.png");
-    test_partial(r, "color_wheel.png");
-    test_partial(r, "mandrill_256.png");
-    test_partial(r, "mandrill_32.png");
-    test_partial(r, "arrow.png");
-    test_partial(r, "randPixels.png");
-    test_partial(r, "baby_tux.png");
-}
diff --git a/tests/CodecTest.cpp b/tests/CodecTest.cpp
index 52ddbff801..12d6ac8d3d 100644
--- a/tests/CodecTest.cpp
+++ b/tests/CodecTest.cpp
@@ -83,62 +83,6 @@ SkIRect generate_random_subset(SkRandom* rand, int w, int h) {
     return rect;
 }
 
-static void test_incremental_decode(skiatest::Reporter* r, SkCodec* codec, const SkImageInfo& info,
-        const SkMD5::Digest& goodDigest) {
-    SkBitmap bm;
-    bm.allocPixels(info);
-    SkAutoLockPixels autoLockPixels(bm);
-
-    REPORTER_ASSERT(r, SkCodec::kSuccess == codec->startIncrementalDecode(info, bm.getPixels(),
-                                                                          bm.rowBytes()));
-
-    REPORTER_ASSERT(r, SkCodec::kSuccess == codec->incrementalDecode());
-
-    compare_to_good_digest(r, goodDigest, bm);
-}
-
-// Test in stripes, similar to DM's kStripe_Mode
-static void test_in_stripes(skiatest::Reporter* r, SkCodec* codec, const SkImageInfo& info,
-                            const SkMD5::Digest& goodDigest) {
-    SkBitmap bm;
-    bm.allocPixels(info);
-    bm.eraseColor(SK_ColorYELLOW);
-
-    const int height = info.height();
-    // Note that if numStripes does not evenly divide height there will be an extra
-    // stripe.
-    const int numStripes = 4;
-
-    if (numStripes > height) {
-        // Image is too small.
-        return;
-    }
-
-    const int stripeHeight = height / numStripes;
-
-    // Iterate through the image twice. Once to decode odd stripes, and once for even.
-    for (int oddEven = 1; oddEven >= 0; oddEven--) {
-        for (int y = oddEven * stripeHeight; y < height; y += 2 * stripeHeight) {
-            SkIRect subset = SkIRect::MakeLTRB(0, y, info.width(),
-                                               SkTMin(y + stripeHeight, height));
-            SkCodec::Options options;
-            options.fSubset = &subset;
-            if (SkCodec::kSuccess != codec->startIncrementalDecode(info, bm.getAddr(0, y),
-                        bm.rowBytes(), &options)) {
-                ERRORF(r, "failed to start incremental decode!\ttop: %i\tbottom%i\n",
-                       subset.top(), subset.bottom());
-                return;
-            }
-            if (SkCodec::kSuccess != codec->incrementalDecode()) {
-                ERRORF(r, "failed incremental decode starting from line %i\n", y);
-                return;
-            }
-        }
-    }
-
-    compare_to_good_digest(r, goodDigest, bm);
-}
-
 template<typename Codec>
 static void test_codec(skiatest::Reporter* r, Codec* codec, SkBitmap& bm, const SkImageInfo& info,
         const SkISize& size, SkCodec::Result expectedResult, SkMD5::Digest* digest,
@@ -247,14 +191,12 @@ static bool supports_partial_scanlines(const char path[]) {
     return false;
 }
 
-// FIXME: Break up this giant function
 static void check(skiatest::Reporter* r,
                   const char path[],
                   SkISize size,
                   bool supportsScanlineDecoding,
                   bool supportsSubsetDecoding,
-                  bool supportsIncomplete,
-                  bool supportsNewScanlineDecoding = false) {
+                  bool supportsIncomplete = true) {
 
     SkAutoTDelete<SkStream> stream(resource(path));
     if (!stream) {
@@ -284,15 +226,12 @@ static void check(skiatest::Reporter* r,
     test_codec(r, codec.get(), bm, info, size, expectedResult, &codecDigest, nullptr);
 
     // Scanline decoding follows.
-
-    if (supportsNewScanlineDecoding && !isIncomplete) {
-        test_incremental_decode(r, codec, info, codecDigest);
-        test_in_stripes(r, codec, info, codecDigest);
-    }
-
     // Need to call startScanlineDecode() first.
-    REPORTER_ASSERT(r, codec->getScanlines(bm.getAddr(0, 0), 1, 0) == 0);
-    REPORTER_ASSERT(r, !codec->skipScanlines(1));
+    REPORTER_ASSERT(r, codec->getScanlines(bm.getAddr(0, 0), 1, 0)
+            == 0);
+    REPORTER_ASSERT(r, codec->skipScanlines(1)
+            == 0);
+
     const SkCodec::Result startResult = codec->startScanlineDecode(info);
     if (supportsScanlineDecoding) {
         bm.eraseColor(SK_ColorYELLOW);
@@ -387,7 +326,7 @@ static void check(skiatest::Reporter* r,
     }
 
     // SkAndroidCodec tests
-    if (supportsScanlineDecoding || supportsSubsetDecoding || supportsNewScanlineDecoding) {
+    if (supportsScanlineDecoding || supportsSubsetDecoding) {
 
         SkAutoTDelete<SkStream> stream(resource(path));
         if (!stream) {
@@ -425,23 +364,14 @@ static void check(skiatest::Reporter* r,
         REPORTER_ASSERT(r, gen->getPixels(info, bm.getPixels(), bm.rowBytes()));
         compare_to_good_digest(r, codecDigest, bm);
 
-#if PNG_LIBPNG_VER_MAJOR == 1 && PNG_LIBPNG_VER_MINOR < 5
-        // FIXME: With older versions of libpng, SkPngCodec requires being able to call
-        // SkStream::move(), which is not supported by SkFrontBufferedStream. (Android
-        // has a more recent version of libpng which uses png_process_data_pause to
-        // avoid calling move().)
-        if (!SkStrEndsWith(path, ".png"))
-#endif
-        {
-            // Test using SkFrontBufferedStream, as Android does
-            SkStream* bufferedStream = SkFrontBufferedStream::Create(new SkMemoryStream(fullData),
-                    SkCodec::MinBufferedBytesNeeded());
-            REPORTER_ASSERT(r, bufferedStream);
-            codec.reset(SkCodec::NewFromStream(bufferedStream));
-            REPORTER_ASSERT(r, codec);
-            if (codec) {
-                test_info(r, codec.get(), info, SkCodec::kSuccess, &codecDigest);
-            }
+        // Test using SkFrontBufferedStream, as Android does
+        SkStream* bufferedStream = SkFrontBufferedStream::Create(new SkMemoryStream(fullData),
+                SkCodec::MinBufferedBytesNeeded());
+        REPORTER_ASSERT(r, bufferedStream);
+        codec.reset(SkCodec::NewFromStream(bufferedStream));
+        REPORTER_ASSERT(r, codec);
+        if (codec) {
+            test_info(r, codec.get(), info, SkCodec::kSuccess, &codecDigest);
         }
     }
 
@@ -453,16 +383,16 @@ static void check(skiatest::Reporter* r,
 
 DEF_TEST(Codec, r) {
     // WBMP
-    check(r, "mandrill.wbmp", SkISize::Make(512, 512), true, false, true);
+    check(r, "mandrill.wbmp", SkISize::Make(512, 512), true, false);
 
     // WEBP
-    check(r, "baby_tux.webp", SkISize::Make(386, 395), false, true, true);
-    check(r, "color_wheel.webp", SkISize::Make(128, 128), false, true, true);
-    check(r, "yellow_rose.webp", SkISize::Make(400, 301), false, true, true);
+    check(r, "baby_tux.webp", SkISize::Make(386, 395), false, true);
+    check(r, "color_wheel.webp", SkISize::Make(128, 128), false, true);
+    check(r, "yellow_rose.webp", SkISize::Make(400, 301), false, true);
 
     // BMP
-    check(r, "randPixels.bmp", SkISize::Make(8, 8), true, false, true);
-    check(r, "rle.bmp", SkISize::Make(320, 240), true, false, true);
+    check(r, "randPixels.bmp", SkISize::Make(8, 8), true, false);
+    check(r, "rle.bmp", SkISize::Make(320, 240), true, false);
 
     // ICO
     // FIXME: We are not ready to test incomplete ICOs
@@ -470,7 +400,7 @@ DEF_TEST(Codec, r) {
     // Decodes an embedded BMP image
     check(r, "color_wheel.ico", SkISize::Make(128, 128), true, false, false);
     // Decodes an embedded PNG image
-    check(r, "google_chrome.ico", SkISize::Make(256, 256), false, false, false, true);
+    check(r, "google_chrome.ico", SkISize::Make(256, 256), true, false, false);
 
     // GIF
     // FIXME: We are not ready to test incomplete GIFs
@@ -480,30 +410,30 @@ DEF_TEST(Codec, r) {
     check(r, "randPixels.gif", SkISize::Make(8, 8), true, false, false);
 
     // JPG
-    check(r, "CMYK.jpg", SkISize::Make(642, 516), true, false, true);
-    check(r, "color_wheel.jpg", SkISize::Make(128, 128), true, false, true);
+    check(r, "CMYK.jpg", SkISize::Make(642, 516), true, false);
+    check(r, "color_wheel.jpg", SkISize::Make(128, 128), true, false);
     // grayscale.jpg is too small to test incomplete
     check(r, "grayscale.jpg", SkISize::Make(128, 128), true, false, false);
-    check(r, "mandrill_512_q075.jpg", SkISize::Make(512, 512), true, false, true);
+    check(r, "mandrill_512_q075.jpg", SkISize::Make(512, 512), true, false);
     // randPixels.jpg is too small to test incomplete
     check(r, "randPixels.jpg", SkISize::Make(8, 8), true, false, false);
 
     // PNG
-    check(r, "arrow.png", SkISize::Make(187, 312), false, false, true, true);
-    check(r, "baby_tux.png", SkISize::Make(240, 246), false, false, true, true);
-    check(r, "color_wheel.png", SkISize::Make(128, 128), false, false, true, true);
-    // half-transparent-white-pixel.png is too small to test incomplete
-    check(r, "half-transparent-white-pixel.png", SkISize::Make(1, 1), false, false, false, true);
-    check(r, "mandrill_128.png", SkISize::Make(128, 128), false, false, true, true);
-    check(r, "mandrill_16.png", SkISize::Make(16, 16), false, false, true, true);
-    check(r, "mandrill_256.png", SkISize::Make(256, 256), false, false, true, true);
-    check(r, "mandrill_32.png", SkISize::Make(32, 32), false, false, true, true);
-    check(r, "mandrill_512.png", SkISize::Make(512, 512), false, false, true, true);
-    check(r, "mandrill_64.png", SkISize::Make(64, 64), false, false, true, true);
-    check(r, "plane.png", SkISize::Make(250, 126), false, false, true, true);
-    check(r, "plane_interlaced.png", SkISize::Make(250, 126), false, false, true, true);
-    check(r, "randPixels.png", SkISize::Make(8, 8), false, false, true, true);
-    check(r, "yellow_rose.png", SkISize::Make(400, 301), false, false, true, true);
+    check(r, "arrow.png", SkISize::Make(187, 312), true, false, false);
+    check(r, "baby_tux.png", SkISize::Make(240, 246), true, false, false);
+    check(r, "color_wheel.png", SkISize::Make(128, 128), true, false, false);
+    check(r, "half-transparent-white-pixel.png", SkISize::Make(1, 1), true, false, false);
+    check(r, "mandrill_128.png", SkISize::Make(128, 128), true, false, false);
+    check(r, "mandrill_16.png", SkISize::Make(16, 16), true, false, false);
+    check(r, "mandrill_256.png", SkISize::Make(256, 256), true, false, false);
+    check(r, "mandrill_32.png", SkISize::Make(32, 32), true, false, false);
+    check(r, "mandrill_512.png", SkISize::Make(512, 512), true, false, false);
+    check(r, "mandrill_64.png", SkISize::Make(64, 64), true, false, false);
+    check(r, "plane.png", SkISize::Make(250, 126), true, false, false);
+    // FIXME: We are not ready to test incomplete interlaced pngs
+    check(r, "plane_interlaced.png", SkISize::Make(250, 126), true, false, false);
+    check(r, "randPixels.png", SkISize::Make(8, 8), true, false, false);
+    check(r, "yellow_rose.png", SkISize::Make(400, 301), true, false, false);
 
     // RAW
 // Disable RAW tests for Win32.
@@ -514,6 +444,97 @@ DEF_TEST(Codec, r) {
 #endif
 }
 
+// Test interlaced PNG in stripes, similar to DM's kStripe_Mode
+DEF_TEST(Codec_stripes, r) {
+    const char * path = "plane_interlaced.png";
+    SkAutoTDelete<SkStream> stream(resource(path));
+    if (!stream) {
+        SkDebugf("Missing resource '%s'\n", path);
+    }
+
+    SkAutoTDelete<SkCodec> codec(SkCodec::NewFromStream(stream.release()));
+    REPORTER_ASSERT(r, codec);
+
+    if (!codec) {
+        return;
+    }
+
+    switch (codec->getScanlineOrder()) {
+        case SkCodec::kBottomUp_SkScanlineOrder:
+        case SkCodec::kOutOfOrder_SkScanlineOrder:
+            ERRORF(r, "This scanline order will not match the original.");
+            return;
+        default:
+            break;
+    }
+
+    // Baseline for what the image should look like, using N32.
+    const SkImageInfo info = codec->getInfo().makeColorType(kN32_SkColorType);
+
+    SkBitmap bm;
+    bm.allocPixels(info);
+    SkAutoLockPixels autoLockPixels(bm);
+    SkCodec::Result result = codec->getPixels(info, bm.getPixels(), bm.rowBytes());
+    REPORTER_ASSERT(r, result == SkCodec::kSuccess);
+
+    SkMD5::Digest digest;
+    md5(bm, &digest);
+
+    // Now decode in stripes
+    const int height = info.height();
+    const int numStripes = 4;
+    int stripeHeight;
+    int remainingLines;
+    SkTDivMod(height, numStripes, &stripeHeight, &remainingLines);
+
+    bm.eraseColor(SK_ColorYELLOW);
+
+    result = codec->startScanlineDecode(info);
+    REPORTER_ASSERT(r, result == SkCodec::kSuccess);
+
+    // Odd stripes
+    for (int i = 1; i < numStripes; i += 2) {
+        // Skip the even stripes
+        bool skipResult = codec->skipScanlines(stripeHeight);
+        REPORTER_ASSERT(r, skipResult);
+
+        int linesDecoded = codec->getScanlines(bm.getAddr(0, i * stripeHeight), stripeHeight,
+                                     bm.rowBytes());
+        REPORTER_ASSERT(r, linesDecoded == stripeHeight);
+    }
+
+    // Even stripes
+    result = codec->startScanlineDecode(info);
+    REPORTER_ASSERT(r, result == SkCodec::kSuccess);
+
+    for (int i = 0; i < numStripes; i += 2) {
+        int linesDecoded = codec->getScanlines(bm.getAddr(0, i * stripeHeight), stripeHeight,
+                                     bm.rowBytes());
+        REPORTER_ASSERT(r, linesDecoded == stripeHeight);
+
+        // Skip the odd stripes
+        if (i + 1 < numStripes) {
+            bool skipResult = codec->skipScanlines(stripeHeight);
+            REPORTER_ASSERT(r, skipResult);
+        }
+    }
+
+    // Remainder at the end
+    if (remainingLines > 0) {
+        result = codec->startScanlineDecode(info);
+        REPORTER_ASSERT(r, result == SkCodec::kSuccess);
+
+        bool skipResult = codec->skipScanlines(height - remainingLines);
+        REPORTER_ASSERT(r, skipResult);
+
+        int linesDecoded = codec->getScanlines(bm.getAddr(0, height - remainingLines),
+                                     remainingLines, bm.rowBytes());
+        REPORTER_ASSERT(r, linesDecoded == remainingLines);
+    }
+
+    compare_to_good_digest(r, digest, bm);
+}
+
 static void test_invalid_stream(skiatest::Reporter* r, const void* stream, size_t len) {
     // Neither of these calls should return a codec. Bots should catch us if we leaked anything.
     SkCodec* codec = SkCodec::NewFromStream(new SkMemoryStream(stream, len, false));
@@ -656,41 +677,26 @@ static void test_invalid_parameters(skiatest::Reporter* r, const char path[]) {
         return;
     }
     SkAutoTDelete<SkCodec> decoder(SkCodec::NewFromStream(stream.release()));
-    if (!decoder) {
-        SkDebugf("Missing codec for %s\n", path);
-        return;
-    }
-
-    const SkImageInfo info = decoder->getInfo().makeColorType(kIndex_8_SkColorType);
 
     // This should return kSuccess because kIndex8 is supported.
     SkPMColor colorStorage[256];
     int colorCount;
-    SkCodec::Result result = decoder->startScanlineDecode(info, nullptr, colorStorage,
-                                                          &colorCount);
-    if (SkCodec::kSuccess == result) {
-        // This should return kInvalidParameters because, in kIndex_8 mode, we must pass in a valid
-        // colorPtr and a valid colorCountPtr.
-        result = decoder->startScanlineDecode(info, nullptr, nullptr, nullptr);
-        REPORTER_ASSERT(r, SkCodec::kInvalidParameters == result);
-        result = decoder->startScanlineDecode(info);
-        REPORTER_ASSERT(r, SkCodec::kInvalidParameters == result);
-    } else if (SkCodec::kUnimplemented == result) {
-        // New method should be supported:
-        SkBitmap bm;
-        sk_sp<SkColorTable> colorTable(new SkColorTable(colorStorage, 256));
-        bm.allocPixels(info, nullptr, colorTable.get());
-        result = decoder->startIncrementalDecode(info, bm.getPixels(), bm.rowBytes(), nullptr,
-                                                 colorStorage, &colorCount);
-        REPORTER_ASSERT(r, SkCodec::kSuccess == result);
-        result = decoder->startIncrementalDecode(info, bm.getPixels(), bm.rowBytes());
-        REPORTER_ASSERT(r, SkCodec::kInvalidParameters == result);
-    } else {
-        // The test is uninteresting if kIndex8 is not supported
-        ERRORF(r, "Should not call test_invalid_parameters for non-Index8 file: %s\n", path);
+    SkCodec::Result result = decoder->startScanlineDecode(
+        decoder->getInfo().makeColorType(kIndex_8_SkColorType), nullptr, colorStorage, &colorCount);
+    REPORTER_ASSERT(r, SkCodec::kSuccess == result);
+    // The rest of the test is uninteresting if kIndex8 is not supported
+    if (SkCodec::kSuccess != result) {
         return;
     }
 
+    // This should return kInvalidParameters because, in kIndex_8 mode, we must pass in a valid
+    // colorPtr and a valid colorCountPtr.
+    result = decoder->startScanlineDecode(
+        decoder->getInfo().makeColorType(kIndex_8_SkColorType), nullptr, nullptr, nullptr);
+    REPORTER_ASSERT(r, SkCodec::kInvalidParameters == result);
+    result = decoder->startScanlineDecode(
+        decoder->getInfo().makeColorType(kIndex_8_SkColorType));
+    REPORTER_ASSERT(r, SkCodec::kInvalidParameters == result);
 }
 
 DEF_TEST(Codec_Params, r) {
@@ -1006,91 +1012,3 @@ DEF_TEST(Codec_wbmp_max_size, r) {
 
     REPORTER_ASSERT(r, !codec);
 }
-
-// Only rewinds up to a limit.
-class LimitedRewindingStream : public SkStream {
-public:
-    static SkStream* Make(const char path[], size_t limit) {
-        SkStream* stream = resource(path);
-        if (!stream) {
-            return nullptr;
-        }
-        return new LimitedRewindingStream(stream, limit);
-    }
-
-    size_t read(void* buffer, size_t size) override {
-        const size_t bytes = fStream->read(buffer, size);
-        fPosition += bytes;
-        return bytes;
-    }
-
-    bool isAtEnd() const override {
-        return fStream->isAtEnd();
-    }
-
-    bool rewind() override {
-        if (fPosition <= fLimit && fStream->rewind()) {
-            fPosition = 0;
-            return true;
-        }
-
-        return false;
-    }
-
-private:
-    SkAutoTDelete<SkStream> fStream;
-    const size_t            fLimit;
-    size_t                  fPosition;
-
-    LimitedRewindingStream(SkStream* stream, size_t limit)
-        : fStream(stream)
-        , fLimit(limit)
-        , fPosition(0)
-    {
-        SkASSERT(fStream);
-    }
-};
-
-DEF_TEST(Codec_fallBack, r) {
-    // SkAndroidCodec needs to be able to fall back to scanline decoding
-    // if incremental decoding does not work. Make sure this does not
-    // require a rewind.
-
-    // Formats that currently do not support incremental decoding
-    auto files = {
-            "box.gif",
-            "CMYK.jpg",
-            "color_wheel.ico",
-            "mandrill.wbmp",
-            "randPixels.bmp",
-            };
-    for (auto file : files) {
-        SkStream* stream = LimitedRewindingStream::Make(file, 14);
-        if (!stream) {
-            SkDebugf("Missing resources (%s). Set --resourcePath.\n", file);
-            return;
-        }
-
-        SkAutoTDelete<SkCodec> codec(SkCodec::NewFromStream(stream));
-        if (!codec) {
-            ERRORF(r, "Failed to create codec for %s,", file);
-            continue;
-        }
-
-        SkImageInfo info = codec->getInfo().makeColorType(kN32_SkColorType);
-        SkBitmap bm;
-        bm.allocPixels(info);
-
-        if (SkCodec::kUnimplemented != codec->startIncrementalDecode(info, bm.getPixels(),
-                bm.rowBytes())) {
-            ERRORF(r, "Is scanline decoding now implemented for %s?", file);
-            continue;
-        }
-
-        // Scanline decoding should not require a rewind.
-        SkCodec::Result result = codec->startScanlineDecode(info);
-        if (SkCodec::kSuccess != result) {
-            ERRORF(r, "Scanline decoding failed for %s with %i", file, result);
-        }
-    }
-}
diff --git a/tests/ColorSpaceTest.cpp b/tests/ColorSpaceTest.cpp
index 51d8457fd1..6dd4789a8e 100644
--- a/tests/ColorSpaceTest.cpp
+++ b/tests/ColorSpaceTest.cpp
@@ -67,10 +67,7 @@ DEF_TEST(ColorSpaceParsePngICCProfile, r) {
     }
 
     SkAutoTDelete<SkCodec> codec(SkCodec::NewFromStream(stream.release()));
-    if (!codec) {
-        ERRORF(r, "Failed to create codec from a PNG. Is PNG support missing (or too old)?");
-        return;
-    }
+    REPORTER_ASSERT(r, nullptr != codec);
 
 #if (PNG_LIBPNG_VER_MAJOR > 1) || (PNG_LIBPNG_VER_MAJOR == 1 && PNG_LIBPNG_VER_MINOR >= 6)
     SkColorSpace* colorSpace = codec->getColorSpace();
diff --git a/tests/ImageTest.cpp b/tests/ImageTest.cpp
index b2bf495338..0922364d0f 100644
--- a/tests/ImageTest.cpp
+++ b/tests/ImageTest.cpp
@@ -161,10 +161,6 @@ static void test_encode(skiatest::Reporter* reporter, SkImage* image) {
     REPORTER_ASSERT(reporter, origEncoded->size() > 0);
 
     sk_sp<SkImage> decoded(SkImage::MakeFromEncoded(origEncoded));
-    if (!decoded) {
-        ERRORF(reporter, "failed to decode image!");
-        return;
-    }
     REPORTER_ASSERT(reporter, decoded);
     assert_equal(reporter, image, nullptr, decoded.get());
 
@@ -522,10 +518,6 @@ static bool has_pixels(const SkPMColor pixels[], int count, SkPMColor expected)
 }
 
 static void test_read_pixels(skiatest::Reporter* reporter, SkImage* image) {
-    if (!image) {
-        ERRORF(reporter, "Failed to create image!");
-        return;
-    }
     const SkPMColor expected = SkPreMultiplyColor(SK_ColorWHITE);
     const SkPMColor notExpected = ~expected;
 
@@ -611,10 +603,6 @@ static void check_legacy_bitmap(skiatest::Reporter* reporter, const SkImage* ima
 }
 
 static void test_legacy_bitmap(skiatest::Reporter* reporter, const SkImage* image, SkImage::LegacyBitmapMode mode) {
-    if (!image) {
-        ERRORF(reporter, "Failed to create image.");
-        return;
-    }
     SkBitmap bitmap;
     REPORTER_ASSERT(reporter, image->asLegacyBitmap(&bitmap, mode));
     check_legacy_bitmap(reporter, image, bitmap, mode);
@@ -666,10 +654,6 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ImageLegacyBitmap_Gpu, reporter, ctxInfo) {
 #endif
 
 static void test_peek(skiatest::Reporter* reporter, SkImage* image, bool expectPeekSuccess) {
-    if (!image) {
-        ERRORF(reporter, "Failed to create image!");
-        return;
-    }
     SkPixmap pm;
     bool success = image->peekPixels(&pm);
     REPORTER_ASSERT(reporter, expectPeekSuccess == success);
@@ -873,10 +857,6 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(DeferredTextureImage, reporter, ctxInfo) {
 
     for (auto testCase : testCases) {
         sk_sp<SkImage> image(testCase.fImageFactory());
-        if (!image) {
-            ERRORF(reporter, "Failed to create image!");
-            continue;
-        }
 
         // This isn't currently used in the implementation, just set any old values.
         SkImage::DeferredTextureImageUsageParams params;