Use IOSImageDataFetcherWrapper for favicon

components/favicon/ios/web_favicon_driver.mm

Index: components/favicon/ios/web_favicon_driver.mm
diff --git a/components/favicon/ios/web_favicon_driver.mm b/components/favicon/ios/web_favicon_driver.mm
index 03777812d270aaab8087036341b4f3035fc340a0..82f3d9f5f9be73150ef1ffd5c829a2f7eb779af2 100644
--- a/components/favicon/ios/web_favicon_driver.mm
+++ b/components/favicon/ios/web_favicon_driver.mm
@@ -5,17 +5,38 @@
 #include "components/favicon/ios/web_favicon_driver.h"
 
 #include "base/bind.h"
+#include "base/memory/ptr_util.h"
 #include "components/favicon/core/favicon_url.h"
 #include "components/favicon/ios/favicon_url_util.h"
+#import "components/image_fetcher/ios/ios_image_data_fetcher_wrapper.h"
 #include "ios/web/public/browser_state.h"
 #include "ios/web/public/favicon_status.h"
 #include "ios/web/public/navigation_item.h"
 #include "ios/web/public/navigation_manager.h"
 #include "ios/web/public/web_state/web_state.h"
+#include "ios/web/public/web_thread.h"
+#include "net/url_request/url_fetcher.h"
+#include "skia/ext/skia_utils_ios.h"
+#include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/image/image.h"
 
 DEFINE_WEB_STATE_USER_DATA_KEY(favicon::WebFaviconDriver);
 
+namespace {

[Marc Treib, 2017/02/06 12:35:49]

nit: no namespace required for a typedef

[gambard, 2017/02/06 15:58:28]

Done.

+// Callback for the download of favicon.
+typedef base::Callback<void(

[Marc Treib, 2017/02/06 12:35:49]

nit: I prefer the C++11-style
using ImageDownloadCallback = ...
That also allows specifying param names the "normal" way, without the comment
hack.

[gambard, 2017/02/06 15:58:28]

Done.

+    int,                          /* id */
+    int,                          /* HTTP status code */
+    const GURL&,                  /* image_url */
+    const std::vector<SkBitmap>&, /* bitmaps */
+    /* The sizes in pixel of the bitmaps before they were resized due to the
+     max bitmap size passed to DownloadImage(). Each entry in the bitmaps
+     vector corresponds to an entry in the sizes vector. If a bitmap was
+     resized, there should be a single returned bitmap. */
+    const std::vector<gfx::Size>&)>
+    ImageDownloadCallback;
+}  // namespace
+
 namespace favicon {
 
 // static
@@ -55,10 +76,31 @@ int WebFaviconDriver::StartDownload(const GURL& url, int max_image_size) {
     return 0;
   }
 
-  return web_state()->DownloadImage(
-      url, true, max_image_size, false,
-      base::Bind(&FaviconDriverImpl::DidDownloadFavicon,
-                 base::Unretained(this)));
+  static int downloaded_image_count = 0;
+  int local_download_id = ++downloaded_image_count;
+
+  ImageDownloadCallback local_image_callback = base::Bind(
+      &FaviconDriverImpl::DidDownloadFavicon, base::Unretained(this));
+  GURL local_url(url);
+
+  image_fetcher::IOSImageDataFetcherCallback local_callback =
+      ^(const int response_code, NSData* data) {
+        std::vector<SkBitmap> frames;
+        std::vector<gfx::Size> sizes;
+        if (data) {
+          frames = skia::ImageDataToSkBitmaps(data);
+          for (auto& frame : frames) {

[Marc Treib, 2017/02/06 12:35:49]

nit: const auto& ?

[gambard, 2017/02/06 15:58:28]

Done.

+            sizes.push_back(gfx::Size(frame.width(), frame.height()));
+          }
+        }
+        if (response_code != net::URLFetcher::RESPONSE_CODE_INVALID) {
+          local_image_callback.Run(local_download_id, response_code, local_url,
+                                   frames, sizes);
+        }
+      };
+  image_fetcher_->FetchImageDataWebpDecoded(url, local_callback);
+
+  return downloaded_image_count;
 }
 
 bool WebFaviconDriver::IsOffTheRecord() {
@@ -97,6 +139,10 @@ WebFaviconDriver::WebFaviconDriver(web::WebState* web_state,
                                    bookmarks::BookmarkModel* bookmark_model)
     : web::WebStateObserver(web_state),
       FaviconDriverImpl(favicon_service, history_service, bookmark_model) {
+  // Set up the image fetcher.
+  image_fetcher_ = base::MakeUnique<image_fetcher::IOSImageDataFetcherWrapper>(

[Marc Treib, 2017/02/06 12:35:49]

Looks like this doesn't need to be a pointer?

[gambard, 2017/02/06 15:58:28]

Done.

+      web_state->GetBrowserState()->GetRequestContext(),
+      web::WebThread::GetBlockingPool());
 }
 
 WebFaviconDriver::~WebFaviconDriver() {