Refactor code in taskbarHost

atom/browser/api/atom_api_window.cc

@@ -438,16 +438,18 @@ void Window::SetOverlayIcon(const gfx::Image& overlay,
   window_->SetOverlayIcon(overlay, description);
 }
 
-void Window::SetThumbarButtons(mate::Arguments* args) {
+bool Window::SetThumbarButtons(mate::Arguments* args) {
 #if defined(OS_WIN)
   std::vector<TaskbarHost::ThumbarButton> buttons;
   if (!args->GetNext(&buttons)) {
     args->ThrowError();
-    return;
+    return false;
   }
   auto window = static_cast<NativeWindowViews*>(window_.get());
-  window->taskbar_host().SetThumbarButtons(window->GetAcceleratedWidget(),
+  return window->taskbar_host().SetThumbarButtons(
-                                           buttons);
+      window->GetAcceleratedWidget(), buttons);
+#else
+  return false;
 #endif
 }
 

atom/browser/api/atom_api_window.h

@@ -130,7 +130,7 @@ class Window : public mate::TrackableObject<Window>,
   void SetProgressBar(double progress);
   void SetOverlayIcon(const gfx::Image& overlay,
                       const std::string& description);
-  void SetThumbarButtons(mate::Arguments* args);
+  bool SetThumbarButtons(mate::Arguments* args);
   void SetMenu(v8::Isolate* isolate, v8::Local<v8::Value> menu);
   void SetAutoHideMenuBar(bool auto_hide);
   bool IsMenuBarAutoHide();

atom/browser/ui/win/taskbar_host.cc

@@ -8,49 +8,40 @@
 
 #include <string>
 
+#include "base/stl_util.h"
 #include "base/win/scoped_comptr.h"
-#include "base/win/win_util.h"
+#include "base/win/scoped_gdi_object.h"
-#include "base/win/wrapped_window_proc.h"
 #include "base/strings/utf_string_conversions.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/icon_util.h"
-#include "ui/gfx/win/hwnd_util.h"
 
 namespace atom {
 
 namespace {
 
-// From MSDN: https://msdn.microsoft.com/en-us/library/windows/desktop/
+// From MSDN: https://msdn.microsoft.com/en-us/library/windows/desktop/dd378460(v=vs.85).aspx#thumbbars
-// dd378460(v=vs.85).aspx#thumbbars
 // The thumbnail toolbar has a maximum of seven buttons due to the limited room.
-const int kMaxButtonsCount = 7;
+const size_t 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()) {
+  THUMBBUTTONFLAGS result = THBF_ENABLED;  // THBF_ENABLED == 0
-    *out = THBF_ENABLED;
-    return true;
-  }
-  THUMBBUTTONFLAGS result = static_cast<THUMBBUTTONFLAGS>(0);
   for (const auto& flag : flags) {
-    if (flag == "enabled") {
+    if (flag == "disabled")
-      result |= THBF_ENABLED;
-    } else if (flag == "disabled") {
       result |= THBF_DISABLED;
-    } else if (flag == "dismissonclick") {
+    else if (flag == "dismissonclick")
       result |= THBF_DISMISSONCLICK;
-    } else if (flag == "nobackground") {
+    else if (flag == "nobackground")
       result |= THBF_NOBACKGROUND;
-    } else if (flag == "hidden") {
+    else if (flag == "hidden")
       result |= THBF_HIDDEN;
-    } else if (flag == "noninteractive") {
+    else if (flag == "noninteractive")
       result |= THBF_NONINTERACTIVE;
-    } else {
+    else
       return false;
-    }
   }
   *out = result;
   return true;
@@ -58,7 +49,7 @@ bool GetThumbarButtonFlags(const std::vector<std::string>& flags,
 
 }  // namespace
 
-TaskbarHost::TaskbarHost() : is_initialized_(false) {
+TaskbarHost::TaskbarHost() : thumbar_buttons_added_(false) {
 }
 
 TaskbarHost::~TaskbarHost() {
@@ -66,9 +57,6 @@ TaskbarHost::~TaskbarHost() {
 
 bool TaskbarHost::SetThumbarButtons(
     HWND window, const std::vector<ThumbarButton>& buttons) {
-  if (buttons.size() > kMaxButtonsCount)
-    return false;
-
   base::win::ScopedComPtr<ITaskbarList3> taskbar;
   if (FAILED(taskbar.CreateInstance(CLSID_TaskbarList,
                                     nullptr,
@@ -77,59 +65,67 @@ bool TaskbarHost::SetThumbarButtons(
     return false;
   }
 
+  callback_map_.clear();
+
+  // The number of buttons in thumbar can not be changed once it is created,
+  // so we have to claim kMaxButtonsCount buttons initialy in case users add
+  // more buttons later.
+  base::win::ScopedHICON icons[kMaxButtonsCount] = {};
   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
+  for (size_t i = 0; i < kMaxButtonsCount; ++i) {
-  // to remove it without re-creating the window.
+    THUMBBUTTON& thumb_button = thumb_buttons[i];
-  // To achieve to re-set thumbar buttons,  we initialize the buttons with
-  // HIDDEN state and only updated the caller's specified buttons.
-  //
-  // Initialize all thumb buttons with HIDDEN state.
-  for (int i = 0; i < kMaxButtonsCount; ++i) {
-    thumb_buttons[i].iId = kButtonIdBase + i;
-    thumb_buttons[i].dwMask = THB_FLAGS;  // dwFlags is valid.
-    thumb_buttons[i].dwFlags = THBF_HIDDEN;
-  }
 
-  // Update the callers' specified buttons.
+    // Set ID.
-  for (size_t i = 0; i < buttons.size(); ++i) {
+    thumb_button.iId = kButtonIdBase + i;
-    if (!GetThumbarButtonFlags(buttons[i].flags, &thumb_buttons[i].dwFlags))
+    thumb_button.dwMask = THB_FLAGS;
-      return false;
+
-    thumb_buttons[i].dwMask = THB_ICON | THB_FLAGS;
+    if (i >= buttons.size()) {
-    thumb_buttons[i].hIcon = IconUtil::CreateHICONFromSkBitmap(
+      // This button is used to occupy the place in toolbar, and it does not
-        buttons[i].icon.AsBitmap());
+      // show.
-    if (!buttons[i].tooltip.empty()) {
+      thumb_button.dwFlags = THBF_HIDDEN;
-      thumb_buttons[i].dwMask |= THB_TOOLTIP;
+      continue;
-      wcscpy_s(thumb_buttons[i].szTip,
-               base::UTF8ToUTF16(buttons[i].tooltip).c_str());
     }
 
-    thumbar_button_clicked_callback_map_[thumb_buttons[i].iId] =
+    // This button is user's button.
-        buttons[i].clicked_callback;
+    const ThumbarButton& button = buttons[i];
+
+    // Generate flags.
+    thumb_button.dwFlags = THBF_ENABLED;
+    if (!GetThumbarButtonFlags(button.flags, &thumb_button.dwFlags))
+      return false;
+
+    // Set icon.
+    if (!button.icon.IsEmpty()) {
+      thumb_button.dwMask |= THB_ICON;
+      icons[i] = IconUtil::CreateHICONFromSkBitmap(button.icon.AsBitmap());
+      thumb_button.hIcon = icons[i].Get();
+    }
+
+    // Set tooltip.
+    if (!button.tooltip.empty()) {
+      thumb_button.dwMask |= THB_TOOLTIP;
+      wcscpy_s(thumb_button.szTip, base::UTF8ToUTF16(button.tooltip).c_str());
+    }
+
+    // Save callback.
+    callback_map_[thumb_button.iId] = button.clicked_callback;
   }
 
-  bool is_success = false;
+  // Finally add them to taskbar.
-  if (!is_initialized_) {
+  HRESULT r;
-    is_initialized_ = true;
+  if (thumbar_buttons_added_)
-    is_success = taskbar->ThumbBarAddButtons(
+    r = taskbar->ThumbBarUpdateButtons(window, kMaxButtonsCount, thumb_buttons);
-        window, buttons.size(), thumb_buttons) == S_OK;
+  else
-  } else {
+    r = taskbar->ThumbBarAddButtons(window, kMaxButtonsCount, thumb_buttons);
-    is_success = taskbar->ThumbBarUpdateButtons(
-        window, kMaxButtonsCount, thumb_buttons) == S_OK;
-  }
 
-  // Release thumb_buttons' icons, the taskbar makes its own copy.
+  thumbar_buttons_added_ = true;
-  for (size_t i = 0; i < buttons.size(); ++i) {
+  return SUCCEEDED(r);
-    ::DestroyIcon(thumb_buttons[i].hIcon);
-  }
-  return is_success;
 }
 
 bool TaskbarHost::HandleThumbarButtonEvent(int button_id) {
-  if (thumbar_button_clicked_callback_map_.find(button_id) !=
+  if (ContainsKey(callback_map_, button_id)) {
-      thumbar_button_clicked_callback_map_.end()) {
+    auto callback = callback_map_[button_id];
-    auto callback = thumbar_button_clicked_callback_map_[button_id];
     if (!callback.is_null())
       callback.Run();
     return true;

atom/browser/ui/win/taskbar_host.h

@@ -8,6 +8,7 @@
 #include <windows.h>
 
 #include <map>
+#include <string>
 #include <vector>
 
 #include "base/callback.h"
@@ -27,15 +28,19 @@ class TaskbarHost {
   TaskbarHost();
   virtual ~TaskbarHost();
 
+  // Add or update the buttons in thumbar
   bool SetThumbarButtons(
       HWND window, const std::vector<ThumbarButton>& buttons);
+
+  // Called by the window that there is a button in thumbar clicked.
   bool HandleThumbarButtonEvent(int button_id);
 
  private:
-  using ThumbarButtonClickedCallbackMap = std::map<int, base::Closure>;
+  using CallbackMap = std::map<int, base::Closure>;
-  ThumbarButtonClickedCallbackMap thumbar_button_clicked_callback_map_;
+  CallbackMap callback_map_;
 
-  bool is_initialized_;
+  // Whether we have already added the buttons to thumbar.
+  bool thumbar_buttons_added_;
 
   DISALLOW_COPY_AND_ASSIGN(TaskbarHost);
 };

docs/api/browser-window.md

@@ -646,23 +646,24 @@ __Note:__ This API is only available on Windows (Windows 7 and above)
 
 ### BrowserWindow.setThumbarButtons(buttons)
 
-* `buttons` Array of `button` objects
+* `buttons` Array
   * `button` Object
     * `icon` [NativeImage](native-image.md) - The icon showing in thumbnail
       toolbar.
-    * `tooltip` String - (Option) - The text of the button's tooltip.
+    * `tooltip` String (optional) - The text of the button's tooltip.
-    * `flags` Array of Strings - (Option) Control specific states and behaviors
+    * `flags` Array (optional) - Control specific states and behaviors
-      of the button. By default, it uses `enabled`.
+      of the button. By default, it uses `enabled`. It can include following
-      *  `enabled` - The button is active and available to the user.
+      Strings:
-      *  `disabled` - The button is disabled. It is present, but has a visual
+      * `enabled` - The button is active and available to the user.
-         state 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
+        state that indicates that it will not respond to user action.
-         flyout 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
+      * `hidden` - The button is not shown to the user.
-         pressed button state is drawn. This value is intended for instances
+      * `noninteractive` - The button is enabled but not interactive; no
-         where the button is used in a notification.
+        pressed button state is drawn. This value is intended for instances
+        where the button is used in a notification.
     * `click` - Function
 
 Add a thumbnail toolbar with a specified set of buttons to the thumbnail image