Respect colorSpace in DecodingImageGenerator::onGetPixels()

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

 /*
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 93 matching lines @@
 }
 
 ImageFrameGenerator::ImageFrameGenerator(const SkISize& fullSize,
                                          bool isMultiFrame,
                                          const ColorBehavior& colorBehavior)
     : m_fullSize(fullSize),
       m_decoderColorBehavior(colorBehavior),
       m_isMultiFrame(isMultiFrame),
       m_decodeFailed(false),
       m_yuvDecodingFailed(false),
-      m_frameCount(0) {}
+      m_frameCount(0),
+      m_alphaOption(ImageDecoder::AlphaPremultiplied) {}
 
 ImageFrameGenerator::~ImageFrameGenerator() {
   ImageDecodingStore::instance().removeCacheIndexedByGenerator(this);
 }
 
 bool ImageFrameGenerator::decodeAndScale(SegmentReader* data,
                                          bool allDataReceived,
                                          size_t index,
                                          const SkImageInfo& info,
                                          void* pixels,
@@ skipping 163 matching lines @@
   // Try to create an ImageDecoder if we are not given one.
   ASSERT(decoder);
   bool newDecoder = false;
   bool shouldCallSetData = true;
   if (!*decoder) {
     newDecoder = true;
     if (m_imageDecoderFactory)
       *decoder = m_imageDecoderFactory->create().release();
 
     if (!*decoder) {
-      *decoder = ImageDecoder::create(data, allDataReceived,
-                                      ImageDecoder::AlphaPremultiplied,
+      *decoder = ImageDecoder::create(data, allDataReceived, m_alphaOption,
                                       m_decoderColorBehavior)
                      .release();
       // The newly created decoder just grabbed the data.  No need to reset it.
       shouldCallSetData = false;
     }
 
     if (!*decoder)
       return false;
   }
 
+  DCHECK((*decoder)->frameBufferAtIndex(index)->premultiplyAlpha() ==

[msarett1, 2017/04/04 23:02:36]

I'm expecting the DCHECK() to warn us if we've already created a decoder with
the wrong alpha behavior.

[scroggo_chromium, 2017/04/05 19:59:01]

frameBufferAtIndex may trigger a decode, so you don't want to call it inside
DCHECK. (And you don't want to trigger the decode until later, either.) And
frameBufferAtIndex might return null, for example in the case where we haven't
set the data yet.

Instead, perhaps calling setAlphaOption should tell the ImageFrameGenerator to
use a different ImageDecoder? Maybe the AlphaOption should be added to the key
for the cache of ImageDecoder in ImageDecodingStore::lockDecoder?

(https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/graph...)

Come to think of it, would it make sense to move code you're adding here instead
of in DecodingImageGenerator? You already pass an SkImageInfo to the
ImageFrameGenerator inside decodeAndScale. It uses the original SkImageInfo
instead of the dstInfo, but maybe it could use the dstInfo and
ImageFrameGenerator can decide how to handle the conversion?

[msarett1, 2017/04/06 22:05:29]

This all sgtm.  Thanks for your thoughts.

+         (m_alphaOption == ImageDecoder::AlphaPremultiplied));
 
   if (shouldCallSetData)
     (*decoder)->setData(data, allDataReceived);
 
   bool usingExternalAllocator = false;
 
   // For multi-frame image decoders, we need to know how many frames are
   // in that image in order to release the decoder when all frames are
   // decoded. frameCount() is reliable only if all data is received and set in
   // decoder, particularly with GIF.
@@ skipping 63 matching lines @@
   // do YUV decoding.
   std::unique_ptr<ImagePlanes> dummyImagePlanes =
       WTF::wrapUnique(new ImagePlanes);
   decoder->setImagePlanes(std::move(dummyImagePlanes));
 
   return updateYUVComponentSizes(decoder.get(), sizeInfo->fSizes,
                                  sizeInfo->fWidthBytes);
 }
 
 }  // namespace blink