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

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

 /*
  * Copyright (C) Research In Motion Limited 2009-2010. All rights reserved.
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Library General Public
  *  License as published by the Free Software Foundation; either
  *  version 2 of the License, or (at your option) any later version.
  *
  *  This library is distributed in the hope that it will be useful,
  *  but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 53 matching lines @@
 }
 
 // This needs to be updated if we ever add a matches*Signature() which requires
 // more characters.
 static constexpr size_t kLongestSignatureLength = sizeof("RIFF????WEBPVP") - 1;
 
 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
   // matcher.
   if (data->size() < kLongestSignatureLength)
     return nullptr;
 
   const size_t maxDecodedBytes =
       Platform::current() ? Platform::current()->maxDecodedImageBytes()
                           : noDecodedImageByteLimit;
@@ skipping 267 matching lines @@
     return;
 
   gTargetColorSpace =
       SkColorSpace::NewICC(profile.data(), profile.size()).release();
 
   // UMA statistics.
   BitmapImageMetrics::countGamma(gTargetColorSpace);
 }
 
 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
   // SkImage to Skia.
   if (RuntimeEnabledFeatures::colorCorrectRenderingEnabled())
     return;
 
   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);
 
   // Initialize the output device profile to sRGB if it has not yet been
   // initialized.
   if (!gTargetColorSpace) {
     gTargetColorSpace =
         SkColorSpace::NewNamed(SkColorSpace::kSRGB_Named).release();
   }
 
   if (SkColorSpace::Equals(srcSpace.get(), gTargetColorSpace)) {
     return;
   }
 
   m_sourceToOutputDeviceColorTransform =
       SkColorSpaceXform::New(srcSpace.get(), gTargetColorSpace);
 }
 
 }  // namespace blink