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

 // Copyright 2014 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 CHROME_BROWSER_SEARCH_SUGGESTIONS_IMAGE_FETCHER_IMPL_H_
 #define CHROME_BROWSER_SEARCH_SUGGESTIONS_IMAGE_FETCHER_IMPL_H_
 
 #include <map>
 #include <string>
 #include <utility>
 #include <vector>
 
 #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"
 
 namespace gfx {
 class Image;
 }
 
 namespace net {
 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;
 
   void SetImageFetcherDelegate(
       image_fetcher::ImageFetcherDelegate* delegate) override;
 
   void StartOrQueueNetworkRequest(
       const std::string& id,
       const GURL& image_url,
       base::Callback<void(const std::string&, const gfx::Image&)> callback)
       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;
   };
 
   using ImageRequestMap = std::map<const GURL, ImageRequest>;
 
+  // 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_;
 
-  image_fetcher::ImageFetcherDelegate* delegate_;
-
-  net::URLRequestContextGetter* url_request_context_;
-
   DISALLOW_COPY_AND_ASSIGN(ImageFetcherImpl);
 };
 
 }  // namespace suggestions
 
 #endif  // CHROME_BROWSER_SEARCH_SUGGESTIONS_IMAGE_FETCHER_IMPL_H_