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

cc/tiles/software_image_decode_controller.cc

Index: cc/tiles/software_image_decode_controller.cc
diff --git a/cc/tiles/software_image_decode_controller.cc b/cc/tiles/software_image_decode_controller.cc
index ee19c9a10168ba690b06c04cae8212f594bcb920..c2dec8039e208015d6c98785ffcae90559e8be3e 100644
--- a/cc/tiles/software_image_decode_controller.cc
+++ b/cc/tiles/software_image_decode_controller.cc
@@ -63,6 +63,24 @@ class AutoRemoveKeyFromTaskMap {
   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,
@@ -550,10 +568,10 @@ SoftwareImageDecodeController::GetSubrectImageDecode(
 
   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_);
@@ -577,8 +595,6 @@ SoftwareImageDecodeController::GetSubrectImageDecode(
     // 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),
@@ -605,10 +621,10 @@ SoftwareImageDecodeController::GetScaledImageDecode(
 
   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);
@@ -643,11 +659,6 @@ SoftwareImageDecodeController::GetScaledImageDecode(
     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()),