Add unit test for color managed ImageBitamp(ImageBitmap) constructor

third_party/WebKit/Source/core/frame/ImageBitmap.cpp

 // Copyright 2013 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.
 
 #include "core/frame/ImageBitmap.h"
 
 #include "core/html/HTMLCanvasElement.h"
 #include "core/html/HTMLVideoElement.h"
 #include "core/html/ImageData.h"
 #include "platform/graphics/skia/SkiaUtils.h"
@@ skipping 592 matching lines @@
   IntSize playerSize;
   if (video->webMediaPlayer())
     playerSize = video->webMediaPlayer()->naturalSize();
   ParsedOptions parsedOptions =
       parseOptions(options, cropRect, video->bitmapSourceSize());
   if (dstBufferSizeHasOverflow(parsedOptions))
     return;
 
   std::unique_ptr<ImageBuffer> buffer = ImageBuffer::create(
       IntSize(parsedOptions.resizeWidth, parsedOptions.resizeHeight), NonOpaque,
-      DoNotInitializeImagePixels);
+      DoNotInitializeImagePixels, parsedOptions.dstColorSpace);
   if (!buffer)
     return;
 
   IntPoint dstPoint =
       IntPoint(-parsedOptions.cropRect.x(), -parsedOptions.cropRect.y());
   if (parsedOptions.flipY) {
     buffer->canvas()->translate(0, buffer->size().height());
     buffer->canvas()->scale(1, -1);
   }
   SkPaint paint;
@@ skipping 56 matching lines @@
     m_image = StaticBitmapImage::create(premulSkImageToUnPremul(
         m_image->imageForCurrentFrame(ColorBehavior::transformToGlobalTarget())
             .get()));
   }
   if (!m_image)
     return;
   m_image->setOriginClean(canvas->originClean());
   m_image->setPremultiplied(parsedOptions.premultiplyAlpha);
 }
 
+// TODO(zakerinasab): Support color space conversion in this constructor when
+// the new serialization module is ready. crbug.com/670703
 ImageBitmap::ImageBitmap(const void* pixelData,
                          uint32_t width,
                          uint32_t height,
                          bool isImageBitmapPremultiplied,
                          bool isImageBitmapOriginClean) {
   SkImageInfo info = SkImageInfo::MakeN32(
       width, height,
       isImageBitmapPremultiplied ? kPremul_SkAlphaType : kUnpremul_SkAlphaType);
   SkPixmap pixmap(info, pixelData, info.bytesPerPixel() * width);
   m_image = StaticBitmapImage::create(SkImage::MakeRasterCopy(pixmap));
   if (!m_image)
     return;
   m_image->setPremultiplied(isImageBitmapPremultiplied);
   m_image->setOriginClean(isImageBitmapOriginClean);
 }
 
 static sk_sp<SkImage> scaleSkImage(sk_sp<SkImage> skImage,
                                    unsigned resizeWidth,
                                    unsigned resizeHeight,
-                                   SkFilterQuality resizeQuality) {
+                                   SkFilterQuality resizeQuality,
+                                   SkColorType colorType = kN32_SkColorType,
+                                   sk_sp<SkColorSpace> colorSpace = nullptr) {
   SkImageInfo resizedInfo = SkImageInfo::Make(
-      resizeWidth, resizeHeight, kN32_SkColorType, kUnpremul_SkAlphaType);
+      resizeWidth, resizeHeight, colorType, kUnpremul_SkAlphaType, colorSpace);
   RefPtr<ArrayBuffer> dstBuffer = ArrayBuffer::createOrNull(
       resizeWidth * resizeHeight, resizedInfo.bytesPerPixel());
   if (!dstBuffer)
     return nullptr;
   RefPtr<Uint8Array> resizedPixels =
       Uint8Array::create(dstBuffer, 0, dstBuffer->byteLength());
   SkPixmap pixmap(
       resizedInfo, resizedPixels->data(),
       static_cast<unsigned>(resizeWidth) * resizedInfo.bytesPerPixel());
   skImage->scalePixels(pixmap, resizeQuality);
   return SkImage::MakeFromRaster(pixmap,
                                  [](const void*, void* pixels) {
                                    static_cast<Uint8Array*>(pixels)->deref();
                                  },
                                  resizedPixels.release().leakRef());
 }
 
+// TODO(zakerinasab): Add color space conversion to this constructor when
+// ImageData supports color spaces. crbug.com/670715.

[Justin Novosad, 2016/12/14 20:53:46]

It seems this constructor is now in a half fixed state: the code paths that call
scaleSkImage will have their colors converted.  Is the right? Could you document
this in the comment?

[zakerinasab1, 2016/12/16 20:25:34]

No, but it can be if we pass the color space parameter. I left it null to not
leave the constructor half fixed.

 ImageBitmap::ImageBitmap(ImageData* data,
                          Optional<IntRect> cropRect,
                          const ImageBitmapOptions& options) {
   // TODO(xidachen): implement the resize option
   IntRect dataSrcRect = IntRect(IntPoint(), data->size());
   ParsedOptions parsedOptions =
       parseOptions(options, cropRect, data->bitmapSourceSize());
   if (dstBufferSizeHasOverflow(parsedOptions))
     return;
   IntRect srcRect = cropRect ? intersection(parsedOptions.cropRect, dataSrcRect)
@@ skipping 245 matching lines @@
   m_isNeutered = true;
 }
 
 // static
 ImageBitmap* ImageBitmap::take(ScriptPromiseResolver*, sk_sp<SkImage> image) {
   return ImageBitmap::create(StaticBitmapImage::create(std::move(image)));
 }
 
 PassRefPtr<Uint8Array> ImageBitmap::copyBitmapData(AlphaDisposition alphaOp,
                                                    DataColorFormat format) {
-  SkImageInfo info = SkImageInfo::Make(
-      width(), height(),
-      (format == RGBAColorType) ? kRGBA_8888_SkColorType : kN32_SkColorType,
-      (alphaOp == PremultiplyAlpha) ? kPremul_SkAlphaType
-                                    : kUnpremul_SkAlphaType);
-  // TODO(ccameron): Canvas should operate in sRGB and not display space.
-  // https://crbug.com/667431
+  SkColorType colorType = kRGBA_8888_SkColorType;
+  if (format == N32ColorType)
+    colorType = kN32_SkColorType;
+  else if (format == F16ColorType)
+    colorType = kRGBA_F16_SkColorType;
+
+  // TODO(zakerinasab): Fix this to use the SkColorSpace from
+  // m_image->imageForCurrentFrame() instead of nullptr. crbug.com/671356.
+  sk_sp<SkColorSpace> colorSpace = nullptr;
+  SkImageInfo info =
+      SkImageInfo::Make(width(), height(), colorType,
+                        (alphaOp == PremultiplyAlpha) ? kPremul_SkAlphaType
+                                                      : kUnpremul_SkAlphaType,
+                        colorSpace);
+  // TODO(zakerinasab): Bitmap data must be returned in the current color space

[Justin Novosad, 2016/12/14 20:53:46]

Note: This will require making StaticBitmapImage and its subclasses to not
ignore the ColorBehavior argument of imageForCurrentFrame.  You could use the
caching behavior that ccameron put in BitmapImage::imageForCurrentFrame, perhaps
move it up into Image so that other subclasses of Image can use it.

[zakerinasab1, 2016/12/16 20:25:34]

I added your comments to the TODO to be considered when implementing.

+  // of the ImageBitmap and not display space. crbug.com/671356.
+  // When this is fixed, also fix ShapeDetector::detect(...).
   RefPtr<Uint8Array> dstPixels = copySkImageData(
       m_image->imageForCurrentFrame(ColorBehavior::transformToGlobalTarget())
           .get(),
       info);
   return dstPixels.release();
 }
 
 unsigned long ImageBitmap::width() const {
   if (!m_image)
     return 0;
@@ skipping 47 matching lines @@
 void ImageBitmap::adjustDrawRects(FloatRect* srcRect,
                                   FloatRect* dstRect) const {}
 
 FloatSize ImageBitmap::elementSize(const FloatSize&) const {
   return FloatSize(width(), height());
 }
 
 DEFINE_TRACE(ImageBitmap) {}
 
 }  // namespace blink