Add support for multiple icon sizes for Mac platform apps

chrome/browser/web_applications/web_app_mac.mm

 // 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/web_applications/web_app_mac.h"
 
 #import <Cocoa/Cocoa.h>
 
 #include "base/file_util.h"
 #include "base/mac/bundle_locations.h"
@@ skipping 15 matching lines @@
 // 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]);
 }
 
+// Adds |image_rep| to |icon_family|. Returns true on success, false on failure.
+bool AddBitmapImageRepToIconFamily(IconFamily* icon_family,
+                                   NSBitmapImageRep* image_rep) {
+  NSSize size = [image_rep size];
+  if (size.width != size.height)
+    return false;
+
+  switch (static_cast<int>(size.width)) {
+    case 512:
+      return [icon_family setIconFamilyElement:kIconServices512PixelDataARGB
+                            fromBitmapImageRep:image_rep];
+    case 256:
+      return [icon_family setIconFamilyElement:kIconServices256PixelDataARGB
+                            fromBitmapImageRep:image_rep];
+    case 128:
+      return [icon_family setIconFamilyElement:kThumbnail32BitData
+                            fromBitmapImageRep:image_rep] &&
+             [icon_family setIconFamilyElement:kThumbnail8BitMask
+                            fromBitmapImageRep:image_rep];
+    case 32:
+      return [icon_family setIconFamilyElement:kLarge32BitData
+                            fromBitmapImageRep:image_rep] &&
+             [icon_family setIconFamilyElement:kLarge8BitData
+                            fromBitmapImageRep:image_rep] &&
+             [icon_family setIconFamilyElement:kLarge8BitMask
+                            fromBitmapImageRep:image_rep] &&
+             [icon_family setIconFamilyElement:kLarge1BitMask
+                            fromBitmapImageRep:image_rep];
+    case 16:
+      return [icon_family setIconFamilyElement:kSmall32BitData
+                            fromBitmapImageRep:image_rep] &&
+             [icon_family setIconFamilyElement:kSmall8BitData
+                            fromBitmapImageRep:image_rep] &&
+             [icon_family setIconFamilyElement:kSmall8BitMask
+                            fromBitmapImageRep:image_rep] &&
+             [icon_family setIconFamilyElement:kSmall1BitMask
+                            fromBitmapImageRep:image_rep];
+    default:
+      return false;
+  }
+}
+
 }  // namespace
 
 
 namespace web_app {
 
 WebAppShortcutCreator::WebAppShortcutCreator(
     const FilePath& user_data_dir,
     const ShellIntegration::ShortcutInfo& shortcut_info)
     : user_data_dir_(user_data_dir),
       info_(shortcut_info) {
@@ skipping 70 matching lines @@
   [dict setObject:base::SysUTF8ToNSString(info_.url.spec())
            forKey:app_mode::kCrAppModeShortcutURLKey];
   [dict setObject:base::mac::FilePathToNSString(user_data_dir_)
            forKey:app_mode::kCrAppModeUserDataDirKey];
   [dict setObject:base::mac::FilePathToNSString(info_.extension_path)
            forKey:app_mode::kCrAppModeExtensionPathKey];
   return [dict writeToFile:plist_path atomically:YES];
 }
 
 bool WebAppShortcutCreator::UpdateIcon(const FilePath& app_path) const {
-  // TODO(sail): Add support for multiple icon sizes.
-  if (info_.favicon.empty() || info_.favicon.width() != 32 ||
-      info_.favicon.height() != 32) {
+  if (info_.favicon.IsEmpty())
     return true;
-  }
-
-  NSBitmapImageRep* image_rep = SkBitmapToImageRep(info_.favicon);
-  if (!image_rep)
-    return false;
 
   scoped_nsobject<IconFamily> icon_family([[IconFamily alloc] init]);
-  bool success = [icon_family setIconFamilyElement:kLarge32BitData
-                                fromBitmapImageRep:image_rep] &&
-                 [icon_family setIconFamilyElement:kLarge8BitData
-                                fromBitmapImageRep:image_rep] &&
-                 [icon_family setIconFamilyElement:kLarge8BitMask
-                                fromBitmapImageRep:image_rep] &&
-                 [icon_family setIconFamilyElement:kLarge1BitMask
-                                fromBitmapImageRep:image_rep];
-  if (!success)
-    return false;
+  bool image_added = false;

[jeremy, 2012/02/29 07:13:47]

Doesn't look like you check the value of this?

[sail, 2012/02/29 19:45:16]

Ouch good catch. I dropped this when rebasing. Re-added.

+  for (size_t i = 0; i < info_.favicon.GetNumberOfSkBitmaps(); ++i) {
+    NSBitmapImageRep* image_rep =
+        SkBitmapToImageRep(*info_.favicon.GetSkBitmapAtIndex(i));
+    if (!image_rep)
+      continue;
+
+    if (!AddBitmapImageRepToIconFamily(icon_family, image_rep))

[jeremy, 2012/02/29 07:13:47]

Could you add a comment on why it's ok to continue without failing here?

Is it possible that the favicon doesn't contain square images or images of the
wrong size?

[sail, 2012/02/29 19:45:16]

Done.

Added a comment.

+      continue;
+
+    image_added = true;
+  }
 
   FilePath resources_path = app_path.Append("Contents").Append("Resources");
   if (!file_util::CreateDirectory(resources_path))
     return false;
   FilePath icon_path = resources_path.Append("app.icns");
   return [icon_family writeToFile:base::mac::FilePathToNSString(icon_path)];
 }
 
 }  // namespace
 
 namespace web_app {
 namespace internals {
 
 void CreateShortcutTask(const FilePath& web_app_path,
                         const FilePath& profile_path,
                         const ShellIntegration::ShortcutInfo& shortcut_info) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
   WebAppShortcutCreator shortcut_creator(web_app_path, shortcut_info);
   shortcut_creator.CreateShortcut();
 }
 
 }  // namespace internals
 }  // namespace web_app