[omnibox][ui simplification] one-line suggestion display

Fixes issue with uniform row height feature, implements one-line
display for non-chip suggestions when the appropriate flag is enabled.

Adds feature flags for uniform omnibox suggest heights.
Available options are 36 and 40 px.

Bug: 1368852
Change-Id: Ib13eb232cdd14eb622188aee2eae102b625b8ece

After (36 px rows):
After (40 px rows):

Before: https://screenshot.googleplex.com/BEBdU2YzvS9pZEU
https: //screenshot.googleplex.com/7AtAottoV2ujTP4.png
https: //screenshot.googleplex.com/Brqq3FaRuemsMut.png
Change-Id: Ib13eb232cdd14eb622188aee2eae102b625b8ece
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3993832
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Yohanes Shimelis <yohanes@google.com>
Cr-Commit-Position: refs/heads/main@{#1071421}

chrome/browser/about_flags.cc

@@ -1604,14 +1604,14 @@ const FeatureEntry::FeatureVariation
          std::size(kOmniboxDynamicMaxAutocomplete102), nullptr}};
 
 const FeatureEntry::FeatureParam kOmniboxUniformRowHeight36[] = {
-    {"OmniboxUniformRowHeight", "36"}};
+    {"OmniboxSuggestionVerticalMargin", "8"}};
 const FeatureEntry::FeatureParam kOmniboxUniformRowHeight40[] = {
-    {"OmniboxUniformRowHeight", "40"}};
+    {"OmniboxSuggestionVerticalMargin", "10"}};
 
 const FeatureEntry::FeatureVariation kOmniboxSuggestionHeightVariations[] = {
-    {"36 px height for all omnibox suggestions", kOmniboxUniformRowHeight36,
+    {"36px height for all omnibox suggestions", kOmniboxUniformRowHeight36,
      std::size(kOmniboxUniformRowHeight36), nullptr},
-    {"40 px height for all omnibox suggestions", kOmniboxUniformRowHeight40,
+    {"40px height for all omnibox suggestions", kOmniboxUniformRowHeight40,
      std::size(kOmniboxUniformRowHeight40), nullptr},
 };
 

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

@@ -16,7 +16,10 @@
 #include "chrome/grit/generated_resources.h"
 #include "chrome/grit/theme_resources.h"
 #include "components/omnibox/browser/autocomplete_match.h"
+#include "components/omnibox/browser/autocomplete_match_type.h"
+#include "components/omnibox/browser/omnibox_field_trial.h"
 #include "components/omnibox/browser/vector_icons.h"
+#include "components/omnibox/common/omnibox_features.h"
 #include "content/public/common/color_parser.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -25,6 +28,8 @@
 #include "ui/base/pointer/touch_ui_controller.h"
 #include "ui/color/color_provider.h"
 #include "ui/gfx/canvas.h"
+#include "ui/gfx/geometry/insets.h"
+#include "ui/gfx/geometry/size.h"
 #include "ui/gfx/geometry/skia_conversions.h"
 #include "ui/gfx/image/canvas_image_source.h"
 #include "ui/gfx/image/image_skia_operations.h"
@@ -33,6 +38,7 @@
 #include "ui/views/border.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/layout/layout_provider.h"
+#include "ui/views/layout/layout_types.h"
 
 namespace {
 
@@ -41,6 +47,7 @@ static constexpr int kAnswerImageSize = 24;
 
 // The edge length of the entity suggestions images.
 static constexpr int kEntityImageSize = 32;
+// TODO: set image size to 28 px if UniformRowHeight feature is enabled.
 
 ////////////////////////////////////////////////////////////////////////////////
 // PlaceholderImageSource:
@@ -191,7 +198,7 @@ int OmniboxMatchCellView::GetTextIndent() {
 }
 
 // static
-bool OmniboxMatchCellView::IsTwoLineLayout(const AutocompleteMatch& match) {
+bool OmniboxMatchCellView::ShouldDisplayImage(const AutocompleteMatch& match) {
   return match.answer || match.type == AutocompleteMatchType::CALCULATOR ||
          !match.image_url.is_empty();
 }
@@ -199,17 +206,21 @@ bool OmniboxMatchCellView::IsTwoLineLayout(const AutocompleteMatch& match) {
 void OmniboxMatchCellView::OnMatchUpdate(const OmniboxResultView* result_view,
                                          const AutocompleteMatch& match) {
   is_search_type_ = AutocompleteMatch::IsSearchType(match.type);
+  has_image_ = ShouldDisplayImage(match);
   // Decide layout style once before Layout, while match data is available.
-  const bool two_line = IsTwoLineLayout(match);
-  layout_style_ = two_line ? LayoutStyle::TWO_LINE_SUGGESTION
-                           : LayoutStyle::ONE_LINE_SUGGESTION;
+  layout_style_ = has_image_ && !OmniboxFieldTrial::IsUniformRowHeightEnabled()
+                      ? LayoutStyle::TWO_LINE_SUGGESTION
+                      : LayoutStyle::ONE_LINE_SUGGESTION;
 
   // Set up the separator.
-  separator_view_->SetSize(two_line ? gfx::Size()
-                                    : separator_view_->GetPreferredSize());
+  separator_view_->SetSize(layout_style_ == LayoutStyle::TWO_LINE_SUGGESTION ||
+                                   match.description.empty()
+                               ? gfx::Size()
+                               : separator_view_->GetPreferredSize());
 
   // Set up the small icon.
-  icon_view_->SetSize(two_line ? gfx::Size() : icon_view_->GetPreferredSize());
+  icon_view_->SetSize(has_image_ ? gfx::Size()
+                                 : icon_view_->GetPreferredSize());
 
   const auto apply_vector_icon = [=](const gfx::VectorIcon& vector_icon) {
     const auto* color_provider = GetColorProvider();
@@ -225,7 +236,7 @@ void OmniboxMatchCellView::OnMatchUpdate(const OmniboxResultView* result_view,
   };
   if (match.type == AutocompleteMatchType::CALCULATOR) {
     apply_vector_icon(omnibox::kAnswerCalculatorIcon);
-  } else if (!two_line) {
+  } else if (!has_image_) {
     answer_image_view_->SetImage(gfx::ImageSkia());
     answer_image_view_->SetSize(gfx::Size());
   } else {
@@ -275,9 +286,13 @@ void OmniboxMatchCellView::SetImage(const gfx::ImageSkia& image) {
 
 gfx::Insets OmniboxMatchCellView::GetInsets() const {
   const bool single_line = layout_style_ == LayoutStyle::ONE_LINE_SUGGESTION;
-  const int vertical_margin = ChromeLayoutProvider::Get()->GetDistanceMetric(
-      single_line ? DISTANCE_OMNIBOX_CELL_VERTICAL_PADDING
-                  : DISTANCE_OMNIBOX_TWO_LINE_CELL_VERTICAL_PADDING);
+
+  const int vertical_margin =
+      OmniboxFieldTrial::IsUniformRowHeightEnabled()
+          ? OmniboxFieldTrial::kSuggestionVerticalMargin.Get()
+          : ChromeLayoutProvider::Get()->GetDistanceMetric(
+                single_line ? DISTANCE_OMNIBOX_CELL_VERTICAL_PADDING
+                            : DISTANCE_OMNIBOX_TWO_LINE_CELL_VERTICAL_PADDING);
   return gfx::Insets::TLBR(vertical_margin, OmniboxMatchCellView::kMarginLeft,
                            vertical_margin, OmniboxMatchCellView::kMarginRight);
 }
@@ -289,9 +304,11 @@ void OmniboxMatchCellView::Layout() {
   const gfx::Rect child_area = GetContentsBounds();
   int x = child_area.x();
   int y = child_area.y();
+
   const int row_height = child_area.height();
+
   views::ImageView* const image_view =
-      two_line ? answer_image_view_.get() : icon_view_.get();
+      has_image_ ? answer_image_view_.get() : icon_view_.get();
   image_view->SetBounds(x, y, OmniboxMatchCellView::kImageBoundsWidth,
                         row_height);
 

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

@@ -60,8 +60,11 @@ class OmniboxMatchCellView : public views::View {
 
   static int GetTextIndent();
 
-  // Determine whether `match` should be displayed on 2 lines.
-  static bool IsTwoLineLayout(const AutocompleteMatch& match);
+  // Determines if `match` should display an answer, calculator, or entity
+  // image.
+  // If #omnibox-uniform-suggestion-height experiment flag is disabled, also
+  // determines whether `match` should be displayed on 1 or 2 lines.
+  static bool ShouldDisplayImage(const AutocompleteMatch& match);
 
   void OnMatchUpdate(const OmniboxResultView* result_view,
                      const AutocompleteMatch& match);
@@ -85,6 +88,7 @@ class OmniboxMatchCellView : public views::View {
   void SetTailSuggestCommonPrefixWidth(const std::u16string& common_prefix);
 
   bool is_search_type_ = false;
+  bool has_image_ = false;
   LayoutStyle layout_style_ = LayoutStyle::ONE_LINE_SUGGESTION;
 
   // Weak pointers for easy reference.

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

@@ -264,7 +264,7 @@ void OmniboxResultView::SetMatch(const AutocompleteMatch& match) {
     // calculator answers are 2-line but not deemphasized.
     const bool deemphasize =
         match_.type == AutocompleteMatchType::SEARCH_SUGGEST_ENTITY &&
-        OmniboxMatchCellView::IsTwoLineLayout(match_);
+        OmniboxMatchCellView::ShouldDisplayImage(match_);
     suggestion_view_->description()->SetTextWithStyling(
         match_.description, match_.description_class, deemphasize);
   }

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

@@ -17,6 +17,7 @@
 #include "chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.h"
 #include "components/omnibox/browser/actions/omnibox_pedal.h"
 #include "components/omnibox/browser/omnibox_edit_model.h"
+#include "components/omnibox/browser/omnibox_field_trial.h"
 #include "components/omnibox/browser/vector_icons.h"
 #include "components/strings/grit/components_strings.h"
 #include "components/vector_icons/vector_icons.h"
@@ -154,14 +155,15 @@ OmniboxSuggestionButtonRowView::OmniboxSuggestionButtonRowView(
     : popup_contents_view_(popup_contents_view),
       model_(model),
       model_index_(model_index) {
+  int bottom_margin = OmniboxFieldTrial::IsUniformRowHeightEnabled()
+                          ? OmniboxFieldTrial::kSuggestionVerticalMargin.Get()
+                          : ChromeLayoutProvider::Get()->GetDistanceMetric(
+                                DISTANCE_OMNIBOX_CELL_VERTICAL_PADDING);
   SetLayoutManager(std::make_unique<views::FlexLayout>())
       ->SetCrossAxisAlignment(views::LayoutAlignment::kStart)
       .SetCollapseMargins(true)
-      .SetInteriorMargin(
-          gfx::Insets::TLBR(0, OmniboxMatchCellView::GetTextIndent(),
-                            ChromeLayoutProvider::Get()->GetDistanceMetric(
-                                DISTANCE_OMNIBOX_CELL_VERTICAL_PADDING),
-                            0))
+      .SetInteriorMargin(gfx::Insets::TLBR(
+          0, OmniboxMatchCellView::GetTextIndent(), bottom_margin, 0))
       .SetDefault(
           views::kMarginsKey,
           gfx::Insets::VH(0, ChromeLayoutProvider::Get()->GetDistanceMetric(

components/omnibox/browser/omnibox_field_trial.cc

@@ -604,14 +604,14 @@ bool OmniboxFieldTrial::IsSiteSearchStarterPackEnabled() {
 
 // Omnibox UI simplification - Uniform Suggestion Row Heights
 
-bool IsUniformRowHeightEnabled() {
+bool OmniboxFieldTrial::IsUniformRowHeightEnabled() {
   return base::FeatureList::IsEnabled(omnibox::kUniformRowHeight);
 }
 
-const base::FeatureParam<int> OmniboxFieldTrial::kSuggestionRowHeight(
+const base::FeatureParam<int> OmniboxFieldTrial::kSuggestionVerticalMargin(
     &omnibox::kUniformRowHeight,
-    "OmniboxUniformRowHeight",
-    36);
+    "OmniboxSuggestionVerticalMargin",
+    8);
 
 const char OmniboxFieldTrial::kBundledExperimentFieldTrialName[] =
     "OmniboxBundledExperimentV1";

components/omnibox/browser/omnibox_field_trial.h

@@ -345,7 +345,7 @@ bool IsSiteSearchStarterPackEnabled();
 // Returns true if the feature to enable uniform row height is enabled.
 bool IsUniformRowHeightEnabled();
 // Specifies the row height in pixels for omnibox suggestions.
-extern const base::FeatureParam<int> kSuggestionRowHeight;
+extern const base::FeatureParam<int> kSuggestionVerticalMargin;
 
 // ---------------------------------------------------------
 // Clipboard URL suggestions: