Remove ShouldHideTopMatch

BUG=492452

Review URL: https://codereview.chromium.org/1154063003

Cr-Commit-Position: refs/heads/master@{#333772}

7 files changed, 4 insertions, 159 deletions

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