Use SkPngEncoder and SkWebpEncoder in WebKit platform

third_party/WebKit/Source/platform/graphics/ImageBuffer.cpp

Index: third_party/WebKit/Source/platform/graphics/ImageBuffer.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/ImageBuffer.cpp b/third_party/WebKit/Source/platform/graphics/ImageBuffer.cpp
index 287c350d222eacfe5211bda71b87f54524378618..e0b4c02f01dc6bb371b78df185d0b79b764518a2 100644
--- a/third_party/WebKit/Source/platform/graphics/ImageBuffer.cpp
+++ b/third_party/WebKit/Source/platform/graphics/ImageBuffer.cpp
@@ -50,8 +50,6 @@
 #include "platform/graphics/paint/PaintRecord.h"
 #include "platform/graphics/skia/SkiaUtils.h"
 #include "platform/image-encoders/ImageEncoder.h"
-#include "platform/image-encoders/PNGImageEncoder.h"
-#include "platform/image-encoders/WEBPImageEncoder.h"
 #include "platform/network/mime/MIMETypeRegistry.h"
 #include "platform/wtf/CheckedNumeric.h"
 #include "platform/wtf/MathExtras.h"
@@ -546,14 +544,13 @@ void ImageBuffer::SetNeedsCompositingUpdate() {
 bool ImageDataBuffer::EncodeImage(const String& mime_type,
                                   const double& quality,
                                   Vector<unsigned char>* encoded_image) const {
-  if (mime_type == "image/jpeg") {
-    SkImageInfo info =
-        SkImageInfo::Make(Width(), Height(), kRGBA_8888_SkColorType,
-                          kUnpremul_SkAlphaType, nullptr);
-    size_t rowBytes =
-        Width() * SkColorTypeBytesPerPixel(kRGBA_8888_SkColorType);
-    SkPixmap src(info, Pixels(), rowBytes);
+  SkImageInfo info =
+      SkImageInfo::Make(Width(), Height(), kRGBA_8888_SkColorType,

[scroggo_chromium, 2017/05/22 13:35:04]

It seems weird to me that the color type and alpha type are hardcoded, but it
looks like there's an implicit contract about using ImageDataBuffer.

[msarett1, 2017/05/22 15:33:56]

Yeah that seems to be part of the implicit contract.  Now that the image encoder
supports SkImageInfos, this could become more flexible.

But that might be better for another change.

+                        kUnpremul_SkAlphaType, nullptr);
+  size_t rowBytes = Width() * SkColorTypeBytesPerPixel(kRGBA_8888_SkColorType);

[scroggo_chromium, 2017/05/22 13:35:04]

Should this be info.minRowBytes?

Also, nit: Could be const?

[msarett1, 2017/05/22 15:33:56]

Yeah, that's better.

+  SkPixmap src(info, Pixels(), rowBytes);

[scroggo_chromium, 2017/05/22 13:35:04]

This kind of seems like it should be in the constructor. That said, it seems
like we really just want two functions (one that encodes, and the other that
writes to DataUrl); the only callsites I saw use a temporary object.

No need to address in this CL, but I wonder if we should clean this up someday?

[msarett1, 2017/05/22 15:33:56]

I agree.  This could/should just be two methods.  Will consider a future
cleanup.

 
+  if (mime_type == "image/jpeg") {
     SkJpegEncoder::Options options;
     options.fQuality = ImageEncoder::ComputeJpegQuality(quality);
     options.fAlphaOption = SkJpegEncoder::AlphaOption::kBlendOnBlack;
@@ -565,14 +562,17 @@ bool ImageDataBuffer::EncodeImage(const String& mime_type,
   }
 
   if (mime_type == "image/webp") {
-    int compression_quality = WEBPImageEncoder::kDefaultCompressionQuality;
-    if (quality >= 0.0 && quality <= 1.0)
-      compression_quality = static_cast<int>(quality * 100 + 0.5);
-    return WEBPImageEncoder::Encode(*this, compression_quality, encoded_image);
+    SkWebpEncoder::Options options = ImageEncoder::ComputeWebpOptions(
+        quality, SkTransferFunctionBehavior::kIgnore);
+    return ImageEncoder::Encode(encoded_image, src, options);
   }
 
   DCHECK_EQ(mime_type, "image/png");
-  return PNGImageEncoder::Encode(*this, encoded_image);
+  SkPngEncoder::Options options;
+  options.fFilterFlags = SkPngEncoder::FilterFlag::kSub;
+  options.fZLibLevel = 3;
+  options.fUnpremulBehavior = SkTransferFunctionBehavior::kIgnore;
+  return ImageEncoder::Encode(encoded_image, src, options);
 }
 
 String ImageDataBuffer::ToDataURL(const String& mime_type,