Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1024)

Unified Diff: chrome/browser/ui/views/create_application_shortcut_view.cc

Issue 1038573002: Fixed thread-unsafe use of gfx::Image in app shortcut creation. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Rebase. Created 5 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 bc3b9f6f2d93f197873306c8bd19cd9f81e46af3..cfe0841464407508c1deb24a5f54a2d4f1d59bde 100644
--- a/chrome/browser/ui/views/create_application_shortcut_view.cc
+++ b/chrome/browser/ui/views/create_application_shortcut_view.cc
@@ -252,15 +252,16 @@ 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) {
+}
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));
@@ -370,6 +371,12 @@ base::string16 CreateApplicationShortcutView::GetWindowTitle() const {
}
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;
@@ -392,8 +399,7 @@ bool CreateApplicationShortcutView::Accept() {
#endif
web_app::CreateShortcutsWithInfo(web_app::SHORTCUT_CREATION_BY_USER,
- creation_locations,
- shortcut_info_,
+ creation_locations, shortcut_info_.Pass(),
file_handlers_info_);
return true;
}
@@ -430,7 +436,7 @@ CreateUrlApplicationShortcutView::CreateUrlApplicationShortcutView(
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()) {
@@ -448,12 +454,15 @@ 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)
@@ -492,6 +501,10 @@ void CreateUrlApplicationShortcutView::DidDownloadFavicon(
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,
@@ -501,8 +514,8 @@ void CreateUrlApplicationShortcutView::DidDownloadFavicon(
// 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();
}
@@ -544,8 +557,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;
}

Powered by Google App Engine
This is Rietveld 408576698