Revert software image cache rework patches.

This patch reverts two commits:
Revert "Reland "cc: Rework software image decode cache.""
  This reverts commit 0a6f9bcd.
Revert "cc/images: Resuse the closest available decode for scaling sw images."
  This reverts commit 7a28b96c.

The reason is that the rework missed a few cases that are causing DCHECKs
or incorrect behavior in release builds. Since these are not currently
blocking any work, I will reland them with the proper fixes.

TBR=khushalsagar@chromium.org, ericrk@chromium.org

Bug: 786285
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I7fc81026e162d0854fc828bdd4457a3212b27db9
Reviewed-on: https://chromium-review.googlesource.com/779939
Commit-Queue: vmpstr <vmpstr@chromium.org>
Reviewed-by: Eric Karl <ericrk@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517920}

cc/paint/paint_image.cc

@@ -142,6 +142,13 @@ SkISize PaintImage::GetSupportedDecodeSize(
   return SkISize::Make(width(), height());
 }
 
+SkImageInfo PaintImage::CreateDecodeImageInfo(const SkISize& size,
+                                              SkColorType color_type) const {
+  DCHECK(GetSupportedDecodeSize(size) == size);
+  return SkImageInfo::Make(size.width(), size.height(), color_type,
+                           kPremul_SkAlphaType);
+}
+
 bool PaintImage::Decode(void* memory,
                         SkImageInfo* info,
                         sk_sp<SkColorSpace> color_space,

cc/paint/paint_image.h

@@ -116,6 +116,11 @@ class CC_PAINT_EXPORT PaintImage {
   // GetSupportedDecodeSize(size).
   SkISize GetSupportedDecodeSize(const SkISize& requested_size) const;
 
+  // Returns SkImageInfo that should be used to decode this image to the given
+  // size and color type. The size must be supported.
+  SkImageInfo CreateDecodeImageInfo(const SkISize& size,
+                                    SkColorType color_type) const;
+
   // Decode the image into the given memory for the given SkImageInfo.
   // - Size in |info| must be supported.
   // - The amount of memory allocated must be at least

cc/tiles/software_image_decode_cache.cc

@@ -53,7 +53,7 @@ const size_t kMemoryThresholdToSubrect = 64 * 1024 * 1024;
 const int kMinDimensionToSubrect = 4 * 1024;
 const float kMemoryRatioToSubrect = 0.5f;
 
-// Tracing ID sequence for use in CacheEntry.
+// Tracing ID sequence for use in DecodedImage.
 base::AtomicSequenceNumber g_next_tracing_id_;
 
 class AutoRemoveKeyFromTaskMap {
@@ -73,17 +73,35 @@ class AutoRemoveKeyFromTaskMap {
   const SoftwareImageDecodeCache::ImageKey& key_;
 };
 
+class AutoDrawWithImageFinished {
+ public:
+  AutoDrawWithImageFinished(SoftwareImageDecodeCache* cache,
+                            const DrawImage& draw_image,
+                            const DecodedDrawImage& decoded_draw_image)
+      : cache_(cache),
+        draw_image_(draw_image),
+        decoded_draw_image_(decoded_draw_image) {}
+  ~AutoDrawWithImageFinished() {
+    cache_->DrawWithImageFinished(draw_image_, decoded_draw_image_);
+  }
+
+ private:
+  SoftwareImageDecodeCache* cache_;
+  const DrawImage& draw_image_;
+  const DecodedDrawImage& decoded_draw_image_;
+};
+
 class ImageDecodeTaskImpl : public TileTask {
  public:
   ImageDecodeTaskImpl(SoftwareImageDecodeCache* cache,
                       const SoftwareImageDecodeCache::ImageKey& image_key,
-                      const PaintImage& paint_image,
+                      const DrawImage& image,
                       SoftwareImageDecodeCache::DecodeTaskType task_type,
                       const ImageDecodeCache::TracingInfo& tracing_info)
       : TileTask(true),
         cache_(cache),
         image_key_(image_key),
-        paint_image_(paint_image),
+        image_(image),
         task_type_(task_type),
         tracing_info_(tracing_info) {}
 
@@ -93,15 +111,15 @@ class ImageDecodeTaskImpl : public TileTask {
                  "software", "source_prepare_tiles_id",
                  tracing_info_.prepare_tiles_id);
     devtools_instrumentation::ScopedImageDecodeTask image_decode_task(
-        paint_image_.GetSkImage().get(),
+        image_.paint_image().GetSkImage().get(),
         devtools_instrumentation::ScopedImageDecodeTask::kSoftware,
         ImageDecodeCache::ToScopedTaskType(tracing_info_.task_type));
-    cache_->DecodeImageInTask(image_key_, paint_image_, task_type_);
+    cache_->DecodeImage(image_key_, image_, task_type_);
   }
 
   // Overridden from TileTask:
   void OnTaskCompleted() override {
-    cache_->OnImageDecodeTaskCompleted(image_key_, task_type_);
+    cache_->RemovePendingTask(image_key_, task_type_);
   }
 
  protected:
@@ -110,7 +128,7 @@ class ImageDecodeTaskImpl : public TileTask {
  private:
   SoftwareImageDecodeCache* cache_;
   SoftwareImageDecodeCache::ImageKey image_key_;
-  PaintImage paint_image_;
+  DrawImage image_;
   SoftwareImageDecodeCache::DecodeTaskType task_type_;
   const ImageDecodeCache::TracingInfo tracing_info_;
 
@@ -138,9 +156,10 @@ SkFilterQuality GetDecodedFilterQuality(const ImageDecodeCacheKey& key) {
   return std::min(key.filter_quality(), kLow_SkFilterQuality);
 }
 
-SkImageInfo CreateImageInfo(const SkISize& size, SkColorType color_type) {
-  return SkImageInfo::Make(size.width(), size.height(), color_type,
-                           kPremul_SkAlphaType);
+SkImageInfo CreateImageInfo(size_t width,
+                            size_t height,
+                            SkColorType color_type) {
+  return SkImageInfo::Make(width, height, color_type, kPremul_SkAlphaType);
 }
 
 void RecordLockExistingCachedImageHistogram(TilePriority::PriorityBin bin,
@@ -169,19 +188,13 @@ gfx::Rect GetSrcRect(const DrawImage& image) {
   return gfx::Rect(x, y, right - x, bottom - y);
 }
 
-std::unique_ptr<base::DiscardableMemory> AllocateDiscardable(
-    const SkImageInfo& info) {
-  TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"), "AllocateDiscardable");
-  return base::DiscardableMemoryAllocator::GetInstance()
-      ->AllocateLockedDiscardableMemory(info.minRowBytes() * info.height());
-}
-
 }  // namespace
 
 SoftwareImageDecodeCache::SoftwareImageDecodeCache(
     SkColorType color_type,
     size_t locked_memory_limit_bytes)
     : decoded_images_(ImageMRUCache::NO_AUTO_EVICT),
+      at_raster_decoded_images_(ImageMRUCache::NO_AUTO_EVICT),
       locked_images_budget_(locked_memory_limit_bytes),
       color_type_(color_type),
       max_items_in_cache_(kNormalMaxItemsInCache) {
@@ -197,6 +210,10 @@ SoftwareImageDecodeCache::SoftwareImageDecodeCache(
 }
 
 SoftwareImageDecodeCache::~SoftwareImageDecodeCache() {
+  // Debugging crbug.com/650234
+  CHECK_EQ(0u, decoded_images_ref_counts_.size());
+  CHECK_EQ(0u, at_raster_decoded_images_ref_counts_.size());
+
   // It is safe to unregister, even if we didn't register in the constructor.
   base::trace_event::MemoryDumpManager::GetInstance()->UnregisterDumpProvider(
       this);
@@ -236,368 +253,522 @@ SoftwareImageDecodeCache::GetTaskForImageAndRefInternal(
     const DrawImage& image,
     const TracingInfo& tracing_info,
     DecodeTaskType task_type) {
+  // If the image already exists or if we're going to create a task for it, then
+  // we'll likely need to ref this image (the exception is if we're prerolling
+  // the image only). That means the image is or will be in the cache. When the
+  // ref goes to 0, it will be unpinned but will remain in the cache. If the
+  // image does not fit into the budget, then we don't ref this image, since it
+  // will be decoded at raster time which is when it will be temporarily put in
+  // the cache.
   ImageKey key = ImageKey::FromDrawImage(image, color_type_);
   TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
-               "SoftwareImageDecodeCache::GetTaskForImageAndRefInternal", "key",
+               "SoftwareImageDecodeCache::GetTaskForImageAndRef", "key",
                key.ToString());
 
   // If the target size is empty, we can skip this image during draw (and thus
   // we don't need to decode it or ref it).
-  if (key.target_size().IsEmpty())
+  if (key.target_size().IsEmpty()) {
     return TaskResult(false);
+  }
 
   base::AutoLock lock(lock_);
 
+  // If we already have the image in cache, then we can return it.
+  auto decoded_it = decoded_images_.Get(key);
   bool new_image_fits_in_memory =
       locked_images_budget_.AvailableMemoryBytes() >= key.locked_bytes();
-
-  // Get or generate the cache entry.
-  auto decoded_it = decoded_images_.Get(key);
-  CacheEntry* cache_entry = nullptr;
-  if (decoded_it == decoded_images_.end()) {
-    // There is no reason to create a new entry if we know it won't fit anyway.
-    if (!new_image_fits_in_memory)
-      return TaskResult(false);
-    cache_entry = AddCacheEntry(key);
-    if (task_type == DecodeTaskType::USE_OUT_OF_RASTER_TASKS)
-      cache_entry->mark_out_of_raster();
-  } else {
-    cache_entry = decoded_it->second.get();
+  if (decoded_it != decoded_images_.end()) {
+    bool image_was_locked = decoded_it->second->is_locked();
+    if (image_was_locked ||
+        (new_image_fits_in_memory && decoded_it->second->Lock())) {
+      RefImage(key);
+
+      // If the image wasn't locked, then we just succeeded in locking it.
+      if (!image_was_locked) {
+        RecordLockExistingCachedImageHistogram(tracing_info.requesting_tile_bin,
+                                               true);
+      }
+      return TaskResult(true);
     }
-  DCHECK(cache_entry);
 
-  if (!cache_entry->is_budgeted) {
-    if (!new_image_fits_in_memory) {
-      // We don't need to ref anything here because this image will be at
-      // raster.
-      return TaskResult(false);
+    // If the image fits in memory, then we at least tried to lock it and
+    // failed. This means that it's not valid anymore.
+    if (new_image_fits_in_memory) {
+      RecordLockExistingCachedImageHistogram(tracing_info.requesting_tile_bin,
+                                             false);
+      CleanupDecodedImagesCache(key, decoded_it);
     }
-    AddBudgetForImage(key, cache_entry);
   }
-  DCHECK(cache_entry->is_budgeted);
-
-  // The rest of the code will return either true or a task, so we should ref
-  // the image once now for the raster to unref.
-  ++cache_entry->ref_count;
 
-  // If we already have a locked entry, then we can just use that. Otherwise
-  // we'll have to create a task.
-  if (cache_entry->is_locked)
-    return TaskResult(true);
+  DCHECK(task_type == DecodeTaskType::USE_IN_RASTER_TASKS ||
+         task_type == DecodeTaskType::USE_OUT_OF_RASTER_TASKS);
+  // If the task exists, return it. Note that if we always need to create a new
+  // task, then just set |existing_task| to reference the passed in task (which
+  // is set to nullptr above).
+  scoped_refptr<TileTask>& existing_task =
+      (task_type == DecodeTaskType::USE_IN_RASTER_TASKS)
+          ? pending_in_raster_image_tasks_[key]
+          : pending_out_of_raster_image_tasks_[key];
+  if (existing_task) {
+    RefImage(key);
+    return TaskResult(existing_task);
+  }
 
-  scoped_refptr<TileTask>& task =
-      task_type == DecodeTaskType::USE_IN_RASTER_TASKS
-          ? cache_entry->in_raster_task
-          : cache_entry->out_of_raster_task;
-  if (!task) {
-    // Ref image once for the task.
-    ++cache_entry->ref_count;
-    task = base::MakeRefCounted<ImageDecodeTaskImpl>(
-        this, key, image.paint_image(), task_type, tracing_info);
+  // At this point, we have to create a new image/task, so we need to abort if
+  // it doesn't fit into memory and there are currently no raster tasks that
+  // would have already accounted for memory. The latter part is possible if
+  // there's a running raster task that could not be canceled, and still has a
+  // ref to the image that is now being reffed for the new schedule.
+  if (!new_image_fits_in_memory && (decoded_images_ref_counts_.find(key) ==
+                                    decoded_images_ref_counts_.end())) {
+    return TaskResult(false);
   }
-  return TaskResult(task);
+
+  // Actually create the task. RefImage will account for memory on the first
+  // ref.
+  RefImage(key);
+  existing_task = base::MakeRefCounted<ImageDecodeTaskImpl>(
+      this, key, image, task_type, tracing_info);
+  return TaskResult(existing_task);
 }
 
-void SoftwareImageDecodeCache::AddBudgetForImage(const ImageKey& key,
-                                                 CacheEntry* entry) {
+void SoftwareImageDecodeCache::RefImage(const ImageKey& key) {
   TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
-               "SoftwareImageDecodeCache::AddBudgetForImage", "key",
-               key.ToString());
+               "SoftwareImageDecodeCache::RefImage", "key", key.ToString());
   lock_.AssertAcquired();
-
-  DCHECK(!entry->is_budgeted);
+  int ref = ++decoded_images_ref_counts_[key];
+  if (ref == 1) {
     DCHECK_GE(locked_images_budget_.AvailableMemoryBytes(), key.locked_bytes());
     locked_images_budget_.AddUsage(key.locked_bytes());
-  entry->is_budgeted = true;
-}
-
-void SoftwareImageDecodeCache::RemoveBudgetForImage(const ImageKey& key,
-                                                    CacheEntry* entry) {
-  TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
-               "SoftwareImageDecodeCache::RemoveBudgetForImage", "key",
-               key.ToString());
-  lock_.AssertAcquired();
-
-  DCHECK(entry->is_budgeted);
-  locked_images_budget_.SubtractUsage(key.locked_bytes());
-  entry->is_budgeted = false;
+  }
 }
 
 void SoftwareImageDecodeCache::UnrefImage(const DrawImage& image) {
+  // When we unref the image, there are several situations we need to consider:
+  // 1. The ref did not reach 0, which means we have to keep the image locked.
+  // 2. The ref reached 0, we should unlock it.
+  //   2a. The image isn't in the locked cache because we didn't get to decode
+  //       it yet (or failed to decode it).
+  //   2b. Unlock the image but keep it in list.
   const ImageKey& key = ImageKey::FromDrawImage(image, color_type_);
   TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
                "SoftwareImageDecodeCache::UnrefImage", "key", key.ToString());
 
   base::AutoLock lock(lock_);
-  UnrefImage(key);
-}
+  auto ref_count_it = decoded_images_ref_counts_.find(key);
+  DCHECK(ref_count_it != decoded_images_ref_counts_.end());
+
+  --ref_count_it->second;
+  if (ref_count_it->second == 0) {
+    decoded_images_ref_counts_.erase(ref_count_it);
+    locked_images_budget_.SubtractUsage(key.locked_bytes());
 
-void SoftwareImageDecodeCache::UnrefImage(const ImageKey& key) {
-  lock_.AssertAcquired();
     auto decoded_image_it = decoded_images_.Peek(key);
-  DCHECK(decoded_image_it != decoded_images_.end());
-  auto* entry = decoded_image_it->second.get();
-  DCHECK_GT(entry->ref_count, 0);
-  if (--entry->ref_count == 0) {
-    if (entry->is_budgeted)
-      RemoveBudgetForImage(key, entry);
-    if (entry->is_locked)
-      entry->Unlock();
+    // If we've never decoded the image before ref reached 0, then we wouldn't
+    // have it in our cache. This would happen if we canceled tasks.
+    if (decoded_image_it == decoded_images_.end())
+      return;
+    DCHECK(decoded_image_it->second->is_locked());
+    decoded_image_it->second->Unlock();
   }
 }
 
-void SoftwareImageDecodeCache::DecodeImageInTask(const ImageKey& key,
-                                                 const PaintImage& paint_image,
+void SoftwareImageDecodeCache::DecodeImage(const ImageKey& key,
+                                           const DrawImage& image,
                                            DecodeTaskType task_type) {
-  TRACE_EVENT1("cc", "SoftwareImageDecodeCache::DecodeImageInTask", "key",
+  TRACE_EVENT1("cc", "SoftwareImageDecodeCache::DecodeImage", "key",
                key.ToString());
   base::AutoLock lock(lock_);
+  AutoRemoveKeyFromTaskMap remove_key_from_task_map(
+      (task_type == DecodeTaskType::USE_IN_RASTER_TASKS)
+          ? &pending_in_raster_image_tasks_
+          : &pending_out_of_raster_image_tasks_,
+      key);
+
+  // We could have finished all of the raster tasks (cancelled) while the task
+  // was just starting to run. Since this task already started running, it
+  // wasn't cancelled. So, if the ref count for the image is 0 then we can just
+  // abort.
+  if (decoded_images_ref_counts_.find(key) ==
+      decoded_images_ref_counts_.end()) {
+    return;
+  }
 
   auto image_it = decoded_images_.Peek(key);
-  DCHECK(image_it != decoded_images_.end());
-  auto* cache_entry = image_it->second.get();
-  // These two checks must be true because we're running this from a task, which
-  // means that we've budgeted this entry when we got the task and the ref count
-  // is also held by the task (released in OnTaskCompleted).
-  DCHECK_GT(cache_entry->ref_count, 0);
-  DCHECK(cache_entry->is_budgeted);
-
-  DecodeImageIfNecessary(key, paint_image, cache_entry);
-  DCHECK(cache_entry->decode_failed || cache_entry->is_locked);
+  if (image_it != decoded_images_.end()) {
+    if (image_it->second->is_locked() || image_it->second->Lock())
+      return;
+    CleanupDecodedImagesCache(key, image_it);
+  }
+
+  std::unique_ptr<DecodedImage> decoded_image;
+  {
+    base::AutoUnlock unlock(lock_);
+    decoded_image = DecodeImageInternal(key, image);
+  }
+
+  // Abort if we failed to decode the image.
+  if (!decoded_image)
+    return;
+
+  // At this point, it could have been the case that this image was decoded in
+  // place by an already running raster task from a previous schedule. If that's
+  // the case, then it would have already been placed into the cache (possibly
+  // locked). Remove it if that was the case.
+  image_it = decoded_images_.Peek(key);
+  if (image_it != decoded_images_.end()) {
+    if (image_it->second->is_locked() || image_it->second->Lock()) {
+      // Make sure to unlock the decode we did in this function.
+      decoded_image->Unlock();
+      return;
+    }
+    CleanupDecodedImagesCache(key, image_it);
+  }
+
+  // We could have finished all of the raster tasks (cancelled) while this image
+  // decode task was running, which means that we now have a locked image but no
+  // ref counts. Unlock it immediately in this case.
+  if (decoded_images_ref_counts_.find(key) ==
+      decoded_images_ref_counts_.end()) {
+    decoded_image->Unlock();
+  }
+
+  if (task_type == DecodeTaskType::USE_OUT_OF_RASTER_TASKS)
+    decoded_image->mark_out_of_raster();
+
   RecordImageMipLevelUMA(
       MipMapUtil::GetLevelForSize(key.src_rect().size(), key.target_size()));
+
+  CacheDecodedImages(key, std::move(decoded_image));
 }
 
-void SoftwareImageDecodeCache::DecodeImageIfNecessary(
-    const ImageKey& key,
-    const PaintImage& paint_image,
-    CacheEntry* entry) {
+std::unique_ptr<SoftwareImageDecodeCache::DecodedImage>
+SoftwareImageDecodeCache::DecodeImageInternal(const ImageKey& key,
+                                              const DrawImage& draw_image) {
   TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
-               "SoftwareImageDecodeCache::DecodeImageIfNecessary", "key",
+               "SoftwareImageDecodeCache::DecodeImageInternal", "key",
                key.ToString());
-  lock_.AssertAcquired();
-  DCHECK_GT(entry->ref_count, 0);
-
-  if (key.target_size().IsEmpty())
-    entry->decode_failed = true;
+  const PaintImage& paint_image = draw_image.paint_image();
+  if (!paint_image)
+    return nullptr;
 
-  if (entry->decode_failed)
-    return;
+  // Special case subrect into a special function.
+  if (key.should_use_subrect())
+    return GetSubrectImageDecode(key, paint_image);
+
+  // There are two cases where we can use the exact size image decode:
+  // - If we we're using full image (no subset) and we can decode natively to
+  // that scale, or
+  // - If we're not doing a scale at all (which is supported by all decoders and
+  // subsetting is handled in the draw calls).
+  // TODO(vmpstr): See if we can subrect the result of decoded to scale.
+  SkIRect full_size_rect =
+      SkIRect::MakeWH(paint_image.width(), paint_image.height());
+  bool need_subset = (gfx::RectToSkIRect(key.src_rect()) != full_size_rect);
+  SkISize exact_size =
+      SkISize::Make(key.target_size().width(), key.target_size().height());
+  // TODO(vmpstr): If an image of a bigger size is already decoded and is
+  // lock-able then it might be faster to just scale that instead of redecoding
+  // to exact scale. We need to profile this.
+  if ((!need_subset &&
+       exact_size == paint_image.GetSupportedDecodeSize(exact_size)) ||
+      SkIRect::MakeSize(exact_size) == full_size_rect) {
+    return GetExactSizeImageDecode(key, paint_image);
+  }
+  return GetScaledImageDecode(key, paint_image);
+}
 
-  if (entry->memory) {
-    if (entry->is_locked)
-      return;
+DecodedDrawImage SoftwareImageDecodeCache::GetDecodedImageForDraw(
+    const DrawImage& draw_image) {
+  ImageKey key = ImageKey::FromDrawImage(draw_image, color_type_);
+  TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
+               "SoftwareImageDecodeCache::GetDecodedImageForDraw", "key",
+               key.ToString());
+  // If the target size is empty, we can skip this image draw.
+  if (key.target_size().IsEmpty())
+    return DecodedDrawImage(nullptr, kNone_SkFilterQuality);
 
-    bool lock_succeeded = entry->Lock();
-    // TODO(vmpstr): Deprecate the prepaint split, since it doesn't matter.
-    RecordLockExistingCachedImageHistogram(TilePriority::NOW, lock_succeeded);
+  return GetDecodedImageForDrawInternal(key, draw_image);
+}
 
-    if (lock_succeeded)
-      return;
-    entry->memory = nullptr;
-  }
+DecodedDrawImage SoftwareImageDecodeCache::GetDecodedImageForDrawInternal(
+    const ImageKey& key,
+    const DrawImage& draw_image) {
+  TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
+               "SoftwareImageDecodeCache::GetDecodedImageForDrawInternal",
+               "key", key.ToString());
 
-  const SkIRect full_size_rect =
-      SkIRect::MakeWH(paint_image.width(), paint_image.height());
-  const bool need_subset =
-      (gfx::RectToSkIRect(key.src_rect()) != full_size_rect);
-  const SkIRect target_size_rect =
-      SkIRect::MakeWH(key.target_size().width(), key.target_size().height());
-
-  std::unique_ptr<CacheEntry> local_cache_entry;
-  // If this is a full size, unsubrected image, we will definitely need to do
-  // a decode.
-  if (!need_subset && target_size_rect == full_size_rect) {
-    base::AutoUnlock release(lock_);
-    local_cache_entry = DoDecodeImage(key, paint_image);
+  base::AutoLock lock(lock_);
+  auto decoded_images_it = decoded_images_.Get(key);
+  // If we found the image and it's locked, then return it. If it's not locked,
+  // erase it from the cache since it might be put into the at-raster cache.
+  std::unique_ptr<DecodedImage> scoped_decoded_image;
+  DecodedImage* decoded_image = nullptr;
+  if (decoded_images_it != decoded_images_.end()) {
+    decoded_image = decoded_images_it->second.get();
+    if (decoded_image->is_locked()) {
+      RefImage(key);
+      decoded_image->mark_used();
+      return DecodedDrawImage(
+          decoded_image->image(), decoded_image->src_rect_offset(),
+          GetScaleAdjustment(key), GetDecodedFilterQuality(key));
     } else {
-    // Use the full image decode to generate a scaled/subrected decode.
-    // TODO(vmpstr): Change this part to find a good candidate first other
-    // than the full decode. Also this part needs to handle decode to scale.
-    base::Optional<ImageKey> candidate_key;
-    auto image_keys_it = frame_key_to_image_keys_.find(key.frame_key());
-    // We know that we must have at least our own |entry| in this list, so it
-    // won't be empty.
-    DCHECK(image_keys_it != frame_key_to_image_keys_.end());
-
-    auto& available_keys = image_keys_it->second;
-    std::sort(available_keys.begin(), available_keys.end(),
-              [](const ImageKey& one, const ImageKey& two) {
-                // Return true if |one| scale is less than |two| scale.
-                return one.target_size().width() < two.target_size().width() &&
-                       one.target_size().height() < two.target_size().height();
-              });
-    for (auto& available_key : available_keys) {
-      // Only consider keys coming from the same src rect, since otherwise the
-      // resulting image was extracted using a different src. Only consider keys
-      if (available_key.src_rect() != key.src_rect())
-        continue;
-
-      // that are at least as big as the required |key|.
-      if (available_key.target_size().width() < key.target_size().width() ||
-          available_key.target_size().height() < key.target_size().height()) {
-        continue;
+      scoped_decoded_image = std::move(decoded_images_it->second);
+      CleanupDecodedImagesCache(key, decoded_images_it);
     }
-      auto image_it = decoded_images_.Peek(available_key);
-      DCHECK(image_it != decoded_images_.end());
-      auto* available_entry = image_it->second.get();
-      if (available_entry->is_locked || available_entry->Lock()) {
-        candidate_key.emplace(available_key);
-        break;
-      }
-    }
-
-    if (!candidate_key) {
-      DrawImage candidate_draw_image(
-          paint_image, full_size_rect, kNone_SkFilterQuality, SkMatrix::I(),
-          key.frame_key().frame_index(), key.target_color_space());
-      candidate_key.emplace(
-          ImageKey::FromDrawImage(candidate_draw_image, color_type_));
   }
 
+  // See if another thread already decoded this image at raster time. If so, we
+  // can just use that result directly.
+  auto at_raster_images_it = at_raster_decoded_images_.Get(key);
+  if (at_raster_images_it != at_raster_decoded_images_.end()) {
+    DCHECK(at_raster_images_it->second->is_locked());
+    RefAtRasterImage(key);
+    DecodedImage* at_raster_decoded_image = at_raster_images_it->second.get();
+    at_raster_decoded_image->mark_used();
     auto decoded_draw_image =
-        GetDecodedImageForDrawInternal(*candidate_key, paint_image);
-
-    if (!decoded_draw_image.image()) {
-      local_cache_entry = nullptr;
-    } else {
-      base::AutoUnlock release(lock_);
-      local_cache_entry =
-          GenerateCacheEntryFromCandidate(key, decoded_draw_image);
+        DecodedDrawImage(at_raster_decoded_image->image(),
+                         at_raster_decoded_image->src_rect_offset(),
+                         GetScaleAdjustment(key), GetDecodedFilterQuality(key));
+    decoded_draw_image.set_at_raster_decode(true);
+    return decoded_draw_image;
   }
 
-    // Unref to balance the GetDecodedImageForDrawInternal() call.
-    UnrefImage(*candidate_key);
+  // Now we know that we don't have a locked image, and we seem to be the first
+  // thread encountering this image (that might not be true, since other threads
+  // might be decoding it already). This means that we need to decode the image
+  // assuming we can't lock the one we found in the cache.
+  bool check_at_raster_cache = false;
+  if (!decoded_image || !decoded_image->Lock()) {
+    // Note that we have to release the lock, since this lock is also accessed
+    // on the compositor thread. This means holding on to the lock might stall
+    // the compositor thread for the duration of the decode!
+    base::AutoUnlock unlock(lock_);
+    scoped_decoded_image = DecodeImageInternal(key, draw_image);
+    decoded_image = scoped_decoded_image.get();
+
+    // Skip the image if we couldn't decode it.
+    if (!decoded_image)
+      return DecodedDrawImage(nullptr, kNone_SkFilterQuality);
+    check_at_raster_cache = true;
   }
 
-  if (!local_cache_entry) {
-    entry->decode_failed = true;
-    return;
+  DCHECK(decoded_image == scoped_decoded_image.get());
+
+  // While we unlocked the lock, it could be the case that another thread
+  // already decoded this already and put it in the at-raster cache. Look it up
+  // first.
+  if (check_at_raster_cache) {
+    at_raster_images_it = at_raster_decoded_images_.Get(key);
+    if (at_raster_images_it != at_raster_decoded_images_.end()) {
+      // We have to drop our decode, since the one in the cache is being used by
+      // another thread.
+      decoded_image->Unlock();
+      decoded_image = at_raster_images_it->second.get();
+      scoped_decoded_image = nullptr;
     }
-
-  // Just in case someone else did this already, just unlock our work.
-  // TODO(vmpstr): It's possible to have a pending decode state where the
-  // thread would just block on a cv and wait for that decode to finish
-  // instead of actually doing the work.
-  if (entry->memory) {
-    // This would have to be locked because we hold a ref count on the entry. So
-    // if someone ever populated the entry with memory, they would not be able
-    // to unlock it.
-    DCHECK(entry->is_locked);
-    // Unlock our local memory though.
-    local_cache_entry->Unlock();
-  } else {
-    local_cache_entry->MoveImageMemoryTo(entry);
-    DCHECK(entry->is_locked);
   }
+
+  // If we really are the first ones, or if the other thread already unlocked
+  // the image, then put our work into at-raster time cache.
+  if (scoped_decoded_image)
+    at_raster_decoded_images_.Put(key, std::move(scoped_decoded_image));
+
+  DCHECK(decoded_image);
+  DCHECK(decoded_image->is_locked());
+  RefAtRasterImage(key);
+  decoded_image->mark_used();
+  auto decoded_draw_image =
+      DecodedDrawImage(decoded_image->image(), decoded_image->src_rect_offset(),
+                       GetScaleAdjustment(key), GetDecodedFilterQuality(key));
+  decoded_draw_image.set_at_raster_decode(true);
+  return decoded_draw_image;
 }
 
-std::unique_ptr<SoftwareImageDecodeCache::CacheEntry>
-SoftwareImageDecodeCache::DoDecodeImage(const ImageKey& key,
+std::unique_ptr<SoftwareImageDecodeCache::DecodedImage>
+SoftwareImageDecodeCache::GetExactSizeImageDecode(
+    const ImageKey& key,
     const PaintImage& paint_image) {
-  SkISize target_size =
+  SkISize decode_size =
       SkISize::Make(key.target_size().width(), key.target_size().height());
-  DCHECK(target_size == paint_image.GetSupportedDecodeSize(target_size));
+  DCHECK(decode_size == paint_image.GetSupportedDecodeSize(decode_size));
 
-  SkImageInfo target_info = CreateImageInfo(target_size, color_type_);
-  std::unique_ptr<base::DiscardableMemory> target_pixels =
-      AllocateDiscardable(target_info);
-  DCHECK(target_pixels);
+  SkImageInfo decoded_info =
+      paint_image.CreateDecodeImageInfo(decode_size, color_type_);
 
+  std::unique_ptr<base::DiscardableMemory> decoded_pixels;
+  {
+    TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
+                 "SoftwareImageDecodeCache::GetOriginalSizeImageDecode - "
+                 "allocate decoded pixels");
+    decoded_pixels =
+        base::DiscardableMemoryAllocator::GetInstance()
+            ->AllocateLockedDiscardableMemory(decoded_info.minRowBytes() *
+                                              decoded_info.height());
+  }
+  {
     TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
-               "SoftwareImageDecodeCache::DoDecodeImage - "
+                 "SoftwareImageDecodeCache::GetOriginalSizeImageDecode - "
                  "decode");
-  DCHECK(!key.should_use_subrect());
-  bool result = paint_image.Decode(target_pixels->data(), &target_info,
+    bool result = paint_image.Decode(decoded_pixels->data(), &decoded_info,
                                      key.target_color_space().ToSkColorSpace(),
                                      key.frame_key().frame_index());
     if (!result) {
-    target_pixels->Unlock();
+      decoded_pixels->Unlock();
       return nullptr;
     }
-  return std::make_unique<CacheEntry>(target_info, std::move(target_pixels),
+  }
+
+  return std::make_unique<DecodedImage>(decoded_info, std::move(decoded_pixels),
                                         SkSize::Make(0, 0));
 }
 
-std::unique_ptr<SoftwareImageDecodeCache::CacheEntry>
-SoftwareImageDecodeCache::GenerateCacheEntryFromCandidate(
-    const ImageKey& key,
-    const DecodedDrawImage& candidate_image) {
-  SkISize target_size =
-      SkISize::Make(key.target_size().width(), key.target_size().height());
-  SkImageInfo target_info = CreateImageInfo(target_size, color_type_);
-  std::unique_ptr<base::DiscardableMemory> target_pixels =
-      AllocateDiscardable(target_info);
-  DCHECK(target_pixels);
+std::unique_ptr<SoftwareImageDecodeCache::DecodedImage>
+SoftwareImageDecodeCache::GetSubrectImageDecode(const ImageKey& key,
+                                                const PaintImage& image) {
+  // Construct a key to use in GetDecodedImageForDrawInternal().
+  // This allows us to reuse an image in any cache if available.
+  // This uses the original sized image, since when we're subrecting, there is
+  // no scale.
+  // TODO(vmpstr): This doesn't have to be true, but the Key generation makes it
+  // so. We could also subrect scaled images.
+  SkIRect exact_size_rect = SkIRect::MakeWH(image.width(), image.height());
+  DrawImage exact_size_draw_image(image, exact_size_rect, kNone_SkFilterQuality,
+                                  SkMatrix::I(), key.frame_key().frame_index(),
+                                  key.target_color_space());
+  ImageKey exact_size_key =
+      ImageKey::FromDrawImage(exact_size_draw_image, color_type_);
+
+// Sanity checks.
+#if DCHECK_IS_ON()
+  SkISize exact_target_size =
+      SkISize::Make(exact_size_key.target_size().width(),
+                    exact_size_key.target_size().height());
+  DCHECK(image.GetSupportedDecodeSize(exact_target_size) == exact_target_size);
+  DCHECK(!exact_size_key.should_use_subrect());
+#endif
 
-  if (key.should_use_subrect()) {
+  auto decoded_draw_image =
+      GetDecodedImageForDrawInternal(exact_size_key, exact_size_draw_image);
+  AutoDrawWithImageFinished auto_finish_draw(this, exact_size_draw_image,
+                                             decoded_draw_image);
+  if (!decoded_draw_image.image())
+    return nullptr;
+
+  DCHECK(SkColorSpace::Equals(decoded_draw_image.image()->colorSpace(),
+                              key.target_color_space().ToSkColorSpace().get()));
+  SkImageInfo subrect_info = CreateImageInfo(
+      key.target_size().width(), key.target_size().height(), color_type_);
+  std::unique_ptr<base::DiscardableMemory> subrect_pixels;
+  {
     TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
-                 "SoftwareImageDecodeCache::GenerateCacheEntryFromCandidate - "
-                 "subrect");
-    bool result = candidate_image.image()->readPixels(
-        target_info, target_pixels->data(), target_info.minRowBytes(),
+                 "SoftwareImageDecodeCache::GetSubrectImageDecode - "
+                 "allocate subrect pixels");
+    // TODO(vmpstr): This is using checked math to diagnose a problem reported
+    // in crbug.com/662217. If this is causing crashes, then it should be fixed
+    // elsewhere by skipping images that are too large.
+    base::CheckedNumeric<size_t> byte_size = subrect_info.minRowBytes();
+    byte_size *= subrect_info.height();
+    subrect_pixels =
+        base::DiscardableMemoryAllocator::GetInstance()
+            ->AllocateLockedDiscardableMemory(byte_size.ValueOrDie());
+  }
+  {
+    TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
+                 "SoftwareImageDecodeCache::GetSubrectImageDecode - "
+                 "read pixels");
+    bool result = decoded_draw_image.image()->readPixels(
+        subrect_info, subrect_pixels->data(), subrect_info.minRowBytes(),
         key.src_rect().x(), key.src_rect().y(), SkImage::kDisallow_CachingHint);
     // We have a decoded image, and we're reading into already allocated memory.
     // This should never fail.
     DCHECK(result);
-    return std::make_unique<CacheEntry>(
-        target_info.makeColorSpace(candidate_image.image()->refColorSpace()),
-        std::move(target_pixels),
-        SkSize::Make(-key.src_rect().x(), -key.src_rect().y()));
   }
 
-  TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
-               "SoftwareImageDecodeCache::GenerateCacheEntryFromCandidate - "
-               "scale");
-  SkPixmap decoded_pixmap;
-  bool result = candidate_image.image()->peekPixels(&decoded_pixmap);
-  result = result && decoded_pixmap.extractSubset(
-                         &decoded_pixmap, gfx::RectToSkIRect(key.src_rect()));
-  DCHECK(result) << key.ToString();
-
-  SkPixmap target_pixmap(target_info, target_pixels->data(),
-                         target_info.minRowBytes());
-  result = decoded_pixmap.scalePixels(target_pixmap, key.filter_quality());
-  DCHECK(result) << key.ToString();
-  return std::make_unique<CacheEntry>(
-      target_info.makeColorSpace(candidate_image.image()->refColorSpace()),
-      std::move(target_pixels),
+  return std::make_unique<DecodedImage>(
+      subrect_info.makeColorSpace(decoded_draw_image.image()->refColorSpace()),
+      std::move(subrect_pixels),
       SkSize::Make(-key.src_rect().x(), -key.src_rect().y()));
 }
 
-DecodedDrawImage SoftwareImageDecodeCache::GetDecodedImageForDraw(
-    const DrawImage& draw_image) {
-  base::AutoLock hold(lock_);
-  return GetDecodedImageForDrawInternal(
-      ImageKey::FromDrawImage(draw_image, color_type_),
-      draw_image.paint_image());
-}
+std::unique_ptr<SoftwareImageDecodeCache::DecodedImage>
+SoftwareImageDecodeCache::GetScaledImageDecode(const ImageKey& key,
+                                               const PaintImage& image) {
+  // Construct a key to use in GetDecodedImageForDrawInternal().
+  // This allows us to reuse an image in any cache if available.
+  // TODO(vmpstr): If we're using a subrect, then we need to decode the original
+  // image and subset it since the subset might be strict. If we plumb whether
+  // the subrect is strict or not, we can loosen this condition.
+  SkIRect full_size_rect = SkIRect::MakeWH(image.width(), image.height());
+  bool need_subset = (gfx::RectToSkIRect(key.src_rect()) != full_size_rect);
+  SkIRect exact_size_rect =
+      need_subset
+          ? full_size_rect
+          : SkIRect::MakeSize(image.GetSupportedDecodeSize(SkISize::Make(
+                key.target_size().width(), key.target_size().height())));
+
+  DrawImage exact_size_draw_image(image, exact_size_rect, kNone_SkFilterQuality,
+                                  SkMatrix::I(), key.frame_key().frame_index(),
+                                  key.target_color_space());
+  ImageKey exact_size_key =
+      ImageKey::FromDrawImage(exact_size_draw_image, color_type_);
+
+// Sanity checks.
+#if DCHECK_IS_ON()
+  SkISize exact_target_size =
+      SkISize::Make(exact_size_key.target_size().width(),
+                    exact_size_key.target_size().height());
+  DCHECK(image.GetSupportedDecodeSize(exact_target_size) == exact_target_size);
+  DCHECK(!need_subset || exact_size_key.target_size() ==
+                             gfx::Size(image.width(), image.height()));
+  DCHECK(!exact_size_key.should_use_subrect());
+#endif
 
-DecodedDrawImage SoftwareImageDecodeCache::GetDecodedImageForDrawInternal(
-    const ImageKey& key,
-    const PaintImage& paint_image) {
-  TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
-               "SoftwareImageDecodeCache::GetDecodedImageForDrawInternal",
-               "key", key.ToString());
+  auto decoded_draw_image =
+      GetDecodedImageForDrawInternal(exact_size_key, exact_size_draw_image);
+  AutoDrawWithImageFinished auto_finish_draw(this, exact_size_draw_image,
+                                             decoded_draw_image);
+  if (!decoded_draw_image.image())
+    return nullptr;
 
-  lock_.AssertAcquired();
-  auto decoded_it = decoded_images_.Get(key);
-  CacheEntry* cache_entry = nullptr;
-  if (decoded_it == decoded_images_.end())
-    cache_entry = AddCacheEntry(key);
-  else
-    cache_entry = decoded_it->second.get();
+  SkPixmap decoded_pixmap;
+  bool result = decoded_draw_image.image()->peekPixels(&decoded_pixmap);
+  DCHECK(result) << key.ToString();
+  if (need_subset) {
+    result = decoded_pixmap.extractSubset(&decoded_pixmap,
+                                          gfx::RectToSkIRect(key.src_rect()));
+    DCHECK(result) << key.ToString();
+  }
 
-  // We'll definitely ref this cache entry and use it.
-  ++cache_entry->ref_count;
-  cache_entry->mark_used();
+  DCHECK(!key.target_size().IsEmpty());
+  DCHECK(SkColorSpace::Equals(decoded_draw_image.image()->colorSpace(),
+                              key.target_color_space().ToSkColorSpace().get()));
+  SkImageInfo scaled_info = CreateImageInfo(
+      key.target_size().width(), key.target_size().height(), color_type_);
+  std::unique_ptr<base::DiscardableMemory> scaled_pixels;
+  {
+    TRACE_EVENT0(
+        TRACE_DISABLED_BY_DEFAULT("cc.debug"),
+        "SoftwareImageDecodeCache::ScaleImage - allocate scaled pixels");
+    scaled_pixels = base::DiscardableMemoryAllocator::GetInstance()
+                        ->AllocateLockedDiscardableMemory(
+                            scaled_info.minRowBytes() * scaled_info.height());
+  }
+  SkPixmap scaled_pixmap(scaled_info, scaled_pixels->data(),
+                         scaled_info.minRowBytes());
+  DCHECK(key.filter_quality() == kMedium_SkFilterQuality);
+  {
+    TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
+                 "SoftwareImageDecodeCache::ScaleImage - scale pixels");
+    bool result =
+        decoded_pixmap.scalePixels(scaled_pixmap, key.filter_quality());
+    DCHECK(result) << key.ToString();
+  }
 
-  DecodeImageIfNecessary(key, paint_image, cache_entry);
-  auto decoded_draw_image =
-      DecodedDrawImage(cache_entry->image(), cache_entry->src_rect_offset(),
-                       GetScaleAdjustment(key), GetDecodedFilterQuality(key));
-  decoded_draw_image.set_at_raster_decode(!cache_entry->is_budgeted);
-  return decoded_draw_image;
+  return std::make_unique<DecodedImage>(
+      scaled_info.makeColorSpace(decoded_draw_image.image()->refColorSpace()),
+      std::move(scaled_pixels),
+      SkSize::Make(-key.src_rect().x(), -key.src_rect().y()));
 }
 
 void SoftwareImageDecodeCache::DrawWithImageFinished(
@@ -606,31 +777,91 @@ void SoftwareImageDecodeCache::DrawWithImageFinished(
   TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
                "SoftwareImageDecodeCache::DrawWithImageFinished", "key",
                ImageKey::FromDrawImage(image, color_type_).ToString());
+  ImageKey key = ImageKey::FromDrawImage(image, color_type_);
+  if (!decoded_image.image())
+    return;
+
+  if (decoded_image.is_at_raster_decode())
+    UnrefAtRasterImage(key);
+  else
     UnrefImage(image);
 }
 
+void SoftwareImageDecodeCache::RefAtRasterImage(const ImageKey& key) {
+  TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
+               "SoftwareImageDecodeCache::RefAtRasterImage", "key",
+               key.ToString());
+  DCHECK(at_raster_decoded_images_.Peek(key) !=
+         at_raster_decoded_images_.end());
+  ++at_raster_decoded_images_ref_counts_[key];
+}
+
+void SoftwareImageDecodeCache::UnrefAtRasterImage(const ImageKey& key) {
+  TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
+               "SoftwareImageDecodeCache::UnrefAtRasterImage", "key",
+               key.ToString());
+  base::AutoLock lock(lock_);
+
+  auto ref_it = at_raster_decoded_images_ref_counts_.find(key);
+  DCHECK(ref_it != at_raster_decoded_images_ref_counts_.end());
+  --ref_it->second;
+  if (ref_it->second == 0) {
+    at_raster_decoded_images_ref_counts_.erase(ref_it);
+    auto at_raster_image_it = at_raster_decoded_images_.Peek(key);
+    DCHECK(at_raster_image_it != at_raster_decoded_images_.end());
+
+    // The ref for our image reached 0 and it's still locked. We need to figure
+    // out what the best thing to do with the image. There are several
+    // situations:
+    // 1. The image is not in the main cache and...
+    //    1a. ... its ref count is 0: unlock our image and put it in the main
+    //    cache.
+    //    1b. ... ref count is not 0: keep the image locked and put it in the
+    //    main cache.
+    // 2. The image is in the main cache...
+    //    2a. ... and is locked: unlock our image and discard it
+    //    2b. ... and is unlocked and...
+    //       2b1. ... its ref count is 0: unlock our image and replace the
+    //       existing one with ours.
+    //       2b2. ... its ref count is not 0: this shouldn't be possible.
+    auto image_it = decoded_images_.Peek(key);
+    if (image_it == decoded_images_.end()) {
+      if (decoded_images_ref_counts_.find(key) ==
+          decoded_images_ref_counts_.end()) {
+        at_raster_image_it->second->Unlock();
+      }
+      CacheDecodedImages(key, std::move(at_raster_image_it->second));
+    } else if (image_it->second->is_locked()) {
+      at_raster_image_it->second->Unlock();
+    } else {
+      DCHECK(decoded_images_ref_counts_.find(key) ==
+             decoded_images_ref_counts_.end());
+      at_raster_image_it->second->Unlock();
+      // Access decoded_images_ directly here to avoid deletion and entry
+      // of same key in ImageKey vector of decoded_images_unique_ids_.
+      decoded_images_.Erase(image_it);
+      decoded_images_.Put(key, std::move(at_raster_image_it->second));
+    }
+    at_raster_decoded_images_.Erase(at_raster_image_it);
+  }
+}
+
 void SoftwareImageDecodeCache::ReduceCacheUsageUntilWithinLimit(size_t limit) {
   TRACE_EVENT0("cc",
                "SoftwareImageDecodeCache::ReduceCacheUsageUntilWithinLimit");
   lifetime_max_items_in_cache_ =
       std::max(lifetime_max_items_in_cache_, decoded_images_.size());
+  size_t num_to_remove =
+      (decoded_images_.size() > limit) ? (decoded_images_.size() - limit) : 0;
   for (auto it = decoded_images_.rbegin();
-       decoded_images_.size() > limit && it != decoded_images_.rend();) {
-    if (it->second->ref_count != 0) {
+       num_to_remove != 0 && it != decoded_images_.rend();) {
+    if (it->second->is_locked()) {
       ++it;
       continue;
     }
 
-    const ImageKey& key = it->first;
-    auto vector_it = frame_key_to_image_keys_.find(key.frame_key());
-    auto item_it =
-        std::find(vector_it->second.begin(), vector_it->second.end(), key);
-    DCHECK(item_it != vector_it->second.end());
-    vector_it->second.erase(item_it);
-    if (vector_it->second.empty())
-      frame_key_to_image_keys_.erase(vector_it);
-
     it = decoded_images_.Erase(it);
+    --num_to_remove;
   }
 }
 
@@ -656,37 +887,29 @@ void SoftwareImageDecodeCache::NotifyImageUnused(
   if (it == frame_key_to_image_keys_.end())
     return;
 
-  for (auto key_it = it->second.begin(); key_it != it->second.end();) {
+  for (auto key = it->second.begin(); key != it->second.end(); ++key) {
     // This iterates over the ImageKey vector for the given skimage_id,
     // and deletes all entries from decoded_images_ corresponding to the
     // skimage_id.
-    auto image_it = decoded_images_.Peek(*key_it);
-    // TODO(sohanjg): Find an optimized way to cleanup locked images.
-    if (image_it != decoded_images_.end() && image_it->second->ref_count == 0) {
+    auto image_it = decoded_images_.Peek(*key);
+    // TODO(sohanjg) :Find an optimized way to cleanup locked images.
+    if (image_it != decoded_images_.end() && !image_it->second->is_locked())
       decoded_images_.Erase(image_it);
-      key_it = it->second.erase(key_it);
-    } else {
-      ++key_it;
-    }
   }
-  if (it->second.empty())
   frame_key_to_image_keys_.erase(it);
 }
 
-void SoftwareImageDecodeCache::OnImageDecodeTaskCompleted(
-    const ImageKey& key,
+void SoftwareImageDecodeCache::RemovePendingTask(const ImageKey& key,
                                                  DecodeTaskType task_type) {
-  base::AutoLock hold(lock_);
-
-  auto image_it = decoded_images_.Peek(key);
-  DCHECK(image_it != decoded_images_.end());
-  CacheEntry* cache_entry = image_it->second.get();
-  auto& task = task_type == DecodeTaskType::USE_IN_RASTER_TASKS
-                   ? cache_entry->in_raster_task
-                   : cache_entry->out_of_raster_task;
-  task = nullptr;
-
-  UnrefImage(key);
+  base::AutoLock lock(lock_);
+  switch (task_type) {
+    case DecodeTaskType::USE_IN_RASTER_TASKS:
+      pending_in_raster_image_tasks_.erase(key);
+      break;
+    case DecodeTaskType::USE_OUT_OF_RASTER_TASKS:
+      pending_out_of_raster_image_tasks_.erase(key);
+      break;
+  }
 }
 
 bool SoftwareImageDecodeCache::OnMemoryDump(
@@ -701,35 +924,38 @@ bool SoftwareImageDecodeCache::OnMemoryDump(
     dump->AddScalar("locked_size", MemoryAllocatorDump::kUnitsBytes,
                     locked_images_budget_.GetCurrentUsageSafe());
   } else {
-    for (const auto& image_pair : decoded_images_) {
-      int image_id = static_cast<int>(image_pair.first.frame_key().hash());
-      CacheEntry* entry = image_pair.second.get();
-      DCHECK(entry);
-      // We might not have memory for this cache entry, depending on where int
-      // he CacheEntry lifecycle we are. If we don't have memory, then we don't
-      // have to record it in the dump.
-      if (!entry->memory)
-        continue;
+    // Dump each of our caches.
+    DumpImageMemoryForCache(decoded_images_, "cached", pmd);
+    DumpImageMemoryForCache(at_raster_decoded_images_, "at_raster", pmd);
+  }
 
+  // Memory dump can't fail, always return true.
+  return true;
+}
+
+void SoftwareImageDecodeCache::DumpImageMemoryForCache(
+    const ImageMRUCache& cache,
+    const char* cache_name,
+    base::trace_event::ProcessMemoryDump* pmd) const {
+  lock_.AssertAcquired();
+
+  for (const auto& image_pair : cache) {
+    int image_id = static_cast<int>(image_pair.first.frame_key().hash());
     std::string dump_name = base::StringPrintf(
         "cc/image_memory/cache_0x%" PRIXPTR "/%s/image_%" PRIu64 "_id_%d",
-          reinterpret_cast<uintptr_t>(this),
-          entry->is_budgeted ? "budgeted" : "at_raster", entry->tracing_id(),
-          image_id);
-      // CreateMemoryAllocatorDump will automatically add tracking values for
-      // the total size. We also add a "locked_size" below.
+        reinterpret_cast<uintptr_t>(this), cache_name,
+        image_pair.second->tracing_id(), image_id);
+    // CreateMemoryAllocatorDump will automatically add tracking values for the
+    // total size. We also add a "locked_size" below.
     MemoryAllocatorDump* dump =
-          entry->memory->CreateMemoryAllocatorDump(dump_name.c_str(), pmd);
+        image_pair.second->memory()->CreateMemoryAllocatorDump(
+            dump_name.c_str(), pmd);
     DCHECK(dump);
     size_t locked_bytes =
-          entry->is_locked ? image_pair.first.locked_bytes() : 0u;
+        image_pair.second->is_locked() ? image_pair.first.locked_bytes() : 0u;
     dump->AddScalar("locked_size", MemoryAllocatorDump::kUnitsBytes,
                     locked_bytes);
   }
-  }
-
-  // Memory dump can't fail, always return true.
-  return true;
 }
 
 // SoftwareImageDecodeCacheKey
@@ -882,26 +1108,23 @@ std::string ImageDecodeCacheKey::ToString() const {
   return str.str();
 }
 
-// CacheEntry
-SoftwareImageDecodeCache::CacheEntry::CacheEntry()
-    : tracing_id_(g_next_tracing_id_.GetNext()) {}
-SoftwareImageDecodeCache::CacheEntry::CacheEntry(
+// DecodedImage
+SoftwareImageDecodeCache::DecodedImage::DecodedImage(
     const SkImageInfo& info,
-    std::unique_ptr<base::DiscardableMemory> in_memory,
+    std::unique_ptr<base::DiscardableMemory> memory,
     const SkSize& src_rect_offset)
-    : is_locked(true),
-      memory(std::move(in_memory)),
+    : locked_(true),
       image_info_(info),
+      memory_(std::move(memory)),
       src_rect_offset_(src_rect_offset),
       tracing_id_(g_next_tracing_id_.GetNext()) {
-  DCHECK(memory);
-  SkPixmap pixmap(image_info_, memory->data(), image_info_.minRowBytes());
+  SkPixmap pixmap(image_info_, memory_->data(), image_info_.minRowBytes());
   image_ = SkImage::MakeFromRaster(
       pixmap, [](const void* pixels, void* context) {}, nullptr);
 }
 
-SoftwareImageDecodeCache::CacheEntry::~CacheEntry() {
-  DCHECK(!is_locked);
+SoftwareImageDecodeCache::DecodedImage::~DecodedImage() {
+  DCHECK(!locked_);
   // lock_count | used  | last lock failed | result state
   // ===========+=======+==================+==================
   //  1         | false | false            | WASTED
@@ -947,44 +1170,22 @@ SoftwareImageDecodeCache::CacheEntry::~CacheEntry() {
         usage_stats_.first_lock_wasted);
 }
 
-void SoftwareImageDecodeCache::CacheEntry::MoveImageMemoryTo(
-    CacheEntry* entry) {
-  DCHECK(!is_budgeted);
-  DCHECK_EQ(ref_count, 0);
-
-  // Copy/move most things except budgeted and ref counts.
-  entry->decode_failed = decode_failed;
-  entry->is_locked = is_locked;
-  is_locked = false;
-
-  entry->memory = std::move(memory);
-  entry->image_info_ = std::move(image_info_);
-  entry->src_rect_offset_ = std::move(src_rect_offset_);
-  entry->image_ = std::move(image_);
-}
-
-bool SoftwareImageDecodeCache::CacheEntry::Lock() {
-  if (!memory)
-    return false;
-
-  DCHECK(!is_locked);
-  bool success = memory->Lock();
+bool SoftwareImageDecodeCache::DecodedImage::Lock() {
+  DCHECK(!locked_);
+  bool success = memory_->Lock();
   if (!success) {
     usage_stats_.last_lock_failed = true;
     return false;
   }
-  is_locked = true;
+  locked_ = true;
   ++usage_stats_.lock_count;
   return true;
 }
 
-void SoftwareImageDecodeCache::CacheEntry::Unlock() {
-  if (!memory)
-    return;
-
-  DCHECK(is_locked);
-  memory->Unlock();
-  is_locked = false;
+void SoftwareImageDecodeCache::DecodedImage::Unlock() {
+  DCHECK(locked_);
+  memory_->Unlock();
+  locked_ = false;
   if (usage_stats_.lock_count == 1)
     usage_stats_.first_lock_wasted = !usage_stats_.used;
 }
@@ -1040,12 +1241,31 @@ void SoftwareImageDecodeCache::OnPurgeMemory() {
   ReduceCacheUsageUntilWithinLimit(0);
 }
 
-SoftwareImageDecodeCache::CacheEntry* SoftwareImageDecodeCache::AddCacheEntry(
-    const ImageKey& key) {
+void SoftwareImageDecodeCache::CleanupDecodedImagesCache(
+    const ImageKey& key,
+    ImageMRUCache::iterator it) {
+  lock_.AssertAcquired();
+  auto vector_it = frame_key_to_image_keys_.find(key.frame_key());
+
+  // TODO(sohanjg): Check if we can DCHECK here.
+  if (vector_it != frame_key_to_image_keys_.end()) {
+    auto iter =
+        std::find(vector_it->second.begin(), vector_it->second.end(), key);
+    DCHECK(iter != vector_it->second.end());
+    vector_it->second.erase(iter);
+    if (vector_it->second.empty())
+      frame_key_to_image_keys_.erase(vector_it);
+  }
+
+  decoded_images_.Erase(it);
+}
+
+void SoftwareImageDecodeCache::CacheDecodedImages(
+    const ImageKey& key,
+    std::unique_ptr<DecodedImage> decoded_image) {
   lock_.AssertAcquired();
   frame_key_to_image_keys_[key.frame_key()].push_back(key);
-  auto it = decoded_images_.Put(key, std::make_unique<CacheEntry>());
-  return it->second.get();
+  decoded_images_.Put(key, std::move(decoded_image));
 }
 
 }  // namespace cc

cc/tiles/software_image_decode_cache.h

@@ -142,12 +142,11 @@ class CC_EXPORT SoftwareImageDecodeCache
 
   // Decode the given image and store it in the cache. This is only called by an
   // image decode task from a worker thread.
-  void DecodeImageInTask(const ImageKey& key,
-                         const PaintImage& paint_image,
+  void DecodeImage(const ImageKey& key,
+                   const DrawImage& image,
                    DecodeTaskType task_type);
 
-  void OnImageDecodeTaskCompleted(const ImageKey& key,
-                                  DecodeTaskType task_type);
+  void RemovePendingTask(const ImageKey& key, DecodeTaskType task_type);
 
   // MemoryDumpProvider overrides.
   bool OnMemoryDump(const base::trace_event::MemoryDumpArgs& args,
@@ -156,30 +155,29 @@ class CC_EXPORT SoftwareImageDecodeCache
   size_t GetNumCacheEntriesForTesting() const { return decoded_images_.size(); }
 
  private:
-  // CacheEntry is a convenience storage for discardable memory. It can also
+  // DecodedImage is a convenience storage for discardable memory. It can also
   // construct an image out of SkImageInfo and stored discardable memory.
-  class CacheEntry {
+  class DecodedImage {
    public:
-    CacheEntry();
-    CacheEntry(const SkImageInfo& info,
+    DecodedImage(const SkImageInfo& info,
                  std::unique_ptr<base::DiscardableMemory> memory,
                  const SkSize& src_rect_offset);
-    ~CacheEntry();
-
-    void MoveImageMemoryTo(CacheEntry* entry);
+    ~DecodedImage();
 
-    sk_sp<SkImage> image() const {
-      if (!memory)
-        return nullptr;
-      DCHECK(is_locked);
+    const sk_sp<SkImage>& image() const {
+      DCHECK(locked_);
       return image_;
     }
+
     const SkSize& src_rect_offset() const { return src_rect_offset_; }
 
+    bool is_locked() const { return locked_; }
     bool Lock();
     void Unlock();
 
-    // An ID which uniquely identifies this CacheEntry within the image decode
+    const base::DiscardableMemory* memory() const { return memory_.get(); }
+
+    // An ID which uniquely identifies this DecodedImage within the image decode
     // cache. Used in memory tracing.
     uint64_t tracing_id() const { return tracing_id_; }
     // Mark this image as being used in either a draw or as a source for a
@@ -188,21 +186,6 @@ class CC_EXPORT SoftwareImageDecodeCache
     void mark_used() { usage_stats_.used = true; }
     void mark_out_of_raster() { usage_stats_.first_lock_out_of_raster = true; }
 
-    // Since this is an inner class, we expose these variables publicly for
-    // simplicity.
-    // TODO(vmpstr): A good simple clean-up would be to rethink this class
-    // and its interactions to instead expose a few functions which would also
-    // facilitate easier DCHECKs.
-    int ref_count = 0;
-    bool decode_failed = false;
-    bool is_locked = false;
-    bool is_budgeted = false;
-
-    scoped_refptr<TileTask> in_raster_task;
-    scoped_refptr<TileTask> out_of_raster_task;
-
-    std::unique_ptr<base::DiscardableMemory> memory;
-
    private:
     struct UsageStats {
       // We can only create a decoded image in a locked state, so the initial
@@ -214,7 +197,9 @@ class CC_EXPORT SoftwareImageDecodeCache
       bool first_lock_out_of_raster = false;
     };
 
+    bool locked_;
     SkImageInfo image_info_;
+    std::unique_ptr<base::DiscardableMemory> memory_;
     sk_sp<SkImage> image_;
     SkSize src_rect_offset_;
     uint64_t tracing_id_;
@@ -240,22 +225,62 @@ class CC_EXPORT SoftwareImageDecodeCache
   };
 
   using ImageMRUCache = base::
-      HashingMRUCache<ImageKey, std::unique_ptr<CacheEntry>, ImageKeyHash>;
+      HashingMRUCache<ImageKey, std::unique_ptr<DecodedImage>, ImageKeyHash>;
+
+  // Looks for the key in the cache and returns true if it was found and was
+  // successfully locked (or if it was already locked). Note that if this
+  // function returns true, then a ref count is increased for the image.
+  bool LockDecodedImageIfPossibleAndRef(const ImageKey& key);
 
   // Actually decode the image. Note that this function can (and should) be
   // called with no lock acquired, since it can do a lot of work. Note that it
   // can also return nullptr to indicate the decode failed.
-  std::unique_ptr<CacheEntry> DecodeImageInternal(const ImageKey& key,
+  std::unique_ptr<DecodedImage> DecodeImageInternal(
+      const ImageKey& key,
       const DrawImage& draw_image);
 
-  // Get the decoded draw image for the given key and paint_image. Note that
-  // this function has to be called with no lock acquired, since it will acquire
-  // its own locks and might call DecodeImageInternal above. Note that
-  // when used internally, we still require that DrawWithImageFinished() is
-  // called afterwards.
-  DecodedDrawImage GetDecodedImageForDrawInternal(
+  // Get the decoded draw image for the given key and draw_image. Note that this
+  // function has to be called with no lock acquired, since it will acquire its
+  // own locks and might call DecodeImageInternal above. Also note that this
+  // function will use the provided key, even if
+  // ImageKey::FromDrawImage(draw_image) would return a different key.
+  // Note that when used internally, we still require that
+  // DrawWithImageFinished() is called afterwards.
+  DecodedDrawImage GetDecodedImageForDrawInternal(const ImageKey& key,
+                                                  const DrawImage& draw_image);
+
+  // GetExactSizeImageDecode is called by DecodeImageInternal when the
+  // quality does not scale the image. Like DecodeImageInternal, it should be
+  // called with no lock acquired and it returns nullptr if the decoding failed.
+  std::unique_ptr<DecodedImage> GetExactSizeImageDecode(
       const ImageKey& key,
-      const PaintImage& paint_image);
+      const PaintImage& image);
+
+  // GetSubrectImageDecode is similar to GetExactSizeImageDecode in that the
+  // image is decoded to exact scale. However, we extract a subrect (copy it
+  // out) and only return this subrect in order to cache a smaller amount of
+  // memory. Note that this uses GetExactSizeImageDecode to get the initial
+  // data, which ensures that we cache an unlocked version of the original image
+  // in case we need to extract multiple subrects (as would be the case in an
+  // atlas).
+  std::unique_ptr<DecodedImage> GetSubrectImageDecode(const ImageKey& key,
+                                                      const PaintImage& image);
+
+  // GetScaledImageDecode is called by DecodeImageInternal when the quality
+  // requires the image be scaled. Like DecodeImageInternal, it should be
+  // called with no lock acquired and it returns nullptr if the decoding or
+  // scaling failed.
+  std::unique_ptr<DecodedImage> GetScaledImageDecode(const ImageKey& key,
+                                                     const PaintImage& image);
+
+  void RefImage(const ImageKey& key);
+  void RefAtRasterImage(const ImageKey& key);
+  void UnrefAtRasterImage(const ImageKey& key);
+
+  // Helper function which dumps all images in a specific ImageMRUCache.
+  void DumpImageMemoryForCache(const ImageMRUCache& cache,
+                               const char* cache_name,
+                               base::trace_event::ProcessMemoryDump* pmd) const;
 
   // Removes unlocked decoded images until the number of decoded images is
   // reduced within the given limit.
@@ -271,21 +296,10 @@ class CC_EXPORT SoftwareImageDecodeCache
                                            const TracingInfo& tracing_info,
                                            DecodeTaskType type);
 
-  CacheEntry* AddCacheEntry(const ImageKey& key);
-
-  void DecodeImageIfNecessary(const ImageKey& key,
-                              const PaintImage& paint_image,
-                              CacheEntry* cache_entry);
-  void AddBudgetForImage(const ImageKey& key, CacheEntry* entry);
-  void RemoveBudgetForImage(const ImageKey& key, CacheEntry* entry);
-
-  std::unique_ptr<CacheEntry> DoDecodeImage(const ImageKey& key,
-                                            const PaintImage& image);
-  std::unique_ptr<CacheEntry> GenerateCacheEntryFromCandidate(
-      const ImageKey& key,
-      const DecodedDrawImage& candidate);
-
-  void UnrefImage(const ImageKey& key);
+  void CacheDecodedImages(const ImageKey& key,
+                          std::unique_ptr<DecodedImage> decoded_image);
+  void CleanupDecodedImagesCache(const ImageKey& key,
+                                 ImageMRUCache::iterator it);
 
   std::unordered_map<ImageKey, scoped_refptr<TileTask>, ImageKeyHash>
       pending_in_raster_image_tasks_;
@@ -300,6 +314,7 @@ class CC_EXPORT SoftwareImageDecodeCache
 
   // Decoded images and ref counts (predecode path).
   ImageMRUCache decoded_images_;
+  std::unordered_map<ImageKey, int, ImageKeyHash> decoded_images_ref_counts_;
 
   // A map of PaintImage::FrameKey to the ImageKeys for cached decodes of this
   // PaintImage.
@@ -308,6 +323,11 @@ class CC_EXPORT SoftwareImageDecodeCache
                      PaintImage::FrameKeyHash>
       frame_key_to_image_keys_;
 
+  // Decoded image and ref counts (at-raster decode path).
+  ImageMRUCache at_raster_decoded_images_;
+  std::unordered_map<ImageKey, int, ImageKeyHash>
+      at_raster_decoded_images_ref_counts_;
+
   MemoryBudget locked_images_budget_;
 
   SkColorType color_type_;

cc/tiles/software_image_decode_cache_unittest.cc

@@ -654,36 +654,6 @@ TEST(SoftwareImageDecodeCacheTest, GetTaskForImageSameImage) {
   cache.UnrefImage(draw_image);
 }
 
-TEST(SoftwareImageDecodeCacheTest, GetTaskForImageProcessUnrefCancel) {
-  TestSoftwareImageDecodeCache cache;
-  PaintImage paint_image = CreatePaintImage(100, 100);
-  bool is_decomposable = true;
-  SkFilterQuality quality = kHigh_SkFilterQuality;
-
-  DrawImage draw_image(
-      paint_image, SkIRect::MakeWH(paint_image.width(), paint_image.height()),
-      quality, CreateMatrix(SkSize::Make(0.5f, 0.5f), is_decomposable),
-      PaintImage::kDefaultFrameIndex, DefaultColorSpace());
-  ImageDecodeCache::TaskResult result =
-      cache.GetTaskForImageAndRef(draw_image, ImageDecodeCache::TracingInfo());
-  EXPECT_TRUE(result.need_unref);
-  EXPECT_TRUE(result.task);
-
-  TestTileTaskRunner::ProcessTask(result.task.get());
-  cache.UnrefImage(draw_image);
-
-  result =
-      cache.GetTaskForImageAndRef(draw_image, ImageDecodeCache::TracingInfo());
-  EXPECT_TRUE(result.need_unref);
-  EXPECT_TRUE(result.task);
-
-  TestTileTaskRunner::CancelTask(result.task.get());
-  TestTileTaskRunner::CompleteTask(result.task.get());
-  // This is expected to pass instead of DCHECKing since we're reducing the ref
-  // for an image which isn't locked to begin with.
-  cache.UnrefImage(draw_image);
-}
-
 TEST(SoftwareImageDecodeCacheTest, GetTaskForImageSameImageDifferentQuality) {
   TestSoftwareImageDecodeCache cache;
   PaintImage paint_image = CreatePaintImage(100, 100);
@@ -1112,6 +1082,97 @@ TEST(SoftwareImageDecodeCacheTest,
   cache.DrawWithImageFinished(draw_image, another_decoded_draw_image);
 }
 
+TEST(SoftwareImageDecodeCacheTest,
+     GetDecodedImageForDrawAtRasterDecodeDoesNotPreventTasks) {
+  TestSoftwareImageDecodeCache cache;
+  bool is_decomposable = true;
+  SkFilterQuality quality = kHigh_SkFilterQuality;
+
+  PaintImage paint_image = CreatePaintImage(100, 100);
+  DrawImage draw_image(
+      paint_image, SkIRect::MakeWH(paint_image.width(), paint_image.height()),
+      quality, CreateMatrix(SkSize::Make(0.5f, 0.5f), is_decomposable),
+      PaintImage::kDefaultFrameIndex, DefaultColorSpace());
+
+  DecodedDrawImage decoded_draw_image =
+      cache.GetDecodedImageForDraw(draw_image);
+  EXPECT_TRUE(decoded_draw_image.image());
+  EXPECT_EQ(50, decoded_draw_image.image()->width());
+  EXPECT_EQ(50, decoded_draw_image.image()->height());
+  EXPECT_FLOAT_EQ(0.5f, decoded_draw_image.scale_adjustment().width());
+  EXPECT_FLOAT_EQ(0.5f, decoded_draw_image.scale_adjustment().height());
+  EXPECT_EQ(kLow_SkFilterQuality, decoded_draw_image.filter_quality());
+  EXPECT_FALSE(decoded_draw_image.is_scale_adjustment_identity());
+  EXPECT_TRUE(decoded_draw_image.is_at_raster_decode());
+
+  ImageDecodeCache::TaskResult result =
+      cache.GetTaskForImageAndRef(draw_image, ImageDecodeCache::TracingInfo());
+  EXPECT_TRUE(result.need_unref);
+  EXPECT_TRUE(result.task);
+
+  TestTileTaskRunner::ProcessTask(result.task.get());
+
+  DecodedDrawImage another_decoded_draw_image =
+      cache.GetDecodedImageForDraw(draw_image);
+  // This should get the new decoded/locked image, not the one we're using at
+  // raster.
+  // TODO(vmpstr): We can possibly optimize this so that the decode simply moves
+  // the image to the right spot.
+  EXPECT_NE(decoded_draw_image.image()->uniqueID(),
+            another_decoded_draw_image.image()->uniqueID());
+  EXPECT_FALSE(another_decoded_draw_image.is_at_raster_decode());
+
+  cache.DrawWithImageFinished(draw_image, decoded_draw_image);
+  cache.DrawWithImageFinished(draw_image, another_decoded_draw_image);
+  cache.UnrefImage(draw_image);
+}
+
+TEST(SoftwareImageDecodeCacheTest,
+     GetDecodedImageForDrawAtRasterDecodeIsUsedForLockedCache) {
+  TestSoftwareImageDecodeCache cache;
+  bool is_decomposable = true;
+  SkFilterQuality quality = kHigh_SkFilterQuality;
+
+  PaintImage paint_image = CreatePaintImage(100, 100);
+  DrawImage draw_image(
+      paint_image, SkIRect::MakeWH(paint_image.width(), paint_image.height()),
+      quality, CreateMatrix(SkSize::Make(0.5f, 0.5f), is_decomposable),
+      PaintImage::kDefaultFrameIndex, DefaultColorSpace());
+
+  DecodedDrawImage decoded_draw_image =
+      cache.GetDecodedImageForDraw(draw_image);
+  EXPECT_TRUE(decoded_draw_image.image());
+  EXPECT_EQ(50, decoded_draw_image.image()->width());
+  EXPECT_EQ(50, decoded_draw_image.image()->height());
+  EXPECT_FLOAT_EQ(0.5f, decoded_draw_image.scale_adjustment().width());
+  EXPECT_FLOAT_EQ(0.5f, decoded_draw_image.scale_adjustment().height());
+  EXPECT_EQ(kLow_SkFilterQuality, decoded_draw_image.filter_quality());
+  EXPECT_FALSE(decoded_draw_image.is_scale_adjustment_identity());
+  EXPECT_TRUE(decoded_draw_image.is_at_raster_decode());
+
+  ImageDecodeCache::TaskResult result =
+      cache.GetTaskForImageAndRef(draw_image, ImageDecodeCache::TracingInfo());
+  EXPECT_TRUE(result.need_unref);
+  EXPECT_TRUE(result.task);
+
+  // If we finish the draw here, then we will use it for the locked decode
+  // instead of decoding again.
+  cache.DrawWithImageFinished(draw_image, decoded_draw_image);
+
+  TestTileTaskRunner::ProcessTask(result.task.get());
+
+  DecodedDrawImage another_decoded_draw_image =
+      cache.GetDecodedImageForDraw(draw_image);
+  // This should get the decoded/locked image which we originally decoded at
+  // raster time, since it's now in the locked cache.
+  EXPECT_EQ(decoded_draw_image.image()->uniqueID(),
+            another_decoded_draw_image.image()->uniqueID());
+  EXPECT_FALSE(another_decoded_draw_image.is_at_raster_decode());
+
+  cache.DrawWithImageFinished(draw_image, another_decoded_draw_image);
+  cache.UnrefImage(draw_image);
+}
+
 TEST(SoftwareImageDecodeCacheTest, ZeroSizedImagesAreSkipped) {
   TestSoftwareImageDecodeCache cache;
   bool is_decomposable = true;
@@ -1577,18 +1638,20 @@ TEST(SoftwareImageDecodeCacheTest, RemoveUnusedImage) {
   std::vector<PaintImage::FrameKey> frame_keys;
 
   for (int i = 0; i < 10; ++i) {
-    SCOPED_TRACE(i);
     PaintImage paint_image = CreatePaintImage(100, 100);
     DrawImage draw_image(
         paint_image, SkIRect::MakeWH(paint_image.width(), paint_image.height()),
         quality, CreateMatrix(SkSize::Make(1.0f, 1.0f), is_decomposable),
         PaintImage::kDefaultFrameIndex, DefaultColorSpace());
     frame_keys.push_back(draw_image.frame_key());
+    DecodedDrawImage decoded_draw_image =
+        cache.GetDecodedImageForDraw(draw_image);
     ImageDecodeCache::TaskResult result = cache.GetTaskForImageAndRef(
         draw_image, ImageDecodeCache::TracingInfo());
     EXPECT_TRUE(result.need_unref);
     EXPECT_TRUE(result.task);
     TestTileTaskRunner::ProcessTask(result.task.get());
+    cache.DrawWithImageFinished(draw_image, decoded_draw_image);
     cache.UnrefImage(draw_image);
   }
 
@@ -1655,314 +1718,5 @@ TEST(SoftwareImageDecodeCacheTest, CacheDecodesExpectedFrames) {
   cache.DrawWithImageFinished(subset_draw_image, decoded_image);
 }
 
-TEST(SoftwareImageDecodeCacheTest, UseClosestAvailableDecode) {
-  TestSoftwareImageDecodeCache cache;
-  bool is_decomposable = true;
-  SkFilterQuality quality = kMedium_SkFilterQuality;
-  PaintImage paint_image = CreatePaintImage(512, 512);
-  SkIRect src_rect = SkIRect::MakeWH(paint_image.width(), paint_image.height());
-
-  // Populate a 0.5 scale image in the cache.
-  DrawImage draw_image_50(
-      paint_image, src_rect, quality,
-      CreateMatrix(SkSize::Make(0.5f, 0.5f), is_decomposable),
-      PaintImage::kDefaultFrameIndex, DefaultColorSpace());
-  ImageDecodeCache::TaskResult result_50 = cache.GetTaskForImageAndRef(
-      draw_image_50, ImageDecodeCache::TracingInfo());
-  EXPECT_TRUE(result_50.task);
-  EXPECT_TRUE(result_50.need_unref);
-
-  TestTileTaskRunner::ProcessTask(result_50.task.get());
-  // Clear all (unlocked) images from the cache.
-  cache.ClearCache();
-
-  // Verify that we indeed have a cached item at that scale.
-  {
-    DecodedDrawImage decoded = cache.GetDecodedImageForDraw(draw_image_50);
-    EXPECT_EQ(256, decoded.image()->width());
-    EXPECT_EQ(256, decoded.image()->height());
-    EXPECT_FALSE(decoded.is_at_raster_decode());
-    cache.DrawWithImageFinished(draw_image_50, decoded);
-  }
-
-  // At this point we should only have one image in the cache.
-  EXPECT_EQ(1u, cache.GetNumCacheEntriesForTesting());
-
-  // Get a task for 0.125 scale, which should use the 0.5 scale decode and scale
-  // it 4x.
-  DrawImage draw_image_125(
-      paint_image, src_rect, quality,
-      CreateMatrix(SkSize::Make(0.125f, 0.125f), is_decomposable),
-      PaintImage::kDefaultFrameIndex, DefaultColorSpace());
-  ImageDecodeCache::TaskResult result_125 = cache.GetTaskForImageAndRef(
-      draw_image_125, ImageDecodeCache::TracingInfo());
-  EXPECT_TRUE(result_125.task);
-  EXPECT_TRUE(result_125.need_unref);
-
-  TestTileTaskRunner::ProcessTask(result_125.task.get());
-
-  // Note that we don't clear the cache, since there should be no transient
-  // entries, just the locked ones.
-  // Verify that the draw image is at the right scale.
-  {
-    DecodedDrawImage decoded = cache.GetDecodedImageForDraw(draw_image_125);
-    EXPECT_EQ(64, decoded.image()->width());
-    EXPECT_EQ(64, decoded.image()->height());
-    EXPECT_FALSE(decoded.is_at_raster_decode());
-    cache.DrawWithImageFinished(draw_image_125, decoded);
-  }
-
-  // Now we should only have 2 images in the cache (0.5 and 0.125 scales).
-  EXPECT_EQ(2u, cache.GetNumCacheEntriesForTesting());
-
-  // Clean up.
-  cache.UnrefImage(draw_image_50);
-  cache.UnrefImage(draw_image_125);
-}
-
-TEST(SoftwareImageDecodeCacheTest, UseClosestAvailableDecodeAtRaster) {
-  TestSoftwareImageDecodeCache cache;
-  bool is_decomposable = true;
-  SkFilterQuality quality = kMedium_SkFilterQuality;
-  PaintImage paint_image = CreatePaintImage(512, 512);
-  SkIRect src_rect = SkIRect::MakeWH(paint_image.width(), paint_image.height());
-
-  // Populate a 0.5 scale image in the cache.
-  DrawImage draw_image_50(
-      paint_image, src_rect, quality,
-      CreateMatrix(SkSize::Make(0.5f, 0.5f), is_decomposable),
-      PaintImage::kDefaultFrameIndex, DefaultColorSpace());
-  ImageDecodeCache::TaskResult result_50 = cache.GetTaskForImageAndRef(
-      draw_image_50, ImageDecodeCache::TracingInfo());
-  EXPECT_TRUE(result_50.task);
-  EXPECT_TRUE(result_50.need_unref);
-
-  TestTileTaskRunner::ProcessTask(result_50.task.get());
-  // Clear all (unlocked) images from the cache.
-  cache.ClearCache();
-
-  // Verify that we indeed have a cached item at that scale.
-  {
-    DecodedDrawImage decoded = cache.GetDecodedImageForDraw(draw_image_50);
-    EXPECT_EQ(256, decoded.image()->width());
-    EXPECT_EQ(256, decoded.image()->height());
-    EXPECT_FALSE(decoded.is_at_raster_decode());
-    cache.DrawWithImageFinished(draw_image_50, decoded);
-  }
-
-  // At this point we should only have one image in the cache.
-  EXPECT_EQ(1u, cache.GetNumCacheEntriesForTesting());
-
-  // Get a task for 0.125 scale, which should use the 0.5 scale decode and scale
-  // it 4x.
-  DrawImage draw_image_125(
-      paint_image, src_rect, quality,
-      CreateMatrix(SkSize::Make(0.125f, 0.125f), is_decomposable),
-      PaintImage::kDefaultFrameIndex, DefaultColorSpace());
-
-  // Verify that the draw image at raster is at the right scale.
-  {
-    DecodedDrawImage decoded = cache.GetDecodedImageForDraw(draw_image_125);
-    EXPECT_EQ(64, decoded.image()->width());
-    EXPECT_EQ(64, decoded.image()->height());
-    EXPECT_TRUE(decoded.is_at_raster_decode());
-    cache.DrawWithImageFinished(draw_image_125, decoded);
-  }
-
-  // Now we should only have 2 images in the cache (0.5 and 0.125 scales).
-  EXPECT_EQ(2u, cache.GetNumCacheEntriesForTesting());
-
-  // Clean up.
-  cache.UnrefImage(draw_image_50);
-}
-
-TEST(SoftwareImageDecodeCacheTest, UseClosestAvailableDecodeNotSmaller) {
-  TestSoftwareImageDecodeCache cache;
-  bool is_decomposable = true;
-  SkFilterQuality quality = kMedium_SkFilterQuality;
-  PaintImage paint_image = CreatePaintImage(512, 512);
-  SkIRect src_rect = SkIRect::MakeWH(paint_image.width(), paint_image.height());
-
-  // Populate a 0.25 scale image in the cache.
-  DrawImage draw_image_25(
-      paint_image, src_rect, quality,
-      CreateMatrix(SkSize::Make(0.25f, 0.25f), is_decomposable),
-      PaintImage::kDefaultFrameIndex, DefaultColorSpace());
-  ImageDecodeCache::TaskResult result_25 = cache.GetTaskForImageAndRef(
-      draw_image_25, ImageDecodeCache::TracingInfo());
-  EXPECT_TRUE(result_25.task);
-  EXPECT_TRUE(result_25.need_unref);
-
-  TestTileTaskRunner::ProcessTask(result_25.task.get());
-  // Clear all (unlocked) images from the cache.
-  cache.ClearCache();
-
-  // Verify that we indeed have a cached item at that scale.
-  {
-    DecodedDrawImage decoded = cache.GetDecodedImageForDraw(draw_image_25);
-    EXPECT_EQ(128, decoded.image()->width());
-    EXPECT_EQ(128, decoded.image()->height());
-    EXPECT_FALSE(decoded.is_at_raster_decode());
-    cache.DrawWithImageFinished(draw_image_25, decoded);
-  }
-
-  // At this point we should only have one image in the cache.
-  EXPECT_EQ(1u, cache.GetNumCacheEntriesForTesting());
-
-  // Get a task for 0.5 scale, which can't use the 0.125 scale, so it would
-  // generate a 1.0 scale first and use that.
-  DrawImage draw_image_50(
-      paint_image, src_rect, quality,
-      CreateMatrix(SkSize::Make(0.50, 0.50), is_decomposable),
-      PaintImage::kDefaultFrameIndex, DefaultColorSpace());
-  ImageDecodeCache::TaskResult result_50 = cache.GetTaskForImageAndRef(
-      draw_image_50, ImageDecodeCache::TracingInfo());
-  EXPECT_TRUE(result_50.task);
-  EXPECT_TRUE(result_50.need_unref);
-
-  TestTileTaskRunner::ProcessTask(result_50.task.get());
-
-  // Verify that the draw image at raster is at the right scale.
-  {
-    DecodedDrawImage decoded = cache.GetDecodedImageForDraw(draw_image_50);
-    EXPECT_EQ(256, decoded.image()->width());
-    EXPECT_EQ(256, decoded.image()->height());
-    EXPECT_FALSE(decoded.is_at_raster_decode());
-    cache.DrawWithImageFinished(draw_image_50, decoded);
-  }
-
-  // Now we should only have 3 images in the cache (1.0, 0.5 and 0.25 scales).
-  EXPECT_EQ(3u, cache.GetNumCacheEntriesForTesting());
-
-  // Clean up.
-  cache.UnrefImage(draw_image_25);
-  cache.UnrefImage(draw_image_50);
-}
-
-TEST(SoftwareImageDecodeCacheTest,
-     UseClosestAvailableDecodeNotSmallerAtRaster) {
-  TestSoftwareImageDecodeCache cache;
-  bool is_decomposable = true;
-  SkFilterQuality quality = kMedium_SkFilterQuality;
-  PaintImage paint_image = CreatePaintImage(512, 512);
-  SkIRect src_rect = SkIRect::MakeWH(paint_image.width(), paint_image.height());
-
-  // Populate a 0.25 scale image in the cache.
-  DrawImage draw_image_25(
-      paint_image, src_rect, quality,
-      CreateMatrix(SkSize::Make(0.25f, 0.25f), is_decomposable),
-      PaintImage::kDefaultFrameIndex, DefaultColorSpace());
-  ImageDecodeCache::TaskResult result_25 = cache.GetTaskForImageAndRef(
-      draw_image_25, ImageDecodeCache::TracingInfo());
-  EXPECT_TRUE(result_25.task);
-  EXPECT_TRUE(result_25.need_unref);
-
-  TestTileTaskRunner::ProcessTask(result_25.task.get());
-  // Clear all (unlocked) images from the cache.
-  cache.ClearCache();
-
-  // Verify that we indeed have a cached item at that scale.
-  {
-    DecodedDrawImage decoded = cache.GetDecodedImageForDraw(draw_image_25);
-    EXPECT_EQ(128, decoded.image()->width());
-    EXPECT_EQ(128, decoded.image()->height());
-    EXPECT_FALSE(decoded.is_at_raster_decode());
-    cache.DrawWithImageFinished(draw_image_25, decoded);
-  }
-
-  // At this point we should only have one image in the cache.
-  EXPECT_EQ(1u, cache.GetNumCacheEntriesForTesting());
-
-  // Get a task for 0.5 scale, which can't use the 0.125 scale, so it would
-  // generate a 1.0 scale first and use that.
-  DrawImage draw_image_50(
-      paint_image, src_rect, quality,
-      CreateMatrix(SkSize::Make(0.50, 0.50), is_decomposable),
-      PaintImage::kDefaultFrameIndex, DefaultColorSpace());
-
-  // Verify that the draw image at raster is at the right scale.
-  {
-    DecodedDrawImage decoded = cache.GetDecodedImageForDraw(draw_image_50);
-    EXPECT_EQ(256, decoded.image()->width());
-    EXPECT_EQ(256, decoded.image()->height());
-    EXPECT_TRUE(decoded.is_at_raster_decode());
-    cache.DrawWithImageFinished(draw_image_50, decoded);
-  }
-
-  // Now we should only have 3 images in the cache (1.0, 0.5 and 0.25 scales).
-  EXPECT_EQ(3u, cache.GetNumCacheEntriesForTesting());
-
-  // Clean up.
-  cache.UnrefImage(draw_image_25);
-}
-
-TEST(SoftwareImageDecodeCache, UseClosestAvailableDecodeWithTheSameSrc) {
-  TestSoftwareImageDecodeCache cache;
-  bool is_decomposable = true;
-  SkFilterQuality quality = kMedium_SkFilterQuality;
-  PaintImage paint_image = CreatePaintImage(512, 512);
-  SkIRect full_src_rect =
-      SkIRect::MakeWH(paint_image.width(), paint_image.height());
-  SkIRect decoded_src_rect = SkIRect::MakeXYWH(1, 1, paint_image.width() - 2,
-                                               paint_image.height() - 2);
-
-  // Populate a 0.5f scale image in the cache with a non-full src rect.
-  DrawImage draw_image_50(
-      paint_image, decoded_src_rect, quality,
-      CreateMatrix(SkSize::Make(0.5f, 0.5f), is_decomposable),
-      PaintImage::kDefaultFrameIndex, DefaultColorSpace());
-  ImageDecodeCache::TaskResult result_50 = cache.GetTaskForImageAndRef(
-      draw_image_50, ImageDecodeCache::TracingInfo());
-  EXPECT_TRUE(result_50.task);
-  EXPECT_TRUE(result_50.need_unref);
-
-  TestTileTaskRunner::ProcessTask(result_50.task.get());
-  // Clear all (unlocked) images from the cache.
-  cache.ClearCache();
-
-  // Verify that we indeed have a cached item at that scale.
-  {
-    DecodedDrawImage decoded = cache.GetDecodedImageForDraw(draw_image_50);
-    // Note the subrect is reflected in the size.
-    EXPECT_EQ(255, decoded.image()->width());
-    EXPECT_EQ(255, decoded.image()->height());
-    EXPECT_FALSE(decoded.is_at_raster_decode());
-    cache.DrawWithImageFinished(draw_image_50, decoded);
-  }
-
-  // At this point we should only have one image in the cache.
-  EXPECT_EQ(1u, cache.GetNumCacheEntriesForTesting());
-
-  // Get a task for 0.25 scale, which can't use the 0.5f scale, since it was
-  // generated a different src rect. It would generate a 1.0 scale first with a
-  // full src rect and use that.
-  DrawImage draw_image_25(
-      paint_image, full_src_rect, quality,
-      CreateMatrix(SkSize::Make(0.25, 0.25), is_decomposable),
-      PaintImage::kDefaultFrameIndex, DefaultColorSpace());
-  ImageDecodeCache::TaskResult result_25 = cache.GetTaskForImageAndRef(
-      draw_image_25, ImageDecodeCache::TracingInfo());
-  EXPECT_TRUE(result_25.task);
-  EXPECT_TRUE(result_25.need_unref);
-
-  TestTileTaskRunner::ProcessTask(result_25.task.get());
-
-  // Verify that the draw image at raster is at the right scale.
-  {
-    DecodedDrawImage decoded = cache.GetDecodedImageForDraw(draw_image_25);
-    EXPECT_EQ(128, decoded.image()->width());
-    EXPECT_EQ(128, decoded.image()->height());
-    EXPECT_FALSE(decoded.is_at_raster_decode());
-    cache.DrawWithImageFinished(draw_image_25, decoded);
-  }
-
-  // Now we should only have 3 images in the cache (1.0, 0.5, and 0.25 scales).
-  EXPECT_EQ(3u, cache.GetNumCacheEntriesForTesting());
-
-  // Clean up.
-  cache.UnrefImage(draw_image_50);
-  cache.UnrefImage(draw_image_25);
-}
-
 }  // namespace
 }  // namespace cc