Create the IOSImageDataFetcherWrapper

components/image_fetcher/ios/ios_image_data_fetcher_wrapper.mm

+// 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.
+
+#import "components/image_fetcher/ios/ios_image_data_fetcher_wrapper.h"
+
+#import "base/mac/bind_objc_block.h"
+#import "base/mac/scoped_nsobject.h"
+#include "base/memory/ptr_util.h"
+#include "base/task_runner.h"
+#include "base/task_runner_util.h"
+#import "components/image_fetcher/image_data_fetcher.h"
+#import "ios/web/public/image_fetcher/webp_decoder.h"
+#include "net/http/http_response_headers.h"
+#include "net/http/http_status_code.h"
+#include "net/url_request/url_fetcher.h"
+#include "url/url_constants.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+#pragma mark - WebpDecoderDelegate
+
+namespace {
+
+class WebpDecoderDelegate : public webp_transcode::WebpDecoder::Delegate {

[markusheintz_, 2017/01/31 15:51:41]

Don't need to do this in this CL. Just want to document it for followups and to
get your thoughts on this:

This is almost an identical copy of the inner class in 
ios/chrome/browser/suggestions/ios_image_decoder_impl.mm

I wonder whether this logic should be shared by having a WebpDecoderDelegate
that is used by both files.

I guess the problem of the ImageDecoder interface is that returns a gfx::Image
which is not what you want.
Maybe this can be tweaked too. probably harder.

[gambard, 2017/01/31 16:55:53]

Yes, I plan to move this code to a shared location with the image decoder and
the tests. But this CL is already getting big so it will be in a future CL.

+ public:
+  NSData* data() const { return decoded_image_; }
+
+  // WebpDecoder::Delegate methods
+  void OnFinishedDecoding(bool success) override {
+    if (!success)
+      decoded_image_.reset();
+  }
+  void SetImageFeatures(
+      size_t total_size,
+      webp_transcode::WebpDecoder::DecodedImageFormat format) override {
+    decoded_image_.reset([[NSMutableData alloc] initWithCapacity:total_size]);
+  }
+  void OnDataDecoded(NSData* data) override {
+    DCHECK(decoded_image_);
+    [decoded_image_ appendData:data];
+  }
+
+ private:
+  ~WebpDecoderDelegate() override {}
+  base::scoped_nsobject<NSMutableData> decoded_image_;
+};
+
+// Content-type header for WebP images.
+static const char kWEBPFirstBytes[4] = {'R', 'I', 'F', 'F'};

[Marc Treib, 2017/01/31 16:17:25]

Is this enough to accurately identify WebPs? The format seems to be
RIFF????WEBP (where ? means any character)
so this might result in some false positives?

[gambard, 2017/01/31 16:55:54]

When I move the WebpDecoderDelegate to a shared location I will use the
IsWebpImage currently in
ios/chrome/browser/suggestions/ios_image_decoder_impl.mm which checks for
correct header.

But I should probably do it the right way now :)

[Marc Treib, 2017/01/31 17:25:24]

Yup, I think that's better :)

[markusheintz_, 2017/02/01 08:09:28]

+1

+
+// Returns a NSData object containing the decoded image.
+// Returns nil in case of failure.
+NSData* DecodeWebpImage(NSData* webp_image) {
+  scoped_refptr<WebpDecoderDelegate> delegate(new WebpDecoderDelegate);
+  scoped_refptr<webp_transcode::WebpDecoder> decoder(
+      new webp_transcode::WebpDecoder(delegate.get()));
+  decoder->OnDataReceived(webp_image);
+  DLOG_IF(ERROR, !delegate->data()) << "WebP image decoding failed.";
+  return base::scoped_nsobject<NSData>(delegate->data());
+}
+
+}  // namespace
+
+#pragma mark - IOSImageDataFetcherWrapper
+
+namespace image_fetcher {
+
+IOSImageDataFetcherWrapper::IOSImageDataFetcherWrapper(
+    net::URLRequestContextGetter* url_request_context_getter,
+    const scoped_refptr<base::TaskRunner>& task_runner)
+    : task_runner_(task_runner) {
+  DCHECK(task_runner_.get());
+  image_data_fetcher_ =
+      base::MakeUnique<ImageDataFetcher>(url_request_context_getter);

[Marc Treib, 2017/01/31 16:17:25]

This could move into the initializer list above, and I think it could be a
direct member rather than a pointer.

[gambard, 2017/01/31 16:55:54]

Done.
I usually use pointer to use forward declaration.

[Marc Treib, 2017/01/31 17:25:24]

IIRC, the style guide recommends against using a pointer for only that reason.
But then, there's so many cases where you need a pointer anyway, so.. meh.

+}
+
+IOSImageDataFetcherWrapper::~IOSImageDataFetcherWrapper() {}
+
+void IOSImageDataFetcherWrapper::FetchImageDataWebpDecoded(
+    const GURL& image_url,
+    IOSImageDataFetcherCallback callback) {
+  DCHECK(callback);
+  if (!callback)

[Marc Treib, 2017/01/31 16:17:25]

Don't handle DCHECK failures, see
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...

[gambard, 2017/01/31 16:55:54]

Done.

+    return;
+
+  scoped_refptr<base::TaskRunner> task_runner = task_runner_;
+  ImageDataFetcher::ImageDataFetcherCallback local_callback =
+      base::BindBlockArc(^(const std::string& image_data) {
+        // Create a NSData from the returned data and notify the callback.
+        NSData* data =
+            [NSData dataWithBytes:image_data.data() length:image_data.size()];
+
+        if (data.length < 4) {
+          callback(data);
+          return;
+        }
+
+        char firstBytes[4] = {0};
+        [data getBytes:&firstBytes length:4];
+
+        if (!memcmp(firstBytes, kWEBPFirstBytes, 4)) {
+          // The image is a webp image.
+          base::PostTaskAndReplyWithResult(task_runner.get(), FROM_HERE,
+                                           base::Bind(&DecodeWebpImage, data),
+                                           base::BindBlockArc(callback));
+          return;
+        }
+        callback(data);
+      });
+  image_data_fetcher_->FetchImageData(image_url, local_callback);
+}
+
+void IOSImageDataFetcherWrapper::SetDataUseServiceName(
+    DataUseServiceName data_use_service_name) {
+  image_data_fetcher_->SetDataUseServiceName(data_use_service_name);
+}
+
+}  // namespace image_fetcher