blink: Refactor ImageFrameGenerator to support multi-thread use.

Currently ImageFrameGenerator can only be used for decoding from a
single thread at a time, with the access to any decoder for this
generator protected using the same mutex. This can result in janks where
the same image is being animated by multiple compositors in the same
process, since the decode tasks from these compositors can not execute
in parallel.

In order to allow multiple threads to use different decoders backed by
the same generator, this change splits the piece in the generator which
uses the decoder into a ImageDecoderWrapper. This makes it explicit that
different decoders can be used concurrently, as long as the access is
synchronized with a mutex per decoder.

The change should be a no-op.

R=chrishtr@chromium.org

Bug: 872753
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ib31ca739fbf858e0dc292dc9b47f9fd4a087956d
Reviewed-on: https://chromium-review.googlesource.com/1185629Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585740}

third_party/blink/renderer/platform/BUILD.gn

@@ -974,6 +974,8 @@ jumbo_component("platform") {
     "graphics/image_animation_policy.h",
     "graphics/image_data_buffer.cc",
     "graphics/image_data_buffer.h",
+    "graphics/image_decoder_wrapper.cc",
+    "graphics/image_decoder_wrapper.h",
     "graphics/image_decoding_store.cc",
     "graphics/image_decoding_store.h",
     "graphics/image_frame_generator.cc",

third_party/blink/renderer/platform/graphics/image_decoder_wrapper.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "third_party/blink/renderer/platform/graphics/image_decoder_wrapper.h"
+
+#include "third_party/blink/renderer/platform/graphics/image_decoding_store.h"
+#include "third_party/blink/renderer/platform/graphics/image_frame_generator.h"
+
+namespace blink {
+namespace {
+
+void CopyPixels(void* dst_addr,
+                size_t dst_row_bytes,
+                const void* src_addr,
+                size_t src_row_bytes,
+                const SkImageInfo& info) {
+  size_t row_bytes = info.bytesPerPixel() * info.width();
+  for (int y = 0; y < info.height(); ++y) {
+    memcpy(dst_addr, src_addr, row_bytes);
+    src_addr = static_cast<const char*>(src_addr) + src_row_bytes;
+    dst_addr = static_cast<char*>(dst_addr) + dst_row_bytes;
+  }
+}
+
+bool CompatibleInfo(const SkImageInfo& src, const SkImageInfo& dst) {
+  if (src == dst)
+    return true;
+
+  // It is legal to write kOpaque_SkAlphaType pixels into a kPremul_SkAlphaType
+  // buffer. This can happen when DeferredImageDecoder allocates an
+  // kOpaque_SkAlphaType image generator based on cached frame info, while the
+  // ImageFrame-allocated dest bitmap stays kPremul_SkAlphaType.
+  if (src.alphaType() == kOpaque_SkAlphaType &&
+      dst.alphaType() == kPremul_SkAlphaType) {
+    const SkImageInfo& tmp = src.makeAlphaType(kPremul_SkAlphaType);
+    return tmp == dst;
+  }
+
+  return false;
+}
+
+// Creates a SkPixelRef such that the memory for pixels is given by an external
+// body. This is used to write directly to the memory given by Skia during
+// decoding.
+class ExternalMemoryAllocator final : public SkBitmap::Allocator {
+  USING_FAST_MALLOC(ExternalMemoryAllocator);
+
+ public:
+  ExternalMemoryAllocator(const SkImageInfo& info,
+                          void* pixels,
+                          size_t row_bytes)
+      : info_(info), pixels_(pixels), row_bytes_(row_bytes) {}
+
+  bool allocPixelRef(SkBitmap* dst) override {
+    const SkImageInfo& info = dst->info();
+    if (kUnknown_SkColorType == info.colorType())
+      return false;
+
+    if (!CompatibleInfo(info_, info) || row_bytes_ != dst->rowBytes())
+      return false;
+
+    return dst->installPixels(info, pixels_, row_bytes_);
+  }
+
+ private:
+  SkImageInfo info_;
+  void* pixels_;
+  size_t row_bytes_;
+
+  DISALLOW_COPY_AND_ASSIGN(ExternalMemoryAllocator);
+};
+
+}  // namespace
+
+ImageDecoderWrapper::ImageDecoderWrapper(
+    ImageFrameGenerator* generator,
+    SegmentReader* data,
+    const SkISize& scaled_size,
+    ImageDecoder::AlphaOption alpha_option,
+    ColorBehavior decoder_color_behavior,
+    ImageDecoder::HighBitDepthDecodingOption decoding_option,
+    size_t index,
+    const SkImageInfo& info,
+    void* pixels,
+    size_t row_bytes,
+    bool all_data_received)
+    : generator_(generator),
+      data_(data),
+      scaled_size_(scaled_size),
+      alpha_option_(alpha_option),
+      decoder_color_behavior_(decoder_color_behavior),
+      decoding_option_(decoding_option),
+      frame_index_(index),
+      info_(info),
+      pixels_(pixels),
+      row_bytes_(row_bytes),
+      all_data_received_(all_data_received) {}
+
+ImageDecoderWrapper::~ImageDecoderWrapper() = default;
+
+bool ImageDecoderWrapper::Decode(ImageDecoderFactory* factory,
+                                 size_t* frame_count,
+                                 bool* has_alpha) {
+  DCHECK(frame_count);
+  DCHECK(has_alpha);
+
+  ImageDecoder* decoder = nullptr;
+  std::unique_ptr<ImageDecoder> new_decoder;
+
+  const bool resume_decoding = ImageDecodingStore::Instance().LockDecoder(
+      generator_, scaled_size_, alpha_option_, &decoder);
+  DCHECK(!resume_decoding || decoder);
+
+  if (resume_decoding) {
+    decoder->SetData(data_, all_data_received_);
+  } else {
+    new_decoder = CreateDecoderWithData(factory);
+    if (!new_decoder)
+      return false;
+    decoder = new_decoder.get();
+  }
+
+  // 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 (all_data_received_)
+    *frame_count = decoder->FrameCount();
+
+  const bool decode_to_external_memory =
+      ShouldDecodeToExternalMemory(*frame_count, resume_decoding);
+
+  ExternalMemoryAllocator external_memory_allocator(info_, pixels_, row_bytes_);
+  if (decode_to_external_memory)
+    decoder->SetMemoryAllocator(&external_memory_allocator);
+  ImageFrame* frame = decoder->DecodeFrameBufferAtIndex(frame_index_);
+  // SetMemoryAllocator() can try to access decoder's data, so we have to
+  // clear it before clearing SegmentReader.
+  if (decode_to_external_memory)
+    decoder->SetMemoryAllocator(nullptr);
+  // Verify we have the only ref-count.
+  DCHECK(external_memory_allocator.unique());
+
+  decoder->SetData(scoped_refptr<SegmentReader>(nullptr), false);
+  decoder->ClearCacheExceptFrame(frame_index_);
+
+  const bool has_decoded_frame =
+      frame && frame->GetStatus() != ImageFrame::kFrameEmpty &&
+      !frame->Bitmap().isNull();
+  if (!has_decoded_frame) {
+    decode_failed_ = decoder->Failed();
+    if (resume_decoding)
+      ImageDecodingStore::Instance().UnlockDecoder(generator_, decoder);
+    return false;
+  }
+
+  SkBitmap scaled_size_bitmap = frame->Bitmap();
+  DCHECK_EQ(scaled_size_bitmap.width(), scaled_size_.width());
+  DCHECK_EQ(scaled_size_bitmap.height(), scaled_size_.height());
+
+// If we decoded into external memory, the bitmap should be backed by the
+// pixels passed to the allocator.
+#if DCHECK_IS_ON()
+  if (!decoder->IsForTesting() && decode_to_external_memory) {
+    DCHECK_EQ(scaled_size_bitmap.getPixels(), pixels_);
+  }
+#endif
+
+  *has_alpha = !scaled_size_bitmap.isOpaque();
+  if (!decode_to_external_memory) {
+    CopyPixels(pixels_, row_bytes_, scaled_size_bitmap.getPixels(),
+               scaled_size_bitmap.rowBytes(), info_);
+  }
+
+  // Free as much memory as possible.  For single-frame images, we can
+  // just delete the decoder entirely if they use the external allocator.
+  // For multi-frame images, we keep the decoder around in order to preserve
+  // decoded information such as the required previous frame indexes, but if
+  // we've reached the last frame we can at least delete all the cached frames.
+  // (If we were to do this before reaching the last frame, any subsequent
+  // requested frames which relied on the current frame would trigger extra
+  // re-decoding of all frames in the dependency chain).
+  const bool frame_was_completely_decoded =
+      frame->GetStatus() == ImageFrame::kFrameComplete || all_data_received_;
+  PurgeAllFramesIfNecessary(decoder, frame_was_completely_decoded,
+                            *frame_count);
+  const bool should_remove_decoder = ShouldRemoveDecoder(
+      frame_was_completely_decoded, decode_to_external_memory);
+  if (resume_decoding) {
+    if (should_remove_decoder)
+      ImageDecodingStore::Instance().RemoveDecoder(generator_, decoder);
+    else
+      ImageDecodingStore::Instance().UnlockDecoder(generator_, decoder);
+  } else if (!should_remove_decoder) {
+    // If we have a newly created decoder which we don't want to remove, add
+    // it to the cache.
+    ImageDecodingStore::Instance().InsertDecoder(generator_,
+                                                 std::move(new_decoder));
+  }
+
+  return true;
+}
+
+bool ImageDecoderWrapper::ShouldDecodeToExternalMemory(
+    size_t frame_count,
+    bool resume_decoding) const {
+  // Multi-frame images need their decode cached in the decoder to allow using
+  // subsequent frames to be decoded by caching dependent frames.
+  // Also external allocators don't work for multi-frame images right now.
+  if (generator_->IsMultiFrame())
+    return false;
+
+  // On low-end devices, always use the external allocator, to avoid storing
+  // duplicate copies of the data for partial decodes in the ImageDecoder's
+  // cache.
+  if (Platform::Current()->IsLowEndDevice()) {
+    DCHECK(!resume_decoding);
+    return true;
+  }
+
+  // TODO (scroggo): If !is_multi_frame_ && new_decoder && frame_count_, it
+  // should always be the case that 1u == frame_count_. But it looks like it
+  // is currently possible for frame_count_ to be another value.
+  if (1u == frame_count && all_data_received_ && !resume_decoding) {
+    // Also use external allocator in situations when all of the data has been
+    // received and there is not already a partial cache in the image decoder.
+    return true;
+  }
+
+  return false;
+}
+
+bool ImageDecoderWrapper::ShouldRemoveDecoder(
+    bool frame_was_completely_decoded,
+    bool decoded_to_external_memory) const {
+  // Mult-frame images need the decode cached to allow decoding subsequent
+  // frames without having to decode the complete dependency chain. For this
+  // reason, we should never be decoding directly to external memory for these
+  // images.
+  if (generator_->IsMultiFrame()) {
+    DCHECK(!decoded_to_external_memory);
+    return false;
+  }
+
+  // If the decode was done directly to external memory, the decoder has no
+  // data to cache. Remove it.
+  if (decoded_to_external_memory)
+    return true;
+
+  // If we were caching a decoder with a partially decoded frame which has
+  // now been completely decoded, we don't need to cache this decoder anymore.
+  if (frame_was_completely_decoded)
+    return true;
+
+  return false;
+}
+
+void ImageDecoderWrapper::PurgeAllFramesIfNecessary(
+    ImageDecoder* decoder,
+    bool frame_was_completely_decoded,
+    size_t frame_count) const {
+  // We only purge all frames when we have decoded the last frame for a
+  // multi-frame image. This is because once the last frame is decoded, the
+  // animation will loop back to the first frame which does not need the last
+  // frame as a dependency and therefore can be purged.
+  // For single-frame images, the complete decoder is removed once it has been
+  // completely decoded.
+  if (!generator_->IsMultiFrame())
+    return;
+
+  // The frame was only partially decoded, we need to retain it to be able to
+  // resume the decoder.
+  if (!frame_was_completely_decoded)
+    return;
+
+  const size_t last_frame_index = frame_count - 1;
+  if (frame_index_ == last_frame_index)
+    decoder->ClearCacheExceptFrame(kNotFound);
+}
+
+std::unique_ptr<ImageDecoder> ImageDecoderWrapper::CreateDecoderWithData(
+    ImageDecoderFactory* factory) const {
+  if (factory) {
+    auto decoder = factory->Create();
+    if (decoder)
+      decoder->SetData(data_, all_data_received_);
+    return decoder;
+  }
+
+  // The newly created decoder just grabbed the data.  No need to reset it.
+  return ImageDecoder::Create(data_, all_data_received_, alpha_option_,
+                              decoding_option_, decoder_color_behavior_,
+                              scaled_size_);
+}
+
+}  // namespace blink

third_party/blink/renderer/platform/graphics/image_decoder_wrapper.h

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef THIRD_PARTY_BLINK_RENDERER_PLATFORM_GRAPHICS_IMAGE_DECODER_WRAPPER_H_
+#define THIRD_PARTY_BLINK_RENDERER_PLATFORM_GRAPHICS_IMAGE_DECODER_WRAPPER_H_
+
+#include "third_party/blink/renderer/platform/image-decoders/image_decoder.h"
+#include "third_party/skia/include/core/SkSize.h"
+
+namespace blink {
+class ImageDecoderFactory;
+class ImageFrameGenerator;
+class SegmentReader;
+
+class ImageDecoderWrapper {
+ public:
+  ImageDecoderWrapper(ImageFrameGenerator* generator,
+                      SegmentReader* data,
+                      const SkISize& scaled_size,
+                      ImageDecoder::AlphaOption alpha_option,
+                      ColorBehavior decoder_color_behavior,
+                      ImageDecoder::HighBitDepthDecodingOption decoding_option,
+                      size_t index,
+                      const SkImageInfo& info,
+                      void* pixels,
+                      size_t row_bytes,
+                      bool all_data_received);
+  ~ImageDecoderWrapper();
+
+  // Returns true if the decode succeeded.
+  bool Decode(ImageDecoderFactory* factory,
+              size_t* frame_count,
+              bool* has_alpha);
+
+  // Indicates that the decode failed due to a corrupt image.
+  bool decode_failed() const { return decode_failed_; }
+
+ private:
+  bool ShouldDecodeToExternalMemory(size_t frame_count,
+                                    bool has_cached_decoder) const;
+  bool ShouldRemoveDecoder(bool frame_was_completely_decoded,
+                           bool decoded_to_external_memory) const;
+  void PurgeAllFramesIfNecessary(ImageDecoder* decoder,
+                                 bool frame_was_completely_decoded,
+                                 size_t frame_count) const;
+  std::unique_ptr<ImageDecoder> CreateDecoderWithData(
+      ImageDecoderFactory* factory) const;
+
+  const ImageFrameGenerator* const generator_;
+  SegmentReader* data_;
+  const SkISize scaled_size_;
+  ImageDecoder::AlphaOption alpha_option_;
+  const ColorBehavior decoder_color_behavior_;
+  const ImageDecoder::HighBitDepthDecodingOption decoding_option_;
+  size_t frame_index_;
+  const SkImageInfo info_;
+  void* pixels_;
+  size_t row_bytes_;
+  bool all_data_received_;
+
+  bool decode_failed_ = false;
+};
+
+}  // namespace blink
+
+#endif  // THIRD_PARTY_BLINK_RENDERER_PLATFORM_GRAPHICS_IMAGE_DECODER_WRAPPER_H_

third_party/blink/renderer/platform/graphics/image_frame_generator.cc

@@ -30,6 +30,7 @@
 
 #include "SkData.h"
 #include "base/macros.h"
+#include "third_party/blink/renderer/platform/graphics/image_decoder_wrapper.h"
 #include "third_party/blink/renderer/platform/graphics/image_decoding_store.h"
 #include "third_party/blink/renderer/platform/image-decoders/image_decoder.h"
 #include "third_party/blink/renderer/platform/instrumentation/tracing/trace_event.h"
@@ -37,67 +38,6 @@
 
 namespace blink {
 
-static void CopyPixels(void* dst_addr,
-                       size_t dst_row_bytes,
-                       const void* src_addr,
-                       size_t src_row_bytes,
-                       const SkImageInfo& info) {
-  size_t row_bytes = info.bytesPerPixel() * info.width();
-  for (int y = 0; y < info.height(); ++y) {
-    memcpy(dst_addr, src_addr, row_bytes);
-    src_addr = static_cast<const char*>(src_addr) + src_row_bytes;
-    dst_addr = static_cast<char*>(dst_addr) + dst_row_bytes;
-  }
-}
-
-static bool CompatibleInfo(const SkImageInfo& src, const SkImageInfo& dst) {
-  if (src == dst)
-    return true;
-
-  // It is legal to write kOpaque_SkAlphaType pixels into a kPremul_SkAlphaType
-  // buffer. This can happen when DeferredImageDecoder allocates an
-  // kOpaque_SkAlphaType image generator based on cached frame info, while the
-  // ImageFrame-allocated dest bitmap stays kPremul_SkAlphaType.
-  if (src.alphaType() == kOpaque_SkAlphaType &&
-      dst.alphaType() == kPremul_SkAlphaType) {
-    const SkImageInfo& tmp = src.makeAlphaType(kPremul_SkAlphaType);
-    return tmp == dst;
-  }
-
-  return false;
-}
-
-// Creates a SkPixelRef such that the memory for pixels is given by an external
-// body. This is used to write directly to the memory given by Skia during
-// decoding.
-class ExternalMemoryAllocator final : public SkBitmap::Allocator {
-  USING_FAST_MALLOC(ExternalMemoryAllocator);
-
- public:
-  ExternalMemoryAllocator(const SkImageInfo& info,
-                          void* pixels,
-                          size_t row_bytes)
-      : info_(info), pixels_(pixels), row_bytes_(row_bytes) {}
-
-  bool allocPixelRef(SkBitmap* dst) override {
-    const SkImageInfo& info = dst->info();
-    if (kUnknown_SkColorType == info.colorType())
-      return false;
-
-    if (!CompatibleInfo(info_, info) || row_bytes_ != dst->rowBytes())
-      return false;
-
-    return dst->installPixels(info, pixels_, row_bytes_);
-  }
-
- private:
-  SkImageInfo info_;
-  void* pixels_;
-  size_t row_bytes_;
-
-  DISALLOW_COPY_AND_ASSIGN(ExternalMemoryAllocator);
-};
-
 static bool UpdateYUVComponentSizes(ImageDecoder* decoder,
                                     SkISize component_sizes[3],
                                     size_t component_width_bytes[3]) {
@@ -120,9 +60,6 @@ ImageFrameGenerator::ImageFrameGenerator(const SkISize& full_size,
     : full_size_(full_size),
       decoder_color_behavior_(color_behavior),
       is_multi_frame_(is_multi_frame),
-      decode_failed_(false),
-      yuv_decoding_failed_(false),
-      frame_count_(0),
       supported_sizes_(std::move(supported_sizes)) {
 #if DCHECK_IS_ON()
   // Verify that sizes are in an increasing order, since
@@ -147,44 +84,62 @@ bool ImageFrameGenerator::DecodeAndScale(
     void* pixels,
     size_t row_bytes,
     ImageDecoder::AlphaOption alpha_option) {
-  if (decode_failed_)
-    return false;
+  {
+    MutexLocker lock(generator_mutex_);
+
+    if (decode_failed_)
+      return false;
+  }
 
   TRACE_EVENT1("blink", "ImageFrameGenerator::decodeAndScale", "frame index",
                static_cast<int>(index));
 
-  // Lock the mutex, so only one thread can use the decoder at once.
-  MutexLocker lock(decode_mutex_);
-
   // This implementation does not support arbitrary scaling so check the
   // requested size.
   SkISize scaled_size = SkISize::Make(info.width(), info.height());
   CHECK(GetSupportedDecodeSize(scaled_size) == scaled_size);
 
-  // It is okay to allocate ref-counted ExternalMemoryAllocator on the stack,
-  // because 1) it contains references to memory that will be invalid after
-  // returning (i.e. a pointer to |pixels|) and therefore 2) should not live
-  // longer than the call to the current method.
-  ExternalMemoryAllocator external_allocator(info, pixels, row_bytes);
   ImageDecoder::HighBitDepthDecodingOption high_bit_depth_decoding_option =
       ImageDecoder::kDefaultBitDepth;
   if (info.colorType() == kRGBA_F16_SkColorType) {
     high_bit_depth_decoding_option = ImageDecoder::kHighBitDepthToHalfFloat;
   }
-  SkBitmap bitmap = TryToResumeDecode(
-      data, all_data_received, index, scaled_size, external_allocator,
-      alpha_option, high_bit_depth_decoding_option);
-  DCHECK(external_allocator.unique());  // Verify we have the only ref-count.
 
-  if (bitmap.isNull())
+  size_t frame_count = 0u;
+  bool has_alpha = true;
+
+  // |decode_failed| indicates a failure due to a corrupt image.
+  bool decode_failed = false;
+  // |current_decode_succeeded| indicates a failure to decode the current frame.
+  // Its possible to have a valid but fail to decode a frame in the case where
+  // we don't have enough data to decode this particular frame yet.
+  bool current_decode_succeeded = false;
+  {
+    // Lock the mutex, so only one thread can use the decoder at once.
+    MutexLocker lock(decode_mutex_);
+    ImageDecoderWrapper decoder_wrapper(
+        this, data, scaled_size, alpha_option, decoder_color_behavior_,
+        high_bit_depth_decoding_option, index, info, pixels, row_bytes,
+        all_data_received);
+    current_decode_succeeded = decoder_wrapper.Decode(
+        image_decoder_factory_.get(), &frame_count, &has_alpha);
+    decode_failed = decoder_wrapper.decode_failed();
+  }
+
+  MutexLocker lock(generator_mutex_);
+  decode_failed_ = decode_failed;
+  if (decode_failed_) {
+    DCHECK(!current_decode_succeeded);
+    return false;
+  }
+
+  if (!current_decode_succeeded)
     return false;
 
-  // Check to see if the decoder has written directly to the pixel memory
-  // provided. If not, make a copy.
-  DCHECK_EQ(bitmap.width(), scaled_size.width());
-  DCHECK_EQ(bitmap.height(), scaled_size.height());
-  if (bitmap.getPixels() != pixels)
-    CopyPixels(pixels, row_bytes, bitmap.getPixels(), bitmap.rowBytes(), info);
+  SetHasAlpha(index, has_alpha);
+  if (frame_count != 0u)
+    frame_count_ = frame_count;
+
   return true;
 }
 
@@ -193,6 +148,8 @@ bool ImageFrameGenerator::DecodeToYUV(SegmentReader* data,
                                       const SkISize component_sizes[3],
                                       void* planes[3],
                                       const size_t row_bytes[3]) {
+  MutexLocker lock(generator_mutex_);
+
   // TODO (scroggo): The only interesting thing this uses from the
   // ImageFrameGenerator is m_decodeFailed. Move this into
   // DecodingImageGenerator, which is the only class that calls it.
@@ -231,88 +188,9 @@ bool ImageFrameGenerator::DecodeToYUV(SegmentReader* data,
   return false;
 }
 
-SkBitmap ImageFrameGenerator::TryToResumeDecode(
-    SegmentReader* data,
-    bool all_data_received,
-    size_t index,
-    const SkISize& scaled_size,
-    SkBitmap::Allocator& allocator,
-    ImageDecoder::AlphaOption alpha_option,
-    ImageDecoder::HighBitDepthDecodingOption high_bit_depth_decoding_option) {
-  decode_mutex_.AssertAcquired();
-
-  TRACE_EVENT1("blink", "ImageFrameGenerator::tryToResumeDecode", "frame index",
-               static_cast<int>(index));
-
-  ImageDecoder* decoder = nullptr;
-  const bool resume_decoding = ImageDecodingStore::Instance().LockDecoder(
-      this, scaled_size, alpha_option, &decoder);
-  DCHECK(!resume_decoding || decoder);
-
-  bool used_external_allocator = false;
-  ImageFrame* current_frame = Decode(
-      data, all_data_received, index, &decoder, allocator, alpha_option,
-      high_bit_depth_decoding_option, scaled_size, used_external_allocator);
-
-  if (!decoder)
-    return SkBitmap();
-
-  // 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.
-  std::unique_ptr<ImageDecoder> decoder_container;
-  if (!resume_decoding)
-    decoder_container = base::WrapUnique(decoder);
-
-  if (!current_frame || current_frame->Bitmap().isNull()) {
-    // If decoding has failed, we can save work in the future by
-    // ignoring further requests to decode the image.
-    decode_failed_ = decoder->Failed();
-    if (resume_decoding)
-      ImageDecodingStore::Instance().UnlockDecoder(this, decoder);
-    return SkBitmap();
-  }
-
-  SkBitmap scaled_size_bitmap = current_frame->Bitmap();
-  DCHECK_EQ(scaled_size_bitmap.width(), scaled_size.width());
-  DCHECK_EQ(scaled_size_bitmap.height(), scaled_size.height());
-  SetHasAlpha(index, !scaled_size_bitmap.isOpaque());
-
-  // Free as much memory as possible.  For single-frame images, we can
-  // just delete the decoder entirely if they use the external allocator.
-  // For multi-frame images, we keep the decoder around in order to preserve
-  // decoded information such as the required previous frame indexes, but if
-  // we've reached the last frame we can at least delete all the cached frames.
-  // (If we were to do this before reaching the last frame, any subsequent
-  // requested frames which relied on the current frame would trigger extra
-  // re-decoding of all frames in the dependency chain.
-  bool remove_decoder = false;
-  if (current_frame->GetStatus() == ImageFrame::kFrameComplete ||
-      all_data_received) {
-    if (!is_multi_frame_) {
-      remove_decoder = true;
-    } else if (index == frame_count_ - 1) {
-      decoder->ClearCacheExceptFrame(kNotFound);
-    }
-  } else if (used_external_allocator) {
-    remove_decoder = true;
-  }
-
-  if (resume_decoding) {
-    if (remove_decoder)
-      ImageDecodingStore::Instance().RemoveDecoder(this, decoder);
-    else
-      ImageDecodingStore::Instance().UnlockDecoder(this, decoder);
-  } else if (!remove_decoder) {
-    ImageDecodingStore::Instance().InsertDecoder(this,
-                                                 std::move(decoder_container));
-  }
-
-  return scaled_size_bitmap;
-}
-
 void ImageFrameGenerator::SetHasAlpha(size_t index, bool has_alpha) {
-  MutexLocker lock(alpha_mutex_);
+  generator_mutex_.AssertAcquired();
+
   if (index >= has_alpha_.size()) {
     const size_t old_size = has_alpha_.size();
     has_alpha_.resize(index + 1);
@@ -322,93 +200,9 @@ void ImageFrameGenerator::SetHasAlpha(size_t index, bool has_alpha) {
   has_alpha_[index] = has_alpha;
 }
 
-ImageFrame* ImageFrameGenerator::Decode(
-    SegmentReader* data,
-    bool all_data_received,
-    size_t index,
-    ImageDecoder** decoder,
-    SkBitmap::Allocator& allocator,
-    ImageDecoder::AlphaOption alpha_option,
-    ImageDecoder::HighBitDepthDecodingOption high_bit_depth_decoding_option,
-    const SkISize& scaled_size,
-    bool& used_external_allocator) {
-  decode_mutex_.AssertAcquired();
-  TRACE_EVENT2("blink", "ImageFrameGenerator::decode", "width",
-               full_size_.width(), "height", full_size_.height());
-
-  // Try to create an ImageDecoder if we are not given one.
-  DCHECK(decoder);
-  bool new_decoder = false;
-  bool should_call_set_data = true;
-  if (!*decoder) {
-    new_decoder = true;
-    // TODO(vmpstr): The factory is only used for tests. We can convert all
-    // calls to use a factory so that we don't need to worry about this call.
-    if (image_decoder_factory_)
-      *decoder = image_decoder_factory_->Create().release();
-
-    if (!*decoder) {
-      *decoder = ImageDecoder::Create(data, all_data_received, alpha_option,
-                                      high_bit_depth_decoding_option,
-                                      decoder_color_behavior_, scaled_size)
-                     .release();
-      // The newly created decoder just grabbed the data.  No need to reset it.
-      should_call_set_data = false;
-    }
-
-    if (!*decoder)
-      return nullptr;
-  }
-
-  if (should_call_set_data)
-    (*decoder)->SetData(data, all_data_received);
-
-  // 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 (all_data_received)
-    frame_count_ = (*decoder)->FrameCount();
-
-  used_external_allocator = false;
-  // External allocators don't work for multi-frame images right now.
-  if (!is_multi_frame_) {
-    if (Platform::Current()->IsLowEndDevice()) {
-      // On low-end devices, always use the external allocator, to avoid
-      // storing duplicate copies of the data in the ImageDecoder's cache.
-      used_external_allocator = true;
-      DCHECK(new_decoder);
-      // TODO (scroggo): If !is_multi_frame_ && new_decoder && frame_count_, it
-      // should always be the case that 1u == frame_count_. But it looks like it
-      // is currently possible for frame_count_ to be another value.
-    } else if (1u == frame_count_ && all_data_received && new_decoder) {
-      // Also use external allocator situations when all of the data has been
-      // received and there is not already a partial cache in the image decoder.
-      used_external_allocator = true;
-    }
-  }
-
-  if (used_external_allocator)
-    (*decoder)->SetMemoryAllocator(&allocator);
-
-  ImageFrame* frame = (*decoder)->DecodeFrameBufferAtIndex(index);
-
-  // SetMemoryAllocator() can try to access decoder's data, so
-  // we have to do it before clearing SegmentReader.
-  if (used_external_allocator)
-    (*decoder)->SetMemoryAllocator(nullptr);
-  (*decoder)->SetData(scoped_refptr<SegmentReader>(nullptr),
-                      false);  // Unref SegmentReader from ImageDecoder.
-  (*decoder)->ClearCacheExceptFrame(index);
-
-  if (!frame || frame->GetStatus() == ImageFrame::kFrameEmpty)
-    return nullptr;
-
-  return frame;
-}
-
 bool ImageFrameGenerator::HasAlpha(size_t index) {
-  MutexLocker lock(alpha_mutex_);
+  MutexLocker lock(generator_mutex_);
+
   if (index < has_alpha_.size())
     return has_alpha_[index];
   return true;
@@ -419,6 +213,8 @@ bool ImageFrameGenerator::GetYUVComponentSizes(SegmentReader* data,
   TRACE_EVENT2("blink", "ImageFrameGenerator::getYUVComponentSizes", "width",
                full_size_.width(), "height", full_size_.height());
 
+  MutexLocker lock(generator_mutex_);
+
   if (yuv_decoding_failed_)
     return false;
 

third_party/blink/renderer/platform/graphics/image_frame_generator.h

@@ -128,46 +128,25 @@ class PLATFORM_EXPORT ImageFrameGenerator final
 
   void SetHasAlpha(size_t index, bool has_alpha);
 
-  SkBitmap TryToResumeDecode(SegmentReader*,
-                             bool all_data_received,
-                             size_t index,
-                             const SkISize& scaled_size,
-                             SkBitmap::Allocator&,
-                             ImageDecoder::AlphaOption,
-                             ImageDecoder::HighBitDepthDecodingOption);
-  // This method should only be called while decode_mutex_ is locked.
-  // Returns a pointer to frame |index|'s ImageFrame, if available.
-  // Sets |used_external_allocator| to true if the the image was decoded into
-  // |external_allocator|'s memory.
-  ImageFrame* Decode(SegmentReader*,
-                     bool all_data_received,
-                     size_t index,
-                     ImageDecoder**,
-                     SkBitmap::Allocator& external_allocator,
-                     ImageDecoder::AlphaOption,
-                     ImageDecoder::HighBitDepthDecodingOption,
-                     const SkISize& scaled_size,
-                     bool& used_external_allocator);
-
   const SkISize full_size_;
-
   // Parameters used to create internal ImageDecoder objects.
   const ColorBehavior decoder_color_behavior_;
-
   const bool is_multi_frame_;
-  bool decode_failed_;
-  bool yuv_decoding_failed_;
-  size_t frame_count_;
-  Vector<bool> has_alpha_;
-  std::vector<SkISize> supported_sizes_;
-
-  std::unique_ptr<ImageDecoderFactory> image_decoder_factory_;
+  const std::vector<SkISize> supported_sizes_;
 
-  // Prevents multiple decode operations on the same data.
+  // Prevents multiple decode operations on the same data. Used in non-yuv
+  // decoding which supports concurrent decoding from the same generator.
   Mutex decode_mutex_;
 
-  // Protect concurrent access to has_alpha_.
-  Mutex alpha_mutex_;
+  // Prevents concurrent access to all variables below.
+  Mutex generator_mutex_;
+
+  bool decode_failed_ = false;
+  bool yuv_decoding_failed_ = false;
+  size_t frame_count_ = 0u;
+  Vector<bool> has_alpha_;
+
+  std::unique_ptr<ImageDecoderFactory> image_decoder_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ImageFrameGenerator);
 };

third_party/blink/renderer/platform/graphics/test/mock_image_decoder.h

@@ -111,6 +111,8 @@ class MockImageDecoder : public ImageDecoder {
     client_->MemoryAllocatorSet();
   }
 
+  bool IsForTesting() const override { return true; }
+
  private:
   void DecodeSize() override {}
 

third_party/blink/renderer/platform/image-decoders/image_decoder.cc

@@ -150,6 +150,8 @@ size_t ImageDecoder::FrameCount() {
 }
 
 ImageFrame* ImageDecoder::DecodeFrameBufferAtIndex(size_t index) {
+  TRACE_EVENT0("blink", "ImageDecoder::DecodeFrameBufferAtIndex");
+
   if (index >= FrameCount())
     return nullptr;
   ImageFrame* frame = &frame_buffer_cache_[index];

third_party/blink/renderer/platform/image-decoders/image_decoder.h

@@ -333,6 +333,8 @@ class PLATFORM_EXPORT ImageDecoder {
   virtual bool DecodeToYUV() { return false; }
   virtual void SetImagePlanes(std::unique_ptr<ImagePlanes>) {}
 
+  virtual bool IsForTesting() const { return false; }
+
  protected:
   ImageDecoder(AlphaOption alpha_option,
                HighBitDepthDecodingOption high_bit_depth_decoding_option,