Fixed thread-unsafe use of gfx::Image in app shortcut creation.

chrome/browser/ui/views/create_application_shortcut_view.cc

Index: chrome/browser/ui/views/create_application_shortcut_view.cc
diff --git a/chrome/browser/ui/views/create_application_shortcut_view.cc b/chrome/browser/ui/views/create_application_shortcut_view.cc
index 6361d207a4d3ecf1aef86ccb9ce8236b53f71508..7611d344b821aed87f0d3f2f887fdd63f0642290 100644
--- a/chrome/browser/ui/views/create_application_shortcut_view.cc
+++ b/chrome/browser/ui/views/create_application_shortcut_view.cc
@@ -252,15 +252,17 @@ CreateApplicationShortcutView::CreateApplicationShortcutView(Profile* profile)
       create_shortcuts_label_(NULL),
       desktop_check_box_(NULL),
       menu_check_box_(NULL),
-      quick_launch_check_box_(NULL) {}
+      quick_launch_check_box_(NULL),
+      shortcut_info_(new web_app::ShortcutInfo) {
+}
 
 CreateApplicationShortcutView::~CreateApplicationShortcutView() {}
 
 void CreateApplicationShortcutView::InitControls(DialogLayout dialog_layout) {
   if (dialog_layout == DIALOG_LAYOUT_URL_SHORTCUT) {
-    app_info_ = new AppInfoView(shortcut_info_.title,
-                                shortcut_info_.description,
-                                shortcut_info_.favicon);
+    app_info_ = new AppInfoView(shortcut_info_->title,
+                                shortcut_info_->description,
+                                shortcut_info_->favicon);
   }
   create_shortcuts_label_ = new views::Label(
       l10n_util::GetStringUTF16(IDS_CREATE_SHORTCUTS_LABEL));
@@ -393,8 +395,11 @@ bool CreateApplicationShortcutView::Accept() {
 
   web_app::CreateShortcutsWithInfo(web_app::SHORTCUT_CREATION_BY_USER,
                                    creation_locations,
-                                   shortcut_info_,
+                                   shortcut_info_.Pass(),
                                    file_handlers_info_);
+  // CreateShortcutsWithInfo stole ownership of |shortcut_info_|, so create a
+  // new blank one (to satisfy the non-null constraint).
+  shortcut_info_.reset(new web_app::ShortcutInfo);

[benwells, 2015/03/26 05:28:51]

Why do this? Put another way, why is there a constraint to have shortcut_info_
not null?

[Matt Giuca, 2015/03/27 08:14:46]

Partly laziness :)

It would mean having to introduce null checks throughout this whole file. Seems
OK.

Done.

   return true;
 }
 
@@ -430,7 +435,7 @@ CreateUrlApplicationShortcutView::CreateUrlApplicationShortcutView(
       web_contents_(web_contents),
       pending_download_id_(-1),
       weak_ptr_factory_(this)  {
-  web_app::GetShortcutInfoForTab(web_contents_, &shortcut_info_);
+  web_app::GetShortcutInfoForTab(web_contents_, shortcut_info_.get());
   const WebApplicationInfo& app_info =
       extensions::TabHelper::FromWebContents(web_contents_)->web_app_info();
   if (!app_info.icons.empty()) {
@@ -452,7 +457,7 @@ bool CreateUrlApplicationShortcutView::Accept() {
     return false;
 
   // Get the smallest icon in the icon family (should have only 1).
-  const gfx::Image* icon = shortcut_info_.favicon.GetBest(0, 0);
+  const gfx::Image* icon = shortcut_info_->favicon.GetBest(0, 0);
   SkBitmap bitmap = icon ? icon->AsBitmap() : SkBitmap();
   extensions::TabHelper::FromWebContents(web_contents_)->SetAppIcon(bitmap);
   Browser* browser = chrome::FindBrowserWithWebContents(web_contents_);
@@ -498,8 +503,8 @@ void CreateUrlApplicationShortcutView::DidDownloadFavicon(
       requested_size,
       NULL);
   if (!image_skia.isNull()) {
-    shortcut_info_.favicon.Add(image_skia);
-    static_cast<AppInfoView*>(app_info_)->UpdateIcon(shortcut_info_.favicon);
+    shortcut_info_->favicon.Add(image_skia);
+    static_cast<AppInfoView*>(app_info_)->UpdateIcon(shortcut_info_->favicon);
   } else {
     FetchIcon();
   }
@@ -541,8 +546,8 @@ bool CreateChromeApplicationShortcutView::Cancel() {
 }
 
 void CreateChromeApplicationShortcutView::OnAppInfoLoaded(
-    const web_app::ShortcutInfo& shortcut_info,
+    scoped_ptr<web_app::ShortcutInfo> shortcut_info,
     const extensions::FileHandlersInfo& file_handlers_info) {
-  shortcut_info_ = shortcut_info;
+  shortcut_info_ = shortcut_info.Pass();
   file_handlers_info_ = file_handlers_info;
 }