Polish thumbar code.

* Fix a memory leak in thumbar initialization.
* Check the number of thumbar buttons, should be <= 7.
* Correct to check thumbar button click event.

atom/browser/native_window_views.cc

@@ -27,7 +27,6 @@
 #include "ui/views/controls/webview/unhandled_keyboard_event_handler.h"
 #include "ui/views/controls/webview/webview.h"
 #include "ui/views/window/client_view.h"
-#include "ui/views/widget/desktop_aura/desktop_native_widget_aura.h"
 #include "ui/views/widget/native_widget_private.h"
 #include "ui/views/widget/widget.h"
 #include "ui/wm/core/shadow_types.h"
@@ -52,6 +51,7 @@
 #include "ui/base/win/shell.h"
 #include "ui/gfx/icon_util.h"
 #include "ui/gfx/win/dpi.h"
+#include "ui/views/widget/desktop_aura/desktop_native_widget_aura.h"
 #include "ui/views/win/hwnd_util.h"
 #endif
 

atom/browser/ui/win/atom_desktop_window_tree_host_win.cc

@@ -3,6 +3,9 @@
 // found in the LICENSE file.
 
 #include "atom/browser/ui/win/atom_desktop_window_tree_host_win.h"
+
+#include <shobjidl.h>
+
 #include "atom/browser/ui/win/thumbar_host.h"
 
 namespace atom {
@@ -32,8 +35,11 @@ bool AtomDesktopWindowTreeHostWin::PreHandleMSG(UINT message,
                                                 LRESULT* result) {
   switch (message) {
     case WM_COMMAND: {
+       // Handle thumbar button click message.
        int id = LOWORD(w_param);
-       if (thumbar_host_ && thumbar_host_->HandleThumbarButtonEvent(id))
+       int thbn_message = HIWORD(w_param);
+       if (thbn_message == THBN_CLICKED && thumbar_host_ &&
+           thumbar_host_->HandleThumbarButtonEvent(id))
          return true;
     }
   }

atom/browser/ui/win/thumbar_host.cc

@@ -18,9 +18,14 @@ namespace atom {
 
 namespace {
 
+// From MSDN: https://msdn.microsoft.com/en-us/library/windows/desktop/
+// dd378460(v=vs.85).aspx#thumbbars
 // The thumbnail toolbar has a maximum of seven buttons due to the limited room.
 const int kMaxButtonsCount = 7;
 
+// The base id of Thumbar button.
+const int kButtonIdBase = 40001;
+
 bool GetThumbarButtonFlags(const std::vector<std::string>& flags,
                            THUMBBUTTONFLAGS* out) {
   if (flags.empty()) {
@@ -60,7 +65,18 @@ ThumbarHost::~ThumbarHost() {
 
 bool ThumbarHost::SetThumbarButtons(
     const std::vector<ThumbarHost::ThumbarButton>& buttons) {
-  THUMBBUTTON thumb_buttons[kMaxButtonsCount];
+  if (buttons.size() > kMaxButtonsCount)
+    return false;
+
+  base::win::ScopedComPtr<ITaskbarList3> taskbar;
+  if (FAILED(taskbar.CreateInstance(CLSID_TaskbarList,
+                                    nullptr,
+                                    CLSCTX_INPROC_SERVER)) ||
+      FAILED(taskbar->HrInit())) {
+    return false;
+  }
+
+  THUMBBUTTON thumb_buttons[kMaxButtonsCount] = {};
   thumbar_button_clicked_callback_map_.clear();
 
   // Once a toolbar with a set of buttons is added to thumbnail, there is no way
@@ -70,7 +86,8 @@ bool ThumbarHost::SetThumbarButtons(
   //
   // Initialize all thumb buttons with HIDDEN state.
   for (int i = 0; i < kMaxButtonsCount; ++i) {
-    thumb_buttons[i].iId = i;
+    thumb_buttons[i].iId = kButtonIdBase + i;
+    thumb_buttons[i].dwMask = THB_FLAGS;  // dwFlags is valid.
     thumb_buttons[i].dwFlags = THBF_HIDDEN;
   }
 
@@ -87,25 +104,25 @@ bool ThumbarHost::SetThumbarButtons(
                base::UTF8ToUTF16(buttons[i].tooltip).c_str());
     }
 
-    thumbar_button_clicked_callback_map_[i] =
+    thumbar_button_clicked_callback_map_[thumb_buttons[i].iId] =
         buttons[i].clicked_callback;
   }
 
-  base::win::ScopedComPtr<ITaskbarList3> taskbar;
+  bool is_success = false;
-  if (FAILED(taskbar.CreateInstance(CLSID_TaskbarList,
-                                    nullptr,
-                                    CLSCTX_INPROC_SERVER)) ||
-      FAILED(taskbar->HrInit())) {
-    return false;
-  }
   if (!is_initialized_) {
     is_initialized_ = true;
-    return taskbar->ThumbBarAddButtons(
+    is_success = taskbar->ThumbBarAddButtons(
+        window_, kMaxButtonsCount, thumb_buttons) == S_OK;
+  } else {
+    is_success = taskbar->ThumbBarUpdateButtons(
         window_, kMaxButtonsCount, thumb_buttons) == S_OK;
   }
 
-  return taskbar->ThumbBarUpdateButtons(
+  // Release thumb_buttons' icons, the taskbar makes its own copy.
-      window_, kMaxButtonsCount, thumb_buttons) == S_OK;
+  for (size_t i = 0; i < buttons.size(); ++i) {
+    ::DestroyIcon(thumb_buttons[i].hIcon);
+  }
+  return is_success;
 }
 
 bool ThumbarHost::HandleThumbarButtonEvent(int button_id) {

atom/browser/ui/win/thumbar_host.h

@@ -18,7 +18,7 @@ namespace atom {
 
 class ThumbarHost {
  public:
-  using ThumbarButtonClickedCallback = base::Callback<void(void)>;
+  using ThumbarButtonClickedCallback = base::Closure;
 
   struct ThumbarButton {
     std::string tooltip;

docs/api/browser-window.md

@@ -648,29 +648,32 @@ __Note:__ This API is only available on Windows (Windows 7 and above)
 
 * `buttons` Array of `button` objects
   * `button` Object
-     * `icon` [NativeImage](native-image.md) - The icon showing in thumbnail toolbar.
+    * `icon` [NativeImage](native-image.md) - The icon showing in thumbnail
-     * `tooltip` String - (Option) - The text of the button's tooltip.
+      toolbar.
-     * `flags` Array of Strings - (Option) Control specific states and behaviors
+    * `tooltip` String - (Option) - The text of the button's tooltip.
-          of the button. By default, it uses `enabled`.
+    * `flags` Array of Strings - (Option) Control specific states and behaviors
-       *  `enabled` - The button is active and available to the user.
+      of the button. By default, it uses `enabled`.
-       *  `disabled` - The button is disabled. It is present, but has a visual state
+      *  `enabled` - The button is active and available to the user.
-          that indicates that it will not respond to user action.
+      *  `disabled` - The button is disabled. It is present, but has a visual
-       *  `dismissonclick` - When the button is clicked, the taskbar button's flyout
+         state that indicates that it will not respond to user action.
-          closes immediately.
+      *  `dismissonclick` - When the button is clicked, the taskbar button's
-       *  `nobackground` - Do not draw a button border, use only the image.
+         flyout closes immediately.
-       *  `hidden` - The button is not shown to the user.
+      *  `nobackground` - Do not draw a button border, use only the image.
-       *  `noninteractive` - The button is enabled but not interactive; no pressed
+      *  `hidden` - The button is not shown to the user.
-          button state is drawn. This value is intended for instances where the button
+      *  `noninteractive` - The button is enabled but not interactive; no
-          is used in a notification.
+         pressed button state is drawn. This value is intended for instances
-     * `click` - Function
+         where the button is used in a notification.
+    * `click` - Function
 
-Add a thumbnail toolbar with a specified set of buttons to the thumbnail image of
+Add a thumbnail toolbar with a specified set of buttons to the thumbnail image
-a window in a taskbar button layout.
+of a window in a taskbar button layout. Returns a `Boolean` object indicates
+whether the thumbnail has been added successfully.
 
 __Note:__ This API is only available on Windows (Windows 7 and above).
-Once you setup the thumbnail toolbar, the toolbar cannot be removed due to the
+The number of buttons in thumbnail toolbar should be no greater than 7 due to
-platform's limitation. But you can call the API with an empty array to clean the
+the limited room. Once you setup the thumbnail toolbar, the toolbar cannot be
-buttons.
+removed due to the platform's limitation. But you can call the API with an empty
+array to clean the buttons.
 
 ### BrowserWindow.showDefinitionForSelection()
 

filenames.gypi

@@ -119,8 +119,6 @@
       'atom/browser/atom_browser_main_parts.h',
       'atom/browser/atom_browser_main_parts_linux.cc',
       'atom/browser/atom_browser_main_parts_mac.mm',
-      "atom/browser/atom_desktop_window_tree_host_win.cc",
-      "atom/browser/atom_desktop_window_tree_host_win.h",
       'atom/browser/atom_javascript_dialog_manager.cc',
       'atom/browser/atom_javascript_dialog_manager.h',
       'atom/browser/atom_quota_permission_context.cc',
@@ -204,6 +202,8 @@
       'atom/browser/ui/views/submenu_button.h',
       'atom/browser/ui/views/win_frame_view.cc',
       'atom/browser/ui/views/win_frame_view.h',
+      "atom/browser/ui/win/atom_desktop_window_tree_host_win.cc",
+      "atom/browser/ui/win/atom_desktop_window_tree_host_win.h",
       'atom/browser/ui/win/notify_icon_host.cc',
       'atom/browser/ui/win/notify_icon_host.h',
       'atom/browser/ui/win/notify_icon.cc',