Merge pull request #5624 from electron/hicon

Feed Windows APIs with ICO icons of appropriate size

atom/browser/api/atom_api_tray.cc

@@ -9,6 +9,7 @@
 #include "atom/browser/api/atom_api_menu.h"
 #include "atom/browser/browser.h"
 #include "atom/browser/ui/tray_icon.h"
+#include "atom/common/api/atom_api_native_image.h"
 #include "atom/common/native_mate_converters/gfx_converter.h"
 #include "atom/common/native_mate_converters/image_converter.h"
 #include "atom/common/native_mate_converters/string16_converter.h"
@@ -22,9 +23,9 @@ namespace atom {
 
 namespace api {
 
-Tray::Tray(v8::Isolate* isolate, const gfx::Image& image)
+Tray::Tray(v8::Isolate* isolate, mate::Handle<NativeImage> image)
     : tray_icon_(TrayIcon::Create()) {
-  tray_icon_->SetImage(image);
+  SetImage(isolate, image);
   tray_icon_->AddObserver(this);
 }
 
@@ -32,7 +33,8 @@ Tray::~Tray() {
 }
 
 // static
-mate::WrappableBase* Tray::New(v8::Isolate* isolate, const gfx::Image& image) {
+mate::WrappableBase* Tray::New(v8::Isolate* isolate,
+                               mate::Handle<NativeImage> image) {
   if (!Browser::Get()->is_ready()) {
     isolate->ThrowException(v8::Exception::Error(mate::StringToV8(
         isolate, "Cannot create Tray before app is ready")));
@@ -94,29 +96,38 @@ void Tray::OnDragEnded() {
   Emit("drag-end");
 }
 
-void Tray::SetImage(mate::Arguments* args, const gfx::Image& image) {
-  tray_icon_->SetImage(image);
+void Tray::SetImage(v8::Isolate* isolate, mate::Handle<NativeImage> image) {
+#if defined(OS_WIN)
+  tray_icon_->SetImage(image->GetHICON(GetSystemMetrics(SM_CXSMICON)));
+#else
+  tray_icon_->SetImage(image->image());
+#endif
 }
 
-void Tray::SetPressedImage(mate::Arguments* args, const gfx::Image& image) {
-  tray_icon_->SetPressedImage(image);
+void Tray::SetPressedImage(v8::Isolate* isolate,
+                           mate::Handle<NativeImage> image) {
+#if defined(OS_WIN)
+  tray_icon_->SetPressedImage(image->GetHICON(GetSystemMetrics(SM_CXSMICON)));
+#else
+  tray_icon_->SetPressedImage(image->image());
+#endif
 }
 
-void Tray::SetToolTip(mate::Arguments* args, const std::string& tool_tip) {
+void Tray::SetToolTip(const std::string& tool_tip) {
   tray_icon_->SetToolTip(tool_tip);
 }
 
-void Tray::SetTitle(mate::Arguments* args, const std::string& title) {
+void Tray::SetTitle(const std::string& title) {
   tray_icon_->SetTitle(title);
 }
 
-void Tray::SetHighlightMode(mate::Arguments* args, bool highlight) {
+void Tray::SetHighlightMode(bool highlight) {
   tray_icon_->SetHighlightMode(highlight);
 }
 
 void Tray::DisplayBalloon(mate::Arguments* args,
                           const mate::Dictionary& options) {
-  gfx::Image icon;
+  mate::Handle<NativeImage> icon;
   options.Get("icon", &icon);
   base::string16 title, content;
   if (!options.Get("title", &title) ||
@@ -125,7 +136,14 @@ void Tray::DisplayBalloon(mate::Arguments* args,
     return;
   }
 
-  tray_icon_->DisplayBalloon(icon, title, content);
+#if defined(OS_WIN)
+  tray_icon_->DisplayBalloon(
+      icon.IsEmpty() ? NULL : icon->GetHICON(GetSystemMetrics(SM_CXSMICON)),
+      title, content);
+#else
+  tray_icon_->DisplayBalloon(
+      icon.IsEmpty() ? gfx::Image() : icon->image(), title, content);
+#endif
 }
 
 void Tray::PopUpContextMenu(mate::Arguments* args) {
@@ -136,7 +154,8 @@ void Tray::PopUpContextMenu(mate::Arguments* args) {
   tray_icon_->PopUpContextMenu(pos, menu.IsEmpty() ? nullptr : menu->model());
 }
 
-void Tray::SetContextMenu(mate::Arguments* args, Menu* menu) {
+void Tray::SetContextMenu(v8::Isolate* isolate, mate::Handle<Menu> menu) {
+  menu_.Reset(isolate, menu.ToV8());
   tray_icon_->SetContextMenu(menu->model());
 }
 
@@ -162,7 +181,7 @@ void Tray::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("setHighlightMode", &Tray::SetHighlightMode)
       .SetMethod("displayBalloon", &Tray::DisplayBalloon)
       .SetMethod("popUpContextMenu", &Tray::PopUpContextMenu)
-      .SetMethod("_setContextMenu", &Tray::SetContextMenu);
+      .SetMethod("setContextMenu", &Tray::SetContextMenu);
 }
 
 }  // namespace api

atom/browser/api/atom_api_tray.h

@@ -11,6 +11,7 @@
 #include "atom/browser/api/trackable_object.h"
 #include "atom/browser/ui/tray_icon_observer.h"
 #include "base/memory/scoped_ptr.h"
+#include "native_mate/handle.h"
 
 namespace gfx {
 class Image;
@@ -28,18 +29,19 @@ class TrayIcon;
 namespace api {
 
 class Menu;
+class NativeImage;
 
 class Tray : public mate::TrackableObject<Tray>,
              public TrayIconObserver {
  public:
   static mate::WrappableBase* New(
-      v8::Isolate* isolate, const gfx::Image& image);
+      v8::Isolate* isolate, mate::Handle<NativeImage> image);
 
   static void BuildPrototype(v8::Isolate* isolate,
                              v8::Local<v8::ObjectTemplate> prototype);
 
  protected:
-  Tray(v8::Isolate* isolate, const gfx::Image& image);
+  Tray(v8::Isolate* isolate, mate::Handle<NativeImage> image);
   ~Tray() override;
 
   // TrayIconObserver:
@@ -55,18 +57,19 @@ class Tray : public mate::TrackableObject<Tray>,
   void OnDragExited() override;
   void OnDragEnded() override;
 
-  void SetImage(mate::Arguments* args, const gfx::Image& image);
-  void SetPressedImage(mate::Arguments* args, const gfx::Image& image);
-  void SetToolTip(mate::Arguments* args, const std::string& tool_tip);
-  void SetTitle(mate::Arguments* args, const std::string& title);
-  void SetHighlightMode(mate::Arguments* args, bool highlight);
+  void SetImage(v8::Isolate* isolate, mate::Handle<NativeImage> image);
+  void SetPressedImage(v8::Isolate* isolate, mate::Handle<NativeImage> image);
+  void SetToolTip(const std::string& tool_tip);
+  void SetTitle(const std::string& title);
+  void SetHighlightMode(bool highlight);
   void DisplayBalloon(mate::Arguments* args, const mate::Dictionary& options);
   void PopUpContextMenu(mate::Arguments* args);
-  void SetContextMenu(mate::Arguments* args, Menu* menu);
+  void SetContextMenu(v8::Isolate* isolate, mate::Handle<Menu> menu);
 
  private:
   v8::Local<v8::Object> ModifiersToObject(v8::Isolate* isolate, int modifiers);
 
+  v8::Global<v8::Object> menu_;
   scoped_ptr<TrayIcon> tray_icon_;
 
   DISALLOW_COPY_AND_ASSIGN(Tray);

atom/browser/api/atom_api_window.cc

@@ -21,8 +21,11 @@
 #include "native_mate/dictionary.h"
 #include "ui/gfx/geometry/rect.h"
 
-#if defined(OS_WIN)
+#if defined(TOOLKIT_VIEWS)
 #include "atom/browser/native_window_views.h"
+#endif
+
+#if defined(OS_WIN)
 #include "atom/browser/ui/win/taskbar_host.h"
 #endif
 
@@ -99,6 +102,13 @@ Window::Window(v8::Isolate* isolate, const mate::Dictionary& options) {
   window_->InitFromOptions(options);
   window_->AddObserver(this);
   AttachAsUserData(window_.get());
+
+#if defined(TOOLKIT_VIEWS)
+  // Sets the window icon.
+  mate::Handle<NativeImage> icon;
+  if (options.Get(options::kIcon, &icon))
+    SetIcon(icon);
+#endif
 }
 
 Window::~Window() {
@@ -617,6 +627,18 @@ void Window::ShowDefinitionForSelection() {
 }
 #endif
 
+#if defined(TOOLKIT_VIEWS)
+void Window::SetIcon(mate::Handle<NativeImage> icon) {
+#if defined(OS_WIN)
+  static_cast<NativeWindowViews*>(window_.get())->SetIcon(
+      icon->GetHICON(GetSystemMetrics(SM_CXSMICON)), icon->GetHICON(256));
+#elif defined(USE_X11)
+  static_cast<NativeWindowViews*>(window_.get())->SetIcon(
+      icon->image().AsImageSkia());
+#endif
+}
+#endif
+
 void Window::SetAspectRatio(double aspect_ratio, mate::Arguments* args) {
   gfx::Size extra_size;
   args->GetNext(&extra_size);
@@ -738,6 +760,9 @@ void Window::BuildPrototype(v8::Isolate* isolate,
 #if defined(OS_MACOSX)
       .SetMethod("showDefinitionForSelection",
                  &Window::ShowDefinitionForSelection)
+#endif
+#if defined(TOOLKIT_VIEWS)
+      .SetMethod("setIcon", &Window::SetIcon)
 #endif
       .SetProperty("id", &Window::ID)
       .SetProperty("webContents", &Window::WebContents);

atom/browser/api/atom_api_window.h

@@ -14,6 +14,7 @@
 #include "atom/browser/api/trackable_object.h"
 #include "atom/browser/native_window.h"
 #include "atom/browser/native_window_observer.h"
+#include "atom/common/api/atom_api_native_image.h"
 #include "native_mate/handle.h"
 
 class GURL;
@@ -172,6 +173,10 @@ class Window : public mate::TrackableObject<Window>,
   void ShowDefinitionForSelection();
 #endif
 
+#if defined(TOOLKIT_VIEWS)
+  void SetIcon(mate::Handle<NativeImage> icon);
+#endif
+
   void SetVisibleOnAllWorkspaces(bool visible);
   bool IsVisibleOnAllWorkspaces();
 

atom/browser/native_window.cc

@@ -12,7 +12,6 @@
 #include "atom/browser/atom_browser_main_parts.h"
 #include "atom/browser/window_list.h"
 #include "atom/common/api/api_messages.h"
-#include "atom/common/native_mate_converters/image_converter.h"
 #include "atom/common/native_mate_converters/file_path_converter.h"
 #include "atom/common/options_switches.h"
 #include "base/files/file_util.h"
@@ -65,9 +64,6 @@ NativeWindow::NativeWindow(
   // mode.
   ui::GpuSwitchingManager::SetTransparent(transparent_);
 
-  // Read icon before window is created.
-  options.Get(options::kIcon, &icon_);
-
   WindowList::AddWindow(this);
 }
 

atom/browser/native_window.h

@@ -251,7 +251,6 @@ class NativeWindow : public base::SupportsUserData,
   bool transparent() const { return transparent_; }
   SkRegion* draggable_region() const { return draggable_region_.get(); }
   bool enable_larger_than_screen() const { return enable_larger_than_screen_; }
-  gfx::ImageSkia icon() const { return icon_; }
 
   void set_has_dialog_attached(bool has_dialog_attached) {
     has_dialog_attached_ = has_dialog_attached;
@@ -307,9 +306,6 @@ class NativeWindow : public base::SupportsUserData,
   // Whether window can be resized larger than screen.
   bool enable_larger_than_screen_;
 
-  // Window icon.
-  gfx::ImageSkia icon_;
-
   // The windows has been closed.
   bool is_closed_;
 

atom/browser/native_window_views.cc

@@ -11,6 +11,7 @@
 #include "atom/browser/ui/views/menu_layout.h"
 #include "atom/common/color_util.h"
 #include "atom/common/draggable_region.h"
+#include "atom/common/native_mate_converters/image_converter.h"
 #include "atom/common/options_switches.h"
 #include "base/strings/utf_string_conversions.h"
 #include "brightray/browser/inspectable_web_contents.h"
@@ -40,6 +41,7 @@
 #include "chrome/browser/ui/libgtk2ui/unity_service.h"
 #include "ui/base/x/x11_util.h"
 #include "ui/gfx/x/x11_types.h"
+#include "ui/views/widget/desktop_aura/desktop_window_tree_host_x11.h"
 #include "ui/views/window/native_frame_view.h"
 #elif defined(OS_WIN)
 #include "atom/browser/ui/views/win_frame_view.h"
@@ -273,7 +275,6 @@ NativeWindowViews::NativeWindowViews(
       use_content_size_)
     size = ContentSizeToWindowSize(size);
 
-  window_->UpdateWindowIcon();
   window_->CenterWindow(size);
   Layout();
 }
@@ -778,6 +779,23 @@ gfx::AcceleratedWidget NativeWindowViews::GetAcceleratedWidget() {
   return GetNativeWindow()->GetHost()->GetAcceleratedWidget();
 }
 
+#if defined(OS_WIN)
+void NativeWindowViews::SetIcon(HICON small_icon, HICON app_icon) {
+  HWND hwnd = GetAcceleratedWidget();
+  SendMessage(hwnd, WM_SETICON, ICON_SMALL,
+              reinterpret_cast<LPARAM>(small_icon));
+  SendMessage(hwnd, WM_SETICON, ICON_BIG,
+              reinterpret_cast<LPARAM>(app_icon));
+}
+#elif defined(USE_X11)
+void NativeWindowViews::SetIcon(const gfx::ImageSkia& icon) {
+  views::DesktopWindowTreeHostX11* tree_host =
+      views::DesktopWindowTreeHostX11::GetHostForXID(GetAcceleratedWidget());
+  static_cast<views::DesktopWindowTreeHost*>(tree_host)->SetWindowIcons(
+      icon, icon);
+}
+#endif
+
 void NativeWindowViews::OnWidgetActivationChanged(
     views::Widget* widget, bool active) {
   if (widget != window_.get())
@@ -842,14 +860,6 @@ bool NativeWindowViews::ShouldHandleSystemCommands() const {
   return true;
 }
 
-gfx::ImageSkia NativeWindowViews::GetWindowAppIcon() {
-  return icon();
-}
-
-gfx::ImageSkia NativeWindowViews::GetWindowIcon() {
-  return GetWindowAppIcon();
-}
-
 views::Widget* NativeWindowViews::GetWidget() {
   return window_.get();
 }

atom/browser/native_window_views.h

@@ -104,6 +104,12 @@ class NativeWindowViews : public NativeWindow,
 
   gfx::AcceleratedWidget GetAcceleratedWidget() override;
 
+#if defined(OS_WIN)
+  void SetIcon(HICON small_icon, HICON app_icon);
+#elif defined(USE_X11)
+  void SetIcon(const gfx::ImageSkia& icon);
+#endif
+
   views::Widget* widget() const { return window_.get(); }
 
 #if defined(OS_WIN)
@@ -125,8 +131,6 @@ class NativeWindowViews : public NativeWindow,
   bool CanMinimize() const override;
   base::string16 GetWindowTitle() const override;
   bool ShouldHandleSystemCommands() const override;
-  gfx::ImageSkia GetWindowAppIcon() override;
-  gfx::ImageSkia GetWindowIcon() override;
   views::Widget* GetWidget() override;
   const views::Widget* GetWidget() const override;
   views::View* GetContentsView() override;
@@ -145,7 +149,6 @@ class NativeWindowViews : public NativeWindow,
   // MessageHandlerDelegate:
   bool PreHandleMSG(
       UINT message, WPARAM w_param, LPARAM l_param, LRESULT* result) override;
-
   void HandleSizeEvent(WPARAM w_param, LPARAM l_param);
 #endif
 
@@ -202,7 +205,6 @@ class NativeWindowViews : public NativeWindow,
 
   // If true we have enabled a11y
   bool enabled_a11y_support_;
-
 #endif
 
   // Handles unhandled keyboard messages coming back from the renderer process.

atom/browser/ui/tray_icon.cc

@@ -12,7 +12,7 @@ TrayIcon::TrayIcon() {
 TrayIcon::~TrayIcon() {
 }
 
-void TrayIcon::SetPressedImage(const gfx::Image& image) {
+void TrayIcon::SetPressedImage(ImageType image) {
 }
 
 void TrayIcon::SetTitle(const std::string& title) {
@@ -21,7 +21,7 @@ void TrayIcon::SetTitle(const std::string& title) {
 void TrayIcon::SetHighlightMode(bool highlight) {
 }
 
-void TrayIcon::DisplayBalloon(const gfx::Image& icon,
+void TrayIcon::DisplayBalloon(ImageType icon,
                               const base::string16& title,
                               const base::string16& contents) {
 }

atom/browser/ui/tray_icon.h

@@ -19,13 +19,19 @@ class TrayIcon {
  public:
   static TrayIcon* Create();
 
+#if defined(OS_WIN)
+  using ImageType = HICON;
+#else
+  using ImageType = const gfx::Image&;
+#endif
+
   virtual ~TrayIcon();
 
   // Sets the image associated with this status icon.
-  virtual void SetImage(const gfx::Image& image) = 0;
+  virtual void SetImage(ImageType image) = 0;
 
   // Sets the image associated with this status icon when pressed.
-  virtual void SetPressedImage(const gfx::Image& image);
+  virtual void SetPressedImage(ImageType image);
 
   // Sets the hover text for this status icon. This is also used as the label
   // for the menu item which is created as a replacement for the status icon
@@ -43,7 +49,7 @@ class TrayIcon {
 
   // Displays a notification balloon with the specified contents.
   // Depending on the platform it might not appear by the icon tray.
-  virtual void DisplayBalloon(const gfx::Image& icon,
+  virtual void DisplayBalloon(ImageType icon,
                               const base::string16& title,
                               const base::string16& contents);
 

atom/browser/ui/win/notify_icon.cc

@@ -9,7 +9,6 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/win/windows_version.h"
 #include "third_party/skia/include/core/SkBitmap.h"
-#include "ui/gfx/icon_util.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/geometry/rect.h"
@@ -91,19 +90,20 @@ void NotifyIcon::ResetIcon() {
     LOG(WARNING) << "Unable to re-create status tray icon.";
 }
 
-void NotifyIcon::SetImage(const gfx::Image& image) {
+void NotifyIcon::SetImage(HICON image) {
+  icon_ = base::win::ScopedHICON(CopyIcon(image));
+
   // Create the icon.
   NOTIFYICONDATA icon_data;
   InitIconData(&icon_data);
   icon_data.uFlags |= NIF_ICON;
-  icon_ = IconUtil::CreateHICONFromSkBitmap(image.AsBitmap());
-  icon_data.hIcon = icon_.get();
+  icon_data.hIcon = image;
   BOOL result = Shell_NotifyIcon(NIM_MODIFY, &icon_data);
   if (!result)
     LOG(WARNING) << "Error setting status tray icon image";
 }
 
-void NotifyIcon::SetPressedImage(const gfx::Image& image) {
+void NotifyIcon::SetPressedImage(HICON image) {
   // Ignore pressed images, since the standard on Windows is to not highlight
   // pressed status icons.
 }
@@ -119,7 +119,7 @@ void NotifyIcon::SetToolTip(const std::string& tool_tip) {
     LOG(WARNING) << "Unable to set tooltip for status tray icon";
 }
 
-void NotifyIcon::DisplayBalloon(const gfx::Image& icon,
+void NotifyIcon::DisplayBalloon(HICON icon,
                                 const base::string16& title,
                                 const base::string16& contents) {
   NOTIFYICONDATA icon_data;
@@ -129,13 +129,8 @@ void NotifyIcon::DisplayBalloon(const gfx::Image& icon,
   wcsncpy_s(icon_data.szInfoTitle, title.c_str(), _TRUNCATE);
   wcsncpy_s(icon_data.szInfo, contents.c_str(), _TRUNCATE);
   icon_data.uTimeout = 0;
-
-  base::win::Version win_version = base::win::GetVersion();
-  if (!icon.IsEmpty() && win_version != base::win::VERSION_PRE_XP) {
-    balloon_icon_ = IconUtil::CreateHICONFromSkBitmap(icon.AsBitmap());
-    icon_data.hBalloonIcon = balloon_icon_.get();
-    icon_data.dwInfoFlags = NIIF_USER | NIIF_LARGE_ICON;
-  }
+  icon_data.hBalloonIcon = icon;
+  icon_data.dwInfoFlags = NIIF_USER | NIIF_LARGE_ICON;
 
   BOOL result = Shell_NotifyIcon(NIM_MODIFY, &icon_data);
   if (!result)

atom/browser/ui/win/notify_icon.h

@@ -45,10 +45,10 @@ class NotifyIcon : public TrayIcon {
   UINT message_id() const { return message_id_; }
 
   // Overridden from TrayIcon:
-  void SetImage(const gfx::Image& image) override;
-  void SetPressedImage(const gfx::Image& image) override;
+  void SetImage(HICON image) override;
+  void SetPressedImage(HICON image) override;
   void SetToolTip(const std::string& tool_tip) override;
-  void DisplayBalloon(const gfx::Image& icon,
+  void DisplayBalloon(HICON icon,
                       const base::string16& title,
                       const base::string16& contents) override;
   void PopUpContextMenu(const gfx::Point& pos,
@@ -73,9 +73,6 @@ class NotifyIcon : public TrayIcon {
   // The currently-displayed icon for the window.
   base::win::ScopedHICON icon_;
 
-  // The currently-displayed icon for the notification balloon.
-  base::win::ScopedHICON balloon_icon_;
-
   // The context menu.
   ui::SimpleMenuModel* menu_model_;
 

atom/common/api/atom_api_native_image.cc

@@ -142,7 +142,7 @@ bool IsTemplateFilename(const base::FilePath& path) {
 #endif
 
 #if defined(OS_WIN)
-bool ReadImageSkiaFromICO(gfx::ImageSkia* image, const base::FilePath& path) {
+base::win::ScopedHICON ReadICOFromPath(int size, const base::FilePath& path) {
   // If file is in asar archive, we extract it to a temp file so LoadImage can
   // load it.
   base::FilePath asar_path, relative_path;
@@ -155,16 +155,15 @@ bool ReadImageSkiaFromICO(gfx::ImageSkia* image, const base::FilePath& path) {
   }
 
   // Load the icon from file.
-  base::win::ScopedHICON icon(static_cast<HICON>(
-      LoadImage(NULL, image_path.value().c_str(), IMAGE_ICON, 0, 0,
-                LR_DEFAULTSIZE | LR_LOADFROMFILE)));
-  if (!icon.get())
-    return false;
+  return base::win::ScopedHICON(static_cast<HICON>(
+      LoadImage(NULL, image_path.value().c_str(), IMAGE_ICON, size, size,
+                LR_LOADFROMFILE)));
+}
 
+void ReadImageSkiaFromICO(gfx::ImageSkia* image, HICON icon) {
   // Convert the icon from the Windows specific HICON to gfx::ImageSkia.
-  scoped_ptr<SkBitmap> bitmap(IconUtil::  CreateSkBitmapFromHICON(icon.get()));
+  scoped_ptr<SkBitmap> bitmap(IconUtil::CreateSkBitmapFromHICON(icon));
   image->AddRepresentation(gfx::ImageSkiaRep(*bitmap, 1.0f));
-  return true;
 }
 #endif
 
@@ -175,8 +174,40 @@ NativeImage::NativeImage(v8::Isolate* isolate, const gfx::Image& image)
   Init(isolate);
 }
 
+#if defined(OS_WIN)
+NativeImage::NativeImage(v8::Isolate* isolate, const base::FilePath& hicon_path)
+    : hicon_path_(hicon_path) {
+  // Use the 256x256 icon as fallback icon.
+  gfx::ImageSkia image_skia;
+  ReadImageSkiaFromICO(&image_skia, GetHICON(256));
+  image_ = gfx::Image(image_skia);
+  Init(isolate);
+}
+#endif
+
 NativeImage::~NativeImage() {}
 
+#if defined(OS_WIN)
+HICON NativeImage::GetHICON(int size) {
+  auto iter = hicons_.find(size);
+  if (iter != hicons_.end())
+    return iter->second.get();
+
+  // First try loading the icon with specified size.
+  if (!hicon_path_.empty()) {
+    hicons_[size] = std::move(ReadICOFromPath(size, hicon_path_));
+    return hicons_[size].get();
+  }
+
+  // Then convert the image to ICO.
+  if (image_.IsEmpty())
+    return NULL;
+  hicons_[size] = std::move(
+      IconUtil::CreateHICONFromSkBitmap(image_.AsBitmap()));
+  return hicons_[size].get();
+}
+#endif
+
 v8::Local<v8::Value> NativeImage::ToPNG(v8::Isolate* isolate) {
   scoped_refptr<base::RefCountedMemory> png = image_.As1xPNGBytes();
   return node::Buffer::Copy(isolate,
@@ -263,16 +294,15 @@ mate::Handle<NativeImage> NativeImage::CreateFromJPEG(
 // static
 mate::Handle<NativeImage> NativeImage::CreateFromPath(
     v8::Isolate* isolate, const base::FilePath& path) {
-  gfx::ImageSkia image_skia;
   base::FilePath image_path = NormalizePath(path);
-
-  if (image_path.MatchesExtension(FILE_PATH_LITERAL(".ico"))) {
 #if defined(OS_WIN)
-    ReadImageSkiaFromICO(&image_skia, image_path);
-#endif
-  } else {
-    PopulateImageSkiaRepsFromPath(&image_skia, image_path);
+  if (image_path.MatchesExtension(FILE_PATH_LITERAL(".ico"))) {
+    return mate::CreateHandle(isolate,
+                              new NativeImage(isolate, image_path));
   }
+#endif
+  gfx::ImageSkia image_skia;
+  PopulateImageSkiaRepsFromPath(&image_skia, image_path);
   gfx::Image image(image_skia);
   mate::Handle<NativeImage> handle = Create(isolate, image);
 #if defined(OS_MACOSX)
@@ -328,6 +358,35 @@ void NativeImage::BuildPrototype(
 
 }  // namespace atom
 
+namespace mate {
+
+v8::Local<v8::Value> Converter<mate::Handle<atom::api::NativeImage>>::ToV8(
+    v8::Isolate* isolate,
+    const mate::Handle<atom::api::NativeImage>& val) {
+  return val.ToV8();
+}
+
+bool Converter<mate::Handle<atom::api::NativeImage>>::FromV8(
+    v8::Isolate* isolate, v8::Local<v8::Value> val,
+    mate::Handle<atom::api::NativeImage>* out) {
+  // Try converting from file path.
+  base::FilePath path;
+  if (ConvertFromV8(isolate, val, &path)) {
+    *out = atom::api::NativeImage::CreateFromPath(isolate, path);
+    // Should throw when failed to initialize from path.
+    return !(*out)->image().IsEmpty();
+  }
+
+  WrappableBase* wrapper = static_cast<WrappableBase*>(internal::FromV8Impl(
+      isolate, val));
+  if (!wrapper)
+    return false;
+
+  *out = CreateHandle(isolate, static_cast<atom::api::NativeImage*>(wrapper));
+  return true;
+}
+
+}  // namespace mate
 
 namespace {
 

atom/common/api/atom_api_native_image.h

@@ -5,12 +5,18 @@
 #ifndef ATOM_COMMON_API_ATOM_API_NATIVE_IMAGE_H_
 #define ATOM_COMMON_API_ATOM_API_NATIVE_IMAGE_H_
 
+#include <map>
 #include <string>
 
 #include "native_mate/handle.h"
 #include "native_mate/wrappable.h"
 #include "ui/gfx/image/image.h"
 
+#if defined(OS_WIN)
+#include "base/files/file_path.h"
+#include "base/win/scoped_gdi_object.h"
+#endif
+
 class GURL;
 
 namespace base {
@@ -48,10 +54,17 @@ class NativeImage : public mate::Wrappable<NativeImage> {
   static void BuildPrototype(v8::Isolate* isolate,
                              v8::Local<v8::ObjectTemplate> prototype);
 
+#if defined(OS_WIN)
+  HICON GetHICON(int size);
+#endif
+
   const gfx::Image& image() const { return image_; }
 
  protected:
   NativeImage(v8::Isolate* isolate, const gfx::Image& image);
+#if defined(OS_WIN)
+  NativeImage(v8::Isolate* isolate, const base::FilePath& hicon_path);
+#endif
   ~NativeImage() override;
 
  private:
@@ -69,6 +82,11 @@ class NativeImage : public mate::Wrappable<NativeImage> {
   // Determine if the image is a template image.
   bool IsTemplateImage();
 
+#if defined(OS_WIN)
+  base::FilePath hicon_path_;
+  std::map<int, base::win::ScopedHICON> hicons_;
+#endif
+
   gfx::Image image_;
 
   DISALLOW_COPY_AND_ASSIGN(NativeImage);
@@ -78,4 +96,19 @@ class NativeImage : public mate::Wrappable<NativeImage> {
 
 }  // namespace atom
 
+namespace mate {
+
+// A custom converter that allows converting path to NativeImage.
+template<>
+struct Converter<mate::Handle<atom::api::NativeImage>> {
+  static v8::Local<v8::Value> ToV8(
+      v8::Isolate* isolate,
+      const mate::Handle<atom::api::NativeImage>& val);
+  static bool FromV8(v8::Isolate* isolate, v8::Local<v8::Value> val,
+                     mate::Handle<atom::api::NativeImage>* out);
+};
+
+}  // namespace mate
+
+
 #endif  // ATOM_COMMON_API_ATOM_API_NATIVE_IMAGE_H_

atom/common/native_mate_converters/image_converter.cc

@@ -28,16 +28,8 @@ bool Converter<gfx::Image>::FromV8(v8::Isolate* isolate,
     return true;
 
   Handle<atom::api::NativeImage> native_image;
-  if (!ConvertFromV8(isolate, val, &native_image)) {
-    // Try converting from file path.
-    base::FilePath path;
-    if (!Converter<base::FilePath>::FromV8(isolate, val, &path))
-      return false;
-
-    native_image = atom::api::NativeImage::CreateFromPath(isolate, path);
-    if (native_image->image().IsEmpty())
-      return false;
-  }
+  if (!ConvertFromV8(isolate, val, &native_image))
+    return false;
 
   *out = native_image->image();
   return true;

docs/api/browser-window.md

@@ -65,8 +65,9 @@ It creates a new `BrowserWindow` with native properties as set by the `options`.
     `false`.
   * `kiosk` Boolean - The kiosk mode. Default is `false`.
   * `title` String - Default window title. Default is `"Electron"`.
-  * `icon` [NativeImage](native-image.md) - The window icon, when omitted on
-    Windows the executable's icon would be used as window icon.
+  * `icon` [NativeImage](native-image.md) - The window icon. On Windows it is
+    recommended to use `ICO` icons to get best visual effects, you can also
+    leave it undefined so the executable's icon will be used.
   * `show` Boolean - Whether window should be shown when created. Default is
     `true`.
   * `frame` Boolean - Specify `false` to create a
@@ -885,6 +886,12 @@ The `flags` is an array that can include following `String`s:
 
 Shows pop-up dictionary that searches the selected word on the page.
 
+### `win.setIcon(icon)` _Windows_ _Linux_
+
+* `icon` [NativeImage](native-image.md)
+
+Changes window icon.
+
 ### `win.setAutoHideMenuBar(hide)`
 
 * `hide` Boolean

docs/api/native-image.md

@@ -25,7 +25,12 @@ const appIcon = new Tray(image);
 Currently `PNG` and `JPEG` image formats are supported. `PNG` is recommended
 because of its support for transparency and lossless compression.
 
-On Windows, you can also load an `ICO` icon from a file path.
+On Windows, you can also load `ICO` icons from file paths, to get best visual
+effects it is recommended to include at least followings sizes in the icon:
+
+* 16x16
+* 32x32
+* 256x256
 
 ## High Resolution Image
 

docs/api/tray.md

@@ -29,6 +29,7 @@ __Platform limitations:__
 * When app indicator is used on Linux, the `click` event is ignored.
 * On Linux in order for changes made to individual `MenuItem`s to take effect,
   you have to call `setContextMenu` again. For example:
+* On Windows it is recommended to use `ICO` icons to get best visual effects.
 
 ```javascript
 contextMenu.items[2].checked = false;

lib/browser/api/tray.js

@@ -3,11 +3,4 @@ const {Tray} = process.atomBinding('tray')
 
 Object.setPrototypeOf(Tray.prototype, EventEmitter.prototype)
 
-Tray.prototype.setContextMenu = function (menu) {
-  this._setContextMenu(menu)
-
-  // Keep a strong reference of menu.
-  this.menu = menu
-}
-
 module.exports = Tray

vendor/native_mate

@@ -1 +1 @@
-Subproject commit ea07d4c6c89d8d460e76b2d2ab9b0ebc51f9a432
+Subproject commit 4ad6ecd19617ac33c09e93ccb6d8e652ac1ac126