Commit 1f0c266b authored by manukh's avatar manukh Committed by Chromium LUCI CQ

[omnibox] [rich-autocompletion] Remove TwoLine param.

We're unlikely to further pursue kRichAutocompletionTwoLineOmniboxParam.

Bug: 1062446
Change-Id: Ia10ccf122faf94c86c912e0c618d4039bf8da84e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2617514Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Commit-Queue: manuk hovanesian <manukh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844851}
parent 94699f0a
......@@ -958,13 +958,7 @@ const FeatureEntry::FeatureVariation kOmniboxDocumentProviderVariations[] = {
{"server and client scores", kOmniboxDocumentProviderServerAndClientScoring,
base::size(kOmniboxDocumentProviderServerAndClientScoring), nullptr}};
// The variations include 5 of the 8 possible permutations of "2-Line UI",
// "Title AC", and "Non-Prefix AC". The remaining 3 permutations would
// effectively be no-ops.
// - 2-Line UI: Stretches the omnibox vertically to fit 2 lines and displays
// titles on a 2nd line
// E.g. en.wikipe | [dia.org/wiki/Space_Shuttle]
// Space Shuttle - Wikipedia
// 3 permutations of the 2 rich autocompletion params:
// - Title AC: Autocompletes suggestions when the input matches the title.
// E.g. Space Sh | [ttle - Wikipedia] (en.wikipedia.org/wiki/Space_Shuttle)
// - Non-Prefix AC: Autocompletes suggestions when the input is not necessarily
......@@ -979,14 +973,6 @@ const FeatureEntry::FeatureVariation kOmniboxRichAutocompletionVariations[] = {
1,
nullptr,
},
{
"2-Line UI & Title AC",
(FeatureEntry::FeatureParam[]){
{"RichAutocompletionTwoLineOmnibox", "true"},
{"RichAutocompletionAutocompleteTitles", "true"}},
2,
nullptr,
},
{
"Non-Prefix AC",
(FeatureEntry::FeatureParam[]){
......@@ -994,7 +980,6 @@ const FeatureEntry::FeatureVariation kOmniboxRichAutocompletionVariations[] = {
1,
nullptr,
},
// Skipping "2-Line UI & Non-Prefix AC" as that would be a no-op
{
"Title AC & Non-Prefix AC",
(FeatureEntry::FeatureParam[]){
......@@ -1002,15 +987,6 @@ const FeatureEntry::FeatureVariation kOmniboxRichAutocompletionVariations[] = {
{"RichAutocompletionAutocompleteNonPrefixAll", "true"}},
2,
nullptr,
},
{
"2-Line UI, Title AC, & Non-Prefix AC",
(FeatureEntry::FeatureParam[]){
{"RichAutocompletionTwoLineOmnibox", "true"},
{"RichAutocompletionAutocompleteTitles", "true"},
{"RichAutocompletionAutocompleteNonPrefixAll", "true"}},
3,
nullptr,
}};
const FeatureEntry::FeatureVariation
......
......@@ -5,7 +5,6 @@
#include "chrome/browser/ui/layout_constants.h"
#include "base/notreached.h"
#include "components/omnibox/browser/omnibox_field_trial.h"
#include "chrome/browser/ui/ui_features.h"
#include "ui/base/pointer/touch_ui_controller.h"
......@@ -56,9 +55,6 @@ int GetLayoutConstant(LayoutConstant constant) {
case LOCATION_BAR_ELEMENT_PADDING:
return touch_ui ? 3 : 2;
case LOCATION_BAR_HEIGHT:
if (OmniboxFieldTrial::RichAutocompletionShowAdditionalText() &&
OmniboxFieldTrial::RichAutocompletionTwoLineOmnibox())
return touch_ui ? 52 : 40;
return touch_ui ? 36 : 28;
case LOCATION_BAR_ICON_SIZE:
return touch_ui ? 20 : 16;
......
......@@ -593,17 +593,12 @@ void LocationBarView::Layout() {
int location_needed_width = omnibox_view_->GetTextWidth();
if (OmniboxFieldTrial::RichAutocompletionShowAdditionalText()) {
// Calculate location_needed_width based on the omnibox view and omnibox
// additional text widths. If RichAutocompletionTwoLineOmnibox is enabled,
// location_needed_width only needs to be large enough to contain the
// larger; otherwise, it must be large enough to contain both in addition to
// the padding in between.
// additional text widths. |location_needed_width| must be large enough to
// contain both in addition to the padding in between.
int omnibox_additional_text_needed_width =
omnibox_additional_text_view_->CalculatePreferredSize().width();
location_needed_width =
OmniboxFieldTrial::RichAutocompletionTwoLineOmnibox()
? std::max(location_needed_width,
omnibox_additional_text_needed_width)
: location_needed_width + omnibox_additional_text_needed_width + 10;
location_needed_width + omnibox_additional_text_needed_width + 10;
// TODO (manukh): If we launch rich autocompletion with the current
// iteration of 1 line UI, the padding (10) should be moved to
// layout_constants.cc. Likewise below.
......@@ -642,23 +637,10 @@ void LocationBarView::Layout() {
std::min(width, entry_width), location_bounds.height());
}
// If rich autocompletion is enabled, split |location_bounds| for the
// |omnibox_view_| and |omnibox_additional_text_view_|.
// If rich autocompletion is enabled, split |location_bounds| horizontally for
// the |omnibox_view_| and |omnibox_additional_text_view_|.
if (OmniboxFieldTrial::RichAutocompletionShowAdditionalText() &&
OmniboxFieldTrial::RichAutocompletionTwoLineOmnibox()) {
// Split vertically.
auto omnibox_bounds = location_bounds;
omnibox_bounds.set_height(location_bounds.height() / 2);
omnibox_view_->SetBoundsRect(omnibox_bounds);
auto omnibox_additional_text_bounds = omnibox_bounds;
omnibox_additional_text_bounds.set_x(location_bounds.x() + 3);
omnibox_additional_text_bounds.set_y(omnibox_bounds.bottom());
omnibox_additional_text_view_->SetBoundsRect(
omnibox_additional_text_bounds);
} else if (OmniboxFieldTrial::RichAutocompletionShowAdditionalText() &&
!omnibox_view_->GetText().empty()) {
// Split horizontally.
auto omnibox_bounds = location_bounds;
omnibox_bounds.set_width(std::min(
omnibox_view_->GetUnelidedTextWidth() + 10, location_bounds.width()));
......
......@@ -603,7 +603,6 @@ TEST(AutocompleteMatchTest, TryRichAutocompletion) {
omnibox::kRichAutocompletion,
{
{"RichAutocompletionAutocompleteTitles", "true"},
{"RichAutocompletionTwoLineOmnibox", "true"},
{"RichAutocompletionAutocompleteNonPrefixAll", "true"},
{"RichAutocompletionAutocompleteTitlesMinChar", "3"},
{"RichAutocompletionAutocompleteNonPrefixMinChar", "2"},
......@@ -649,7 +648,6 @@ TEST(AutocompleteMatchTest, TryRichAutocompletion) {
omnibox::kRichAutocompletion,
{
{"RichAutocompletionAutocompleteTitles", "true"},
{"RichAutocompletionTwoLineOmnibox", "true"},
{"RichAutocompletionAutocompleteNonPrefixAll", "true"},
{"RichAutocompletionAutocompleteTitlesMinChar", "3"},
{"RichAutocompletionAutocompleteNonPrefixMinChar", "2"},
......
......@@ -679,12 +679,6 @@ size_t OmniboxFieldTrial::RichAutocompletionAutocompleteTitlesMinChar() {
kRichAutocompletionAutocompleteTitlesMinCharParam, 0);
}
bool OmniboxFieldTrial::RichAutocompletionTwoLineOmnibox() {
return base::GetFieldTrialParamByFeatureAsBool(
omnibox::kRichAutocompletion, kRichAutocompletionTwoLineOmniboxParam,
false);
}
bool OmniboxFieldTrial::RichAutocompletionAutocompleteNonPrefixAll() {
return base::GetFieldTrialParamByFeatureAsBool(
omnibox::kRichAutocompletion,
......@@ -939,8 +933,6 @@ const char OmniboxFieldTrial::kRichAutocompletionAutocompleteTitlesParam[] =
const char
OmniboxFieldTrial::kRichAutocompletionAutocompleteTitlesMinCharParam[] =
"RichAutocompletionAutocompleteTitlesMinChar";
const char OmniboxFieldTrial::kRichAutocompletionTwoLineOmniboxParam[] =
"RichAutocompletionTwoLineOmnibox";
const char
OmniboxFieldTrial::kRichAutocompletionAutocompleteNonPrefixAllParam[] =
"RichAutocompletionAutocompleteNonPrefixAll";
......
......@@ -401,7 +401,6 @@ bool IsRefinedFocusStateEnabled();
bool IsRichAutocompletionEnabled();
bool RichAutocompletionAutocompleteTitles();
size_t RichAutocompletionAutocompleteTitlesMinChar();
bool RichAutocompletionTwoLineOmnibox();
bool RichAutocompletionAutocompleteNonPrefixAll();
bool RichAutocompletionAutocompleteNonPrefixShortcutProvider();
size_t RichAutocompletionAutocompleteNonPrefixMinChar();
......@@ -538,7 +537,6 @@ extern int kDefaultMinimumTimeBetweenSuggestQueriesMs;
// Parameter names used for rich autocompletion variations.
extern const char kRichAutocompletionAutocompleteTitlesParam[];
extern const char kRichAutocompletionAutocompleteTitlesMinCharParam[];
extern const char kRichAutocompletionTwoLineOmniboxParam[];
extern const char kRichAutocompletionAutocompleteNonPrefixAllParam[];
extern const char
kRichAutocompletionAutocompleteNonPrefixShortcutProviderParam[];
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment