[omnibox] Add some mouse hover handling to omnibox Views headers

This CL adds some mouse hover handling to the omnibox Views headers,
analogous to what OmniboxResultView already has.

The logic is slightly duplicative but I would hesitate to add any
abstraction at this point, given the experimental nature of the
feature, as well as the fact that it's just two locations.

Bug: 1052522
Change-Id: Ib0211c1e168bf17e0ff6ad487cc43c986801e064
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2145077Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758207}

chrome/browser/ui/views/omnibox/omnibox_result_view.cc

@@ -146,6 +146,23 @@ OmniboxResultView::OmniboxResultView(
 
 OmniboxResultView::~OmniboxResultView() {}
 
+// static
+std::unique_ptr<views::Background> OmniboxResultView::GetPopupCellBackground(
+    views::View* view,
+    OmniboxPartState part_state) {
+  DCHECK(view);
+
+  bool high_contrast = view->GetNativeTheme() &&
+                       view->GetNativeTheme()->UsesHighContrastColors();
+  // TODO(tapted): Consider using background()->SetNativeControlColor() and
+  // always have a background.
+  if ((part_state == OmniboxPartState::NORMAL && !high_contrast))
+    return nullptr;
+
+  return views::CreateSolidBackground(GetOmniboxColor(
+      view->GetThemeProvider(), OmniboxPart::RESULTS_BACKGROUND, part_state));
+}
+
 SkColor OmniboxResultView::GetColor(OmniboxPart part) const {
   return GetOmniboxColor(GetThemeProvider(), part, GetThemeState());
 }
@@ -203,14 +220,7 @@ void OmniboxResultView::ShowKeyword(bool show_keyword) {
 }
 
 void OmniboxResultView::ApplyThemeAndRefreshIcons(bool force_reapply_styles) {
-  bool high_contrast =
-      GetNativeTheme() && GetNativeTheme()->UsesHighContrastColors();
-  // TODO(tapted): Consider using background()->SetNativeControlColor() and
-  // always have a background.
-  SetBackground((GetThemeState() == OmniboxPartState::NORMAL && !high_contrast)
-                    ? nullptr
-                    : views::CreateSolidBackground(
-                          GetColor(OmniboxPart::RESULTS_BACKGROUND)));
+  SetBackground(GetPopupCellBackground(this, GetThemeState()));
 
   // Reapply the dim color to account for the highlight state.
   suggestion_view_->separator()->ApplyTextColor(
@@ -231,6 +241,8 @@ void OmniboxResultView::ApplyThemeAndRefreshIcons(bool force_reapply_styles) {
       omnibox::kKeywordSearchIcon, GetLayoutConstant(LOCATION_BAR_ICON_SIZE),
       GetColor(OmniboxPart::RESULTS_ICON)));
 
+  bool high_contrast =
+      GetNativeTheme() && GetNativeTheme()->UsesHighContrastColors();
   if (match_.answer) {
     suggestion_view_->content()->ApplyTextColor(
         OmniboxPart::RESULTS_TEXT_DEFAULT);

chrome/browser/ui/views/omnibox/omnibox_result_view.h

@@ -19,6 +19,7 @@
 #include "ui/gfx/font_list.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/views/animation/animation_delegate_views.h"
+#include "ui/views/background.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/controls/button/image_button.h"
 #include "ui/views/controls/image_view.h"
@@ -48,6 +49,11 @@ class OmniboxResultView : public views::View,
                     size_t model_index);
   ~OmniboxResultView() override;
 
+  // Static method to share logic about how to set backgrounds of popup cells.
+  static std::unique_ptr<views::Background> GetPopupCellBackground(
+      views::View* view,
+      OmniboxPartState part_state);
+
   // Helper to get the color for |part| using the current state.
   SkColor GetColor(OmniboxPart part) const;
 

chrome/browser/ui/views/omnibox/omnibox_row_view.cc

@@ -29,6 +29,7 @@ class OmniboxRowView::HeaderView : public views::Label,
     // TODO(tommycli): Add a focus ring.
     hide_button_ = AddChildView(views::CreateVectorToggleImageButton(this));
     views::InstallCircleHighlightPathGenerator(hide_button_);
+    hide_button_->SetVisible(false);
   }
 
   void SetHeader(int suggestion_group_id, const base::string16& header_text) {
@@ -37,17 +38,24 @@ class OmniboxRowView::HeaderView : public views::Label,
   }
 
   // views::View:
+  gfx::Insets GetInsets() const override { return gfx::Insets(8, 16); }
+  void OnMouseEntered(const ui::MouseEvent& event) override {
+    UpdateUIForHoverState();
+  }
+  void OnMouseExited(const ui::MouseEvent& event) override {
+    UpdateUIForHoverState();
+  }
   void OnThemeChanged() override {
     views::View::OnThemeChanged();
 
-    int dip_size = GetLayoutConstant(LOCATION_BAR_ICON_SIZE);
-    OmniboxPartState part_state =
-        IsMouseHovered() ? OmniboxPartState::HOVERED : OmniboxPartState::NORMAL;
-
-    SkColor color = GetOmniboxColor(GetThemeProvider(),
-                                    OmniboxPart::RESULTS_ICON, part_state);
+    // Since the hide button is only visible when the part is hovered, base the
+    // icon color on the hover state.
+    SkColor color =
+        GetOmniboxColor(GetThemeProvider(), OmniboxPart::RESULTS_ICON,
+                        OmniboxPartState::HOVERED);
     hide_button_->set_ink_drop_base_color(color);
 
+    int dip_size = GetLayoutConstant(LOCATION_BAR_ICON_SIZE);
     const gfx::ImageSkia arrow_down =
         gfx::CreateVectorIcon(omnibox::kChevronIcon, dip_size, color);
     const gfx::ImageSkia arrow_up =
@@ -58,6 +66,9 @@ class OmniboxRowView::HeaderView : public views::Label,
     // The "toggled" button state corresponds with the group being hidden.
     hide_button_->SetImage(views::Button::STATE_NORMAL, arrow_up);
     hide_button_->SetToggledImage(views::Button::STATE_NORMAL, &arrow_down);
+
+    // When the theme is updated, also refresh the hover-specific UI.
+    UpdateUIForHoverState();
   }
 
   // views::ButtonListener:
@@ -68,6 +79,20 @@ class OmniboxRowView::HeaderView : public views::Label,
   }
 
  private:
+  // Some UI changes on-hover, and this function effects those changes.
+  void UpdateUIForHoverState() {
+    bool is_hovered = IsMouseHovered();
+    hide_button_->SetVisible(is_hovered);
+
+    OmniboxPartState part_state =
+        is_hovered ? OmniboxPartState::HOVERED : OmniboxPartState::NORMAL;
+
+    // It's a little hokey that we're stealing the logic for the background
+    // color from OmniboxResultView. If we start doing this is more than just
+    // one place, we should introduce a more elegant abstraction here.
+    SetBackground(OmniboxResultView::GetPopupCellBackground(this, part_state));
+  }
+
   // The Label containing the header text. This is never nullptr.
   views::Label* header_text_;