Add VectorIconButton functionality to ImageButton.

chrome/browser/ui/views/download/download_item_view.cc

Index: chrome/browser/ui/views/download/download_item_view.cc
diff --git a/chrome/browser/ui/views/download/download_item_view.cc b/chrome/browser/ui/views/download/download_item_view.cc
index 77965e3f57245afb1e89dc260bdfb22ebc848814..fd3011d79447630d7411c51db12f3a18b222a69d 100644
--- a/chrome/browser/ui/views/download/download_item_view.cc
+++ b/chrome/browser/ui/views/download/download_item_view.cc
@@ -62,8 +62,9 @@
 #include "ui/views/animation/ink_drop_highlight.h"
 #include "ui/views/animation/ink_drop_impl.h"
 #include "ui/views/border.h"
+#include "ui/views/controls/button/image_button.h"
+#include "ui/views/controls/button/image_button_util.h"
 #include "ui/views/controls/button/md_text_button.h"
-#include "ui/views/controls/button/vector_icon_button.h"
 #include "ui/views/controls/focusable_border.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/mouse_constants.h"
@@ -154,23 +155,6 @@ class SeparatorBorder : public views::FocusableBorder {
 
 }  // namespace
 
-// Allows the DownloadItemView to control the InkDrop on the drop down button.
-class DownloadItemView::DropDownButton : public views::VectorIconButton {
- public:
-  explicit DropDownButton(views::VectorIconButtonDelegate* delegate)
-      : views::VectorIconButton(delegate) {}
-  ~DropDownButton() override {}
-
-  // Promoted visibility to public.
-  void AnimateInkDrop(views::InkDropState state) {
-    // TODO(bruthig): Plumb in the proper Event.
-    views::VectorIconButton::AnimateInkDrop(state, nullptr /* event */);
-  }
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(DropDownButton);
-};
-
 DownloadItemView::DownloadItemView(DownloadItem* download_item,
                                    DownloadShelfView* parent)
     : shelf_(parent),
@@ -182,7 +166,7 @@ DownloadItemView::DownloadItemView(DownloadItem* download_item,
       model_(download_item),
       save_button_(nullptr),
       discard_button_(nullptr),
-      dropdown_button_(new DropDownButton(this)),
+      dropdown_button_(views::CreateImageButtonWithVectorIconStyling(this)),
       dangerous_download_label_(nullptr),
       dangerous_download_label_sized_(false),
       disabled_while_opening_(false),
@@ -388,6 +372,13 @@ void DownloadItemView::Layout() {
   }
 }
 
+void DownloadItemView::UpdateDropdownButton() {
+  views::SetImageFromVectorIcon(
+      dropdown_button_,
+      dropdown_state_ == PUSHED ? kCaretDownIcon : kCaretUpIcon,
+      GetTextColor());
+}
+
 gfx::Size DownloadItemView::GetPreferredSize() const {
   int width = 0;
   // We set the height to the height of two rows or text plus margins.
@@ -504,6 +495,11 @@ void DownloadItemView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
 void DownloadItemView::OnThemeChanged() {
   UpdateColorsFromTheme();
   SchedulePaint();
+  UpdateDropdownButton();
+}
+
+void DownloadItemView::OnNativeThemeChanged(const ui::NativeTheme* theme) {
+  UpdateDropdownButton();

[Evan Stade, 2017/03/17 13:54:44]

this may not be necessary because the color is derived from the browser theme
(ThemeProvider) so the updates in OnThemeChanged ought to be sufficient.

[Bret, 2017/03/17 23:41:41]

The button is initially red without this because the theme provider isn't
plumbed in yet. When it is we get OnNativeThemeChanged. So this is necessary.

[Evan Stade, 2017/03/18 00:27:57]

This may be right but only incidentally so. You'd get OnNativeThemeChanged when
added to a widget hierarchy, at which point you'd also have a ThemeProvider. But
that doesn't mean OnNativeThemeChanged is the right thing to override. You'd
either want to use OnViewHierarchyChanged or make it so OnThemeChanged is called
when the theme is first ready, like OnNativeThemeChanged[1]

[1]
https://cs.chromium.org/chromium/src/ui/views/view.cc?rcl=a5082d6ce45219eba13...

[Bret, 2017/03/18 20:12:53]

Okay. Changed to a ViewHierachyChanged override.

I think in principle we should call OnThemeChanged when the theme provider
changes in that case, but it seems like ToolbarView at least doesn't expect to
have that called right away, so I'm going to let that refactor happen in another
patch.

 }
 
 void DownloadItemView::AddInkDropLayer(ui::Layer* ink_drop_layer) {
@@ -601,10 +597,6 @@ void DownloadItemView::ButtonPressed(views::Button* sender,
   // WARNING: 'this' maybe deleted at this point. Don't access 'this'.
 }
 
-SkColor DownloadItemView::GetVectorIconBaseColor() const {
-  return GetTextColor();
-}
-
 void DownloadItemView::AnimationProgressed(const gfx::Animation* animation) {
   // We don't care if what animation (body button/drop button/complete),
   // is calling back, as they all have to go through the same paint call.
@@ -882,15 +874,14 @@ void DownloadItemView::SetDropdownState(State new_state) {
       !dropdown_button_->GetImage(views::CustomButton::STATE_NORMAL).isNull())
     return;
 
-  dropdown_button_->SetIcon(new_state == PUSHED ? kCaretDownIcon
-                                                : kCaretUpIcon);
   if (new_state != dropdown_state_) {
     dropdown_button_->AnimateInkDrop(new_state == PUSHED
                                          ? views::InkDropState::ACTIVATED
-                                         : views::InkDropState::DEACTIVATED);
+                                         : views::InkDropState::DEACTIVATED,
+                                     nullptr);
   }
-  dropdown_button_->OnThemeChanged();
   dropdown_state_ = new_state;
+  UpdateDropdownButton();
   SchedulePaint();
 }