Split the code for fetching images and for decoding images into two separate classes. This will all…

chrome/browser/search/suggestions/image_fetcher_impl.h

Index: chrome/browser/search/suggestions/image_fetcher_impl.h
diff --git a/chrome/browser/search/suggestions/image_fetcher_impl.h b/chrome/browser/search/suggestions/image_fetcher_impl.h
index b5f8bc590bc72c820ef8d8bf764502986dd2a64e..45b8575ce7bb801deb72420284548b306cfa60a0 100644
--- a/chrome/browser/search/suggestions/image_fetcher_impl.h
+++ b/chrome/browser/search/suggestions/image_fetcher_impl.h
@@ -12,7 +12,8 @@
 
 #include "base/callback.h"
 #include "base/macros.h"
-#include "chrome/browser/bitmap_fetcher/bitmap_fetcher.h"
+#include "components/image_fetcher/image_data_fetcher.h"
+#include "components/image_fetcher/image_decoder.h"
 #include "components/image_fetcher/image_fetcher.h"
 #include "url/gurl.h"
 
@@ -27,8 +28,7 @@ class URLRequestContextGetter;
 namespace suggestions {

[blundell, 2016/06/10 08:38:12]

If this code is intended to stay in //chrome it shouldn't have any namespace at
all. If it's intended to move into //components then this is the right
namespace, but then please add a TODO about moving it into //components. Same
comment on ImageDecoderImpl (but in that case if it's intended to go into
//components why not just add it there now?).

[markusheintz_, 2016/06/10 09:43:14]

Done.

 
 // image_fetcher::ImageFetcher implementation.
-class ImageFetcherImpl : public image_fetcher::ImageFetcher,
-                         public chrome::BitmapFetcherDelegate {
+class ImageFetcherImpl : public image_fetcher::ImageFetcher {
  public:
   explicit ImageFetcherImpl(net::URLRequestContextGetter* url_request_context);
   ~ImageFetcherImpl() override;
@@ -43,31 +43,21 @@ class ImageFetcherImpl : public image_fetcher::ImageFetcher,
       override;
 
  private:
-  // Inherited from BitmapFetcherDelegate.
-  void OnFetchComplete(const GURL& image_url, const SkBitmap* bitmap) override;
-
   using CallbackVector =
       std::vector<base::Callback<void(const std::string&, const gfx::Image&)>>;
 
-  // State related to an image fetch (id, image_url, fetcher, pending
-  // callbacks).
+  // State related to an image fetch (id, pending callbacks).
   struct ImageRequest {
     ImageRequest();
-    // Struct takes ownership of |f|.
-    explicit ImageRequest(chrome::BitmapFetcher* f);
     ImageRequest(const ImageRequest& other);
     ~ImageRequest();
 
     void swap(ImageRequest* other) {

[Marc Treib, 2016/06/10 08:32:37]

I think this can be removed now

[markusheintz_, 2016/06/10 09:43:14]

Done.

[Bernhard Bauer, 2016/06/10 12:48:25]

Not really?

[Marc Treib, 2016/06/10 12:50:29]

It could have been removed if we could use map::emplace, but turns out that's
not available everywhere yet :(
So unfortunately this can't be done yet.

[Bernhard Bauer, 2016/06/10 12:52:11]

Oh, sorry, I misinterpreted the meaning of "Done" 😃

       std::swap(id, other->id);
-      std::swap(image_url, other->image_url);
       std::swap(callbacks, other->callbacks);
-      std::swap(fetcher, other->fetcher);
     }
 
     std::string id;
-    GURL image_url;
-    chrome::BitmapFetcher* fetcher;
     // Queue for pending callbacks, which may accumulate while the request is in
     // flight.
     CallbackVector callbacks;
@@ -75,14 +65,27 @@ class ImageFetcherImpl : public image_fetcher::ImageFetcher,
 
   using ImageRequestMap = std::map<const GURL, ImageRequest>;
 
-  // Map from each image URL to the request information (associated website
-  // url, fetcher, pending callbacks).
-  ImageRequestMap pending_net_requests_;
+  // Processes image URL fetched events. This is the continuation method used
+  // for creating callbacks that are passed to the ImageDataFetcher.
+  void OnImageURLFetched(const GURL& image_url, const std::string& image_data);
+
+  // Processes image decoded events. This is the continuation method used for
+  // creating callbacks that are passed to the ImageDecoder.
+  void OnImageDecoded(const GURL& image_url, const gfx::Image& image);
+
 
   image_fetcher::ImageFetcherDelegate* delegate_;
 
   net::URLRequestContextGetter* url_request_context_;
 
+  std::unique_ptr<image_fetcher::ImageDecoder> image_decoder_;
+
+  std::unique_ptr<image_fetcher::ImageDataFetcher> image_url_fetcher_;

[Marc Treib, 2016/06/10 08:32:37]

image_data_fetcher_ now?

[markusheintz_, 2016/06/10 09:43:14]

Done.

+
+  // Map from each image URL to the request information (associated website
+  // url, fetcher, pending callbacks).
+  ImageRequestMap pending_net_requests_;
+
   DISALLOW_COPY_AND_ASSIGN(ImageFetcherImpl);
 };