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

chrome/browser/shell_integration_linux.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/shell_integration_linux.h"
 
 #include <fcntl.h>
 #include <glib.h>
 #include <stdlib.h>
 #include <sys/stat.h>
@@ skipping 18 matching lines @@
 #include "base/strings/string_tokenizer.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/chrome_constants.h"
 #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;
 
 namespace {
 
 // Helper to launch xdg scripts. We don't want them to ask any questions on the
 // terminal etc. The function returns true if the utility launches and exits
 // cleanly, in which case |exit_code| returns the utility's exit code.
 bool LaunchXdgUtility(const std::vector<std::string>& argv, int* exit_code) {
   // xdg-settings internally runs xdg-mime, which uses mv to move newly-created
@@ skipping 27 matching lines @@
   if (shortcut_info.favicon.IsEmpty())
     return std::string();
 
   // TODO(phajdan.jr): Report errors from this function, possibly as infobars.
   base::ScopedTempDir temp_dir;
   if (!temp_dir.CreateUniqueTempDir())
     return std::string();
 
   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());
+  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();
+    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");
+    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");
+    // 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("--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);
+    argv.push_back(temp_file_path.value());
+    argv.push_back(icon_name);
+    int exit_code;
+    LaunchXdgUtility(argv, &exit_code);
+  }
   return icon_name;
 }
 
 bool CreateShortcutOnDesktop(const FilePath& shortcut_filename,
                              const std::string& contents) {
   // Make sure that we will later call openat in a secure way.
   DCHECK_EQ(shortcut_filename.BaseName().value(), shortcut_filename.value());
 
   FilePath desktop_path;
   if (!PathService::Get(base::DIR_USER_DESKTOP, &desktop_path))
@@ skipping 586 matching lines @@
 
   FilePath shortcut_filename = GetExtensionShortcutFilename(
       profile_path, extension_id);
   DCHECK(!shortcut_filename.empty());
 
   DeleteShortcutOnDesktop(shortcut_filename);
   DeleteShortcutInApplicationsMenu(shortcut_filename);
 }
 
 }  // namespace ShellIntegrationLinux