Deferred GIF image decodes should report decode failures

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 86 matching lines @@
     componentSizes[1].set(size.width(), size.height());
     size = decoder->decodedYUVSize(2, sizeType);
     componentSizes[2].set(size.width(), size.height());
     return true;
 }
 
 ImageFrameGenerator::ImageFrameGenerator(const SkISize& fullSize, PassRefPtr<SharedBuffer> data, bool allDataReceived, bool isMultiFrame)
     : m_fullSize(fullSize)
     , m_data(adoptRef(new ThreadSafeDataTransport()))
     , m_isMultiFrame(isMultiFrame)
-    , m_decodeFailedAndEmpty(false)
-    , m_decodeCount(0)
+    , m_decodeFailed(false)
     , m_frameCount(0)
     , m_encodedData(nullptr)
 {
     setData(data.get(), allDataReceived);
 }
 
 ImageFrameGenerator::~ImageFrameGenerator()
 {
     if (m_encodedData)
         m_encodedData->unref();
@@ skipping 49 matching lines @@
         // While m_encodedData is referenced, prevent disposing m_data and its content.
         // it is dereferenced in sharedSkDataReleaseCallback, called when m_encodedData gets dereferenced.
         m_data->ref();
     }
     // Increase the reference, caller must decrease it. One reference is always kept by ImageFrameGenerator and released
     // in destructor.
     m_encodedData->ref();
     return m_encodedData;
 }
 
-bool ImageFrameGenerator::decodeAndScale(const SkImageInfo& info, size_t index, void* pixels, size_t rowBytes)
+bool ImageFrameGenerator::decodeAndScale(size_t index, const SkImageInfo& info, void* pixels, size_t rowBytes)
 {
-    // This method is called to populate a discardable memory owned by Skia.
+    // Prevent concurrent decode or scale operations on the same image data.
+    MutexLocker lock(m_decodeMutex);
 
-    // Prevents concurrent decode or scale operations on the same image data.
-    MutexLocker lock(m_decodeMutex);
+    if (m_decodeFailed)
+        return false;
+
+    TRACE_EVENT1("blink", "ImageFrameGenerator::decodeAndScale", "frame index", static_cast<int>(index));
+
+    m_externalAllocator = adoptPtr(new ExternalMemoryAllocator(info, pixels, rowBytes));
 
     // This implementation does not support scaling so check the requested size.
     SkISize scaledSize = SkISize::Make(info.width(), info.height());
     ASSERT(m_fullSize == scaledSize);
 
-    if (m_decodeFailedAndEmpty)
-        return false;
-
-    TRACE_EVENT2("blink", "ImageFrameGenerator::decodeAndScale", "generator", this, "decodeCount", m_decodeCount);
-
-    m_externalAllocator = adoptPtr(new ExternalMemoryAllocator(info, pixels, rowBytes));
-
-    SkBitmap bitmap = tryToResumeDecode(scaledSize, index);
+    SkBitmap bitmap = tryToResumeDecode(index, scaledSize);
     if (bitmap.isNull())
         return false;
 
     // Don't keep the allocator because it contains a pointer to memory
     // that we do not own.
     m_externalAllocator.clear();
 
+    // Check to see if the decoder has written directly to the memory provided
+    // by Skia. If not, make a copy.

[scroggo_chromium, 2015/12/18 18:12:06]

I support removing the comment above ("This method is called to populate a
discardable memory owned by Skia") for the exact reason you mentioned - this
method populates whatever memory was handed to it at |pixels|. I think this
comment should be modified for the same reason. You can just remove "by Skia".

[Noel Gordon, 2015/12/21 03:23:22]

Thanks Leon, done.  Removed the "by Skia" bit.

     ASSERT(bitmap.width() == scaledSize.width());
     ASSERT(bitmap.height() == scaledSize.height());
-
-    bool result = true;
     SkAutoLockPixels bitmapLock(bitmap);
-    // Check to see if decoder has written directly to the memory provided
-    // by Skia. If not make a copy.
     if (bitmap.getPixels() != pixels)
-        result = bitmap.copyPixelsTo(pixels, rowBytes * info.height(), rowBytes);
-    return result;
+        return bitmap.copyPixelsTo(pixels, rowBytes * info.height(), rowBytes);
+    return true;
 }
 
-bool ImageFrameGenerator::decodeToYUV(SkISize componentSizes[3], void* planes[3], size_t rowBytes[3])
+bool ImageFrameGenerator::decodeToYUV(size_t index, SkISize componentSizes[3], void* planes[3], size_t rowBytes[3])
 {
-    // This method is called to populate a discardable memory owned by Skia.
-
-    // Prevents concurrent decode or scale operations on the same image data.
+    // Prevent concurrent decode or scale operations on the same image data.
     MutexLocker lock(m_decodeMutex);
 
-    if (m_decodeFailedAndEmpty)
+    if (m_decodeFailed)
         return false;
 
-    TRACE_EVENT2("blink", "ImageFrameGenerator::decodeToYUV", "generator", this, "decodeCount", static_cast<int>(m_decodeCount));
+    TRACE_EVENT1("blink", "ImageFrameGenerator::decodeToYUV", "frame index", static_cast<int>(index));
 
     if (!planes || !planes[0] || !planes[1] || !planes[2]
         || !rowBytes || !rowBytes[0] || !rowBytes[1] || !rowBytes[2]) {
         return false;
     }
 
     SharedBuffer* data = 0;
     bool allDataReceived = false;
     m_data->data(&data, &allDataReceived);
 
     // FIXME: YUV decoding does not currently support progressive decoding.
     ASSERT(allDataReceived);
 
     OwnPtr<ImageDecoder> decoder = ImageDecoder::create(*data, ImageDecoder::AlphaPremultiplied, ImageDecoder::GammaAndColorProfileApplied);
     if (!decoder)
         return false;
 
     decoder->setData(data, allDataReceived);
 
     OwnPtr<ImagePlanes> imagePlanes = adoptPtr(new ImagePlanes(planes, rowBytes));
     decoder->setImagePlanes(imagePlanes.release());
 
     bool sizeUpdated = updateYUVComponentSizes(decoder.get(), componentSizes, ImageDecoder::ActualSize);
     RELEASE_ASSERT(sizeUpdated);
 
-    bool yuvDecoded = decoder->decodeToYUV();
-    if (yuvDecoded)
+    if (decoder->decodeToYUV()) {
         setHasAlpha(0, false); // YUV is always opaque
-    return yuvDecoded;
+        return true;
+    }
+
+    ASSERT(decoder->failed());
+    m_decodeFailed = true;
+    return false;
 }
 
-SkBitmap ImageFrameGenerator::tryToResumeDecode(const SkISize& scaledSize, size_t index)
+SkBitmap ImageFrameGenerator::tryToResumeDecode(size_t index, const SkISize& scaledSize)
 {
-    TRACE_EVENT1("blink", "ImageFrameGenerator::tryToResumeDecodeAndScale", "index", static_cast<int>(index));
+    TRACE_EVENT1("blink", "ImageFrameGenerator::tryToResumeDecode", "frame index", static_cast<int>(index));
 
     ImageDecoder* decoder = 0;
     const bool resumeDecoding = ImageDecodingStore::instance().lockDecoder(this, m_fullSize, &decoder);
     ASSERT(!resumeDecoding || decoder);
 
     SkBitmap fullSizeImage;
     bool complete = decode(index, &decoder, &fullSizeImage);
 
     if (!decoder)
         return SkBitmap();
     if (index >= m_frameComplete.size())
         m_frameComplete.resize(index + 1);
     m_frameComplete[index] = complete;
 
     // If we are not resuming decoding that means the decoder is freshly
     // created and we have ownership. If we are resuming decoding then
     // the decoder is owned by ImageDecodingStore.
     OwnPtr<ImageDecoder> decoderContainer;
     if (!resumeDecoding)
         decoderContainer = adoptPtr(decoder);
 
     if (fullSizeImage.isNull()) {
-        // If decode has failed and resulted an empty image we can save work
-        // in the future by returning early.
-        m_decodeFailedAndEmpty = !m_isMultiFrame && decoder->failed();
-
+        // If decoding has failed, we can save work in the future by
+        // ignoring further requests to decode the image.
+        m_decodeFailed = decoder->failed();
         if (resumeDecoding)
             ImageDecodingStore::instance().unlockDecoder(this, decoder);
         return SkBitmap();
     }
 
     // If the image generated is complete then there is no need to keep
     // the decoder. For multi-frame images, if all frames in the image are
     // decoded, we remove the decoder.
     bool removeDecoder;
 
@@ skipping 30 matching lines @@
         for (size_t i = oldSize; i < m_hasAlpha.size(); ++i)
             m_hasAlpha[i] = true;
     }
     m_hasAlpha[index] = hasAlpha;
 }
 
 bool ImageFrameGenerator::decode(size_t index, ImageDecoder** decoder, SkBitmap* bitmap)
 {
     TRACE_EVENT2("blink", "ImageFrameGenerator::decode", "width", m_fullSize.width(), "height", m_fullSize.height());
 
-    ASSERT(decoder);
     SharedBuffer* data = 0;
     bool allDataReceived = false;
-    bool newDecoder = false;
     m_data->data(&data, &allDataReceived);
 
     // Try to create an ImageDecoder if we are not given one.
+    ASSERT(decoder);
+    bool newDecoder = false;
     if (!*decoder) {
         newDecoder = true;
         if (m_imageDecoderFactory)
             *decoder = m_imageDecoderFactory->create().leakPtr();
 
         if (!*decoder)
             *decoder = ImageDecoder::create(*data, ImageDecoder::AlphaPremultiplied, ImageDecoder::GammaAndColorProfileApplied).leakPtr();
 
         if (!*decoder)
             return false;
     }
 
     if (!m_isMultiFrame && newDecoder && allDataReceived) {
         // If we're using an external memory allocator that means we're decoding
         // directly into the output memory and we can save one memcpy.
         ASSERT(m_externalAllocator.get());
         (*decoder)->setMemoryAllocator(m_externalAllocator.get());
     }
+
     (*decoder)->setData(data, allDataReceived);
+    ImageFrame* frame = (*decoder)->frameBufferAtIndex(index);
 
-    ImageFrame* frame = (*decoder)->frameBufferAtIndex(index);
     // 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.
     if (allDataReceived)
         m_frameCount = (*decoder)->frameCount();
 
     (*decoder)->setData(0, false); // Unref SharedBuffer from ImageDecoder.
     (*decoder)->clearCacheExceptFrame(index);
     (*decoder)->setMemoryAllocator(0);
 
     if (!frame || frame->status() == ImageFrame::FrameEmpty)
         return false;
 
     // A cache object is considered complete if we can decode a complete frame.
     // Or we have received all data. The image might not be fully decoded in
     // the latter case.
     const bool isDecodeComplete = frame->status() == ImageFrame::FrameComplete || allDataReceived;
+
     SkBitmap fullSizeBitmap = frame->getSkBitmap();
-    if (!fullSizeBitmap.isNull())
-    {
+    if (!fullSizeBitmap.isNull()) {
         ASSERT(fullSizeBitmap.width() == m_fullSize.width() && fullSizeBitmap.height() == m_fullSize.height());
         setHasAlpha(index, !fullSizeBitmap.isOpaque());
     }
+
     *bitmap = fullSizeBitmap;
     return isDecodeComplete;
 }
 
 bool ImageFrameGenerator::hasAlpha(size_t index)
 {
     MutexLocker lock(m_alphaMutex);
     if (index < m_hasAlpha.size())
         return m_hasAlpha[index];
     return true;
 }
 
 bool ImageFrameGenerator::getYUVComponentSizes(SkISize componentSizes[3])
 {
-    ASSERT(componentSizes);
-
     TRACE_EVENT2("blink", "ImageFrameGenerator::getYUVComponentSizes", "width", m_fullSize.width(), "height", m_fullSize.height());
 
     SharedBuffer* data = 0;
     bool allDataReceived = false;
     m_data->data(&data, &allDataReceived);
 
     // FIXME: YUV decoding does not currently support progressive decoding.
     if (!allDataReceived)
         return false;
 
     OwnPtr<ImageDecoder> decoder = ImageDecoder::create(*data, ImageDecoder::AlphaPremultiplied, ImageDecoder::GammaAndColorProfileApplied);
     if (!decoder)
         return false;
 
     // Setting a dummy ImagePlanes object signals to the decoder that we want to do YUV decoding.
     decoder->setData(data, allDataReceived);
     OwnPtr<ImagePlanes> dummyImagePlanes = adoptPtr(new ImagePlanes);
     decoder->setImagePlanes(dummyImagePlanes.release());
 
+    ASSERT(componentSizes);
     return updateYUVComponentSizes(decoder.get(), componentSizes, ImageDecoder::SizeForMemoryAllocation);
 }
 
 } // namespace blink