Refactor image decoders to use 'colorSpace' instead of 'colorProfile'

third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp

Index: third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp
diff --git a/third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp b/third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp
index c397e7bcf1af164c711aaa7df78f4c5f4f835eef..40b96118463e07fd3e2c8a89c65dca00c5324a01 100644
--- a/third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp
+++ b/third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp
@@ -71,7 +71,7 @@ std::unique_ptr<ImageDecoder> ImageDecoder::create(
     PassRefPtr<SegmentReader> passData,
     bool dataComplete,
     AlphaOption alphaOption,
-    GammaAndColorProfileOption colorOptions) {
+    ColorSpaceOption colorOptions) {
   RefPtr<SegmentReader> data = passData;
 
   // We need at least kLongestSignatureLength bytes to run the signature
@@ -359,14 +359,22 @@ void ImageDecoder::setTargetColorProfile(const WebVector<char>& profile) {
 }
 
 void ImageDecoder::setColorSpaceAndComputeTransform(const char* iccData,
-                                                    unsigned iccLength,
-                                                    bool useSRGB) {
+                                                    unsigned iccLength) {
   // Sub-classes should not call this if they were instructed to ignore embedded
   // color profiles.
-  DCHECK(!m_ignoreGammaAndColorProfile);
+  DCHECK(!m_ignoreColorSpace);
 
-  m_colorProfile.assign(iccData, iccLength);
-  m_hasColorProfile = true;
+  sk_sp<SkColorSpace> srcSpace = SkColorSpace::NewICC(iccData, iccLength);
+  setColorSpaceAndComputeTransform(std::move(srcSpace));

[f(malita), 2016/10/27 17:57:24]

nit: I would just inline these and skip the move.

[msarett, 2016/10/27 18:21:42]

sgtm

+}
+
+void ImageDecoder::setColorSpaceAndComputeTransform(
+    sk_sp<SkColorSpace> srcSpace) {
+  if (!srcSpace) {

[f(malita), 2016/10/27 17:57:24]

Why is this early out needed?  Don't we want to clear m_srcSpace in this case?

[msarett, 2016/10/27 18:21:42]

I think it makes sense to check before attempting to acquire the SpinLock, but I
agree that this is too soon to check.  I'll move this back where it was before.

Also, I think it makes sense to set the transform to nullptr up at the top.

+    return;
+  }
+
+  m_srcSpace = srcSpace;
 
   // With color correct rendering, we do not transform to the output color space
   // at decode time.  Instead, we tag the raw image pixels and pass the tagged
@@ -376,17 +384,6 @@ void ImageDecoder::setColorSpaceAndComputeTransform(const char* iccData,
 
   m_sourceToOutputDeviceColorTransform = nullptr;
 
-  // Create the input profile.
-  sk_sp<SkColorSpace> srcSpace = nullptr;
-  if (useSRGB) {
-    srcSpace = SkColorSpace::NewNamed(SkColorSpace::kSRGB_Named);
-  } else {
-    srcSpace = SkColorSpace::NewICC(iccData, iccLength);
-  }
-
-  if (!srcSpace)
-    return;
-
   // Take a lock around initializing and accessing the global device color
   // profile.
   SpinLock::Guard guard(gTargetColorSpaceLock);