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

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.
 
 #import "chrome/browser/web_applications/web_app_mac.h"
 
+#import <Carbon/Carbon.h>
 #import <Cocoa/Cocoa.h>
 
 #include "base/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/mac/bundle_locations.h"
 #include "base/mac/foundation_util.h"
 #include "base/mac/mac_logging.h"
 #include "base/mac/mac_util.h"
 #include "base/mac/scoped_cftyperef.h"
 #include "base/memory/scoped_nsobject.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/chrome_paths_internal.h"
 #include "chrome/common/mac/app_mode_common.h"
 #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() {
   base::FilePath path;
   if (base::mac::GetLocalDirectory(NSApplicationDirectory, &path) &&
       file_util::PathIsWritable(path)) {
     return path;
   }
   if (base::mac::GetUserDirectory(NSApplicationDirectory, &path))
     return path;
   return base::FilePath();
 }
 
+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 {
 
 const char kChromeAppDirName[] = "Chrome Apps.localized";
 
 WebAppShortcutCreator::WebAppShortcutCreator(
     const base::FilePath& user_data_dir,
     const ShellIntegration::ShortcutInfo& shortcut_info,
     const string16& chrome_bundle_id)
     : user_data_dir_(user_data_dir),
       info_(shortcut_info),
@@ skipping 118 matching lines @@
             forKey:app_mode::kCrAppModeUserDataDirKey];
   [plist setObject:base::mac::FilePathToNSString(info_.profile_path.BaseName())
             forKey:app_mode::kCrAppModeProfileDirKey];
   return [plist writeToFile:plist_path atomically:YES];
 }
 
 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;
   }
 
   if (!image_added)
     return false;
 
   base::FilePath resources_path =
       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
 {
   NSString* bundle_id_template =
     base::mac::ObjCCast<NSString>(
         [plist objectForKey:base::mac::CFToNSCast(kCFBundleIdentifierKey)]);
   NSString* extension_id = base::SysUTF8ToNSString(info_.extension_id);
   NSString* placeholder =
       [NSString stringWithFormat:@"@%@@", app_mode::kShortcutIdPlaceholder];
@@ skipping 67 matching lines @@
 void UpdatePlatformShortcuts(
     const base::FilePath& web_app_path,
     const ShellIntegration::ShortcutInfo& shortcut_info) {
   // TODO(benwells): Implement this when shortcuts / weblings are enabled on
   // mac.
 }
 
 }  // namespace internals
 
 }  // namespace web_app