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

chrome/browser/ui/views/create_application_shortcut_view.cc

 // Copyright (c) 2012 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 "chrome/browser/ui/views/create_application_shortcut_view.h"
 
 #include <algorithm>
 #include <cmath>
 
 #include "base/bind.h"
@@ skipping 234 matching lines @@
 }
 
 }  // namespace chrome
 
 CreateApplicationShortcutView::CreateApplicationShortcutView(Profile* profile)
     : profile_(profile),
       app_info_(NULL),
       create_shortcuts_label_(NULL),
       desktop_check_box_(NULL),
       menu_check_box_(NULL),
-      quick_launch_check_box_(NULL) {}
+      quick_launch_check_box_(NULL) {
+}
 
 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);
+  if (dialog_layout == DIALOG_LAYOUT_URL_SHORTCUT && shortcut_info_) {
+    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));
   create_shortcuts_label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
 
   desktop_check_box_ = AddCheckbox(
       l10n_util::GetStringUTF16(IDS_CREATE_SHORTCUTS_DESKTOP_CHKBOX),
       profile_->GetPrefs()->GetBoolean(prefs::kWebAppCreateOnDesktop));
 
   menu_check_box_ = NULL;
@@ skipping 89 matching lines @@
 
 ui::ModalType CreateApplicationShortcutView::GetModalType() const {
   return ui::MODAL_TYPE_WINDOW;
 }
 
 base::string16 CreateApplicationShortcutView::GetWindowTitle() const {
   return l10n_util::GetStringUTF16(IDS_CREATE_SHORTCUTS_TITLE);
 }
 
 bool CreateApplicationShortcutView::Accept() {
+  // NOTE: This procedure will reset |shortcut_info_| to null.
+
+  // Can happen if the shortcut data is not yet loaded.
+  if (!shortcut_info_)
+    return false;
+
   if (!IsDialogButtonEnabled(ui::DIALOG_BUTTON_OK))
     return false;
 
   web_app::ShortcutLocations creation_locations;
   creation_locations.on_desktop = desktop_check_box_->checked();
   if (menu_check_box_ != NULL && menu_check_box_->checked()) {
     creation_locations.applications_menu_location =
         create_in_chrome_apps_subdir_ ?
             web_app::APP_MENU_LOCATION_SUBDIR_CHROMEAPPS :
             web_app::APP_MENU_LOCATION_ROOT;
   }
 
 #if defined(OS_WIN)
   creation_locations.in_quick_launch_bar = quick_launch_check_box_ == NULL ?
       NULL : quick_launch_check_box_->checked();
 #elif defined(OS_POSIX)
   // Create shortcut in Mac dock or as Linux (gnome/kde) application launcher
   // are not implemented yet.
   creation_locations.in_quick_launch_bar = false;
 #endif
 
   web_app::CreateShortcutsWithInfo(web_app::SHORTCUT_CREATION_BY_USER,
-                                   creation_locations,
-                                   shortcut_info_,
+                                   creation_locations, shortcut_info_.Pass(),
                                    file_handlers_info_);
   return true;
 }
 
 views::Checkbox* CreateApplicationShortcutView::AddCheckbox(
     const base::string16& text, bool checked) {
   views::Checkbox* checkbox = new views::Checkbox(text);
   checkbox->SetChecked(checked);
   checkbox->set_listener(this);
   return checkbox;
@@ skipping 16 matching lines @@
   GetDialogClientView()->UpdateDialogButtons();
 }
 
 CreateUrlApplicationShortcutView::CreateUrlApplicationShortcutView(
     content::WebContents* web_contents)
     : CreateApplicationShortcutView(
           Profile::FromBrowserContext(web_contents->GetBrowserContext())),
       web_contents_(web_contents),
       pending_download_id_(-1),
       weak_ptr_factory_(this)  {
-  web_app::GetShortcutInfoForTab(web_contents_, &shortcut_info_);
+  shortcut_info_ = web_app::GetShortcutInfoForTab(web_contents_);
   const WebApplicationInfo& app_info =
       extensions::TabHelper::FromWebContents(web_contents_)->web_app_info();
   if (!app_info.icons.empty()) {
     web_app::GetIconsInfo(app_info, &unprocessed_icons_);
     FetchIcon();
   }
 
   // Create URL app shortcuts in the top-level menu.
   create_in_chrome_apps_subdir_ = false;
 
   InitControls(DIALOG_LAYOUT_URL_SHORTCUT);
 }
 
 CreateUrlApplicationShortcutView::~CreateUrlApplicationShortcutView() {
 }
 
 bool CreateUrlApplicationShortcutView::Accept() {
+  // Get the smallest icon in the icon family (should have only 1). This must be
+  // done before the call to Accept(), which will reset |shortcut_info_|.
+  DCHECK(shortcut_info_);
+  const gfx::Image* icon = shortcut_info_->favicon.GetBest(0, 0);
+  SkBitmap bitmap = icon ? icon->AsBitmap() : SkBitmap();
+
   if (!CreateApplicationShortcutView::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);
-  SkBitmap bitmap = icon ? icon->AsBitmap() : SkBitmap();
   extensions::TabHelper::FromWebContents(web_contents_)->SetAppIcon(bitmap);
   Browser* browser = chrome::FindBrowserWithWebContents(web_contents_);
   if (browser)
     chrome::ConvertTabToAppWindow(browser, web_contents_);
   return true;
 }
 
 void CreateUrlApplicationShortcutView::FetchIcon() {
   // There should only be fetch job at a time.
   DCHECK_EQ(-1, pending_download_id_);
@@ skipping 19 matching lines @@
     int requested_size,
     int id,
     int http_status_code,
     const GURL& image_url,
     const std::vector<SkBitmap>& bitmaps,
     const std::vector<gfx::Size>& original_bitmap_sizes) {
   if (id != pending_download_id_)
     return;
   pending_download_id_ = -1;
 
+  // Can happen if the dialog has already been accepted.
+  if (!shortcut_info_)
+    return;
+
   gfx::ImageSkia image_skia = CreateFaviconImageSkia(
       bitmaps,
       original_bitmap_sizes,
       requested_size,
       NULL);
   if (!image_skia.isNull()) {
     // As |shortcut_info_| will be passed to the FILE thread upon accepting the
     // dialog, this image must be made read-only and thread-safe.
     image_skia.MakeThreadSafe();
-    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();
   }
 }
 
 CreateChromeApplicationShortcutView::CreateChromeApplicationShortcutView(
     Profile* profile,
     const extensions::Extension* app,
     const base::Callback<void(bool)>& close_callback)
         : CreateApplicationShortcutView(profile),
@@ skipping 21 matching lines @@
   return CreateApplicationShortcutView::Accept();
 }
 
 bool CreateChromeApplicationShortcutView::Cancel() {
   if (!close_callback_.is_null())
     close_callback_.Run(false);
   return CreateApplicationShortcutView::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;
 }