Fix crashes due to gfx::Image unsafe thread passing

chrome/browser/enhanced_bookmarks/android/bookmark_image_service_android.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 "base/json/json_parser.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/bitmap_fetcher/bitmap_fetcher.h"
 #include "chrome/browser/enhanced_bookmarks/android/bookmark_image_service_android.h"
 #include "chrome/browser/enhanced_bookmarks/enhanced_bookmark_model_factory.h"
@@ skipping 54 matching lines @@
 void BookmarkImageServiceAndroid::RetrieveSalientImage(
     const GURL& page_url,
     const GURL& image_url,
     const std::string& referrer,
     net::URLRequest::ReferrerPolicy referrer_policy,
     bool update_bookmark) {
   const BookmarkNode* bookmark =
       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;
   }
 
   BitmapFetcherHandler* bitmap_fetcher_handler =
       new BitmapFetcherHandler(this, image_url);
   bitmap_fetcher_handler->Start(
       browser_context_, referrer, referrer_policy,
       net::LOAD_DO_NOT_SEND_COOKIES | net::LOAD_DO_NOT_SAVE_COOKIES,
       update_bookmark, page_url);
 }
@@ skipping 118 matching lines @@
         referrer_policy = net::URLRequest::NEVER_CLEAR_REFERRER;
         break;
       default:
         NOTREACHED();
     }
   }
   RetrieveSalientImage(page_url, GURL(image_url), referrer.url.spec(),
                        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);
+    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();
+    **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(
     content::BrowserContext* browser_context,
     const std::string& referrer,
     net::URLRequest::ReferrerPolicy referrer_policy,
     int load_flags,
     bool update_bookmark,
     const GURL& page_url) {
   update_bookmark_ = update_bookmark;
   page_url_ = page_url;
 
   bitmap_fetcher_.Start(browser_context->GetRequestContext(), referrer,
                         referrer_policy, load_flags);
 }
 
 void BookmarkImageServiceAndroid::BitmapFetcherHandler::OnFetchComplete(
     const GURL& url,
     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;
 }
 
 }  // namespace enhanced_bookmarks