Support for interlaced PNGs (in gfx::PNGCodec)

BUG=30339
TEST=Added new unit tests: --gtest_filter=PNGCodec.*. For a manual test, load
up the second test theme attached to the referenced bug (comment #10).

Review URL: http://codereview.chromium.org/9496004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@125602 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 494 insertions, 147 deletions

diff --git a/ui/gfx/codec/png_codec.cc b/ui/gfx/codec/png_codec.cc
index 4b3cf8b..f61ca55 100644
--- a/ui/gfx/codec/png_codec.cc
+++ b/ui/gfx/codec/png_codec.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -54,34 +54,6 @@ void ConvertRGBAtoRGB(const unsigned char* rgba, int pixel_width,
   }
 }
 
-void ConvertRGBtoSkia(const unsigned char* rgb, int pixel_width,
-                      unsigned char* rgba, bool* is_opaque) {
-  for (int x = 0; x < pixel_width; x++) {
-    const unsigned char* pixel_in = &rgb[x * 3];
-    uint32_t* pixel_out = reinterpret_cast<uint32_t*>(&rgba[x * 4]);
-    *pixel_out = SkPackARGB32(0xFF, pixel_in[0], pixel_in[1], pixel_in[2]);
-  }
-}
-
-void ConvertRGBAtoSkia(const unsigned char* rgb, int pixel_width,
-                       unsigned char* rgba, bool* is_opaque) {
-  int total_length = pixel_width * 4;
-  for (int x = 0; x < total_length; x += 4) {
-    const unsigned char* pixel_in = &rgb[x];
-    uint32_t* pixel_out = reinterpret_cast<uint32_t*>(&rgba[x]);
-
-    unsigned char alpha = pixel_in[3];
-    if (alpha != 255) {
-      *is_opaque = false;
-      *pixel_out = SkPreMultiplyARGB(alpha,
-           pixel_in[0], pixel_in[1], pixel_in[2]);
-    } else {
-      *pixel_out = SkPackARGB32(alpha,
-           pixel_in[0], pixel_in[1], pixel_in[2]);
-    }
-  }
-}
-
 void ConvertSkiatoRGB(const unsigned char* skia, int pixel_width,
                       unsigned char* rgb, bool* is_opaque) {
   for (int x = 0; x < pixel_width; x++) {
@@ -148,7 +120,6 @@ class PngDecoderState {
         bitmap(NULL),
         is_opaque(true),
         output(o),
-        row_converter(NULL),
         width(0),
         height(0),
         done(false) {
@@ -161,7 +132,6 @@ class PngDecoderState {
         bitmap(skbitmap),
         is_opaque(true),
         output(NULL),
-        row_converter(NULL),
         width(0),
         height(0),
         done(false) {
@@ -181,11 +151,6 @@ class PngDecoderState {
   // instead of directly to an SkBitmap.
   std::vector<unsigned char>* output;
 
-  // Called to convert a row from the library to the correct output format.
-  // When NULL, no conversion is necessary.
-  void (*row_converter)(const unsigned char* in, int w, unsigned char* out,
-                        bool* is_opaque);
-
   // Size of the image, set in the info callback.
   int width;
   int height;
@@ -197,27 +162,29 @@ class PngDecoderState {
   DISALLOW_COPY_AND_ASSIGN(PngDecoderState);
 };
 
-void ConvertRGBtoRGBA(const unsigned char* rgb, int pixel_width,
-                      unsigned char* rgba, bool* is_opaque) {
-  for (int x = 0; x < pixel_width; x++) {
-    const unsigned char* pixel_in = &rgb[x * 3];
-    unsigned char* pixel_out = &rgba[x * 4];
-    pixel_out[0] = pixel_in[0];
-    pixel_out[1] = pixel_in[1];
-    pixel_out[2] = pixel_in[2];
-    pixel_out[3] = 0xff;
-  }
-}
-
-void ConvertRGBtoBGRA(const unsigned char* rgb, int pixel_width,
-                      unsigned char* bgra, bool* is_opaque) {
-  for (int x = 0; x < pixel_width; x++) {
-    const unsigned char* pixel_in = &rgb[x * 3];
-    unsigned char* pixel_out = &bgra[x * 4];
-    pixel_out[0] = pixel_in[2];
-    pixel_out[1] = pixel_in[1];
-    pixel_out[2] = pixel_in[0];
-    pixel_out[3] = 0xff;
+// User transform (passed to libpng) which converts a row decoded by libpng to
+// Skia format. Expects the row to have 4 channels, otherwise there won't be
+// enough room in |data|.
+void ConvertRGBARowToSkia(png_structp png_ptr,
+                          png_row_infop row_info,
+                          png_bytep data) {
+  const int channels = row_info->channels;
+  DCHECK_EQ(channels, 4);
+
+  PngDecoderState* state =
+      static_cast<PngDecoderState*>(png_get_user_transform_ptr(png_ptr));
+  DCHECK(state) << "LibPNG user transform pointer is NULL";
+
+  unsigned char* const end = data + row_info->rowbytes;
+  for (unsigned char* p = data; p < end; p += channels) {
+    uint32_t* sk_pixel = reinterpret_cast<uint32_t*>(p);
+    const unsigned char alpha = p[channels - 1];
+    if (alpha != 255) {
+      state->is_opaque = false;
+      *sk_pixel = SkPreMultiplyARGB(alpha, p[0], p[1], p[2]);
+    } else {
+      *sk_pixel = SkPackARGB32(alpha, p[0], p[1], p[2]);
+    }
   }
 }
 
@@ -228,7 +195,7 @@ void DecodeInfoCallback(png_struct* png_ptr, png_info* info_ptr) {
       png_get_progressive_ptr(png_ptr));
 
   int bit_depth, color_type, interlace_type, compression_type;
-  int filter_type, channels;
+  int filter_type;
   png_uint_32 w, h;
   png_get_IHDR(png_ptr, info_ptr, &w, &h, &bit_depth, &color_type,
                &interlace_type, &compression_type, &filter_type);
@@ -245,94 +212,99 @@ void DecodeInfoCallback(png_struct* png_ptr, png_info* info_ptr) {
   state->width = static_cast<int>(w);
   state->height = static_cast<int>(h);
 
+  // The following png_set_* calls have to be done in the order dictated by
+  // the libpng docs. Please take care if you have to move any of them. This
+  // is also why certain things are done outside of the switch, even though
+  // they look like they belong there.
+
   // Expand to ensure we use 24-bit for RGB and 32-bit for RGBA.
   if (color_type == PNG_COLOR_TYPE_PALETTE ||
       (color_type == PNG_COLOR_TYPE_GRAY && bit_depth < 8))
     png_set_expand(png_ptr);
 
+  // The '!= 0' is for silencing a Windows compiler warning.
+  bool input_has_alpha = ((color_type & PNG_COLOR_MASK_ALPHA) != 0);
+
   // Transparency for paletted images.
-  if (png_get_valid(png_ptr, info_ptr, PNG_INFO_tRNS))
+  if (png_get_valid(png_ptr, info_ptr, PNG_INFO_tRNS)) {
     png_set_expand(png_ptr);
+    input_has_alpha = true;
+  }
 
   // Convert 16-bit to 8-bit.
   if (bit_depth == 16)
     png_set_strip_16(png_ptr);
 
-  // Expand grayscale to RGB.
-  if (color_type == PNG_COLOR_TYPE_GRAY ||
-      color_type == PNG_COLOR_TYPE_GRAY_ALPHA)
-    png_set_gray_to_rgb(png_ptr);
-
-  // Deal with gamma and keep it under our control.
-  double gamma;
-  if (png_get_gAMA(png_ptr, info_ptr, &gamma)) {
-    if (gamma <= 0.0 || gamma > kMaxGamma) {
-      gamma = kInverseGamma;
-      png_set_gAMA(png_ptr, info_ptr, gamma);
-    }
-    png_set_gamma(png_ptr, kDefaultGamma, gamma);
-  } else {
-    png_set_gamma(png_ptr, kDefaultGamma, kInverseGamma);
-  }
-
-  // Tell libpng to send us rows for interlaced pngs.
-  if (interlace_type == PNG_INTERLACE_ADAM7)
-    png_set_interlace_handling(png_ptr);
-
-  // Update our info now
-  png_read_update_info(png_ptr, info_ptr);
-  channels = png_get_channels(png_ptr, info_ptr);
-
   // Pick our row format converter necessary for this data.
-  if (channels == 3) {
+  if (!input_has_alpha) {
     switch (state->output_format) {
       case PNGCodec::FORMAT_RGB:
-        state->row_converter = NULL;  // no conversion necessary
         state->output_channels = 3;
         break;
       case PNGCodec::FORMAT_RGBA:
-        state->row_converter = &ConvertRGBtoRGBA;
         state->output_channels = 4;
+        png_set_add_alpha(png_ptr, 0xFF, PNG_FILLER_AFTER);
         break;
       case PNGCodec::FORMAT_BGRA:
-        state->row_converter = &ConvertRGBtoBGRA;
         state->output_channels = 4;
+        png_set_bgr(png_ptr);
+        png_set_add_alpha(png_ptr, 0xFF, PNG_FILLER_AFTER);
         break;
       case PNGCodec::FORMAT_SkBitmap:
-        state->row_converter = &ConvertRGBtoSkia;
         state->output_channels = 4;
-        break;
-      default:
-        NOTREACHED() << "Unknown output format";
+        png_set_add_alpha(png_ptr, 0xFF, PNG_FILLER_AFTER);
         break;
     }
-  } else if (channels == 4) {
+  } else {
     switch (state->output_format) {
       case PNGCodec::FORMAT_RGB:
-        state->row_converter = &ConvertRGBAtoRGB;
         state->output_channels = 3;
+        png_set_strip_alpha(png_ptr);
         break;
       case PNGCodec::FORMAT_RGBA:
-        state->row_converter = NULL;  // no conversion necessary
         state->output_channels = 4;
         break;
       case PNGCodec::FORMAT_BGRA:
-        state->row_converter = &ConvertBetweenBGRAandRGBA;
         state->output_channels = 4;
+        png_set_bgr(png_ptr);
         break;
       case PNGCodec::FORMAT_SkBitmap:
-        state->row_converter = &ConvertRGBAtoSkia;
         state->output_channels = 4;
         break;
-      default:
-        NOTREACHED() << "Unknown output format";
-        break;
     }
+  }
+
+  // Expand grayscale to RGB.
+  if (color_type == PNG_COLOR_TYPE_GRAY ||
+      color_type == PNG_COLOR_TYPE_GRAY_ALPHA)
+    png_set_gray_to_rgb(png_ptr);
+
+  // Deal with gamma and keep it under our control.
+  double gamma;
+  if (png_get_gAMA(png_ptr, info_ptr, &gamma)) {
+    if (gamma <= 0.0 || gamma > kMaxGamma) {
+      gamma = kInverseGamma;
+      png_set_gAMA(png_ptr, info_ptr, gamma);
+    }
+    png_set_gamma(png_ptr, kDefaultGamma, gamma);
   } else {
-    NOTREACHED() << "Unknown input channels";
-    longjmp(png_jmpbuf(png_ptr), 1);
+    png_set_gamma(png_ptr, kDefaultGamma, kInverseGamma);
+  }
+
+  // Setting the user transforms here (as opposed to inside the switch above)
+  // because all png_set_* calls need to be done in the specific order
+  // mandated by libpng.
+  if (state->output_format == PNGCodec::FORMAT_SkBitmap) {
+    png_set_read_user_transform_fn(png_ptr, ConvertRGBARowToSkia);
+    png_set_user_transform_info(png_ptr, state, 0, 0);
   }
 
+  // Tell libpng to send us rows for interlaced pngs.
+  if (interlace_type == PNG_INTERLACE_ADAM7)
+    png_set_interlace_handling(png_ptr);
+
+  png_read_update_info(png_ptr, info_ptr);
+
   if (state->bitmap) {
     state->bitmap->setConfig(SkBitmap::kARGB_8888_Config,
                              state->width, state->height);
@@ -345,11 +317,12 @@ void DecodeInfoCallback(png_struct* png_ptr, png_info* info_ptr) {
 
 void DecodeRowCallback(png_struct* png_ptr, png_byte* new_row,
                        png_uint_32 row_num, int pass) {
+  if (!new_row)
+    return;  // Interlaced image; row didn't change this pass.
+
   PngDecoderState* state = static_cast<PngDecoderState*>(
       png_get_progressive_ptr(png_ptr));
 
-  DCHECK(pass == 0) << "We didn't turn on interlace handling, but libpng is "
-                       "giving us interlaced data.";
   if (static_cast<int>(row_num) > state->height) {
     NOTREACHED() << "Invalid row";
     return;
@@ -362,10 +335,7 @@ void DecodeRowCallback(png_struct* png_ptr, png_byte* new_row,
     base = &state->output->front();
 
   unsigned char* dest = &base[state->width * state->output_channels * row_num];
-  if (state->row_converter)
-    state->row_converter(new_row, state->width, dest, &state->is_opaque);
-  else
-    memcpy(dest, new_row, state->width * state->output_channels);
+  png_progressive_combine_row(png_ptr, dest, new_row);
 }
 
 void DecodeEndCallback(png_struct* png_ptr, png_info* info) {
diff --git a/ui/gfx/codec/png_codec_unittest.cc b/ui/gfx/codec/png_codec_unittest.cc
index e737cd2..d8feeb9 100644
--- a/ui/gfx/codec/png_codec_unittest.cc
+++ b/ui/gfx/codec/png_codec_unittest.cc
@@ -2,11 +2,19 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#if defined(USE_SYSTEM_LIBPNG)
+#include <png.h>
+#else
+#include "third_party/libpng/png.h"
+#endif
+
 #include <algorithm>
 #include <cmath>
 
+#include "base/logging.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkBitmap.h"
+#include "third_party/skia/include/core/SkColorPriv.h"
 #include "third_party/skia/include/core/SkUnPreMultiply.h"
 #include "third_party/zlib/zlib.h"
 #include "ui/gfx/codec/png_codec.h"
@@ -14,7 +22,9 @@
 
 namespace gfx {
 
-static void MakeRGBImage(int w, int h, std::vector<unsigned char>* dat) {
+namespace {
+
+void MakeRGBImage(int w, int h, std::vector<unsigned char>* dat) {
   dat->resize(w * h * 3);
   for (int y = 0; y < h; y++) {
     for (int x = 0; x < w; x++) {
@@ -30,8 +40,8 @@ static void MakeRGBImage(int w, int h, std::vector<unsigned char>* dat) {
 // be filled with 0xff. With the alpha channel stripped, this should yield the
 // same image as MakeRGBImage above, so the code below can make reference
 // images for conversion testing.
-static void MakeRGBAImage(int w, int h, bool use_transparency,
-                          std::vector<unsigned char>* dat) {
+void MakeRGBAImage(int w, int h, bool use_transparency,
+                   std::vector<unsigned char>* dat) {
   dat->resize(w * h * 4);
   for (int y = 0; y < h; y++) {
     for (int x = 0; x < w; x++) {
@@ -47,6 +57,144 @@ static void MakeRGBAImage(int w, int h, bool use_transparency,
   }
 }
 
+// User write function (to be passed to libpng by EncodeImage) which writes
+// into a buffer instead of to a file.
+void WriteImageData(png_structp png_ptr,
+                    png_bytep data,
+                    png_size_t length) {
+  std::vector<unsigned char>& v =
+      *static_cast<std::vector<unsigned char>*>(png_get_io_ptr(png_ptr));
+  v.resize(v.size() + length);
+  memcpy(&v[v.size() - length], data, length);
+}
+
+// User flush function; goes with WriteImageData, above.
+void FlushImageData(png_structp /*png_ptr*/) {
+}
+
+// Libpng user error function which allows us to print libpng errors using
+// Chrome's logging facilities instead of stderr.
+void LogLibPNGError(png_structp png_ptr,
+                    png_const_charp error_msg) {
+  DLOG(ERROR) << "libpng encode error: " << error_msg;
+  longjmp(png_jmpbuf(png_ptr), 1);
+}
+
+// Goes with LogLibPNGError, above.
+void LogLibPNGWarning(png_structp png_ptr,
+                      png_const_charp warning_msg) {
+  DLOG(ERROR) << "libpng encode warning: " << warning_msg;
+}
+
+// Color types supported by EncodeImage. Required because neither libpng nor
+// PNGCodec::Encode supports all of the required values.
+enum ColorType {
+  COLOR_TYPE_GRAY = PNG_COLOR_TYPE_GRAY,
+  COLOR_TYPE_GRAY_ALPHA = PNG_COLOR_TYPE_GRAY_ALPHA,
+  COLOR_TYPE_PALETTE = PNG_COLOR_TYPE_PALETTE,
+  COLOR_TYPE_RGB = PNG_COLOR_TYPE_RGB,
+  COLOR_TYPE_RGBA = PNG_COLOR_TYPE_RGBA,
+  COLOR_TYPE_BGR,
+  COLOR_TYPE_BGRA
+};
+
+// PNG encoder used for testing. Required because PNGCodec::Encode doesn't do
+// interlaced, palette-based, or grayscale images, but PNGCodec::Decode is
+// actually asked to decode these types of images by Chrome.
+bool EncodeImage(const std::vector<unsigned char>& input,
+                 const int width,
+                 const int height,
+                 ColorType output_color_type,
+                 std::vector<unsigned char>* output,
+                 const int interlace_type = PNG_INTERLACE_NONE,
+                 std::vector<png_color>* palette = 0,
+                 std::vector<unsigned char>* palette_alpha = 0) {
+  struct ScopedPNGStructs {
+    ScopedPNGStructs(png_struct** s, png_info** i) : s_(s), i_(i) {}
+    ~ScopedPNGStructs() { png_destroy_write_struct(s_, i_); }
+    png_struct** s_;
+    png_info** i_;
+  };
+
+  DCHECK(output);
+  png_struct* png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING,
+                                                NULL, NULL, NULL);
+  if (!png_ptr)
+    return false;
+
+  png_infop info_ptr = png_create_info_struct(png_ptr);
+  if (!info_ptr) {
+    png_destroy_write_struct(&png_ptr, NULL);
+    return false;
+  }
+
+  ScopedPNGStructs scoped_png_structs(&png_ptr, &info_ptr);
+
+  if (setjmp(png_jmpbuf(png_ptr)))
+    return false;
+
+  png_set_error_fn(png_ptr, NULL, LogLibPNGError, LogLibPNGWarning);
+
+  int input_rowbytes = 0;
+  int transforms = PNG_TRANSFORM_IDENTITY;
+
+  switch (output_color_type) {
+    case COLOR_TYPE_GRAY:
+      input_rowbytes = width;
+      break;
+    case COLOR_TYPE_GRAY_ALPHA:
+      input_rowbytes = width * 2;
+      break;
+    case COLOR_TYPE_PALETTE:
+      if (!palette)
+        return false;
+      input_rowbytes = width;
+      break;
+    case COLOR_TYPE_RGB:
+      input_rowbytes = width * 3;
+      break;
+    case COLOR_TYPE_RGBA:
+      input_rowbytes = width * 4;
+      break;
+    case COLOR_TYPE_BGR:
+      input_rowbytes = width * 3;
+      output_color_type = static_cast<ColorType>(PNG_COLOR_TYPE_RGB);
+      transforms |= PNG_TRANSFORM_BGR;
+      break;
+    case COLOR_TYPE_BGRA:
+      input_rowbytes = width * 4;
+      output_color_type = static_cast<ColorType>(PNG_COLOR_TYPE_RGBA);
+      transforms |= PNG_TRANSFORM_BGR;
+      break;
+  };
+
+  std::vector<png_bytep> row_pointers(height);
+  for (int y = 0 ; y < height; y++) {
+    row_pointers[y] = const_cast<unsigned char*>(&input[y * input_rowbytes]);
+  }
+  png_set_rows(png_ptr, info_ptr, &row_pointers[0]);
+  png_set_write_fn(png_ptr, output, WriteImageData, FlushImageData);
+  png_set_IHDR(png_ptr, info_ptr, width, height, 8, output_color_type,
+               interlace_type, PNG_COMPRESSION_TYPE_DEFAULT,
+               PNG_FILTER_TYPE_DEFAULT);
+  if (output_color_type == COLOR_TYPE_PALETTE) {
+    png_set_PLTE(png_ptr, info_ptr, &palette->front(), palette->size());
+    if (palette_alpha) {
+      png_set_tRNS(png_ptr,
+                   info_ptr,
+                   &palette_alpha->front(),
+                   palette_alpha->size(),
+                   NULL);
+    }
+  }
+
+  png_write_png(png_ptr, info_ptr, transforms, NULL);
+
+  return true;
+}
+
+}  // namespace
+
 // Returns true if each channel of the given two colors are "close." This is
 // used for comparing colors where rounding errors may cause off-by-one.
 bool ColorsClose(uint32_t a, uint32_t b) {
@@ -82,7 +230,7 @@ TEST(PNGCodec, EncodeDecodeRGB) {
 
   // encode
   std::vector<unsigned char> encoded;
-  EXPECT_TRUE(PNGCodec::Encode(&original[0], PNGCodec::FORMAT_RGB,
+  ASSERT_TRUE(PNGCodec::Encode(&original[0], PNGCodec::FORMAT_RGB,
                                Size(w, h), w * 3, false,
                                std::vector<PNGCodec::Comment>(),
                                &encoded));
@@ -90,7 +238,7 @@ TEST(PNGCodec, EncodeDecodeRGB) {
   // decode, it should have the same size as the original
   std::vector<unsigned char> decoded;
   int outw, outh;
-  EXPECT_TRUE(PNGCodec::Decode(&encoded[0], encoded.size(),
+  ASSERT_TRUE(PNGCodec::Decode(&encoded[0], encoded.size(),
                                PNGCodec::FORMAT_RGB, &decoded,
                                &outw, &outh));
   ASSERT_EQ(w, outw);
@@ -111,7 +259,7 @@ TEST(PNGCodec, EncodeDecodeRGBA) {
 
   // encode
   std::vector<unsigned char> encoded;
-  EXPECT_TRUE(PNGCodec::Encode(&original[0], PNGCodec::FORMAT_RGBA,
+  ASSERT_TRUE(PNGCodec::Encode(&original[0], PNGCodec::FORMAT_RGBA,
                                Size(w, h), w * 4, false,
                                std::vector<PNGCodec::Comment>(),
                                &encoded));
@@ -119,7 +267,7 @@ TEST(PNGCodec, EncodeDecodeRGBA) {
   // decode, it should have the same size as the original
   std::vector<unsigned char> decoded;
   int outw, outh;
-  EXPECT_TRUE(PNGCodec::Decode(&encoded[0], encoded.size(),
+  ASSERT_TRUE(PNGCodec::Decode(&encoded[0], encoded.size(),
                                PNGCodec::FORMAT_RGBA, &decoded,
                                &outw, &outh));
   ASSERT_EQ(w, outw);
@@ -130,6 +278,264 @@ TEST(PNGCodec, EncodeDecodeRGBA) {
   ASSERT_TRUE(original == decoded);
 }
 
+TEST(PNGCodec, EncodeDecodeBGRA) {
+  const int w = 20, h = 20;
+
+  // Create an image with known values, alpha must be opaque because it will be
+  // lost during encoding.
+  std::vector<unsigned char> original;
+  MakeRGBAImage(w, h, true, &original);
+
+  // Encode.
+  std::vector<unsigned char> encoded;
+  ASSERT_TRUE(PNGCodec::Encode(&original[0], PNGCodec::FORMAT_BGRA,
+                               Size(w, h), w * 4, false,
+                               std::vector<PNGCodec::Comment>(),
+                               &encoded));
+
+  // Decode, it should have the same size as the original.
+  std::vector<unsigned char> decoded;
+  int outw, outh;
+  ASSERT_TRUE(PNGCodec::Decode(&encoded[0], encoded.size(),
+                               PNGCodec::FORMAT_BGRA, &decoded,
+                               &outw, &outh));
+  ASSERT_EQ(w, outw);
+  ASSERT_EQ(h, outh);
+  ASSERT_EQ(original.size(), decoded.size());
+
+  // Images must be exactly equal.
+  ASSERT_TRUE(original == decoded);
+}
+
+TEST(PNGCodec, DecodeInterlacedRGB) {
+  const int w = 20, h = 20;
+
+  // create an image with known values
+  std::vector<unsigned char> original;
+  MakeRGBImage(w, h, &original);
+
+  // encode
+  std::vector<unsigned char> encoded;
+  ASSERT_TRUE(EncodeImage(original,
+                          w, h,
+                          COLOR_TYPE_RGB,
+                          &encoded,
+                          PNG_INTERLACE_ADAM7));
+
+  // decode, it should have the same size as the original
+  std::vector<unsigned char> decoded;
+  int outw, outh;
+  ASSERT_TRUE(PNGCodec::Decode(&encoded[0], encoded.size(),
+                               PNGCodec::FORMAT_RGB, &decoded,
+                               &outw, &outh));
+  ASSERT_EQ(w, outw);
+  ASSERT_EQ(h, outh);
+  ASSERT_EQ(original.size(), decoded.size());
+
+  // Images must be equal
+  ASSERT_EQ(original, decoded);
+}
+
+TEST(PNGCodec, DecodeInterlacedRGBA) {
+  const int w = 20, h = 20;
+
+  // create an image with known values
+  std::vector<unsigned char> original;
+  MakeRGBAImage(w, h, false, &original);
+
+  // encode
+  std::vector<unsigned char> encoded;
+  ASSERT_TRUE(EncodeImage(original,
+                          w, h,
+                          COLOR_TYPE_RGBA,
+                          &encoded,
+                          PNG_INTERLACE_ADAM7));
+
+  // decode, it should have the same size as the original
+  std::vector<unsigned char> decoded;
+  int outw, outh;
+  ASSERT_TRUE(PNGCodec::Decode(&encoded[0], encoded.size(),
+                               PNGCodec::FORMAT_RGBA, &decoded,
+                               &outw, &outh));
+  ASSERT_EQ(w, outw);
+  ASSERT_EQ(h, outh);
+  ASSERT_EQ(original.size(), decoded.size());
+
+  // Images must be equal
+  ASSERT_EQ(original, decoded);
+}
+
+TEST(PNGCodec, DecodeInterlacedRGBADiscardAlpha) {
+  const int w = 20, h = 20;
+
+  // create an image with known values
+  std::vector<unsigned char> original;
+  MakeRGBAImage(w, h, false, &original);
+
+  // encode
+  std::vector<unsigned char> encoded;
+  ASSERT_TRUE(EncodeImage(original,
+                          w, h,
+                          COLOR_TYPE_RGBA,
+                          &encoded,
+                          PNG_INTERLACE_ADAM7));
+
+  // decode
+  std::vector<unsigned char> decoded;
+  int outw, outh;
+  ASSERT_TRUE(PNGCodec::Decode(&encoded[0], encoded.size(),
+                               PNGCodec::FORMAT_RGB, &decoded,
+                               &outw, &outh));
+  ASSERT_EQ(w, outw);
+  ASSERT_EQ(h, outh);
+  ASSERT_EQ(decoded.size(), w * h * 3U);
+
+  // Images must be equal
+  for (int x = 0; x < w; x++) {
+    for (int y = 0; y < h; y++) {
+      unsigned char* orig_px = &original[(y * w + x) * 4];
+      unsigned char* dec_px = &decoded[(y * w + x) * 3];
+      ASSERT_EQ(dec_px[0], orig_px[0]);
+      ASSERT_EQ(dec_px[1], orig_px[1]);
+      ASSERT_EQ(dec_px[2], orig_px[2]);
+    }
+  }
+}
+
+TEST(PNGCodec, DecodeInterlacedBGR) {
+  const int w = 20, h = 20;
+
+  // create an image with known values
+  std::vector<unsigned char> original;
+  MakeRGBImage(w, h, &original);
+
+  // encode
+  std::vector<unsigned char> encoded;
+  ASSERT_TRUE(EncodeImage(original,
+                          w, h,
+                          COLOR_TYPE_BGR,
+                          &encoded,
+                          PNG_INTERLACE_ADAM7));
+
+  // decode, it should have the same size as the original
+  std::vector<unsigned char> decoded;
+  int outw, outh;
+  ASSERT_TRUE(PNGCodec::Decode(&encoded[0], encoded.size(),
+                               PNGCodec::FORMAT_BGRA, &decoded,
+                               &outw, &outh));
+  ASSERT_EQ(w, outw);
+  ASSERT_EQ(h, outh);
+  ASSERT_EQ(decoded.size(), w * h * 4U);
+
+  // Images must be equal
+  for (int x = 0; x < w; x++) {
+    for (int y = 0; y < h; y++) {
+      unsigned char* orig_px = &original[(y * w + x) * 3];
+      unsigned char* dec_px = &decoded[(y * w + x) * 4];
+      ASSERT_EQ(dec_px[0], orig_px[0]);
+      ASSERT_EQ(dec_px[1], orig_px[1]);
+      ASSERT_EQ(dec_px[2], orig_px[2]);
+    }
+  }
+}
+
+TEST(PNGCodec, DecodeInterlacedBGRA) {
+  const int w = 20, h = 20;
+
+  // create an image with known values
+  std::vector<unsigned char> original;
+  MakeRGBAImage(w, h, false, &original);
+
+  // encode
+  std::vector<unsigned char> encoded;
+  ASSERT_TRUE(EncodeImage(original,
+                          w, h,
+                          COLOR_TYPE_BGRA,
+                          &encoded,
+                          PNG_INTERLACE_ADAM7));
+
+  // decode, it should have the same size as the original
+  std::vector<unsigned char> decoded;
+  int outw, outh;
+  ASSERT_TRUE(PNGCodec::Decode(&encoded[0], encoded.size(),
+                               PNGCodec::FORMAT_BGRA, &decoded,
+                               &outw, &outh));
+  ASSERT_EQ(w, outw);
+  ASSERT_EQ(h, outh);
+  ASSERT_EQ(original.size(), decoded.size());
+
+  // Images must be equal
+  ASSERT_EQ(original, decoded);
+}
+
+// Not encoding an interlaced PNG from SkBitmap because we don't do it
+// anywhere, and the ability to do that requires more code changes.
+TEST(PNGCodec, DecodeInterlacedRGBtoSkBitmap) {
+  const int w = 20, h = 20;
+
+  // create an image with known values
+  std::vector<unsigned char> original;
+  MakeRGBImage(w, h, &original);
+
+  // encode
+  std::vector<unsigned char> encoded;
+  ASSERT_TRUE(EncodeImage(original,
+                          w, h,
+                          COLOR_TYPE_RGB,
+                          &encoded,
+                          PNG_INTERLACE_ADAM7));
+
+  // Decode the encoded string.
+  SkBitmap decoded_bitmap;
+  ASSERT_TRUE(PNGCodec::Decode(&encoded.front(), encoded.size(),
+                               &decoded_bitmap));
+
+  for (int x = 0; x < w; x++) {
+    for (int y = 0; y < h; y++) {
+      const unsigned char* original_pixel = &original[(y * w + x) * 3];
+      const uint32_t original_pixel_sk = SkPackARGB32(0xFF,
+                                                      original_pixel[0],
+                                                      original_pixel[1],
+                                                      original_pixel[2]);
+      const uint32_t decoded_pixel = decoded_bitmap.getAddr32(0, y)[x];
+      ASSERT_EQ(original_pixel_sk, decoded_pixel);
+    }
+  }
+}
+
+TEST(PNGCodec, DecodeInterlacedRGBAtoSkBitmap) {
+  const int w = 20, h = 20;
+
+  // create an image with known values
+  std::vector<unsigned char> original;
+  MakeRGBAImage(w, h, false, &original);
+
+  // encode
+  std::vector<unsigned char> encoded;
+  ASSERT_TRUE(EncodeImage(original,
+                          w, h,
+                          COLOR_TYPE_RGBA,
+                          &encoded,
+                          PNG_INTERLACE_ADAM7));
+
+  // Decode the encoded string.
+  SkBitmap decoded_bitmap;
+  ASSERT_TRUE(PNGCodec::Decode(&encoded.front(), encoded.size(),
+                               &decoded_bitmap));
+
+  for (int x = 0; x < w; x++) {
+    for (int y = 0; y < h; y++) {
+      const unsigned char* original_pixel = &original[(y * w + x) * 4];
+      const uint32_t original_pixel_sk = SkPackARGB32(original_pixel[3],
+                                                      original_pixel[0],
+                                                      original_pixel[1],
+                                                      original_pixel[2]);
+      const uint32_t decoded_pixel = decoded_bitmap.getAddr32(0, y)[x];
+      ASSERT_EQ(original_pixel_sk, decoded_pixel);
+    }
+  }
+}
+
 // Test that corrupted data decompression causes failures.
 TEST(PNGCodec, DecodeCorrupted) {
   int w = 20, h = 20;
@@ -147,7 +553,7 @@ TEST(PNGCodec, DecodeCorrupted) {
 
   // Make some compressed data.
   std::vector<unsigned char> compressed;
-  EXPECT_TRUE(PNGCodec::Encode(&original[0], PNGCodec::FORMAT_RGB,
+  ASSERT_TRUE(PNGCodec::Encode(&original[0], PNGCodec::FORMAT_RGB,
                                Size(w, h), w * 3, false,
                                std::vector<PNGCodec::Comment>(),
                                &compressed));
@@ -165,35 +571,6 @@ TEST(PNGCodec, DecodeCorrupted) {
                                 &outw, &outh));
 }
 
-TEST(PNGCodec, EncodeDecodeBGRA) {
-  const int w = 20, h = 20;
-
-  // Create an image with known values, alpha must be opaque because it will be
-  // lost during encoding.
-  std::vector<unsigned char> original;
-  MakeRGBAImage(w, h, true, &original);
-
-  // Encode.
-  std::vector<unsigned char> encoded;
-  EXPECT_TRUE(PNGCodec::Encode(&original[0], PNGCodec::FORMAT_BGRA,
-                               Size(w, h), w * 4, false,
-                               std::vector<PNGCodec::Comment>(),
-                               &encoded));
-
-  // Decode, it should have the same size as the original.
-  std::vector<unsigned char> decoded;
-  int outw, outh;
-  EXPECT_TRUE(PNGCodec::Decode(&encoded[0], encoded.size(),
-                               PNGCodec::FORMAT_BGRA, &decoded,
-                               &outw, &outh));
-  ASSERT_EQ(w, outw);
-  ASSERT_EQ(h, outh);
-  ASSERT_EQ(original.size(), decoded.size());
-
-  // Images must be exactly equal.
-  ASSERT_TRUE(original == decoded);
-}
-
 TEST(PNGCodec, StripAddAlpha) {
   const int w = 20, h = 20;
 
@@ -221,7 +598,7 @@ TEST(PNGCodec, StripAddAlpha) {
   ASSERT_EQ(w, outw);
   ASSERT_EQ(h, outh);
   ASSERT_EQ(original_rgba.size(), decoded.size());
-  ASSERT_TRUE(original_rgba == decoded);
+  ASSERT_EQ(original_rgba, decoded);
 
   // Encode RGBA to RGBA.
   EXPECT_TRUE(PNGCodec::Encode(&original_rgba[0], PNGCodec::FORMAT_RGBA,
@@ -238,7 +615,7 @@ TEST(PNGCodec, StripAddAlpha) {
   ASSERT_EQ(w, outw);
   ASSERT_EQ(h, outh);
   ASSERT_EQ(original_rgb.size(), decoded.size());
-  ASSERT_TRUE(original_rgb == decoded);
+  ASSERT_EQ(original_rgb, decoded);
 }
 
 TEST(PNGCodec, EncodeBGRASkBitmap) {