MacViews: Use Cocoa folder icons in tree views.

chrome/browser/resource_delegate_mac.mm

+// Copyright 2016 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/resource_delegate_mac.h"
+
+#import <AppKit/AppKit.h>
+#import <Carbon/Carbon.h>
+
+#include "blink/grit/devtools_resources.h"
+#include "ui/gfx/image/image_skia.h"
+#include "ui/gfx/image/image_skia_operations.h"
+#include "ui/gfx/image/image_skia_util_mac.h"
+#include "ui/resources/grit/ui_resources.h"
+#include "ui/views/resources/grit/views_resources.h"
+
+namespace ui {
+
+namespace {
+// The singleton instance
+MacResourceDelegate* g_instance = nullptr;

[tapted, 2016/01/14 23:43:50]

I don't think you need this

[karandeepb, 2016/01/18 00:31:18]

Done.

+}  // namespace
+
+// Make intercepted_icons_map_ const?
+MacResourceDelegate::MacResourceDelegate() {
+  DCHECK(!g_instance) << "Cannot initialize resource bundle delegate twice.";
+  intercepted_icons_map_.insert(InterceptedIconsMap::value_type(

[tapted, 2016/01/14 23:43:50]

We should try not to do anything in the constructor, since this is on the Chrome
startup path, and it's super important that that is fast.

[karandeepb, 2016/01/18 00:31:17]

Done.

+      IDR_FOLDER_OPEN,
+      InterceptedIcon(NSFileTypeForHFSTypeCode(kOpenFolderIcon), false,
+                      IconSize::SMALL)));
+  intercepted_icons_map_.insert(InterceptedIconsMap::value_type(
+      IDR_FOLDER_CLOSED,
+      InterceptedIcon(NSFileTypeForHFSTypeCode(kGenericFolderIcon), false,
+                      IconSize::SMALL)));
+  intercepted_icons_map_.insert(InterceptedIconsMap::value_type(
+      IDR_FOLDER_OPEN_RTL,
+      InterceptedIcon(NSFileTypeForHFSTypeCode(kOpenFolderIcon), true,
+                      IconSize::SMALL)));
+  intercepted_icons_map_.insert(InterceptedIconsMap::value_type(
+      IDR_FOLDER_CLOSED_RTL,
+      InterceptedIcon(NSFileTypeForHFSTypeCode(kGenericFolderIcon), true,
+                      IconSize::SMALL)));
+  intercepted_icons_map_.insert(InterceptedIconsMap::value_type(
+      IMAGES_FRAME_PNG,
+      InterceptedIcon(NSFileTypeForHFSTypeCode(kGenericFolderIcon), false,
+                      IconSize::SMALL)));
+  g_instance = this;
+}
+
+MacResourceDelegate::~MacResourceDelegate() {
+  DCHECK_EQ(g_instance, this);
+  g_instance = nullptr;
+}
+
+base::FilePath MacResourceDelegate::GetPathForResourcePack(
+    const base::FilePath& pack_path,
+    ui::ScaleFactor scale_factor) {
+  return pack_path;
+};
+
+base::FilePath MacResourceDelegate::GetPathForLocalePack(
+    const base::FilePath& pack_path,
+    const std::string& locale) {
+  return pack_path;
+};
+
+gfx::Image MacResourceDelegate::GetImageNamed(int resource_id) {
+  return this->GetNativeImageNamed(resource_id,

[tapted, 2016/01/14 23:43:50]

no need for `this->` but also when does this get used? (can it just return
gfx::Image unconditionally?)

[karandeepb, 2016/01/18 00:31:17]

tree_view.cc uses the GetImageNamed(..) call. Should I change it there?

[tapted, 2016/01/18 04:15:42]

So, I *think* what you have is correct. A ResourceBundle OWNER may know for
sure. I'm not certain that it is always OK to return an NSImage-backed resource
from GetImageNamed, but it seems to be supported and I don't think it would work
otherwise.

+                                   ui::ResourceBundle::RTL_DISABLED);
+};
+
+gfx::Image MacResourceDelegate::GetNativeImageNamed(
+    int resource_id,
+    ui::ResourceBundle::ImageRTL rtl) {
+  DCHECK_EQ(rtl, ui::ResourceBundle::RTL_DISABLED);

[tapted, 2016/01/14 23:43:50]

comment why this is safe? (do we need to worry about the mirroring if this is
the case?)

[karandeepb, 2016/01/18 00:31:17]

Done. This is because on Mac and mostly other platforms as well, there are
separate mirrored assets. Hence ImageRTL::RTL_ENABLED is not used. Same is done
in resource_bundle_mac.mm -
https://code.google.com/p/chromium/codesearch#chromium/src/ui/base/resource/r....

+  InterceptedIconsMap::const_iterator it =

[tapted, 2016/01/14 23:43:50]

I would just put a switch statement here for now, like

switch (resource_id) {
  case IDR_FOLDER_OPEN:
    return ImageForHFSType(kOpenFolderIcon);
  case IDR_FOLDER_CLOSED:
    return ImageForHFSType(kGenericFolderIcon);
  ..
}


with

gfx::Image ImageForHFSType(.. hfs_type) {
  return gfx::Image([[[NSWorkspace sharedWorkspace] iconForFileType:hfs_type]);
}

[karandeepb, 2016/01/18 00:31:17]

Done. The default return size is 32X32, which is big for the Edit Bookmark
dialog, hence have provided an IconSize option as well.

+      intercepted_icons_map_.find(resource_id);
+  if (it == intercepted_icons_map_.end())
+    return gfx::Image();
+  return it->second.GetImage();
+};
+
+base::RefCountedStaticMemory* MacResourceDelegate::LoadDataResourceBytes(
+    int resource_id,
+    ui::ScaleFactor scale_factor) {
+  // Currently ignoring scale factor.
+  InterceptedIconsMap::const_iterator it =
+      intercepted_icons_map_.find(resource_id);
+  if (it == intercepted_icons_map_.end())
+    return nullptr;
+
+  gfx::Image image = it->second.GetImage();
+  scoped_refptr<base::RefCountedMemory> image_memory = image.As1xPNGBytes();
+  if (!image_memory->size())
+    return nullptr;
+
+  return new base::RefCountedStaticMemory(image_memory->front(),
+                                          image_memory->size());
+};
+
+bool MacResourceDelegate::GetRawDataResource(int resource_id,
+                                             ui::ScaleFactor scale_factor,
+                                             base::StringPiece* value) {
+  // Currently ignoring scale factor.

[karandeepb, 2016/01/14 04:21:33]

I am not quite sure how to handle scale factor. Generally, its just used to
define scale with respect to the 1X resource. 

[karandeepb, 2016/01/18 00:31:18]

Done.

+  InterceptedIconsMap::const_iterator it =
+      intercepted_icons_map_.find(resource_id);
+  if (it == intercepted_icons_map_.end())
+    return false;
+
+  gfx::Image image = it->second.GetImage();
+  scoped_refptr<base::RefCountedMemory> image_memory = image.As1xPNGBytes();
+  if (!image_memory->size())
+    return false;
+  value->set(image_memory->front_as<char>(), image_memory->size());
+  return true;
+};
+
+bool MacResourceDelegate::GetLocalizedString(int message_id,
+                                             base::string16* value) {
+  return false;
+};
+
+scoped_ptr<gfx::Font> MacResourceDelegate::GetFont(
+    ui::ResourceBundle::FontStyle style) {
+  return scoped_ptr<gfx::Font>();
+};
+
+MacResourceDelegate::InterceptedIcon::InterceptedIcon(NSString* file_type,
+                                                      bool flip_icon,
+                                                      IconSize icon_size)
+    : file_type_(file_type),
+      flip_icon_(flip_icon),
+      icon_size_(icon_size),
+      is_image_loaded_(false) {}
+
+MacResourceDelegate::InterceptedIcon::~InterceptedIcon() {}
+
+const gfx::Image MacResourceDelegate::InterceptedIcon::GetImage() const {

[karandeepb, 2016/01/14 04:21:33]

This can also use chrome/browser/icon_manager.h, with a few caveats - 
1) Will need change to IconManager's public api.
2) Will involve async calls.
Should it be preferred?

+  if (!is_image_loaded_) {
+    is_image_loaded_ = true;
+    NSImage* icon =
+        [[[NSWorkspace sharedWorkspace] iconForFileType:file_type_] retain];
+    if (icon) {
+      NSSize desired_size;
+      switch (icon_size_) {
+        case IconSize::SMALL:
+          desired_size = NSMakeSize(16, 16);
+          break;
+        case IconSize::BIG:
+          desired_size = NSMakeSize(32, 32);
+          break;
+        default:
+          NOTREACHED();
+      }
+      gfx::ImageSkia image_skia =
+          gfx::ImageSkiaFromResizedNSImage(icon, desired_size);
+      if (flip_icon_)
+        image_skia = gfx::ImageSkiaOperations::CreateMirroredImage(image_skia);

[karandeepb, 2016/01/14 04:21:33]

I couldn't find a simple enough way to mirror an image with the current APIs.
Any other easier way?

[karandeepb, 2016/01/18 00:31:18]

Done.

+      icon_image_ = gfx::Image(image_skia);
+    }
+  }
+  return icon_image_;
+}
+
+}  // namespace ui