[omnibox] Add OmniboxUIExperimentVerticalMarginLimitToNonTouchOnly

This CL adds a base::Feature boolean:
OmniboxUIExperimentVerticalMarginLimitToNonTouchOnly

This feature flag, when turned on, limits the vertical margin
experiment to non-touch devices only.

Bug: 955585
Change-Id: If2fe91885c7bf1f44d310da626cb80d75c9759e3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1585088
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Orin Jaworski <orinj@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#655251}

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

@@ -9,6 +9,7 @@
 #include "base/feature_list.h"
 #include "base/macros.h"
 #include "base/metrics/field_trial_params.h"
+#include "base/optional.h"
 #include "chrome/browser/ui/layout_constants.h"
 #include "chrome/browser/ui/omnibox/omnibox_theme.h"
 #include "chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.h"
@@ -69,14 +70,16 @@ gfx::Insets GetMarginInsets(int text_height, bool is_two_line) {
   int vertical_margin =
       is_two_line ? kTwoLineRowMarginHeight : kOneLineRowMarginHeight;
 
-  if (base::FeatureList::IsEnabled(omnibox::kUIExperimentVerticalMargin)) {
+  base::Optional<int> vertical_margin_override =
+      OmniboxFieldTrial::GetSuggestionVerticalMarginFieldTrialOverride();
+  if (vertical_margin_override) {
     // If the vertical margin experiment is on, we purposely set both the
     // one-line and two-line suggestions to have the same vertical margin.
     //
     // There is no vertical margin value we could set to make the new answer
     // style look anything similar to the pre-Refresh style, but setting them to
     // be the same looks reasonable, and is a sane place to start experimenting.
-    vertical_margin = OmniboxFieldTrial::GetSuggestionVerticalMargin();
+    vertical_margin = vertical_margin_override.value();
   }
 
   return gfx::Insets(vertical_margin, kMarginLeft, vertical_margin,

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

@@ -8,6 +8,7 @@
 #include <numeric>
 
 #include "base/bind.h"
+#include "base/optional.h"
 #include "build/build_config.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 #include "chrome/browser/ui/views/omnibox/omnibox_result_view.h"
@@ -398,11 +399,13 @@ gfx::Rect OmniboxPopupContentsView::GetTargetBounds() {
   // interior between each row of text.
   popup_height += RoundedOmniboxResultsFrame::GetNonResultSectionHeight();
 
-  if (base::FeatureList::IsEnabled(omnibox::kUIExperimentVerticalMargin)) {
+  base::Optional<int> vertical_margin_override =
+      OmniboxFieldTrial::GetSuggestionVerticalMarginFieldTrialOverride();
+  if (vertical_margin_override) {
     // If the vertical margin experiment uses a very small value (like a value
     // similar to pre-Refresh), we need to pad up the popup height at the
     // bottom (just like pre-Refresh) to prevent it from looking very bad.
-    if (OmniboxFieldTrial::GetSuggestionVerticalMargin() < 4)
+    if (vertical_margin_override.value() < 4)
       popup_height += 4;
   }
 

components/omnibox/browser/omnibox_field_trial.cc

@@ -579,7 +579,17 @@ bool OmniboxFieldTrial::IsHideSteadyStateUrlTrivialSubdomainsEnabled() {
       omnibox::kHideSteadyStateUrlTrivialSubdomains);
 }
 
-int OmniboxFieldTrial::GetSuggestionVerticalMargin() {
+base::Optional<int>
+OmniboxFieldTrial::GetSuggestionVerticalMarginFieldTrialOverride() {
+  if (!base::FeatureList::IsEnabled(omnibox::kUIExperimentVerticalMargin))
+    return base::nullopt;
+
+  if (base::FeatureList::IsEnabled(
+          omnibox::kUIExperimentVerticalMarginLimitToNonTouchOnly) &&
+      ui::MaterialDesignController::touch_ui()) {
+    return base::nullopt;
+  }
+
   // When the vertical margin is set to 2dp, the suggestion height is the
   // closest to the pre-Refresh height. In fact it's 1dp taller than the
   // pre-Refresh height on Linux.

components/omnibox/browser/omnibox_field_trial.h

@@ -14,6 +14,7 @@
 #include <vector>
 
 #include "base/macros.h"
+#include "base/optional.h"
 #include "components/omnibox/browser/autocomplete_input.h"
 #include "components/omnibox/browser/autocomplete_match_type.h"
 #include "third_party/metrics_proto/omnibox_event.pb.h"
@@ -401,9 +402,10 @@ bool IsHideSteadyStateUrlSchemeEnabled();
 // subdomains is enabled.
 bool IsHideSteadyStateUrlTrivialSubdomainsEnabled();
 
-// Returns the size of the vertical margin that should be used in the
-// suggestion view.
-int GetSuggestionVerticalMargin();
+// Returns the field trial override for the vertical margin size that should be
+// used in the suggestion view. Returns base::nullopt if the UI code should use
+// the default vertical margin.
+base::Optional<int> GetSuggestionVerticalMarginFieldTrialOverride();
 
 // Simply a convenient wrapper for testing a flag. Used downstream for an
 // assortment of keyword mode experiments.

components/omnibox/common/omnibox_features.cc

@@ -203,6 +203,12 @@ const base::Feature kUIExperimentSwapTitleAndUrl{
 const base::Feature kUIExperimentVerticalMargin{
     "OmniboxUIExperimentVerticalMargin", base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Feature used to limit the vertical margin UI experiment to non-touch
+// devices only. Has no effect if kUIExperimentVerticalMargin is not enabled.
+const base::Feature kUIExperimentVerticalMarginLimitToNonTouchOnly{
+    "OmniboxUIExperimentVerticalMarginLimitToNonTouchOnly",
+    base::FEATURE_DISABLED_BY_DEFAULT};
+
 // Feature used to color "blue" the generic search icon and search terms.
 // Technically, this makes the search icon and search terms match the color of
 // Omnibox link text, which is blue by convention.

components/omnibox/common/omnibox_features.h

@@ -38,6 +38,7 @@ extern const base::Feature kUIExperimentMaxAutocompleteMatches;
 extern const base::Feature kUIExperimentShowSuggestionFavicons;
 extern const base::Feature kUIExperimentSwapTitleAndUrl;
 extern const base::Feature kUIExperimentVerticalMargin;
+extern const base::Feature kUIExperimentVerticalMarginLimitToNonTouchOnly;
 extern const base::Feature kUIExperimentBlueSearchLoopAndSearchQuery;
 extern const base::Feature kUIExperimentBlueTitlesAndGrayUrlsOnPageSuggestions;
 extern const base::Feature kUIExperimentBlueTitlesOnPageSuggestions;