From 0b303cc493d3557b5a5e9e5c8dfc6bdc4953ee73 Mon Sep 17 00:00:00 2001 From: "estade@chromium.org" Date: Mon, 28 Sep 2009 22:35:15 +0000 Subject: Use favicon for application shortcut icon. BUG=22528 Review URL: http://codereview.chromium.org/249023 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@27422 0039d316-1c4b-4281-b951-d872f2087c98 --- .../gtk/create_application_shortcuts_dialog_gtk.cc | 14 ++++-- .../gtk/create_application_shortcuts_dialog_gtk.h | 15 ++++-- chrome/browser/shell_integration.h | 7 ++- chrome/browser/shell_integration_linux.cc | 57 +++++++++++++++++++--- chrome/browser/shell_integration_unittest.cc | 30 ++++++++++-- chrome/browser/tab_contents/tab_contents.cc | 17 ++++++- chrome/browser/tab_contents/tab_contents.h | 3 ++ 7 files changed, 125 insertions(+), 18 deletions(-) diff --git a/chrome/browser/gtk/create_application_shortcuts_dialog_gtk.cc b/chrome/browser/gtk/create_application_shortcuts_dialog_gtk.cc index 91052e2..56a5908 100644 --- a/chrome/browser/gtk/create_application_shortcuts_dialog_gtk.cc +++ b/chrome/browser/gtk/create_application_shortcuts_dialog_gtk.cc @@ -12,14 +12,19 @@ // static void CreateApplicationShortcutsDialogGtk::Show(GtkWindow* parent, const GURL& url, - const string16& title) { - new CreateApplicationShortcutsDialogGtk(parent, url, title); + const string16& title, + const SkBitmap& favicon) { + new CreateApplicationShortcutsDialogGtk(parent, url, title, favicon); } CreateApplicationShortcutsDialogGtk::CreateApplicationShortcutsDialogGtk( - GtkWindow* parent, const GURL& url, const string16& title) + GtkWindow* parent, + const GURL& url, + const string16& title, + const SkBitmap& favicon) : url_(url), - title_(title) { + title_(title), + favicon_(favicon) { // Build the dialog. GtkWidget* dialog = gtk_dialog_new_with_buttons( l10n_util::GetStringUTF8(IDS_CREATE_SHORTCUTS_TITLE).c_str(), @@ -68,6 +73,7 @@ void CreateApplicationShortcutsDialogGtk::OnDialogResponse(GtkWidget* widget, ShellIntegration::ShortcutInfo shortcut_info; shortcut_info.url = url_; shortcut_info.title = title_; + shortcut_info.favicon = favicon_; shortcut_info.create_on_desktop = gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(desktop_checkbox_)); shortcut_info.create_in_applications_menu = diff --git a/chrome/browser/gtk/create_application_shortcuts_dialog_gtk.h b/chrome/browser/gtk/create_application_shortcuts_dialog_gtk.h index f1b9081..13bb66a 100644 --- a/chrome/browser/gtk/create_application_shortcuts_dialog_gtk.h +++ b/chrome/browser/gtk/create_application_shortcuts_dialog_gtk.h @@ -8,6 +8,7 @@ #include "base/basictypes.h" #include "base/string16.h" #include "googleurl/src/gurl.h" +#include "third_party/skia/include/core/SkBitmap.h" typedef struct _GtkWidget GtkWidget; typedef struct _GtkWindow GtkWindow; @@ -16,11 +17,16 @@ class CreateApplicationShortcutsDialogGtk { public: // Displays the dialog box to create application shortcuts for |url| with // |title|. - static void Show(GtkWindow* parent, const GURL& url, const string16& title); + static void Show(GtkWindow* parent, + const GURL& url, + const string16& title, + const SkBitmap& favicon); private: - CreateApplicationShortcutsDialogGtk(GtkWindow* parent, const GURL& url, - const string16& title); + CreateApplicationShortcutsDialogGtk(GtkWindow* parent, + const GURL& url, + const string16& title, + const SkBitmap& favicon); ~CreateApplicationShortcutsDialogGtk() { } // Handler to respond to Ok and Cancel responses from the dialog. @@ -41,6 +47,9 @@ class CreateApplicationShortcutsDialogGtk { // Visible title of the shortcut. string16 title_; + // The favicon of the tab contents, used to set the icon on the desktop. + SkBitmap favicon_; + DISALLOW_COPY_AND_ASSIGN(CreateApplicationShortcutsDialogGtk); }; diff --git a/chrome/browser/shell_integration.h b/chrome/browser/shell_integration.h index daecacf..74e45af 100644 --- a/chrome/browser/shell_integration.h +++ b/chrome/browser/shell_integration.h @@ -12,6 +12,10 @@ #include "base/string16.h" #include "googleurl/src/gurl.h" +#if defined(OS_LINUX) +#include "third_party/skia/include/core/SkBitmap.h" +#endif + class FilePath; class MessageLoop; @@ -50,11 +54,12 @@ class ShellIntegration { // used to launch Chrome. static std::string GetDesktopFileContents( const std::string& template_contents, const GURL& url, - const string16& title); + const string16& title, const std::string& icon_name); struct ShortcutInfo { GURL url; string16 title; + SkBitmap favicon; bool create_on_desktop; bool create_in_applications_menu; diff --git a/chrome/browser/shell_integration_linux.cc b/chrome/browser/shell_integration_linux.cc index dab372f..30eab5b 100644 --- a/chrome/browser/shell_integration_linux.cc +++ b/chrome/browser/shell_integration_linux.cc @@ -17,6 +17,7 @@ #include "base/eintr_wrapper.h" #include "base/file_path.h" #include "base/file_util.h" +#include "base/gfx/png_encoder.h" #include "base/message_loop.h" #include "base/path_service.h" #include "base/process_util.h" @@ -121,8 +122,11 @@ class CreateDesktopShortcutTask : public Task { FilePath shortcut_filename = ShellIntegration::GetDesktopShortcutFilename(shortcut_info_.url); + std::string icon_name = CreateIcon(shortcut_filename); + std::string contents = ShellIntegration::GetDesktopFileContents( - template_contents, shortcut_info_.url, shortcut_info_.title); + template_contents, shortcut_info_.url, shortcut_info_.title, + icon_name); if (shortcut_info_.create_on_desktop) CreateOnDesktop(shortcut_filename, contents); @@ -132,6 +136,45 @@ class CreateDesktopShortcutTask : public Task { } private: + std::string CreateIcon(const FilePath& shortcut_filename) { + if (shortcut_info_.favicon.isNull()) + return std::string(); + + // TODO(phajdan.jr): Report errors from this function, possibly as infobars. + ScopedTempDir temp_dir; + if (!temp_dir.CreateUniqueTempDir()) + return std::string(); + + FilePath temp_file_path = temp_dir.path().Append( + shortcut_filename.ReplaceExtension("png")); + + std::vector png_data; + PNGEncoder::EncodeBGRASkBitmap(shortcut_info_.favicon, false, &png_data); + int bytes_written = file_util::WriteFile(temp_file_path, + reinterpret_cast(png_data.data()), png_data.size()); + + if (bytes_written != static_cast(png_data.size())) + return std::string(); + + std::vector argv; + argv.push_back("xdg-icon-resource"); + argv.push_back("install"); + + // Always install in user mode, even if someone runs the browser as root + // (people do that). + argv.push_back("--mode"); + argv.push_back("user"); + + argv.push_back("--size"); + argv.push_back(IntToString(shortcut_info_.favicon.width())); + + argv.push_back(temp_file_path.value()); + std::string icon_name = temp_file_path.BaseName().RemoveExtension().value(); + argv.push_back(icon_name); + LaunchXdgUtility(argv); + return icon_name; + } + void CreateOnDesktop(const FilePath& shortcut_filename, const std::string& contents) { // TODO(phajdan.jr): Report errors from this function, possibly as infobars. @@ -183,7 +226,7 @@ class CreateDesktopShortcutTask : public Task { contents.length()); if (bytes_written != static_cast(contents.length())) - return; + return; std::vector argv; argv.push_back("xdg-desktop-menu"); @@ -261,15 +304,13 @@ FilePath ShellIntegration::GetDesktopShortcutFilename(const GURL& url) { std::string ShellIntegration::GetDesktopFileContents( const std::string& template_contents, const GURL& url, - const string16& title) { + const string16& title, const std::string& icon_name) { // See http://standards.freedesktop.org/desktop-entry-spec/latest/ // Although not required by the spec, Nautilus on Ubuntu Karmic creates its // launchers with an xdg-open shebang. Follow that convention. std::string output_buffer("#!/usr/bin/env xdg-open\n"); StringTokenizer tokenizer(template_contents, "\n"); while (tokenizer.GetNext()) { - // TODO(phajdan.jr): Add the icon. - if (tokenizer.token().substr(0, 5) == "Exec=") { std::string exec_path = tokenizer.token().substr(5); StringTokenizer exec_tokenizer(exec_path, " "); @@ -294,13 +335,17 @@ std::string ShellIntegration::GetDesktopFileContents( // use the URL as a default when the title is empty. if (final_title.empty() || final_title.find("\n") != std::string::npos || - final_title.find("\r") != std::string::npos) + final_title.find("\r") != std::string::npos) { final_title = url.spec(); + } output_buffer += StringPrintf("Name=%s\n", final_title.c_str()); } else if (tokenizer.token().substr(0, 11) == "GenericName" || tokenizer.token().substr(0, 7) == "Comment" || tokenizer.token().substr(0, 1) == "#") { // Skip comment lines. + } else if (tokenizer.token().substr(0, 5) == "Icon=" && + !icon_name.empty()) { + output_buffer += StringPrintf("Icon=%s\n", icon_name.c_str()); } else { output_buffer += tokenizer.token() + "\n"; } diff --git a/chrome/browser/shell_integration_unittest.cc b/chrome/browser/shell_integration_unittest.cc index 3b01c30..4f8ddef 100644 --- a/chrome/browser/shell_integration_unittest.cc +++ b/chrome/browser/shell_integration_unittest.cc @@ -40,15 +40,17 @@ TEST(ShellIntegrationTest, GetDesktopFileContents) { const struct { const char* url; const char* title; + const char* icon_name; const char* template_contents; const char* expected_output; } test_cases[] = { // Dumb case. - { "ignored", "ignored", "", "#!/usr/bin/env xdg-open\n" }, + { "ignored", "ignored", "ignored", "", "#!/usr/bin/env xdg-open\n" }, // Real-world case. { "http://gmail.com", "GMail", + "chrome-http__gmail.com", "[Desktop Entry]\n" "Version=1.0\n" @@ -69,7 +71,7 @@ TEST(ShellIntegrationTest, GetDesktopFileContents) { "Name=GMail\n" "Exec=/opt/google/chrome/google-chrome \"--app=http://gmail.com/\"\n" "Terminal=false\n" - "Icon=/opt/google/chrome/product_logo_48.png\n" + "Icon=chrome-http__gmail.com\n" "Type=Application\n" "Categories=Application;Network;WebBrowser;\n" "MimeType=text/html;text/xml;application/xhtml_xml;\n" @@ -78,6 +80,7 @@ TEST(ShellIntegrationTest, GetDesktopFileContents) { // Make sure we don't insert duplicate shebangs. { "http://gmail.com", "GMail", + "chrome-http__gmail.com", "#!/some/shebang\n" "Name=Google Chrome\n" @@ -91,6 +94,7 @@ TEST(ShellIntegrationTest, GetDesktopFileContents) { // Make sure i18n-ed comments are removed. { "http://gmail.com", "GMail", + "chrome-http__gmail.com", "Name=Google Chrome\n" "Exec=/opt/google/chrome/google-chrome %U\n" @@ -101,9 +105,26 @@ TEST(ShellIntegrationTest, GetDesktopFileContents) { "Exec=/opt/google/chrome/google-chrome \"--app=http://gmail.com/\"\n" }, + // Make sure that empty icons are replaced by the chrome icon. + { "http://gmail.com", + "GMail", + "", + + "Name=Google Chrome\n" + "Exec=/opt/google/chrome/google-chrome %U\n" + "Comment[pl]=Jakis komentarz.\n" + "Icon=/opt/google/chrome/product_logo_48.png\n", + + "#!/usr/bin/env xdg-open\n" + "Name=GMail\n" + "Exec=/opt/google/chrome/google-chrome \"--app=http://gmail.com/\"\n" + "Icon=/opt/google/chrome/product_logo_48.png\n" + }, + // Now we're starting to be more evil... { "http://evil.com/evil --join-the-b0tnet", "Ownz0red\nExec=rm -rf /", + "chrome-http__evil.com_evil", "Name=Google Chrome\n" "Exec=/opt/google/chrome/google-chrome %U\n", @@ -115,6 +136,7 @@ TEST(ShellIntegrationTest, GetDesktopFileContents) { }, { "http://evil.com/evil; rm -rf /; \"; rm -rf $HOME >ownz0red", "Innocent Title", + "chrome-http__evil.com_evil", "Name=Google Chrome\n" "Exec=/opt/google/chrome/google-chrome %U\n", @@ -127,6 +149,7 @@ TEST(ShellIntegrationTest, GetDesktopFileContents) { }, { "http://evil.com/evil | cat `echo ownz0red` >/dev/null\\", "Innocent Title", + "chrome-http__evil.com_evil", "Name=Google Chrome\n" "Exec=/opt/google/chrome/google-chrome %U\n", @@ -143,7 +166,8 @@ TEST(ShellIntegrationTest, GetDesktopFileContents) { ShellIntegration::GetDesktopFileContents( test_cases[i].template_contents, GURL(test_cases[i].url), - ASCIIToUTF16(test_cases[i].title))); + ASCIIToUTF16(test_cases[i].title), + test_cases[i].icon_name)); } } #endif // defined(OS_LINUX) diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc index 4dd83dc..73a1fa6 100644 --- a/chrome/browser/tab_contents/tab_contents.cc +++ b/chrome/browser/tab_contents/tab_contents.cc @@ -534,6 +534,18 @@ SkBitmap TabContents::GetFavIcon() const { return SkBitmap(); } +bool TabContents::FavIconIsValid() const { + NavigationEntry* entry = controller_.GetTransientEntry(); + if (entry) + return entry->favicon().is_valid(); + + entry = controller_.GetLastCommittedEntry(); + if (entry) + return entry->favicon().is_valid(); + + return false; +} + bool TabContents::ShouldDisplayFavIcon() { // Always display a throbber during pending loads. if (controller_.GetLastCommittedEntry() && controller_.pending_entry()) @@ -766,8 +778,11 @@ void TabContents::CreateShortcut() { return; #if defined(OS_LINUX) && !defined(TOOLKIT_VIEWS) + SkBitmap bitmap; + if (FavIconIsValid()) + bitmap = GetFavIcon(); CreateApplicationShortcutsDialogGtk::Show(view()->GetTopLevelNativeWindow(), - GetURL(), GetTitle()); + GetURL(), GetTitle(), bitmap); #else // We only allow one pending install request. By resetting the page id we // effectively cancel the pending install request. diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h index 26d69f5..87aeb76 100644 --- a/chrome/browser/tab_contents/tab_contents.h +++ b/chrome/browser/tab_contents/tab_contents.h @@ -221,6 +221,9 @@ class TabContents : public PageNavigator, // entry. SkBitmap GetFavIcon() const; + // Returns true if we are not using the default favicon. + bool FavIconIsValid() const; + // Returns whether the favicon should be displayed. If this returns false, no // space is provided for the favicon, and the favicon is never displayed. virtual bool ShouldDisplayFavIcon(); -- cgit v1.1