[omnibox] Show Remove Suggestion button only on-hover.

This CL makes the Remove Suggestion button only appear on hover.

It also prohibits showing the Remove Suggestion button when the
Tab Switch button is visible. I did this because it insert the X
on the far-right on-hover, since it shifts the Tab Switch button.

Additionally, it seems like if the user has the tab open, it's
not important to offer to delete that suggestion. The user is
still clearly interested in that content.

Bug: 1014788, 929477, 1205
Change-Id: I49684914e94ce5d6e884597c3d3b0bb612c5ebfa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1869555Reviewed-by: Kevin Bailey <krb@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707925}

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

@@ -53,7 +53,6 @@ OmniboxResultView::OmniboxResultView(
       popup_contents_view_(popup_contents_view),
       model_index_(model_index),
       theme_provider_(theme_provider),
-      is_hovered_(false),
       animation_(new gfx::SlideAnimation(this)) {
   CHECK_GE(model_index, 0);
 
@@ -107,11 +106,6 @@ void OmniboxResultView::SetMatch(const AutocompleteMatch& match) {
   suggestion_view_->OnMatchUpdate(this, match_);
   keyword_view_->OnMatchUpdate(this, match_);
   suggestion_tab_switch_button_->SetVisible(match.ShouldShowTabMatchButton());
-  // To avoid clutter, don't show the Remove button for matches with keyword.
-  remove_suggestion_button_->SetVisible(
-      match_.SupportsDeletion() && !match.associated_keyword &&
-      base::FeatureList::IsEnabled(
-          omnibox::kOmniboxSuggestionTransparencyOptions));
 
   suggestion_view_->content()->SetText(match_.contents, match_.contents_class);
   if (match_.answer) {
@@ -277,9 +271,14 @@ void OmniboxResultView::Layout() {
     keyword_view_->SetBounds(suggestion_width, 0, width() - suggestion_width,
                              height());
   }
-
   // Add buttons from right to left, shrinking the suggestion width as we go.
+  // To avoid clutter, don't show either button for matches with keyword.
   // TODO(tommycli): We should probably use a layout manager here.
+  remove_suggestion_button_->SetVisible(
+      match_.SupportsDeletion() && !match_.associated_keyword &&
+      IsMouseHovered() && !match_.ShouldShowTabMatchButton() &&
+      base::FeatureList::IsEnabled(
+          omnibox::kOmniboxSuggestionTransparencyOptions));
   if (remove_suggestion_button_->GetVisible()) {
     const gfx::Size button_size = remove_suggestion_button_->GetPreferredSize();
     suggestion_width -=
@@ -292,6 +291,7 @@ void OmniboxResultView::Layout() {
                                          button_size.width(),
                                          button_size.height());
   }
+
   if (match_.ShouldShowTabMatchButton()) {
     suggestion_tab_switch_button_->ProvideWidthHint(suggestion_width);
     const gfx::Size ts_button_size =
@@ -454,6 +454,7 @@ void OmniboxResultView::SetHovered(bool hovered) {
   if (is_hovered_ != hovered) {
     is_hovered_ = hovered;
     Invalidate();
+    InvalidateLayout();
   }
 }
 

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

@@ -129,8 +129,14 @@ class OmniboxResultView : public views::View,
   // The theme provider associated with this view.
   const ui::ThemeProvider* theme_provider_;
 
-  // Whether this view is in the hovered state.
-  bool is_hovered_;
+  // Whether this view is in the hovered state. Note: This is false when a
+  // child button is hovered, and therefore this is different from
+  // View::IsMouseHovered(). This is useful for a contrasting highlight between
+  // the tab-switch button and the row, but is a non-standard hover meaning.
+  //
+  // TODO(tommycli): Investigate if we can get the desired tab-switch button
+  // behavior while using just plain View::IsMouseHovered().
+  bool is_hovered_ = false;
 
   // The data this class is built to display (the "Omnibox Result").
   AutocompleteMatch match_;