Fix crashes due to gfx::Image unsafe thread passing

chrome/browser/enhanced_bookmarks/android/bookmark_image_service_android.cc

Index: chrome/browser/enhanced_bookmarks/android/bookmark_image_service_android.cc
diff --git a/chrome/browser/enhanced_bookmarks/android/bookmark_image_service_android.cc b/chrome/browser/enhanced_bookmarks/android/bookmark_image_service_android.cc
index c39859d8d8f94dc8d1648adf16ec02290b84053a..268ef65fb4cd8f380ce97ea388df164e8b8fcb36 100644
--- a/chrome/browser/enhanced_bookmarks/android/bookmark_image_service_android.cc
+++ b/chrome/browser/enhanced_bookmarks/android/bookmark_image_service_android.cc
@@ -72,7 +72,8 @@ void BookmarkImageServiceAndroid::RetrieveSalientImage(
       enhanced_bookmark_model_->bookmark_model()
           ->GetMostRecentlyAddedUserNodeForURL(page_url);
   if (!bookmark || !image_url.is_valid()) {
-    ProcessNewImage(page_url, update_bookmark, image_url, gfx::Image());
+    ProcessNewImage(page_url, update_bookmark, image_url,
+                    scoped_ptr<gfx::Image>(new gfx::Image()));
     return;
   }
 
@@ -211,26 +212,28 @@ void BookmarkImageServiceAndroid::RetrieveSalientImageFromContextCallback(
                        referrer_policy, update_bookmark);
 }
 
-gfx::Image BookmarkImageServiceAndroid::ResizeImage(gfx::Image image) {
+void BookmarkImageServiceAndroid::ResizeImage(
+    scoped_ptr<gfx::Image> *image) {

[noyau (Ping after 24h), 2015/04/01 11:07:12]

I'm not a fan of this API. It'd prefer the argument to be a const gfx::Image and
returning a scoped_ptr<>. It would make it explicit that this is creating a new
image, and I'm pretty sure it is not the responsibility of this function to
delete the old one.

[Kibeom Kim (inactive), 2015/04/01 19:09:06]

agree. Done.

   DCHECK(!content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
-  gfx::Image resized_image = image;
-  if (image.Width() > max_size_.width() &&
-      image.Height() > max_size_.height()) {
-    float resize_ratio = std::min(((float)max_size_.width()) / image.Width(),
-                                  ((float)max_size_.height()) / image.Height());
+  if ((*image)->Width() > max_size_.width() &&
+      (*image)->Height() > max_size_.height()) {
+    float resize_ratio = std::min(
+        ((float)max_size_.width()) / (*image)->Width(),
+        ((float)max_size_.height()) / (*image)->Height());
     // +0.5f is for correct rounding. Without it, it's possible that dest_width
     // is smaller than max_size_.Width() by one.
-    int dest_width = static_cast<int>(resize_ratio * image.Width() + 0.5f);
-    int dest_height = static_cast<int>(resize_ratio * image.Height() + 0.5f);
-
-    resized_image =
-        gfx::Image::CreateFrom1xBitmap(skia::ImageOperations::Resize(
-            image.AsBitmap(), skia::ImageOperations::RESIZE_BEST, dest_width,
-            dest_height));
-    resized_image.AsImageSkia().MakeThreadSafe();
+    int dest_width = static_cast<int>(resize_ratio * (*image)->Width() + 0.5f);
+    int dest_height = static_cast<int>(
+        resize_ratio * (*image)->Height() + 0.5f);
+
+    **image = gfx::Image::CreateFrom1xBitmap(
+        skia::ImageOperations::Resize((*image)->AsBitmap(),
+                                      skia::ImageOperations::RESIZE_BEST,
+                                      dest_width,
+                                      dest_height));
+    (*image)->AsImageSkia().MakeThreadSafe();
   }
-  return resized_image;
 }
 
 void BookmarkImageServiceAndroid::BitmapFetcherHandler::Start(
@@ -252,13 +255,15 @@ void BookmarkImageServiceAndroid::BitmapFetcherHandler::OnFetchComplete(
     const SkBitmap* bitmap) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
-  gfx::Image image;
+  scoped_ptr<gfx::Image> image;
   if (bitmap) {
     gfx::ImageSkia imageSkia = gfx::ImageSkia::CreateFrom1xBitmap(*bitmap);
     imageSkia.MakeThreadSafe();
-    image = gfx::Image(imageSkia);
+    image.reset(new gfx::Image(imageSkia));
+  } else {
+    image.reset(new gfx::Image());
   }
-  service_->ProcessNewImage(page_url_, update_bookmark_, url, image);
+  service_->ProcessNewImage(page_url_, update_bookmark_, url, image.Pass());
 
   delete this;
 }