Replace SkBitmap with gfx::Image in the ImageFetcher API.

ios/chrome/browser/suggestions/image_fetcher_impl.mm

Index: ios/chrome/browser/suggestions/image_fetcher_impl.mm
diff --git a/ios/chrome/browser/suggestions/image_fetcher_impl.mm b/ios/chrome/browser/suggestions/image_fetcher_impl.mm
index 553f02cbb1fbd7c15b2c637ffd6d7667a73cdf9a..cd5e633daeedb406e2130b5684621ee988fe80c8 100644
--- a/ios/chrome/browser/suggestions/image_fetcher_impl.mm
+++ b/ios/chrome/browser/suggestions/image_fetcher_impl.mm
@@ -11,6 +11,7 @@
 #include "ios/chrome/browser/net/image_fetcher.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "skia/ext/skia_utils_ios.h"
+#include "ui/gfx/image/image.h"
 
 namespace suggestions {
 
@@ -33,11 +34,12 @@ void ImageFetcherImpl::SetImageFetcherDelegate(
 void ImageFetcherImpl::StartOrQueueNetworkRequest(
     const GURL& url,
     const GURL& image_url,
-    base::Callback<void(const GURL&, const SkBitmap*)> callback) {
+    base::Callback<void(const GURL&, const gfx::Image&)> callback) {
+  gfx::Image empty_image;
   if (image_url.is_empty()) {
-    callback.Run(url, nullptr);
+    callback.Run(url, empty_image);
     if (delegate_) {
-      delegate_->OnImageFetched(url, nullptr);
+      delegate_->OnImageFetched(url, empty_image);
     }
     return;
   }
@@ -47,20 +49,19 @@ void ImageFetcherImpl::StartOrQueueNetworkRequest(
       ^(const GURL& original_url, int response_code, NSData* data) {
       if (data) {
         // Most likely always returns 1x images.
-        UIImage* image = [UIImage imageWithData:data scale:1];
-        if (image) {
-          SkBitmap bitmap =
-              skia::CGImageToSkBitmap(image.CGImage, [image size], YES);
-          callback.Run(page_url, &bitmap);
+        UIImage* ui_image = [UIImage imageWithData:data scale:1];
+        if (ui_image) {
+          gfx::Image image = gfx::Image(ui_image);

[noyau (Ping after 24h), 2016/05/17 14:44:05]

Can't you do:

gfx::Image image(ui_image);

instead ?

[markusheintz_, 2016/05/17 15:00:43]

Yes :-) Sorry.

+          callback.Run(page_url, image);
           if (delegate_) {
-            delegate_->OnImageFetched(page_url, &bitmap);
+            delegate_->OnImageFetched(page_url, gfx_image);

[noyau (Ping after 24h), 2016/05/17 14:44:05]

s/gfx_image/image/ ?

Or better, use gfx_image instead of image in the block.

[markusheintz_, 2016/05/17 15:00:43]

Done.

           }
           return;

[noyau (Ping after 24h), 2016/05/17 14:44:05]

Nit, and optional as it predates your changes. In case of early return, I always
favor putting the shorter code first. This is not the case here, as the empty
image case is definitely smaller but is relegated at the end. Can you reverse
the condition and treat the empty case first? It would help readability.

[markusheintz_, 2016/05/17 15:00:43]

I have no strong feelings and I'm happy to revers this is you prefer.

The rule I'm following here is: Let's have a positive (non negated conditions:
if (data) { ....  because they are easy to read (in this context this is trivial
of course). If I reverse it then I have if (!data) { ...   .

As an alternative I could also use an else

if (data) {
} else {
}

then I don't need the early return and the code is also very readable.

As I said I have no strong feelings. So feel free to pick your favorite
solution.

[noyau (Ping after 24h), 2016/05/18 05:48:53]

Leave it as it is. I'll revisit this code later anyway (I don't like the scale
set to 1 by default there).

         }
       }
-      callback.Run(page_url, nullptr);
+      callback.Run(page_url, empty_image);

[noyau (Ping after 24h), 2016/05/17 14:44:05]

Please create a different, local, empty_image here, to avoid capturing and
copying the variable from outside the block.

[markusheintz_, 2016/05/17 15:00:43]

I moved the "gfx::Image empty_image;" inside the if block and create a new local
instance with the same name here now.

Blocks are really crazy. 

       if (delegate_) {
-        delegate_->OnImageFetched(page_url, nullptr);
+        delegate_->OnImageFetched(page_url, empty_image);
       }
   };
   imageFetcher_->StartDownload(image_url, fetcher_callback);