Refactor SoftwareImageDecodeController

cc/tiles/software_image_decode_controller.h

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CC_TILES_SOFTWARE_IMAGE_DECODE_CONTROLLER_H_
 #define CC_TILES_SOFTWARE_IMAGE_DECODE_CONTROLLER_H_
 
 #include <stdint.h>
 
 #include <unordered_map>
@@ skipping 169 matching lines @@
   // 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);
 
+  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);
   void UnrefAtRasterImage(const ImageKey& key);
 
   // These functions indicate whether the images can be handled and cached by
   // ImageDecodeController or whether they will fall through to Skia (with
   // exception of possibly prerolling them). Over time these should return
   // "false" in less cases, as the ImageDecodeController should start handling
   // more of them.
@@ skipping 24 matching lines @@
   // image the first time we see it. This mimics the previous behavior and
   // should over time change as the compositor starts to handle more cases.
   std::unordered_set<uint32_t> prerolled_images_;
 
   ResourceFormat format_;
 };
 
 }  // namespace cc
 
 #endif  // CC_TILES_SOFTWARE_IMAGE_DECODE_CONTROLLER_H_