ShortcutInfo::favicon is now a gfx::ImageFamily instead of gfx::Image.

This fixes the bad usage where a single Image contained all icon images at the
"100P" scale factor. Now each Image in the family contains a single
representation.

Updated all usages of ShortcutInfo::favicon to use the ImageFamily. Several
places where the "get best size icon" algorithm was duplicated have been
replaced with a call to ImageFamily::GetBest.

BUG=189137

Review URL: https://chromiumcodereview.appspot.com/12881003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@193558 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 50 insertions, 29 deletions

diff --git a/chrome/browser/web_applications/web_app.h b/chrome/browser/web_applications/web_app.h
index ceacac2..4e5f288 100644
--- a/chrome/browser/web_applications/web_app.h
+++ b/chrome/browser/web_applications/web_app.h
@@ -17,6 +17,10 @@ namespace extensions {
 class Extension;
 }
 
+namespace gfx {
+class ImageFamily;
+}
+
 namespace web_app {
 
 // Gets the user data directory for given web app. The path for the directory is
@@ -90,7 +94,8 @@ std::string GetWMClassFromAppName(std::string app_name);
 namespace internals {
 
 #if defined(OS_WIN)
-bool CheckAndSaveIcon(const base::FilePath& icon_file, const SkBitmap& image);
+bool CheckAndSaveIcon(const base::FilePath& icon_file,
+                      const gfx::ImageFamily& image);
 #endif
 
 // Implemented for each platform, does the platform specific parts of creating
diff --git a/chrome/browser/web_applications/web_app_mac.mm b/chrome/browser/web_applications/web_app_mac.mm
index 45fa13d..6da9795 100644
--- a/chrome/browser/web_applications/web_app_mac.mm
+++ b/chrome/browser/web_applications/web_app_mac.mm
@@ -24,18 +24,23 @@
 #include "skia/ext/skia_utils_mac.h"
 #include "third_party/icon_family/IconFamily.h"
 #include "ui/base/l10n/l10n_util_mac.h"
-#include "ui/gfx/image/image_skia.h"
+#include "ui/gfx/image/image_family.h"
 
 namespace {
 
-// Creates a NSBitmapImageRep from |bitmap|.
-NSBitmapImageRep* SkBitmapToImageRep(const SkBitmap& bitmap) {
-  base::mac::ScopedCFTypeRef<CGColorSpaceRef> color_space(
-      CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB));
-  NSImage* image = gfx::SkBitmapToNSImageWithColorSpace(
-    bitmap, color_space.get());
-  return base::mac::ObjCCast<NSBitmapImageRep>(
-      [[image representations] lastObject]);
+// Get the 100% image representation for |image|.
+// This returns the representation with the same width and height as |image|
+// itself. If there is no such representation, returns nil.
+NSBitmapImageRep* NSImageGet100PRepresentation(NSImage* image) {
+  NSSize image_size = [image size];
+  for (NSBitmapImageRep* image_rep in [image representations]) {
+    NSSize image_rep_size = [image_rep size];
+    if (image_rep_size.width == image_size.width &&
+        image_rep_size.height == image_size.height) {
+      return image_rep;
+    }
+  }
+  return nil;
 }
 
 // Adds |image_rep| to |icon_family|. Returns true on success, false on failure.
@@ -220,17 +225,16 @@ bool WebAppShortcutCreator::UpdatePlist(const base::FilePath& app_path) const {
 }
 
 bool WebAppShortcutCreator::UpdateIcon(const base::FilePath& app_path) const {
-  if (info_.favicon.IsEmpty())
+  if (info_.favicon.empty())
     return true;
 
   scoped_nsobject<IconFamily> icon_family([[IconFamily alloc] init]);
   bool image_added = false;
-  info_.favicon.ToImageSkia()->EnsureRepsForSupportedScaleFactors();
-  std::vector<gfx::ImageSkiaRep> image_reps =
-      info_.favicon.ToImageSkia()->image_reps();
-  for (size_t i = 0; i < image_reps.size(); ++i) {
-    NSBitmapImageRep* image_rep = SkBitmapToImageRep(
-        image_reps[i].sk_bitmap());
+  for (gfx::ImageFamily::const_iterator it = info_.favicon.begin();
+       it != info_.favicon.end(); ++it) {
+    if (it->IsEmpty())
+      continue;
+    NSBitmapImageRep* image_rep = NSImageGet100PRepresentation(it->ToNSImage());
     if (!image_rep)
       continue;
 
diff --git a/chrome/browser/web_applications/web_app_mac_unittest.mm b/chrome/browser/web_applications/web_app_mac_unittest.mm
index eae79e5..2579cb36 100644
--- a/chrome/browser/web_applications/web_app_mac_unittest.mm
+++ b/chrome/browser/web_applications/web_app_mac_unittest.mm
@@ -160,19 +160,21 @@ TEST(WebAppShortcutCreatorTest, UpdateIcon) {
   base::FilePath dst_path = scoped_temp_dir.path();
 
   ShellIntegration::ShortcutInfo info = GetShortcutInfo();
-  info.favicon = ui::ResourceBundle::GetSharedInstance().GetImageNamed(
-      IDR_PRODUCT_LOGO_32);
+  gfx::Image product_logo =
+      ui::ResourceBundle::GetSharedInstance().GetNativeImageNamed(
+          IDR_PRODUCT_LOGO_32);
+  info.favicon.Add(product_logo);
   WebAppShortcutCreatorMock shortcut_creator(info);
 
-  shortcut_creator.UpdateIcon(dst_path);
+  ASSERT_TRUE(shortcut_creator.UpdateIcon(dst_path));
   base::FilePath icon_path =
       dst_path.Append("Contents").Append("Resources").Append("app.icns");
 
   scoped_nsobject<NSImage> image([[NSImage alloc] initWithContentsOfFile:
       base::mac::FilePathToNSString(icon_path)]);
   EXPECT_TRUE(image);
-  EXPECT_EQ(info.favicon.ToSkBitmap()->width(), [image size].width);
-  EXPECT_EQ(info.favicon.ToSkBitmap()->height(), [image size].height);
+  EXPECT_EQ(product_logo.Width(), [image size].width);
+  EXPECT_EQ(product_logo.Height(), [image size].height);
 }
 
 }  // namespace web_app
diff --git a/chrome/browser/web_applications/web_app_win.cc b/chrome/browser/web_applications/web_app_win.cc
index 4b543f1..92ef88e 100644
--- a/chrome/browser/web_applications/web_app_win.cc
+++ b/chrome/browser/web_applications/web_app_win.cc
@@ -20,12 +20,19 @@
 #include "chrome/installer/util/browser_distribution.h"
 #include "content/public/browser/browser_thread.h"
 #include "ui/gfx/icon_util.h"
+#include "ui/gfx/image/image.h"
+#include "ui/gfx/image/image_family.h"
 
 namespace {
 
 const base::FilePath::CharType kIconChecksumFileExt[] =
     FILE_PATH_LITERAL(".ico.md5");
 
+// Width and height of icons exported to .ico files.
+// TODO(mgiuca): Remove when icon_util has the capability to save all icon
+// sizes, not just a single particular size.
+const int kIconExportSize = 32;
+
 // Calculates image checksum using MD5.
 void GetImageCheckSum(const SkBitmap& image, base::MD5Digest* digest) {
   DCHECK(digest);
@@ -173,9 +180,14 @@ namespace internals {
 // Saves |image| to |icon_file| if the file is outdated and refresh shell's
 // icon cache to ensure correct icon is displayed. Returns true if icon_file
 // is up to date or successfully updated.
-bool CheckAndSaveIcon(const base::FilePath& icon_file, const SkBitmap& image) {
-  if (ShouldUpdateIcon(icon_file, image)) {
-    if (SaveIconWithCheckSum(icon_file, image)) {
+bool CheckAndSaveIcon(const base::FilePath& icon_file,
+                      const gfx::ImageFamily& image) {
+  // TODO(mgiuca): Save an icon with all icon sizes, not just an icon at a
+  // hard-coded fixed size. http://crbug.com/163864.
+  const gfx::Image* icon = image.GetBest(kIconExportSize, kIconExportSize);
+  SkBitmap bitmap = icon ? icon->AsBitmap() : SkBitmap();
+  if (ShouldUpdateIcon(icon_file, bitmap)) {
+    if (SaveIconWithCheckSum(icon_file, bitmap)) {
       // Refresh shell's icon cache. This call is quite disruptive as user would
       // see explorer rebuilding the icon cache. It would be great that we find
       // a better way to achieve this.
@@ -236,8 +248,7 @@ bool CreatePlatformShortcuts(
   // Creates an ico file to use with shortcut.
   base::FilePath icon_file = web_app_path.Append(file_name).AddExtension(
       FILE_PATH_LITERAL(".ico"));
-  if (!web_app::internals::CheckAndSaveIcon(icon_file,
-        *shortcut_info.favicon.ToSkBitmap())) {
+  if (!web_app::internals::CheckAndSaveIcon(icon_file, shortcut_info.favicon)) {
     return false;
   }
 
@@ -319,8 +330,7 @@ void UpdatePlatformShortcuts(
   base::FilePath icon_file = web_app_path.Append(file_name).AddExtension(
       FILE_PATH_LITERAL(".ico"));
   if (file_util::PathExists(icon_file)) {
-    web_app::internals::CheckAndSaveIcon(icon_file,
-        *shortcut_info.favicon.ToSkBitmap());
+    web_app::internals::CheckAndSaveIcon(icon_file, shortcut_info.favicon);
   }
 }