Create the IOSImageDataFetcher

components/image_fetcher/ios/ios_image_data_fetcher.h

Index: components/image_fetcher/ios/ios_image_data_fetcher.h
diff --git a/components/image_fetcher/ios/ios_image_data_fetcher.h b/components/image_fetcher/ios/ios_image_data_fetcher.h
new file mode 100644
index 0000000000000000000000000000000000000000..5db66889b60a8d6431c903ca0b1ebe92b956df8e
--- /dev/null
+++ b/components/image_fetcher/ios/ios_image_data_fetcher.h
@@ -0,0 +1,35 @@
+// Copyright 2017 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 COMPONENTS_IMAGE_FETCHER_IOS_IOS_IMAGE_DATA_FETCHER_H_
+#define COMPONENTS_IMAGE_FETCHER_IOS_IOS_IMAGE_DATA_FETCHER_H_
+
+#include "components/image_fetcher/image_data_fetcher.h"
+
+namespace image_fetcher {
+
+// Callback that informs of the download of an image encoded in |data|.
+using IOSImageDataFetcherCallback = void (^)(NSData* data);
+
+class IOSImageDataFetcher : public ImageDataFetcher {

[Marc Treib, 2017/01/25 10:37:55]

I don't think inheritance is the right tool here. As it is, this class exposes
two interfaces: The base ImageDataFetcher one, and the new method
FetchImageDataWebpDecoded. And on ios, using the base interface would be wrong
(at least in most circumstances), because it'll return unusable WebP data.

I see a few alternatives; not sure yet which is the best:

- Use composition instead of inheritance: Make this a separate class which just
uses an instance of ImageDataFetcher. This is probably the least effort coming
from the current CL, but it'll make ios very different from other platforms -
essentially there won't be a common image_fetcher interface anymore.

- Actually implement the ImageDataFetcher interface: Instead of defining a new
method, override the existing FetchImageData to do the right thing. Then we can
define a factory method somewhere that will instantiate the right thing based on
the platform, and the rest of the code doesn't have to know. Downside: It's
implementation inheritance, which is mostly considered a bad thing.

- Similar, but maybe cleaner design-wise: Make ImageDataFetcher an interface,
with different implementations for ios vs other platforms. This means factoring
the common URL fetching code into a separate helper class.

- Implement the transcoding in ios's ImageFetcherImpl, rather than in
ImageDataFetcher.

WDYT?

[blundell, 2017/01/25 10:46:46]

With the caveat that I'm not close to the details here, this one ^^^ reads as
the cleanest option.

[gambard, 2017/01/30 13:37:53]

Moving to composition.

+ public:
+  // The TaskRunner is used to eventually decode the image.
+  IOSImageDataFetcher(net::URLRequestContextGetter* url_request_context_getter,
+                      const scoped_refptr<base::TaskRunner>& task_runner);
+  ~IOSImageDataFetcher() override;
+
+  // Start downloading the image at the given |image_url|. The |callback| will
+  // be called with the downloaded image, or nil if any error happened or the
+  // http response header is not HTTP_OK (200). If the url is a data URL, the
+  // http response header is considered to be HTTP_OK.
+  void FetchImageDataWebpDecoded(const GURL& image_url,
+                                 IOSImageDataFetcherCallback callback);
+
+ private:
+  // The task runner used to decode images if necessary.
+  const scoped_refptr<base::TaskRunner> task_runner_;
+};
+}  // namespace image_fetcher
+
+#endif  // COMPONENTS_IMAGE_FETCHER_IOS_IOS_IMAGE_DATA_FETCHER_H_