Add VectorIconButton functionality to ImageButton.

chrome/browser/ui/views/find_bar_view.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/find_bar_view.h"
 
 #include <algorithm>
 
 #include "base/i18n/number_formatting.h"
 #include "base/macros.h"
@@ skipping 18 matching lines @@
 #include "ui/base/theme_provider.h"
 #include "ui/events/event.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/native_theme/native_theme.h"
 #include "ui/vector_icons/vector_icons.h"
 #include "ui/views/background.h"
 #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"
 #include "ui/views/painter.h"
 #include "ui/views/view_targeter.h"
 #include "ui/views/widget/widget.h"
 
 namespace {
 
 // These layout constants are all in dp.
@@ skipping 60 matching lines @@
 
 ////////////////////////////////////////////////////////////////////////////////
 // FindBarView, public:
 
 FindBarView::FindBarView(FindBarHost* host)
     : find_bar_host_(host),
       find_text_(new views::Textfield),
       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);
   find_text_->SetAccessibleName(l10n_util::GetStringUTF16(IDS_ACCNAME_FIND));
   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(
       l10n_util::GetStringUTF16(IDS_ACCNAME_PREVIOUS));
   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(
       l10n_util::GetStringUTF16(IDS_ACCNAME_NEXT));
   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(
       l10n_util::GetStringUTF16(IDS_ACCNAME_CLOSE));
   close_button_->SetAnimationDuration(0);
   AddChildView(close_button_);
 
   AddChildView(focus_forwarder_view_);
 
   EnableCanvasFlippingForRTLUI(true);
 
   match_count_text_->SetEventTargeter(
       base::MakeUnique<views::ViewTargeter>(this));
   AddChildViewAt(match_count_text_, 1);
 
   separator_->SetBorder(views::CreateEmptyBorder(0, kSeparatorLeftSpacing, 0,
                                                  kSeparatorRightSpacing));
+  separator_->SetPreferredHeight(find_text_->GetPreferredSize().height());
   AddChildViewAt(separator_, 2);
 
   find_text_->SetBorder(views::NullBorder());
 
   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);
 }
 
 FindBarView::~FindBarView() {
 }
 
 void FindBarView::SetFindTextAndSelectedRange(
     const base::string16& find_text,
     const gfx::Range& selected_range) {
@@ skipping 114 matching lines @@
       find_bar_host_->GetFindBarController()->EndFindSession(
           FindBarController::kKeepSelectionOnPage,
           FindBarController::kKeepResultsInFindBox);
       break;
     default:
       NOTREACHED() << "Unknown button";
       break;
   }
 }
 
-SkColor FindBarView::GetVectorIconBaseColor() const {
-  return GetNativeTheme()->GetSystemColor(
-      ui::NativeTheme::kColorId_TextfieldDefaultColor);
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // FindBarView, views::TextfieldController implementation:
 
 bool FindBarView::HandleKeyEvent(views::Textfield* sender,
                                  const ui::KeyEvent& key_event) {
   // If the dialog is not visible, there is no reason to process keyboard input.
   if (!find_bar_host_->IsVisible())
     return false;
 
   if (find_bar_host_->MaybeForwardKeyEventToWebpage(key_event))
@@ skipping 30 matching lines @@
   // a paste operation, even if the pasted text is the same as before.
   // See http://crbug.com/79002
   last_searched_text_.clear();
 }
 
 views::View* FindBarView::TargetForRect(View* root, const gfx::Rect& rect) {
   DCHECK_EQ(match_count_text_, root);
   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);
   content::WebContents* web_contents = controller->web_contents();
   // We must guard against a NULL web_contents, which can happen if the text
   // in the Find box is changed right after the tab is destroyed. Otherwise, it
   // can lead to crashes, as exposed by automation testing in issue 8048.
   if (!web_contents)
     return;
   FindTabHelper* find_tab_helper = FindTabHelper::FromWebContents(web_contents);
@@ skipping 42 matching lines @@
   set_background(new views::BubbleBackground(border.get()));
   SetBorder(std::move(border));
 
   match_count_text_->SetBackgroundColor(bg_color);
   SkColor text_color =
       theme->GetSystemColor(ui::NativeTheme::kColorId_TextfieldDefaultColor);
   match_count_text_->SetEnabledColor(SkColorSetA(text_color, 0x69));
   separator_->SetColor(SkColorSetA(text_color, 0x26));
 }