Add unit test for color managed ImageBitamp(ImageBitmap) constructor

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

Index: third_party/WebKit/Source/core/frame/ImageBitmap.cpp
diff --git a/third_party/WebKit/Source/core/frame/ImageBitmap.cpp b/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
index feaa25995291526cdc0937ba73b09dacdba9bd2e..560a03b8393d4baaf59fe7ef4d4eff4f61d1827a 100644
--- a/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
+++ b/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
@@ -610,7 +610,7 @@ ImageBitmap::ImageBitmap(HTMLVideoElement* video,
 
   std::unique_ptr<ImageBuffer> buffer = ImageBuffer::create(
       IntSize(parsedOptions.resizeWidth, parsedOptions.resizeHeight), NonOpaque,
-      DoNotInitializeImagePixels);
+      DoNotInitializeImagePixels, parsedOptions.dstColorSpace);
   if (!buffer)
     return;
 
@@ -687,6 +687,8 @@ ImageBitmap::ImageBitmap(HTMLCanvasElement* canvas,
   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,
@@ -706,9 +708,11 @@ ImageBitmap::ImageBitmap(const void* pixelData,
 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)
@@ -726,6 +730,8 @@ static sk_sp<SkImage> scaleSkImage(sk_sp<SkImage> skImage,
                                  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) {
@@ -991,13 +997,23 @@ ImageBitmap* ImageBitmap::take(ScriptPromiseResolver*, sk_sp<SkImage> 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(),