Pass the ImageDecoder to the ImageFetcher at creation time.

components/image_fetcher/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_
+#ifndef COMPONENTS_IMAGE_FETCHER_IMAGE_FETCHER_IMPL_H_
+#define COMPONENTS_IMAGE_FETCHER_IMAGE_FETCHER_IMPL_H_
 
 #include <map>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/callback.h"
 #include "base/macros.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;
 }
 
-// TODO(markusheintz): Move the ImageFetcherImpl to components/image_fetcher
-namespace suggestions {
+namespace image_fetcher {
+
+// TODO(markusheintz): Once the iOS implementation of the ImageFetcher is
+// removed merge the two classes ImageFetcher and ImageFetcherImpl.
 
 // image_fetcher::ImageFetcher implementation.

[Marc Treib, 2016/06/29 13:57:33]

nit: This comment is a bit redundant now :)

[markusheintz_, 2016/06/29 14:36:45]

Comment removed

 class ImageFetcherImpl : public image_fetcher::ImageFetcher {
  public:
-  explicit ImageFetcherImpl(net::URLRequestContextGetter* url_request_context);
+  ImageFetcherImpl(
+      image_fetcher::ImageDecoder* image_decoder,

[Bernhard Bauer, 2016/06/29 13:55:58]

Pass |image_decoder| as a unique_ptr? For ease of construction you can use
base::MakeUnique<ImageDecoderImpl>() at call sites.

[Marc Treib, 2016/06/29 13:57:33]

This should be a unique_ptr, since the ImageFetcherImpl takes ownership

[markusheintz_, 2016/06/29 14:36:45]

Done.

[markusheintz_, 2016/06/29 14:36:45]

Done.

+      net::URLRequestContextGetter* url_request_context);
   ~ImageFetcherImpl() override;
 
   void SetImageFetcherDelegate(
       image_fetcher::ImageFetcherDelegate* delegate) override;

[Bernhard Bauer, 2016/06/29 13:55:58]

Could you add a comment that the |delegate| has to be alive during the lifetime
of this object and that it's the caller's responsibility to ensure that?

[markusheintz_, 2016/06/29 14:36:45]

Done.

 
   void StartOrQueueNetworkRequest(
       const std::string& id,
       const GURL& image_url,
       base::Callback<void(const std::string&, const gfx::Image&)> callback)
       override;
 
  private:
   using CallbackVector =
       std::vector<base::Callback<void(const std::string&, const gfx::Image&)>>;
@@ skipping 21 matching lines @@
   // 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_;

[Bernhard Bauer, 2016/06/29 13:55:58]

Should this keep a scoped_refptr? 

[markusheintz_, 2016/06/29 14:36:46]

Done.

 
   std::unique_ptr<image_fetcher::ImageDecoder> image_decoder_;
 
   std::unique_ptr<image_fetcher::ImageDataFetcher> image_data_fetcher_;
 
   // Map from each image URL to the request information (associated website
   // url, fetcher, pending callbacks).
   ImageRequestMap pending_net_requests_;
 
   DISALLOW_COPY_AND_ASSIGN(ImageFetcherImpl);
 };
 
-}  // namespace suggestions
+}  // namespace image_fetcher
 
-#endif  // CHROME_BROWSER_SEARCH_SUGGESTIONS_IMAGE_FETCHER_IMPL_H_
+#endif  // COMPONENTS_IMAGE_FETCHER_IMAGE_FETCHER_IMPL_H_