cc, images: Add auto finish draw class to ensure robustness.

cc/tiles/software_image_decode_controller.cc

 // Copyright 2015 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 "cc/tiles/software_image_decode_controller.h"
 
 #include <inttypes.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 45 matching lines @@
       : task_map_(task_map), key_(key) {}
   ~AutoRemoveKeyFromTaskMap() { task_map_->erase(key_); }
 
  private:
   std::unordered_map<SoftwareImageDecodeController::ImageKey,
                      scoped_refptr<TileTask>,
                      SoftwareImageDecodeController::ImageKeyHash>* task_map_;
   SoftwareImageDecodeController::ImageKey key_;
 };
 
+class AutoDrawWithImageFinished {
+ public:
+  AutoDrawWithImageFinished(SoftwareImageDecodeController* controller,
+                            const DrawImage& draw_image,
+                            const DecodedDrawImage& decoded_draw_image)
+      : controller_(controller),
+        draw_image_(draw_image),
+        decoded_draw_image_(decoded_draw_image) {}
+  ~AutoDrawWithImageFinished() {
+    controller_->DrawWithImageFinished(draw_image_, decoded_draw_image_);
+  }
+
+ private:
+  SoftwareImageDecodeController* controller_;
+  DrawImage draw_image_;

[ericrk, 2016/08/19 20:38:43]

nit: make these const, as they are only set in the constructor.

also, given that this is restricted in scope, it seems like you can store const
Decoded/DrawImage& rather than copies?

[vmpstr, 2016/08/21 03:59:33]

Done. Did this for the key above as well. 

+  DecodedDrawImage decoded_draw_image_;
+};
+
 class ImageDecodeTaskImpl : public TileTask {
  public:
   ImageDecodeTaskImpl(SoftwareImageDecodeController* controller,
                       const SoftwareImageDecodeController::ImageKey& image_key,
                       const DrawImage& image,
                       const ImageDecodeController::TracingInfo& tracing_info)
       : TileTask(true),
         controller_(controller),
         image_key_(image_key),
         image_(image),
@@ skipping 467 matching lines @@
                                      kNone_SkFilterQuality, SkMatrix::I());
   ImageKey original_size_key =
       ImageKey::FromDrawImage(original_size_draw_image);
   // Sanity checks.
   DCHECK(original_size_key.can_use_original_decode())
       << original_size_key.ToString();
   DCHECK(full_image_rect.size() == original_size_key.target_size());
 
   auto decoded_draw_image = GetDecodedImageForDrawInternal(
       original_size_key, original_size_draw_image);
-  if (!decoded_draw_image.image()) {
-    DrawWithImageFinished(original_size_draw_image, decoded_draw_image);
+  AutoDrawWithImageFinished auto_finish_draw(this, original_size_draw_image,
+                                             decoded_draw_image);
+  if (!decoded_draw_image.image())
     return nullptr;
-  }
 
   SkImageInfo subrect_info = CreateImageInfo(
       key.target_size().width(), key.target_size().height(), format_);
   std::unique_ptr<base::DiscardableMemory> subrect_pixels;
   {
     TRACE_EVENT0("disabled-by-default-cc.debug",
                  "SoftwareImageDecodeController::GetSubrectImageDecode - "
                  "allocate subrect pixels");
     subrect_pixels =
         base::DiscardableMemoryAllocator::GetInstance()
             ->AllocateLockedDiscardableMemory(subrect_info.minRowBytes() *
                                               subrect_info.height());
   }
   {
     TRACE_EVENT0("disabled-by-default-cc.debug",
                  "SoftwareImageDecodeController::GetOriginalImageDecode - "
                  "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);
-    // Release the original image, since we don't need it anymore.
-    DrawWithImageFinished(original_size_draw_image, decoded_draw_image);
   }
   return base::WrapUnique(
       new DecodedImage(subrect_info, std::move(subrect_pixels),
                        SkSize::Make(-key.src_rect().x(), -key.src_rect().y()),
                        next_tracing_id_.GetNext()));
 }
 
 std::unique_ptr<SoftwareImageDecodeController::DecodedImage>
 SoftwareImageDecodeController::GetScaledImageDecode(
     const ImageKey& key,
     sk_sp<const SkImage> image) {
   // Construct a key to use in GetDecodedImageForDrawInternal().
   // This allows us to reuse an image in any cache if available.
   gfx::Rect full_image_rect(image->width(), image->height());
   DrawImage original_size_draw_image(std::move(image),
                                      gfx::RectToSkIRect(full_image_rect),
                                      kNone_SkFilterQuality, SkMatrix::I());
   ImageKey original_size_key =
       ImageKey::FromDrawImage(original_size_draw_image);
   // Sanity checks.
   DCHECK(original_size_key.can_use_original_decode())
       << original_size_key.ToString();
   DCHECK(full_image_rect.size() == original_size_key.target_size());
 
   auto decoded_draw_image = GetDecodedImageForDrawInternal(
       original_size_key, original_size_draw_image);
-  if (!decoded_draw_image.image()) {
-    DrawWithImageFinished(original_size_draw_image, decoded_draw_image);
+  AutoDrawWithImageFinished auto_finish_draw(this, original_size_draw_image,
+                                             decoded_draw_image);
+  if (!decoded_draw_image.image())
     return nullptr;
-  }
 
   SkPixmap decoded_pixmap;
   bool result = decoded_draw_image.image()->peekPixels(&decoded_pixmap);
   DCHECK(result) << key.ToString();
   if (key.src_rect() != full_image_rect) {
     result = decoded_pixmap.extractSubset(&decoded_pixmap,
                                           gfx::RectToSkIRect(key.src_rect()));
     DCHECK(result) << key.ToString();
   }
 
@@ skipping 14 matching lines @@
   DCHECK(key.filter_quality() == kHigh_SkFilterQuality ||
          key.filter_quality() == kMedium_SkFilterQuality);
   {
     TRACE_EVENT0("disabled-by-default-cc.debug",
                  "SoftwareImageDecodeController::ScaleImage - scale pixels");
     bool result =
         decoded_pixmap.scalePixels(scaled_pixmap, key.filter_quality());
     DCHECK(result) << key.ToString();
   }
 
-  // Release the original sized decode. Any other intermediate result to release
-  // would be the subrect memory. However, that's in a scoped_ptr and will be
-  // deleted automatically when we return.
-  DrawWithImageFinished(original_size_draw_image, decoded_draw_image);
-
   return base::WrapUnique(
       new DecodedImage(scaled_info, std::move(scaled_pixels),
                        SkSize::Make(-key.src_rect().x(), -key.src_rect().y()),
                        next_tracing_id_.GetNext()));
 }
 
 void SoftwareImageDecodeController::DrawWithImageFinished(
     const DrawImage& image,
     const DecodedDrawImage& decoded_image) {
   TRACE_EVENT1("disabled-by-default-cc.debug",
@@ skipping 400 matching lines @@
 void SoftwareImageDecodeController::MemoryBudget::ResetUsage() {
   current_usage_bytes_ = 0;
 }
 
 size_t SoftwareImageDecodeController::MemoryBudget::GetCurrentUsageSafe()
     const {
   return current_usage_bytes_.ValueOrDie();
 }
 
 }  // namespace cc