Create app shims with 32-bit icons only, as per OSX 10.5 spec.

chrome/browser/web_applications/web_app_mac.mm

Index: chrome/browser/web_applications/web_app_mac.mm
diff --git a/chrome/browser/web_applications/web_app_mac.mm b/chrome/browser/web_applications/web_app_mac.mm
index 5c933cd5239e7f8d4d1ba6460265ca76067acf2a..343470ba235aa49464bb9448f14d94d4fd1a0817 100644
--- a/chrome/browser/web_applications/web_app_mac.mm
+++ b/chrome/browser/web_applications/web_app_mac.mm
@@ -4,6 +4,7 @@
 
 #import "chrome/browser/web_applications/web_app_mac.h"
 
+#import <Carbon/Carbon.h>
 #import <Cocoa/Cocoa.h>
 
 #include "base/file_util.h"
@@ -22,67 +23,69 @@
 #include "content/public/browser/browser_thread.h"
 #include "grit/chromium_strings.h"
 #include "skia/ext/skia_utils_mac.h"
-#include "third_party/icon_family/IconFamily.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+#include "third_party/skia/include/core/SkColor.h"
 #include "ui/base/l10n/l10n_util_mac.h"
 #include "ui/gfx/image/image_family.h"
 
 namespace {
 
-// 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;
+void ConvertSkiaToARGB(const SkBitmap& bitmap, char* argb) {

[tapted, 2013/05/22 08:27:33]

This is similar to a function in skia_utils.h, but that function makes RGBA, not
the ARGB, which IconFamily wants. There's an alternative implementation in Patch
Set 1 which looks more like that function, but it uses more skia internals for
the 'unpremultiply' step. This one relies on SkBitmap::getColor returning an
unpremultiplied value, which I think is neater and more robust.

+  SkAutoLockPixels lock(bitmap);
+  for (int y = 0; y < bitmap.height(); ++y) {
+    for (int x = 0; x < bitmap.width(); ++x) {
+      SkColor pixel = bitmap.getColor(x, y);  // Unpremultiplies.
+      argb[0] = SkColorGetA(pixel);
+      argb[1] = SkColorGetR(pixel);
+      argb[2] = SkColorGetG(pixel);
+      argb[3] = SkColorGetB(pixel);
+      argb += 4;
     }
   }
-  return nil;
 }
 
-// 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)
+// Adds |image| to |icon_family|. Returns true on success, false on failure.
+bool AddGfxImageToIconFamily(IconFamilyHandle icon_family,
+                             const gfx::Image& image) {
+  // When called via ShowCreateChromeAppShortcutsDialog the ImageFamily will
+  // have all the representations desired here for mac, from the kDesiredSizes
+  // array in web_app_ui.cc.
+  SkBitmap bitmap = image.AsBitmap();
+  if (bitmap.config() != SkBitmap::kARGB_8888_Config ||
+      bitmap.width() != bitmap.height()) {
     return false;
+  }
 
-  switch (static_cast<int>(size.width)) {
+  OSType icon_type;
+  switch (bitmap.width()) {
     case 512:
-      return [icon_family setIconFamilyElement:kIconServices512PixelDataARGB
-                            fromBitmapImageRep:image_rep];
+      icon_type = kIconServices512PixelDataARGB;
+      break;
     case 256:
-      return [icon_family setIconFamilyElement:kIconServices256PixelDataARGB
-                            fromBitmapImageRep:image_rep];
+      icon_type = kIconServices256PixelDataARGB;
+      break;
     case 128:
-      return [icon_family setIconFamilyElement:kThumbnail32BitData
-                            fromBitmapImageRep:image_rep] &&
-             [icon_family setIconFamilyElement:kThumbnail8BitMask
-                            fromBitmapImageRep:image_rep];
+      icon_type = kIconServices128PixelDataARGB;
+      break;
+    case 48:
+      icon_type = kIconServices48PixelDataARGB;
+      break;
     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];
+      icon_type = kIconServices32PixelDataARGB;
+      break;
     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];
+      icon_type = kIconServices16PixelDataARGB;
+      break;
     default:
       return false;
   }
+
+  Handle raw_data = NewHandle(bitmap.getSize());

[benwells, 2013/05/23 00:42:04]

I can see that it is guaranteed by the config check above that bitmap.getSize is
right, but this code makes me uneasy. Consider adding a max size argument to
CovertSkiaToARGB, or maybe add a comment.

[benwells, 2013/05/23 00:42:04]

Is there a scoped version of the handle you can use?

[tapted, 2013/05/23 04:03:43]

Done.

[tapted, 2013/05/23 04:03:43]

Done.

+  ConvertSkiaToARGB(bitmap, *raw_data);
+  OSErr result = SetIconFamilyData(icon_family, icon_type, raw_data);
+  DisposeHandle(raw_data);
+
+  return result == noErr;
 }
 
 base::FilePath GetWritableApplicationsDirectory() {
@@ -96,8 +99,37 @@ base::FilePath GetWritableApplicationsDirectory() {
   return base::FilePath();
 }
 
-}  // namespace
+template <class T>
+class ScopedCarbonHandle {
+ public:
+  ScopedCarbonHandle() {
+    typed_handle_ = reinterpret_cast<T>(NewHandle(0));
+  }
 
+  ~ScopedCarbonHandle() {
+    DisposeHandle(AsHandle());
+  }
+
+  T get() {
+    return typed_handle_;
+  }
+
+  bool WriteDataToFile(const base::FilePath& path) {
+    NSData* data = [NSData dataWithBytes:*AsHandle()
+                                  length:GetHandleSize(AsHandle())];
+    return [data writeToFile:base::mac::FilePathToNSString(path)
+                  atomically:NO];
+  }
+
+ private:
+  Handle AsHandle() {
+    return reinterpret_cast<Handle>(typed_handle_);
+  }
+
+  T typed_handle_;
+};
+
+}  // namespace
 
 namespace web_app {
 
@@ -237,19 +269,16 @@ bool WebAppShortcutCreator::UpdateIcon(const base::FilePath& app_path) const {
   if (info_.favicon.empty())
     return true;
 
-  scoped_nsobject<IconFamily> icon_family([[IconFamily alloc] init]);
+  ScopedCarbonHandle<IconFamilyHandle> icon_family;
   bool image_added = false;
   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;
 
     // Missing an icon size is not fatal so don't fail if adding the bitmap
     // doesn't work.
-    if (!AddBitmapImageRepToIconFamily(icon_family, image_rep))
+    if (!AddGfxImageToIconFamily(icon_family.get(), *it))
       continue;
 
     image_added = true;
@@ -262,8 +291,8 @@ bool WebAppShortcutCreator::UpdateIcon(const base::FilePath& app_path) const {
       app_path.Append("Contents").Append("Resources");
   if (!file_util::CreateDirectory(resources_path))
     return false;
-  base::FilePath icon_path = resources_path.Append("app.icns");
-  return [icon_family writeToFile:base::mac::FilePathToNSString(icon_path)];
+
+  return icon_family.WriteDataToFile(resources_path.Append("app.icns"));
 }
 
 NSString* WebAppShortcutCreator::GetBundleIdentifier(NSDictionary* plist) const