Replace the usage of SkBitmap with gfx::Image in the suggestion service.

components/suggestions/image_manager.cc

 // Copyright 2014 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/suggestions/image_manager.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 12 matching lines @@
 // synchronize with histograms.xml, AND will also become incompatible with older
 // browsers still reporting the previous values.
 const char kDatabaseUMAClientName[] = "ImageManager";
 
 std::unique_ptr<SkBitmap> DecodeImage(
     scoped_refptr<base::RefCountedMemory> encoded_data) {
   return suggestions::DecodeJPEGToSkBitmap(encoded_data->front(),
                                            encoded_data->size());
 }
 
-// Wraps an ImageManager callback so that it can be used with the ImageFetcher.
-// ImageManager callbacks expect SkBitmaps while ImageFetcher callbacks expect
-// gfx::Images. The image can be empty. In this case it is mapped to nullptr.
-void WrapImageManagerCallback(
-    const base::Callback<void(const GURL&, const SkBitmap*)>& wrapped_callback,
-    const GURL& url,
-    const gfx::Image& image) {
-  const SkBitmap* bitmap = nullptr;
-  if (!image.IsEmpty())
-    bitmap = image.ToSkBitmap();
-  wrapped_callback.Run(url, bitmap);
-}
-
 }  // namespace
 
 namespace suggestions {
 
 ImageManager::ImageManager() : weak_ptr_factory_(this) {}
 
 ImageManager::ImageManager(
     std::unique_ptr<image_fetcher::ImageFetcher> image_fetcher,
     std::unique_ptr<ProtoDatabase<ImageData>> database,
     const base::FilePath& database_dir,
@@ skipping 28 matching lines @@
   }
 }
 
 void ImageManager::AddImageURL(const GURL& url, const GURL& image_url) {
   DCHECK(thread_checker_.CalledOnValidThread());
   image_url_map_[url] = image_url;
 }
 
 void ImageManager::GetImageForURL(
     const GURL& url,
-    base::Callback<void(const GURL&, const SkBitmap*)> callback) {
+    base::Callback<void(const GURL&, const gfx::Image&)> callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
   // If |url| is not found in |image_url_map_|, then invoke |callback| with
   // NULL since there is no associated image for this |url|.
   GURL image_url;
   if (!GetImageURL(url, &image_url)) {
-    callback.Run(url, nullptr);
+    const gfx::Image empty_image;
+    callback.Run(url, empty_image);

[Marc Treib, 2016/05/20 14:40:04]

Any reason for declaring an empty_image variable, instead of just passing
gfx::Image() to the callback?

[markusheintz_, 2016/05/23 13:20:46]

Done.

     return;
   }
 
   // |database_| can be NULL if something went wrong in initialization.
   if (database_.get() && !database_ready_) {
     // Once database is initialized, it will serve pending requests from either
     // cache or network.
     QueueCacheRequest(url, image_url, callback);
     return;
   }
@@ skipping 10 matching lines @@
 bool ImageManager::GetImageURL(const GURL& url, GURL* image_url) {
   DCHECK(image_url);
   std::map<GURL, GURL>::iterator it = image_url_map_.find(url);
   if (it == image_url_map_.end()) return false;  // Not found.
   *image_url = it->second;
   return true;
 }
 
 void ImageManager::QueueCacheRequest(
     const GURL& url, const GURL& image_url,
-    base::Callback<void(const GURL&, const SkBitmap*)> callback) {
+    base::Callback<void(const GURL&, const gfx::Image&)> callback) {
   // To be served when the database has loaded.
   ImageCacheRequestMap::iterator it = pending_cache_requests_.find(url);
   if (it == pending_cache_requests_.end()) {
     ImageCacheRequest request;
     request.url = url;
     request.image_url = image_url;
     request.callbacks.push_back(callback);
     pending_cache_requests_[url] = request;
   } else {
     // Request already queued for this url.
     it->second.callbacks.push_back(callback);
   }
 }
 
 void ImageManager::OnCacheImageDecoded(
     const GURL& url,
     const GURL& image_url,
-    base::Callback<void(const GURL&, const SkBitmap*)> callback,
+    const base::Callback<void(const GURL&, const gfx::Image&)>& callback,
     std::unique_ptr<SkBitmap> bitmap) {
   if (bitmap.get()) {
-    callback.Run(url, bitmap.get());
+    const gfx::Image image(gfx::Image::CreateFrom1xBitmap(*bitmap.get()));

[Marc Treib, 2016/05/20 14:40:04]

I think the .get() isn't needed, you can directly dereference the unique_ptr

[markusheintz_, 2016/05/23 13:20:46]

Uups sorry forgot to remove done.

+    callback.Run(url, image);

[Marc Treib, 2016/05/20 14:40:03]

Also here: I'd just inline the
gfx::Image::CreateFrom1xBitmap(*bitmap.get())
into the callback.Run

[markusheintz_, 2016/05/23 13:20:46]

Done.

   } else {
     image_fetcher_->StartOrQueueNetworkRequest(
-        url, image_url, base::Bind(&WrapImageManagerCallback, callback));
+        url, image_url, callback);
   }
 }
 
 scoped_refptr<base::RefCountedMemory> ImageManager::GetEncodedImageFromCache(
     const GURL& url) {
   ImageMap::iterator image_iter = image_map_.find(url.spec());
   if (image_iter != image_map_.end()) {
     return image_iter->second;
   }
   return nullptr;
 }
 
 void ImageManager::ServeFromCacheOrNetwork(
     const GURL& url,
     const GURL& image_url,
-    base::Callback<void(const GURL&, const SkBitmap*)> callback) {
+    base::Callback<void(const GURL&, const gfx::Image&)> callback) {
   scoped_refptr<base::RefCountedMemory> encoded_data =
       GetEncodedImageFromCache(url);
   if (encoded_data.get()) {
     base::PostTaskAndReplyWithResult(
         background_task_runner_.get(), FROM_HERE,
         base::Bind(&DecodeImage, encoded_data),
         base::Bind(&ImageManager::OnCacheImageDecoded,
                    weak_ptr_factory_.GetWeakPtr(), url, image_url, callback));
   } else {
     image_fetcher_->StartOrQueueNetworkRequest(
-        url, image_url, base::Bind(&WrapImageManagerCallback, callback));
+        url, image_url, callback);

[Marc Treib, 2016/05/20 14:40:03]

This should fit on the previous line now :)

[markusheintz_, 2016/05/23 13:20:46]

Done.

   }
 }
 
 void ImageManager::SaveImage(const GURL& url, const SkBitmap& bitmap) {
   scoped_refptr<base::RefCountedBytes> encoded_data(
       new base::RefCountedBytes());
   if (!EncodeSkBitmapToJPEG(bitmap, &encoded_data->data())) {
     return;
   }
 
@@ skipping 67 matching lines @@
        it != pending_cache_requests_.end(); ++it) {
     const ImageCacheRequest& request = it->second;
     for (CallbackVector::const_iterator callback_it = request.callbacks.begin();
          callback_it != request.callbacks.end(); ++callback_it) {
       ServeFromCacheOrNetwork(request.url, request.image_url, *callback_it);
     }
   }
 }
 
 }  // namespace suggestions