SkScaledCodec should implement onRewind()

tests/CodexTest.cpp

Index: tests/CodexTest.cpp
diff --git a/tests/CodexTest.cpp b/tests/CodexTest.cpp
index f1c5bbaa128e1c989bf06af39f27f7a22e211864..7fb95bc1afb9ce3850a47e7b1d1c0fb447bc2f46 100644
--- a/tests/CodexTest.cpp
+++ b/tests/CodexTest.cpp
@@ -220,6 +220,40 @@ static void check(skiatest::Reporter* r,
             REPORTER_ASSERT(r, result == SkCodec::kUnimplemented);
         }
     }
+
+    // SkScaledCodec tests
+    {
+        SkAutoTDelete<SkStream> stream(resource(path));
+        if (!stream) {
+            SkDebugf("Missing resource '%s'\n", path);
+            return;
+        }
+        SkAutoTDelete<SkCodec> codec(SkScaledCodec::NewFromStream(stream.detach()));
+        if (!codec) {
+            ERRORF(r, "Unable to decode '%s'", path);
+            return;
+        }
+
+        SkImageInfo info = codec->getInfo().makeColorType(kN32_SkColorType);
+        REPORTER_ASSERT(r, info.dimensions() == size);
+
+        // FIXME: We will be able to run this test on ICOs as well, after we have implemented
+        //        scanline decoding for ICO.
+        if (SkEncodedFormat::kICO_SkEncodedFormat != codec->getEncodedFormat()) {

[scroggo, 2015/10/02 20:56:06]

Another way you could do this - not sure if it's better:

You could run this whole block only if (supportsScanlineDecoding), which would
eliminate ICO (until we update ICO)... The awkward part is that you need to also
consider webp, so it becomes

if (supportsScanlineDecoding || supportsSubsetDecoding)

[msarett, 2015/10/05 14:26:51]

Done.

+            SkBitmap bm;
+            bm.allocPixels(info);
+            SkAutoLockPixels autoLockPixels(bm);
+            SkCodec::Result result =
+                codec->getPixels(info, bm.getPixels(), bm.rowBytes(), nullptr, nullptr, nullptr);

[scroggo, 2015/10/02 20:56:06]

nit: This is copied and pasted, so I know it's the same as the other code, but
this could all fit on one line if you call the other version of getPixels and
leave out the three nullptrs.

On that note, maybe this should be factored out into another method? (I
considered it to share with test_info, but wasn't sure it would be better.

Actually, wait, shouldn't this look identical to the non SkScaledCodec version?
This whole block can just be:

(Note: "digest" here means the digest you shadowed, which you do not need to
shadow - just use the same one since it should match the original.)

test_info(r, codec, info, SkCodec::kSuccess, &digest);

// Verify that re-decoding (and thus rewinding) gives the same result
test_info(r, codec, info, SkCodec::kSuccess, &digest);

[msarett, 2015/10/05 14:26:51]

I will refactor into a different method and check that SkScaledCodec digests
match SkCodec digests.

+            REPORTER_ASSERT(r, result == SkCodec::kSuccess);
+
+            SkMD5::Digest digest;
+            md5(bm, &digest);
+
+            // Verify that re-decoding gives the same result.
+            test_info(r, codec, info, SkCodec::kSuccess, &digest);
+        }
+    }
 }
 
 DEF_TEST(Codec, r) {