summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--chrome/browser/metrics/omnibox_metrics_provider.cc7
-rw-r--r--chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm18
-rw-r--r--chrome/browser/ui/omnibox/omnibox_controller.cc17
-rw-r--r--chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc9
-rw-r--r--components/metrics/proto/omnibox_event.proto7
-rw-r--r--components/omnibox/autocomplete_result.cc5
-rw-r--r--components/omnibox/autocomplete_result.h25
-rw-r--r--components/omnibox/autocomplete_result_unittest.cc81
-rw-r--r--components/search/search.cc8
-rw-r--r--components/search/search.h5
-rw-r--r--components/search/search_unittest.cc32
11 files changed, 18 insertions, 196 deletions
diff --git a/chrome/browser/metrics/omnibox_metrics_provider.cc b/chrome/browser/metrics/omnibox_metrics_provider.cc
index 6fc3589..ca2f466 100644
--- a/chrome/browser/metrics/omnibox_metrics_provider.cc
+++ b/chrome/browser/metrics/omnibox_metrics_provider.cc
@@ -144,13 +144,8 @@ void OmniboxMetricsProvider::RecordOmniboxOpenedURL(const OmniboxLog& log) {
// (as explained in omnibox_event.proto) even if it was not, because such
// actions ignore the contents of the popup so it doesn't matter that it was
// open.
- const bool consider_popup_open = log.is_popup_open && !log.is_paste_and_go;
- omnibox_event->set_is_popup_open(consider_popup_open);
+ omnibox_event->set_is_popup_open(log.is_popup_open && !log.is_paste_and_go);
omnibox_event->set_is_paste_and_go(log.is_paste_and_go);
- if (consider_popup_open) {
- omnibox_event->set_is_top_result_hidden_in_dropdown(
- log.result.ShouldHideTopMatch());
- }
for (AutocompleteResult::const_iterator i(log.result.begin());
i != log.result.end(); ++i) {
diff --git a/chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm b/chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm
index df0fde6..f7c94fa 100644
--- a/chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm
+++ b/chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm
@@ -75,8 +75,7 @@ bool OmniboxPopupViewMac::IsOpen() const {
void OmniboxPopupViewMac::UpdatePopupAppearance() {
DCHECK([NSThread isMainThread]);
const AutocompleteResult& result = GetResult();
- const size_t start_match = result.ShouldHideTopMatch() ? 1 : 0;
- const size_t rows = result.size() - start_match;
+ const size_t rows = result.size();
if (rows == 0) {
[[popup_ parentWindow] removeChildWindow:popup_];
[popup_ orderOut:nil];
@@ -107,7 +106,7 @@ void OmniboxPopupViewMac::UpdatePopupAppearance() {
CGFloat contents_offset = -1.0f;
for (size_t ii = 0; ii < rows; ++ii) {
OmniboxPopupCell* cell = [matrix_ cellAtRow:ii column:0];
- const AutocompleteMatch& match = GetResult().match_at(ii + start_match);
+ const AutocompleteMatch& match = GetResult().match_at(ii);
[cell setImage:ImageForMatch(match)];
[cell setMatch:match];
// Only set the image on the one cell with match.answer.
@@ -161,19 +160,12 @@ gfx::Rect OmniboxPopupViewMac::GetTargetBounds() {
// This is only called by model in SetSelectedLine() after updating
// everything. Popup should already be visible.
void OmniboxPopupViewMac::PaintUpdatesNow() {
- size_t start_match = model_->result().ShouldHideTopMatch() ? 1 : 0;
- if (start_match > model_->selected_line()) {
- [matrix_ deselectAllCells];
- } else {
- [matrix_ selectCellAtRow:model_->selected_line() - start_match column:0];
- }
-
+ [matrix_ selectCellAtRow:model_->selected_line() column:0];
}
void OmniboxPopupViewMac::OnMatrixRowSelected(OmniboxPopupMatrix* matrix,
size_t row) {
- size_t start_match = model_->result().ShouldHideTopMatch() ? 1 : 0;
- model_->SetSelectedLine(row + start_match, false, false);
+ model_->SetSelectedLine(row, false, false);
}
void OmniboxPopupViewMac::OnMatrixRowClicked(OmniboxPopupMatrix* matrix,
@@ -336,8 +328,6 @@ NSImage* OmniboxPopupViewMac::ImageForMatch(const AutocompleteMatch& match) {
void OmniboxPopupViewMac::OpenURLForRow(size_t row,
WindowOpenDisposition disposition) {
- size_t start_match = model_->result().ShouldHideTopMatch() ? 1 : 0;
- row += start_match;
DCHECK_LT(row, GetResult().size());
omnibox_view_->OpenMatch(GetResult().match_at(row), disposition, GURL(),
base::string16(), row);
diff --git a/chrome/browser/ui/omnibox/omnibox_controller.cc b/chrome/browser/ui/omnibox/omnibox_controller.cc
index db2aaa1..9876ae36 100644
--- a/chrome/browser/ui/omnibox/omnibox_controller.cc
+++ b/chrome/browser/ui/omnibox/omnibox_controller.cc
@@ -39,12 +39,10 @@ namespace {
//
// If the kAllowPrefetchNonDefaultMatch field trial is enabled we return the
// prefetch suggestion even if it is not the default match. Otherwise we only
-// care about matches that are the default or the very first entry in the
-// dropdown (which can happen for non-default matches only if we're hiding a top
-// verbatim match) or the second entry in the dropdown (which can happen for
-// non-default matches when a top verbatim match is shown); for other matches,
-// we think the likelihood of the user selecting them is low enough that
-// prefetching isn't worth doing.
+// care about matches that are the default or the second entry in the dropdown
+// (which can happen for non-default matches when a top verbatim match is
+// shown); for other matches, we think the likelihood of the user selecting
+// them is low enough that prefetching isn't worth doing.
const AutocompleteMatch* GetMatchToPrefetch(const AutocompleteResult& result) {
if (chrome::ShouldAllowPrefetchNonDefaultMatch()) {
const AutocompleteResult::const_iterator prefetch_match = std::find_if(
@@ -53,17 +51,14 @@ const AutocompleteMatch* GetMatchToPrefetch(const AutocompleteResult& result) {
}
// If the default match should be prefetched, do that.
- const AutocompleteResult::const_iterator default_match(
- result.default_match());
+ const auto default_match = result.default_match();
if ((default_match != result.end()) &&
SearchProvider::ShouldPrefetch(*default_match))
return &(*default_match);
// Otherwise, if the top match is a verbatim match and the very next match
// is prefetchable, fetch that.
- if ((result.ShouldHideTopMatch() ||
- result.TopMatchIsStandaloneVerbatimMatch()) &&
- (result.size() > 1) &&
+ if (result.TopMatchIsStandaloneVerbatimMatch() && (result.size() > 1) &&
SearchProvider::ShouldPrefetch(result.match_at(1)))
return &result.match_at(1);
diff --git a/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc b/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc
index 4428faa..fc8cba7 100644
--- a/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc
+++ b/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc
@@ -149,9 +149,7 @@ void OmniboxPopupContentsView::InvalidateLine(size_t line) {
}
void OmniboxPopupContentsView::UpdatePopupAppearance() {
- const size_t hidden_matches = model_->result().ShouldHideTopMatch() ? 1 : 0;
- if (model_->result().size() <= hidden_matches ||
- omnibox_view_->IsImeShowingPopup()) {
+ if (model_->result().empty() || omnibox_view_->IsImeShowingPopup()) {
// No matches or the IME is showing a popup window which may overlap
// the omnibox popup window. Close any existing popup.
if (popup_ != NULL) {
@@ -175,7 +173,7 @@ void OmniboxPopupContentsView::UpdatePopupAppearance() {
OmniboxResultView* view = result_view_at(i);
const AutocompleteMatch& match = GetMatchAtIndex(i);
view->SetMatch(match);
- view->SetVisible(i >= hidden_matches);
+ view->SetVisible(true);
if (match.answer && !model_->answer_bitmap().isNull()) {
view->SetAnswerImage(
gfx::ImageSkia::CreateFrom1xBitmap(model_->answer_bitmap()));
@@ -379,8 +377,7 @@ void OmniboxPopupContentsView::OnGestureEvent(ui::GestureEvent* event) {
int OmniboxPopupContentsView::CalculatePopupHeight() {
DCHECK_GE(static_cast<size_t>(child_count()), model_->result().size());
int popup_height = 0;
- for (size_t i = model_->result().ShouldHideTopMatch() ? 1 : 0;
- i < model_->result().size(); ++i)
+ for (size_t i = 0; i < model_->result().size(); ++i)
popup_height += child_at(i)->GetPreferredSize().height();
// Add enough space on the top and bottom so it looks like there is the same
diff --git a/components/metrics/proto/omnibox_event.proto b/components/metrics/proto/omnibox_event.proto
index b3e6656..bf13d4c 100644
--- a/components/metrics/proto/omnibox_event.proto
+++ b/components/metrics/proto/omnibox_event.proto
@@ -39,9 +39,10 @@ message OmniboxEventProto {
// This corresponds the index of the |suggestion| below.
optional int32 selected_index = 5;
- // Whether or not the top match was hidden in the omnibox suggestions
- // dropdown.
- optional bool is_top_result_hidden_in_dropdown = 14;
+ // DEPRECATED. Whether or not the top match was hidden in the omnibox
+ // suggestions dropdown.
+ optional bool DEPRECATED_is_top_result_hidden_in_dropdown = 14
+ [deprecated = true];
// Whether the omnibox popup is open. It can be closed if, for instance,
// the user clicks in the omnibox and hits return to reload the same page.
diff --git a/components/omnibox/autocomplete_result.cc b/components/omnibox/autocomplete_result.cc
index f2d171f..7ae331e 100644
--- a/components/omnibox/autocomplete_result.cc
+++ b/components/omnibox/autocomplete_result.cc
@@ -294,11 +294,6 @@ AutocompleteMatch* AutocompleteResult::match_at(size_t index) {
return &matches_[index];
}
-bool AutocompleteResult::ShouldHideTopMatch() const {
- return chrome::ShouldHideTopVerbatimMatch() &&
- TopMatchIsStandaloneVerbatimMatch();
-}
-
bool AutocompleteResult::TopMatchIsStandaloneVerbatimMatch() const {
if (empty() || !match_at(0).IsVerbatimType())
return false;
diff --git a/components/omnibox/autocomplete_result.h b/components/omnibox/autocomplete_result.h
index 6d8d746..1ffc40c 100644
--- a/components/omnibox/autocomplete_result.h
+++ b/components/omnibox/autocomplete_result.h
@@ -102,31 +102,6 @@ class AutocompleteResult {
// Returns true if the top match is a verbatim search or URL match (see
// IsVerbatimType() in autocomplete_match.h), and the next match is not also
- // some kind of verbatim match. In this case, the top match will be hidden,
- // and nothing in the dropdown will appear selected by default; hitting enter
- // will navigate to the (hidden) default match, while pressing the down arrow
- // key will select the first visible match, which is actually the second match
- // in the result set.
- //
- // Hiding the top match in these cases is possible because users should
- // already know what will happen on hitting enter from the omnibox text
- // itself, without needing to see the same text appear again, selected, just
- // below their typing. Instead, by hiding the verbatim match, there is one
- // less line to skip over in order to visually scan downwards to see other
- // suggested matches. This makes it more likely that users will see and
- // select useful non-verbatim matches. (Note that hiding the verbatim match
- // this way is similar to how most other browsers' address bars behave.)
- //
- // We avoid hiding when the top two matches are both verbatim in order to
- // avoid potential confusion if a user were to see the second match just below
- // their typing and assume it would be the default action.
- //
- // Note that if the top match should be hidden and it is the only match,
- // the dropdown should be closed.
- bool ShouldHideTopMatch() const;
-
- // Returns true if the top match is a verbatim search or URL match (see
- // IsVerbatimType() in autocomplete_match.h), and the next match is not also
// some kind of verbatim match.
bool TopMatchIsStandaloneVerbatimMatch() const;
diff --git a/components/omnibox/autocomplete_result_unittest.cc b/components/omnibox/autocomplete_result_unittest.cc
index 4cc1385..d72aa4c 100644
--- a/components/omnibox/autocomplete_result_unittest.cc
+++ b/components/omnibox/autocomplete_result_unittest.cc
@@ -647,80 +647,6 @@ TEST_F(AutocompleteResultTest, SortAndCullReorderForDefaultMatch) {
}
}
-TEST_F(AutocompleteResultTest, ShouldHideTopMatch) {
- base::FieldTrialList::CreateFieldTrial("InstantExtended",
- "Group1 hide_verbatim:1");
- ACMatches matches;
-
- // Case 1: Top match is a verbatim match.
- PopulateAutocompleteMatchesFromTestData(kVerbatimMatches, 1, &matches);
- AutocompleteResult result;
- result.AppendMatches(AutocompleteInput(), matches);
- EXPECT_TRUE(result.ShouldHideTopMatch());
- matches.clear();
- result.Reset();
-
- // Case 2: If the verbatim first match is followed by another verbatim match,
- // don't hide the top verbatim match.
- PopulateAutocompleteMatchesFromTestData(kVerbatimMatches,
- arraysize(kVerbatimMatches),
- &matches);
- result.AppendMatches(AutocompleteInput(), matches);
- EXPECT_FALSE(result.ShouldHideTopMatch());
- matches.clear();
- result.Reset();
-
- // Case 3: Top match is not a verbatim match. Do not hide the top match.
- PopulateAutocompleteMatchesFromTestData(kNonVerbatimMatches, 1, &matches);
- PopulateAutocompleteMatchesFromTestData(kVerbatimMatches,
- arraysize(kVerbatimMatches),
- &matches);
- result.AppendMatches(AutocompleteInput(), matches);
- EXPECT_FALSE(result.ShouldHideTopMatch());
-}
-
-TEST_F(AutocompleteResultTest, ShouldHideTopMatchAfterCopy) {
- base::FieldTrialList::CreateFieldTrial("InstantExtended",
- "Group1 hide_verbatim:1");
- ACMatches matches;
-
- // Case 1: Top match is a verbatim match followed by only copied matches.
- PopulateAutocompleteMatchesFromTestData(kVerbatimMatches,
- arraysize(kVerbatimMatches),
- &matches);
- for (size_t i = 1; i < arraysize(kVerbatimMatches); ++i)
- matches[i].from_previous = true;
- AutocompleteResult result;
- result.AppendMatches(AutocompleteInput(), matches);
- EXPECT_TRUE(result.ShouldHideTopMatch());
- result.Reset();
-
- // Case 2: The copied matches are then followed by a non-verbatim match.
- PopulateAutocompleteMatchesFromTestData(kNonVerbatimMatches, 1, &matches);
- result.AppendMatches(AutocompleteInput(), matches);
- EXPECT_TRUE(result.ShouldHideTopMatch());
- result.Reset();
-
- // Case 3: The copied matches are instead followed by a verbatim match.
- matches.back().from_previous = true;
- PopulateAutocompleteMatchesFromTestData(kVerbatimMatches, 1, &matches);
- result.AppendMatches(AutocompleteInput(), matches);
- EXPECT_FALSE(result.ShouldHideTopMatch());
-}
-
-TEST_F(AutocompleteResultTest, DoNotHideTopMatch_FieldTrialFlagDisabled) {
- // This test config is identical to ShouldHideTopMatch test ("Case 1") except
- // that the "hide_verbatim" flag is disabled in the field trials.
- base::FieldTrialList::CreateFieldTrial("InstantExtended",
- "Group1 hide_verbatim:0");
- ACMatches matches;
- PopulateAutocompleteMatchesFromTestData(kVerbatimMatches, 1, &matches);
- AutocompleteResult result;
- result.AppendMatches(AutocompleteInput(), matches);
- // Field trial flag "hide_verbatim" is disabled. Do not hide top match.
- EXPECT_FALSE(result.ShouldHideTopMatch());
-}
-
TEST_F(AutocompleteResultTest, TopMatchIsStandaloneVerbatimMatch) {
ACMatches matches;
AutocompleteResult result;
@@ -750,11 +676,4 @@ TEST_F(AutocompleteResultTest, TopMatchIsStandaloneVerbatimMatch) {
EXPECT_TRUE(result.TopMatchIsStandaloneVerbatimMatch());
result.Reset();
matches.clear();
-
- // Case 5: Multiple verbatim matches found in AutocompleteResult.
- PopulateAutocompleteMatchesFromTestData(kVerbatimMatches,
- arraysize(kVerbatimMatches),
- &matches);
- result.AppendMatches(AutocompleteInput(), matches);
- EXPECT_FALSE(result.ShouldHideTopMatch());
}
diff --git a/components/search/search.cc b/components/search/search.cc
index c2d4b4e..9afd666 100644
--- a/components/search/search.cc
+++ b/components/search/search.cc
@@ -35,8 +35,6 @@ const uint64 kEmbeddedSearchEnabledVersion = 2;
const uint64 kEmbeddedPageVersionDefault = 2;
#endif
-const char kHideVerbatimFlagName[] = "hide_verbatim";
-
// Constants for the field trial name and group prefix.
// Note in M30 and below this field trial was named "InstantExtended" and in
// M31 was renamed to EmbeddedSearch for clarity and cleanliness. Since we
@@ -145,10 +143,4 @@ bool GetBoolValueForFlagWithDefault(const std::string& flag,
return !!GetUInt64ValueForFlagWithDefault(flag, default_value ? 1 : 0, flags);
}
-bool ShouldHideTopVerbatimMatch() {
- FieldTrialFlags flags;
- return GetFieldTrialInfo(&flags) && GetBoolValueForFlagWithDefault(
- kHideVerbatimFlagName, false, flags);
-}
-
} // namespace chrome
diff --git a/components/search/search.h b/components/search/search.h
index 9558ee8..0a7a55b 100644
--- a/components/search/search.h
+++ b/components/search/search.h
@@ -55,11 +55,6 @@ bool GetBoolValueForFlagWithDefault(const std::string& flag,
bool default_value,
const FieldTrialFlags& flags);
-// Returns true if 'hide_verbatim' flag is enabled in field trials
-// to hide the top match in the native suggestions dropdown if it is a verbatim
-// match. See comments on ShouldHideTopMatch in autocomplete_result.h.
-bool ShouldHideTopVerbatimMatch();
-
} // namespace chrome
#endif // COMPONENTS_SEARCH_SEARCH_H_
diff --git a/components/search/search_unittest.cc b/components/search/search_unittest.cc
index d3bd873..2fd5a23 100644
--- a/components/search/search_unittest.cc
+++ b/components/search/search_unittest.cc
@@ -130,36 +130,4 @@ TEST_F(EmbeddedSearchFieldTrialTest, GetFieldTrialInfoControlFlags) {
EXPECT_EQ(3ul, flags.size());
}
-typedef EmbeddedSearchFieldTrialTest ShouldHideTopVerbatimTest;
-
-TEST_F(ShouldHideTopVerbatimTest, DoNotHideByDefault) {
- ASSERT_TRUE(base::FieldTrialList::CreateFieldTrial("EmbeddedSearch",
- "Control"));
- EXPECT_FALSE(ShouldHideTopVerbatimMatch());
-}
-
-TEST_F(ShouldHideTopVerbatimTest, DoNotHideInInstantExtended) {
- ASSERT_TRUE(base::FieldTrialList::CreateFieldTrial("EmbeddedSearch",
- "Group1"));
- EXPECT_FALSE(ShouldHideTopVerbatimMatch());
-}
-
-TEST_F(ShouldHideTopVerbatimTest, EnableByFlagInInstantExtended) {
- ASSERT_TRUE(base::FieldTrialList::CreateFieldTrial("EmbeddedSearch",
- "Group1 hide_verbatim:1"));
- EXPECT_TRUE(ShouldHideTopVerbatimMatch());
-}
-
-TEST_F(ShouldHideTopVerbatimTest, EnableByFlagOutsideInstantExtended) {
- ASSERT_TRUE(base::FieldTrialList::CreateFieldTrial(
- "EmbeddedSearch", "Controll1 hide_verbatim:1"));
- EXPECT_TRUE(ShouldHideTopVerbatimMatch());
-}
-
-TEST_F(ShouldHideTopVerbatimTest, DisableByFlag) {
- ASSERT_TRUE(base::FieldTrialList::CreateFieldTrial("EmbeddedSearch",
- "Group1 hide_verbatim:0"));
- EXPECT_FALSE(ShouldHideTopVerbatimMatch());
-}
-
} // namespace chrome
generated by cgit v1.1 at 2024-09-14 16:47:53 +0000