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

components/image_fetcher/image_data_fetcher.cc

Index: components/image_fetcher/image_data_fetcher.cc
diff --git a/components/image_fetcher/image_data_fetcher.cc b/components/image_fetcher/image_data_fetcher.cc
new file mode 100644
index 0000000000000000000000000000000000000000..29929091e4dfe4bf375c9670dbce95ba50560ffc
--- /dev/null
+++ b/components/image_fetcher/image_data_fetcher.cc
@@ -0,0 +1,107 @@
+// 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.
+
+#include "components/image_fetcher/image_data_fetcher.h"
+
+#include "net/base/load_flags.h"
+#include "net/url_request/url_request_context_getter.h"
+#include "net/url_request/url_request_status.h"
+#include "url/gurl.h"
+
+namespace image_fetcher {
+
+// An active image URL fetcher request. The class contains any related request
+// state and logic for handling a single image url request.
+class ImageDataFetcher::ImageDataFetcherRequest
+    : public net::URLFetcherDelegate {
+ public:
+  ImageDataFetcherRequest(const GURL& url,
+                         const ImageDataFetcherCallback& callback,
+                         ImageDataFetcher* image_url_fetcher);
+  ~ImageDataFetcherRequest() override {}
+
+  // Sends the URL requests.
+  void Start(net::URLRequestContextGetter* request_context);
+
+ private:
+  // Method inherited from URLFetcherDelegate
+  void OnURLFetchComplete(const net::URLFetcher* source) override;
+
+  // The URL of the image to fetch.
+  const GURL url_;
+
+  // The callback to run after the image data was fetched. The callback will
+  // be run even if the image data could not be fetched successfully.
+  ImageDataFetcherCallback callback_;
+
+  // The ImageDataFetcher that owns the ImageDataFetcherRequest.
+  ImageDataFetcher* image_url_fetcher_;
+
+  std::unique_ptr<net::URLFetcher> url_fetcher_;
+
+  DISALLOW_COPY_AND_ASSIGN(ImageDataFetcherRequest);
+};
+
+ImageDataFetcher::ImageDataFetcherRequest::ImageDataFetcherRequest(
+    const GURL& url,
+    const ImageDataFetcherCallback& callback,
+    ImageDataFetcher* image_url_fetcher)
+    : url_(url),
+      callback_(callback),
+      image_url_fetcher_(image_url_fetcher) {
+

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

nit: remove the empty line

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

Done.

+}
+
+void ImageDataFetcher::ImageDataFetcherRequest::Start(
+    net::URLRequestContextGetter* request_context) {
+  DCHECK(!url_fetcher_);
+
+  url_fetcher_ = net::URLFetcher::Create(url_, net::URLFetcher::GET, this);
+  url_fetcher_->SetRequestContext(request_context);
+  url_fetcher_->SetReferrer(std::string());
+  url_fetcher_->SetReferrerPolicy(
+      net::URLRequest::CLEAR_REFERRER_ON_TRANSITION_FROM_SECURE_TO_INSECURE);
+  url_fetcher_->SetLoadFlags(net::LOAD_NORMAL);
+
+  url_fetcher_->Start();
+}
+
+void ImageDataFetcher::ImageDataFetcherRequest::OnURLFetchComplete(
+    const net::URLFetcher* source) {
+  // TODO(markusheintz): Check if the if else condition can be inverted. Extract
+  // common code from the conditions.

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

I guess you could define the "std::string image_data" outside, and then "if
(..success..) GetResponse(&image_data);", and run the callback after the if?

[Bernhard Bauer, 2016/06/10 09:02:08]

+1.

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

Done

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

Done.

+  if (source->GetStatus().status() != net::URLRequestStatus::SUCCESS) {
+    callback_.Run(std::string());
+  } else {
+    std::string image_data;
+    source->GetResponseAsString(&image_data);
+    callback_.Run(image_data);
+  }
+
+  // Remove the completed ImageDataFetcherRequest from the internal request
+  // queue. This must be last in the method.
+  image_url_fetcher_->RemoveImageDataFetcherRequest(url_);
+}
+
+ImageDataFetcher::ImageDataFetcher(
+    net::URLRequestContextGetter* url_request_context_getter)
+    :url_request_context_getter_(url_request_context_getter) {}

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

nit: there should be a space after the :

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

Done.

+
+ImageDataFetcher::~ImageDataFetcher() {}
+
+void ImageDataFetcher::FetchImageURL(const GURL& url,
+                                    const ImageDataFetcherCallback& callback) {

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

misaligned

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

Already Done. I also renamed the method.

+  request_queue_.emplace(url,
+                         std::unique_ptr<ImageDataFetcherRequest>(
+                             new ImageDataFetcherRequest(url, callback, this)));
+
+  request_queue_[url]->Start(url_request_context_getter_);
+}
+
+void ImageDataFetcher::RemoveImageDataFetcherRequest(const GURL& image_url) {
+  size_t count = request_queue_.erase(image_url);
+  DCHECK(count);
+}
+
+}  // namespace image_fetcher