[omnibox] Fix visual bug on OmniboxSuggestionButtonView

Fixes a layout bug with the suggestion button view, see the bug for
screenshots.

I'm not sure if this fixes all instances, but this fixes at least one.

Bug: 1091354
Change-Id: I3e98c9f7ba6aeec24561ad377d6bde484a2fd12a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2321145
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Orin Jaworski <orinj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792350}

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

@@ -86,9 +86,6 @@ OmniboxResultView::OmniboxResultView(
   if (OmniboxFieldTrial::IsSuggestionButtonRowEnabled()) {
     button_row_ = AddChildView(std::make_unique<OmniboxSuggestionButtonRowView>(
         popup_contents_view_, model_index));
-    // The button row shows itself during Layout when appropriate.
-    // TODO(orinj): Visibility should also be revisited when layout is reworked.
-    button_row_->SetVisible(false);
   }
 
   keyword_view_ = AddChildView(std::make_unique<OmniboxMatchCellView>(this));
@@ -145,7 +142,7 @@ void OmniboxResultView::SetMatch(const AutocompleteMatch& match) {
 
   // With button row, its keyword button is used instead of |keyword_view_|.
   if (OmniboxFieldTrial::IsSuggestionButtonRowEnabled()) {
-    button_row_->UpdateKeyword();
+    button_row_->UpdateFromModel();
   } else {
     AutocompleteMatch* keyword_match = match_.associated_keyword.get();
     keyword_view_->SetVisible(keyword_match != nullptr);

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

@@ -130,12 +130,36 @@ OmniboxSuggestionButtonRowView::OmniboxSuggestionButtonRowView(
 
 OmniboxSuggestionButtonRowView::~OmniboxSuggestionButtonRowView() = default;
 
-void OmniboxSuggestionButtonRowView::UpdateKeyword() {
-  const OmniboxEditModel* edit_model = model()->edit_model();
-  const base::string16& keyword = edit_model->keyword();
-  const auto names = SelectedKeywordView::GetKeywordLabelNames(
-      keyword, edit_model->client()->GetTemplateURLService());
-  keyword_button_->SetText(names.full_name);
+void OmniboxSuggestionButtonRowView::UpdateFromModel() {
+  SetPillButtonVisibility(keyword_button_,
+                          OmniboxPopupModel::FOCUSED_BUTTON_KEYWORD);
+  if (keyword_button_->GetVisible()) {
+    const OmniboxEditModel* edit_model = model()->edit_model();
+    base::string16 keyword;
+    bool is_keyword_hint = false;
+    match().GetKeywordUIState(edit_model->client()->GetTemplateURLService(),
+                              &keyword, &is_keyword_hint);
+
+    const auto names = SelectedKeywordView::GetKeywordLabelNames(
+        keyword, edit_model->client()->GetTemplateURLService());
+    keyword_button_->SetText(names.full_name);
+  }
+
+  SetPillButtonVisibility(pedal_button_,
+                          OmniboxPopupModel::FOCUSED_BUTTON_PEDAL);
+  if (pedal_button_->GetVisible()) {
+    pedal_button_->SetText(match().pedal->GetLabelStrings().hint);
+    pedal_button_->SetTooltipText(
+        match().pedal->GetLabelStrings().suggestion_contents);
+  }
+
+  SetPillButtonVisibility(tab_switch_button_,
+                          OmniboxPopupModel::FOCUSED_BUTTON_TAB_SWITCH);
+
+  bool is_any_button_visible = keyword_button_->GetVisible() ||
+                               pedal_button_->GetVisible() ||
+                               tab_switch_button_->GetVisible();
+  SetVisible(is_any_button_visible);
 }
 
 void OmniboxSuggestionButtonRowView::OnStyleRefresh() {
@@ -196,31 +220,6 @@ void OmniboxSuggestionButtonRowView::ButtonPressed(views::Button* button,
   }
 }
 
-void OmniboxSuggestionButtonRowView::Layout() {
-  views::View::Layout();
-
-  SetPillButtonVisibility(keyword_button_,
-                          OmniboxPopupModel::FOCUSED_BUTTON_KEYWORD);
-  SetPillButtonVisibility(pedal_button_,
-                          OmniboxPopupModel::FOCUSED_BUTTON_PEDAL);
-  SetPillButtonVisibility(tab_switch_button_,
-                          OmniboxPopupModel::FOCUSED_BUTTON_TAB_SWITCH);
-
-  if (pedal_button_->GetVisible()) {
-    pedal_button_->SetText(match().pedal->GetLabelStrings().hint);
-    pedal_button_->SetTooltipText(
-        match().pedal->GetLabelStrings().suggestion_contents);
-  }
-
-  bool is_any_button_visible = keyword_button_->GetVisible() ||
-                               pedal_button_->GetVisible() ||
-                               tab_switch_button_->GetVisible();
-  SetVisible(is_any_button_visible);
-
-  // TODO(orinj): Migrate to BoxLayout or FlexLayout. Ideally we don't do any
-  // more layouts ourselves. Also, check visibility management in result view.
-}
-
 const OmniboxPopupModel* OmniboxSuggestionButtonRowView::model() const {
   return popup_contents_view_->model();
 }

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

@@ -23,18 +23,16 @@ class OmniboxSuggestionButtonRowView : public views::View,
                                           int model_index);
   ~OmniboxSuggestionButtonRowView() override;
 
-  // Gets keyword information and applies it to the keyword button label.
-  // TODO(orinj): This should eventually be made private after refactoring.
-  void UpdateKeyword();
-
   // Called when themes, styles, and visibility is refreshed in result view.
   void OnStyleRefresh();
 
+  // Updates the suggestion row buttons based on the model.
+  void UpdateFromModel();
+
   // views::ButtonListener:
   void ButtonPressed(views::Button* sender, const ui::Event& event) override;
 
   // views::View:
-  void Layout() override;
   void OnThemeChanged() override;
 
  private: