Add VectorIconButton functionality to ImageButton.

chrome/browser/ui/views/find_bar_view.cc

Index: chrome/browser/ui/views/find_bar_view.cc
diff --git a/chrome/browser/ui/views/find_bar_view.cc b/chrome/browser/ui/views/find_bar_view.cc
index 6aa3f49774467a369a49f16809fa17f5d9217fee..1c1e3c8f9d257f027c6888e0e06d74e0083b453d 100644
--- a/chrome/browser/ui/views/find_bar_view.cc
+++ b/chrome/browser/ui/views/find_bar_view.cc
@@ -36,7 +36,6 @@
 #include "ui/views/border.h"
 #include "ui/views/bubble/bubble_border.h"
 #include "ui/views/controls/button/image_button.h"
-#include "ui/views/controls/button/vector_icon_button.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/separator.h"
 #include "ui/views/layout/box_layout.h"
@@ -117,9 +116,10 @@ FindBarView::FindBarView(FindBarHost* host)
       match_count_text_(new MatchCountLabel()),
       focus_forwarder_view_(new FocusForwarderView(find_text_)),
       separator_(new views::Separator()),
-      find_previous_button_(new views::VectorIconButton(this)),
-      find_next_button_(new views::VectorIconButton(this)),
-      close_button_(new views::VectorIconButton(this)) {
+      button_observer_(this),
+      find_previous_button_(CreateButton(kCaretUpIcon)),
+      find_next_button_(CreateButton(kCaretDownIcon)),
+      close_button_(CreateButton(ui::kCloseIcon)) {
   find_text_->set_id(VIEW_ID_FIND_IN_PAGE_TEXT_FIELD);
   find_text_->set_default_width_in_chars(kDefaultCharWidth);
   find_text_->set_controller(this);
@@ -127,12 +127,7 @@ FindBarView::FindBarView(FindBarHost* host)
   find_text_->SetTextInputFlags(ui::TEXT_INPUT_FLAG_AUTOCORRECT_OFF);
   AddChildView(find_text_);
 
-  find_previous_button_->SetIcon(kCaretUpIcon);
-  find_next_button_->SetIcon(kCaretDownIcon);
-  close_button_->SetIcon(ui::kCloseIcon);
-
   find_previous_button_->set_id(VIEW_ID_FIND_IN_PAGE_PREVIOUS_BUTTON);
-  find_previous_button_->SetFocusForPlatform();
   find_previous_button_->SetTooltipText(
       l10n_util::GetStringUTF16(IDS_FIND_IN_PAGE_PREVIOUS_TOOLTIP));
   find_previous_button_->SetAccessibleName(
@@ -140,7 +135,6 @@ FindBarView::FindBarView(FindBarHost* host)
   AddChildView(find_previous_button_);
 
   find_next_button_->set_id(VIEW_ID_FIND_IN_PAGE_NEXT_BUTTON);
-  find_next_button_->SetFocusForPlatform();
   find_next_button_->SetTooltipText(
       l10n_util::GetStringUTF16(IDS_FIND_IN_PAGE_NEXT_TOOLTIP));
   find_next_button_->SetAccessibleName(
@@ -148,7 +142,6 @@ FindBarView::FindBarView(FindBarHost* host)
   AddChildView(find_next_button_);
 
   close_button_->set_id(VIEW_ID_FIND_IN_PAGE_CLOSE_BUTTON);
-  close_button_->SetFocusForPlatform();
   close_button_->SetTooltipText(
       l10n_util::GetStringUTF16(IDS_FIND_IN_PAGE_CLOSE_TOOLTIP));
   close_button_->SetAccessibleName(
@@ -166,6 +159,7 @@ FindBarView::FindBarView(FindBarHost* host)
 
   separator_->SetBorder(views::CreateEmptyBorder(0, kSeparatorLeftSpacing, 0,
                                                  kSeparatorRightSpacing));
+  separator_->SetPreferredHeight(find_text_->GetPreferredSize().height());
   AddChildViewAt(separator_, 2);
 
   find_text_->SetBorder(views::NullBorder());
@@ -173,6 +167,8 @@ FindBarView::FindBarView(FindBarHost* host)
   views::BoxLayout* manager =
       new views::BoxLayout(views::BoxLayout::kHorizontal, kInteriorPadding,
                            kInteriorPadding, kInterChildSpacing);
+  manager->set_cross_axis_alignment(
+      views::BoxLayout::CROSS_AXIS_ALIGNMENT_CENTER);
   SetLayoutManager(manager);
   manager->SetFlexForView(find_text_, 1);
 }
@@ -307,11 +303,6 @@ void FindBarView::ButtonPressed(
   }
 }
 
-SkColor FindBarView::GetVectorIconBaseColor() const {
-  return GetNativeTheme()->GetSystemColor(
-      ui::NativeTheme::kColorId_TextfieldDefaultColor);
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // FindBarView, views::TextfieldController implementation:
 
@@ -362,6 +353,31 @@ views::View* FindBarView::TargetForRect(View* root, const gfx::Rect& rect) {
   return find_text_;
 }
 
+FindBarView::FindBarViewButtonObserver::FindBarViewButtonObserver(
+    FindBarView* parent)
+    : parent_(parent) {}
+
+const gfx::VectorIcon& FindBarView::FindBarViewButtonObserver::GetVectorIcon(
+    View* view) const {
+  if (view == parent_->find_previous_button_)

[Evan Stade, 2017/03/15 22:50:43]

to elaborate, this is also objectionable in my eyes. The output of GetVectorIcon
depends on the inner workings of FindBarView, and it should be part of
FindBarView. I'm pretty sure accessing a private member variable is a violation
of style guidelines, and the resolution of providing accessors is also not
great. What this indicates to me is that the FindBarView is the one that ought
to be the observer.

+    return kCaretUpIcon;
+  if (view == parent_->find_next_button_)
+    return kCaretDownIcon;
+  return ui::kCloseIcon;
+}
+
+SkColor FindBarView::FindBarViewButtonObserver::GetVectorIconColor() const {
+  return parent_->GetNativeTheme()->GetSystemColor(
+      ui::NativeTheme::kColorId_TextfieldDefaultColor);
+}
+
+views::ImageButton* FindBarView::CreateButton(const gfx::VectorIcon& icon) {
+  views::ImageButton* button =
+      views::CreateDefaultVectorIconButton(icon, &button_observer_, this);
+  button->SetFocusForPlatform();
+  return button;
+}
+
 void FindBarView::Find(const base::string16& search_text) {
   FindBarController* controller = find_bar_host_->GetFindBarController();
   DCHECK(controller);