CreateShortcutIcon on Linux now creates icons at all available resolutions, not

chrome/browser/shell_integration_linux.cc

Index: chrome/browser/shell_integration_linux.cc
diff --git a/chrome/browser/shell_integration_linux.cc b/chrome/browser/shell_integration_linux.cc
index 41cde767d9bed5e42c418afad129c106c1687b22..319036b2dd038309d8f6cc8399edfcbe454f311d 100644
--- a/chrome/browser/shell_integration_linux.cc
+++ b/chrome/browser/shell_integration_linux.cc
@@ -36,6 +36,8 @@
 #include "content/public/browser/browser_thread.h"
 #include "googleurl/src/gurl.h"
 #include "ui/gfx/codec/png_codec.h"
+#include "ui/gfx/image/image_skia.h"
+#include "ui/gfx/image/image_skia_rep.h"
 
 using content::BrowserThread;
 
@@ -83,33 +85,39 @@ std::string CreateShortcutIcon(
 
   FilePath temp_file_path = temp_dir.path().Append(
       shortcut_filename.ReplaceExtension("png"));
+  std::string icon_name = temp_file_path.BaseName().RemoveExtension().value();
 
   std::vector<unsigned char> png_data;
-  const SkBitmap* bitmap = shortcut_info.favicon.ToSkBitmap();
-  gfx::PNGCodec::EncodeBGRASkBitmap(*bitmap, false, &png_data);
-  int bytes_written = file_util::WriteFile(temp_file_path,
-      reinterpret_cast<char*>(png_data.data()), png_data.size());
-
-  if (bytes_written != static_cast<int>(png_data.size()))
-    return std::string();
-
-  std::vector<std::string> 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(base::IntToString(bitmap->width()));
-
-  argv.push_back(temp_file_path.value());
-  std::string icon_name = temp_file_path.BaseName().RemoveExtension().value();
-  argv.push_back(icon_name);
-  int exit_code;
-  LaunchXdgUtility(argv, &exit_code);
+  std::vector<gfx::ImageSkiaRep> image_reps =
+      shortcut_info.favicon.ToImageSkia()->image_reps();
+  for (std::vector<gfx::ImageSkiaRep>::const_iterator it = image_reps.begin();
+       it != image_reps.end(); ++it)
+  {

[sky, 2013/02/11 17:45:27]

{ on previous line

[Matt Giuca, 2013/02/11 23:58:56]

Done.

+    const SkBitmap& bitmap = it->sk_bitmap();
+    gfx::PNGCodec::EncodeBGRASkBitmap(bitmap, false, &png_data);

[sky, 2013/02/11 17:45:27]

You're not checking return value here. Worse yet, because png_data is declared
outside the loop weird things may happen if this returns false. png_data should
be moved inside the loop.

[Matt Giuca, 2013/02/11 23:58:56]

Good catch. Fixed both.

While we're checking return values, I've also checked the return value and exit
code of LaunchXdgUtility and put in a warning for both failures.

+    int bytes_written = file_util::WriteFile(temp_file_path,
+        reinterpret_cast<char*>(png_data.data()), png_data.size());
+
+    if (bytes_written != static_cast<int>(png_data.size()))
+      return std::string();
+
+    std::vector<std::string> 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(base::IntToString(bitmap.width()));
+
+    argv.push_back(temp_file_path.value());
+    argv.push_back(icon_name);
+    int exit_code;
+    LaunchXdgUtility(argv, &exit_code);
+  }
   return icon_name;
 }