Refactor SoftwareImageDecodeController

cc/tiles/software_image_decode_controller.h

Index: cc/tiles/software_image_decode_controller.h
diff --git a/cc/tiles/software_image_decode_controller.h b/cc/tiles/software_image_decode_controller.h
index 4e959c7f2f8deae238e8e1e1bd9b4435c12e29ff..4a2e8ec703150455a89aab7a0ca96139c06dd440 100644
--- a/cc/tiles/software_image_decode_controller.h
+++ b/cc/tiles/software_image_decode_controller.h
@@ -187,6 +187,31 @@ class CC_EXPORT SoftwareImageDecodeController : public ImageDecodeController {
   DecodedDrawImage GetDecodedImageForDrawInternal(const ImageKey& key,
                                                   const DrawImage& draw_image);
 
+  struct DecodedImageResult {

[vmpstr, 2016/03/31 19:54:32]

Can you move types to the top (next to other types in this access specifier). 

Also, can you explain the general flow of logic here? Ie, why do we need this
extra type?

[cblume, 2016/04/09 06:48:54]

Done.

The extra type was just to facilitate the single responsibility of
DecodeImageOrUseCache and ScaleImage. ScaleImage needed the information which
DecodedImageResult holds.

But after Vlad and I talked about it a more natural division is between getting
the original image and the scaled image. The fact that this struct existed
showed it wasn't a great dividing line.

+    DecodedImageResult(SkPixmap decoded_pixmap,
+                       DrawImage original_size_draw_image,
+                       DecodedDrawImage decoded_draw_image);
+
+    SkPixmap decoded_pixmap_;

[vmpstr, 2016/03/31 19:54:32]

Typically, struct members that are public are named without the trailing
underscore in cc. 

[cblume, 2016/04/09 06:48:54]

Done.

+    DrawImage original_size_draw_image_;
+    DecodedDrawImage decoded_draw_image_;
+  };
+
+  scoped_ptr<DecodedImage> DecodeImageMediumQuality(const ImageKey& key,

[vmpstr, 2016/03/31 19:54:33]

Add comments for each of the functions please. Specifically, it should say if
the function has any side effects such as caching. 

[cblume, 2016/04/09 06:48:54]

Done.

+                                                    const SkImage& image);
+  scoped_ptr<DecodedImage> DecodeImageHighQuality(const ImageKey& key,
+                                                  const SkImage& image);
+
+  scoped_ptr<DecodedImage> GetOriginalImageDecode(const ImageKey& key,
+                                                  const SkImage& image);
+
+  DecodedImageResult DecodeImageOrUseCache(const ImageKey& key,

[vmpstr, 2016/03/31 19:54:33]

This is named somewhat awkwardly. I don't really know what it returns? Is this
different from GetOriginalImageDecode?

+                                           const SkImage& image);
+
+  scoped_ptr<DecodedImage> ScaleImage(
+      const ImageKey& key,
+      const DecodedImageResult& decoded_image_result);
+
   void SanityCheckState(int line, bool lock_acquired);
   void RefImage(const ImageKey& key);
   void RefAtRasterImage(const ImageKey& key);