Modified GetMatchToPrefetch() to return a valid match for prefetching even when hide_verbatim flag i

chrome/browser/autocomplete/autocomplete_result_unittest.cc

Index: chrome/browser/autocomplete/autocomplete_result_unittest.cc
diff --git a/chrome/browser/autocomplete/autocomplete_result_unittest.cc b/chrome/browser/autocomplete/autocomplete_result_unittest.cc
index e3fa9bc5fd52c4c848db3f52cc0fffef7c4f4fd8..01e8afb1278913952e64613b19aceb4d9ac48a49 100644
--- a/chrome/browser/autocomplete/autocomplete_result_unittest.cc
+++ b/chrome/browser/autocomplete/autocomplete_result_unittest.cc
@@ -486,3 +486,221 @@ TEST_F(AutocompleteResultTest, SortAndCullReorderForDefaultMatch) {
     EXPECT_EQ("http://d/", result.match_at(3)->destination_url.spec());
   }
 }
+
+TEST_F(AutocompleteResultTest, ShouldHideTopMatch) {
+  // Add some matches.
+  ACMatches matches;
+  {

[Mark P, 2013/12/12 23:13:07]

Not required but please seriously consider that all these {} block could be a
lot shorter if you create a helper function AddMatch(url, type, &matches)
(it could look at the final relevance score and assign the new match that minus
100; it could also set allowed_to_be_default_match, in addition to setting URL
and type)

[kmadhusu, 2013/12/13 01:00:29]

Done.

+    AutocompleteMatch match;
+    match.destination_url = GURL("http://search-what-you-typed/");
+    match.relevance = 1300;
+    match.allowed_to_be_default_match = true;
+    match.type = AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED;
+    matches.push_back(match);
+  }
+  {
+    AutocompleteMatch match;
+    match.destination_url = GURL("http://history-title/");
+    match.relevance = 1200;
+    match.allowed_to_be_default_match = true;
+    match.type = AutocompleteMatchType::HISTORY_TITLE;
+    matches.push_back(match);
+  }
+  {
+    AutocompleteMatch match;
+    match.destination_url = GURL("http://search-history/");
+    match.relevance = 500;
+    match.allowed_to_be_default_match = true;
+    match.type = AutocompleteMatchType::SEARCH_HISTORY;
+    matches.push_back(match);
+  }
+
+  base::FieldTrialList::CreateFieldTrial("InstantExtended",
+                                         "Group1 hide_verbatim:1");
+  AutocompleteResult result;
+  result.AppendMatches(matches);
+  EXPECT_TRUE(result.ShouldHideTopMatch());
+}
+
+TEST_F(AutocompleteResultTest, DoNotHideTopMatch) {
+  ACMatches matches;
+  {
+    AutocompleteMatch match;
+    match.destination_url = GURL("http://search-what-you-typed/");
+    match.relevance = 1300;
+    match.allowed_to_be_default_match = true;
+    match.type = AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED;
+    matches.push_back(match);
+  }
+  {
+    AutocompleteMatch match;
+    match.destination_url = GURL("http://search-what-you-typed_url/");

[Mark P, 2013/12/12 23:13:07]

I'm sure you meant url-what-you-typed.  Please fix.
here and below

[Mark P, 2013/12/12 23:13:07]

nit: no _ in hostnames
here and below

[kmadhusu, 2013/12/13 01:00:29]

Done.

[kmadhusu, 2013/12/13 01:00:29]

I was just adding a dummy url. Updated the destination URL to match the
AutocompleteMatch type.

+    match.relevance = 1240;
+    match.allowed_to_be_default_match = true;
+    match.type = AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED;

[Mark P, 2013/12/12 23:13:07]

Did you mean this to be URL_WHAT_YOU_TYPED?

here and below

[kmadhusu, 2013/12/13 01:00:29]

I didn't mean that. Now that you mentioned, I have updated it accordingly.

+    matches.push_back(match);
+  }
+  {
+    AutocompleteMatch match;
+    match.destination_url = GURL("http://history-title/");
+    match.relevance = 1200;
+    match.allowed_to_be_default_match = true;
+    match.type = AutocompleteMatchType::HISTORY_TITLE;
+    matches.push_back(match);
+  }
+  {
+    AutocompleteMatch match;
+    match.destination_url = GURL("http://search-history/");
+    match.relevance = 500;
+    match.allowed_to_be_default_match = true;
+    match.type = AutocompleteMatchType::SEARCH_HISTORY;
+    matches.push_back(match);
+  }
+
+  base::FieldTrialList::CreateFieldTrial("InstantExtended",
+                                         "Group1 hide_verbatim:1");
+  AutocompleteResult result;
+  result.AppendMatches(matches);
+  // If the verbatim first match is followed by another verbatim match, don't
+  // hide the top verbatim match.
+  EXPECT_FALSE(result.ShouldHideTopMatch());
+}
+
+TEST_F(AutocompleteResultTest, DoNotHideTopMatch_TopMatchIsNotVerbatim) {
+  ACMatches matches;
+  {
+    AutocompleteMatch match;
+    match.destination_url = GURL("http://search-history/");
+    match.relevance = 500;

[Mark P, 2013/12/12 23:13:07]

500 here is extremely misleading.  Did you mean 1300 judging by the rest of the
tests?

[kmadhusu, 2013/12/13 01:00:29]

Yes. My bad (copy paste error). Fixed.

+    match.allowed_to_be_default_match = true;
+    match.type = AutocompleteMatchType::SEARCH_HISTORY;
+    matches.push_back(match);
+  }
+  {
+    AutocompleteMatch match;
+    match.destination_url = GURL("http://search-what-you-typed_url/");
+    match.relevance = 1240;
+    match.allowed_to_be_default_match = true;
+    match.type = AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED;
+    matches.push_back(match);
+  }
+  {
+    AutocompleteMatch match;
+    match.destination_url = GURL("http://history-title/");
+    match.relevance = 1200;
+    match.allowed_to_be_default_match = true;
+    match.type = AutocompleteMatchType::HISTORY_TITLE;
+    matches.push_back(match);
+  }
+
+  base::FieldTrialList::CreateFieldTrial("InstantExtended",
+                                         "Group1 hide_verbatim:1");
+  AutocompleteResult result;
+  result.AppendMatches(matches);
+  // Top match is not a verbatim type match. Do not hide the top match.
+  EXPECT_FALSE(result.ShouldHideTopMatch());
+}
+
+TEST_F(AutocompleteResultTest, DoNotHideTopMatch_FieldTrialFlagDisabled) {

[Mark P, 2013/12/12 23:13:07]

Please use an identical config to ShouldHideTopMatch; don't introduce a new
config and EXPECT_FALSE it.  It's better to directly test the same setup with
the flag on and the flag off.

[kmadhusu, 2013/12/13 01:00:29]

Done.

+  ACMatches matches;
+  {
+    AutocompleteMatch match;
+    match.destination_url = GURL("http://search-what-you-typed_url/");
+    match.relevance = 1240;
+    match.allowed_to_be_default_match = true;
+    match.type = AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED;
+    matches.push_back(match);
+  }
+  {
+    AutocompleteMatch match;
+    match.destination_url = GURL("http://history-title/");
+    match.relevance = 1200;
+    match.allowed_to_be_default_match = true;
+    match.type = AutocompleteMatchType::HISTORY_TITLE;
+    matches.push_back(match);
+  }
+
+  base::FieldTrialList::CreateFieldTrial("InstantExtended",
+                                         "Group1 hide_verbatim:0");
+  AutocompleteResult result;
+  result.AppendMatches(matches);
+  // Field trial flag "hide_verbatim" is disabled. Do not hide top match.
+  EXPECT_FALSE(result.ShouldHideTopMatch());
+}
+
+TEST_F(AutocompleteResultTest,
+       TopMatchIsVerbatimAndHasNoConsecutiveVerbatimMatches) {
+  ACMatches matches;
+  {
+    AutocompleteMatch match;
+    match.destination_url = GURL("http://search-what-you-typed_url/");
+    match.relevance = 1240;
+    match.allowed_to_be_default_match = true;
+    match.type = AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED;
+    matches.push_back(match);
+  }
+  {
+    AutocompleteMatch match;
+    match.destination_url = GURL("http://history-title/");
+    match.relevance = 1200;
+    match.allowed_to_be_default_match = true;
+    match.type = AutocompleteMatchType::HISTORY_TITLE;
+    matches.push_back(match);
+  }
+  AutocompleteResult result;
+  result.AppendMatches(matches);
+  EXPECT_TRUE(result.TopMatchIsVerbatimAndHasNoConsecutiveVerbatimMatches());
+}
+
+TEST_F(AutocompleteResultTest,
+       TopMatchIsVerbatimAndHasConsecutiveVerbatimMatches) {
+  ACMatches matches;
+  {
+    AutocompleteMatch match;
+    match.destination_url = GURL("http://search-what-you-typed_url/");
+    match.relevance = 1240;
+    match.allowed_to_be_default_match = true;
+    match.type = AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED;
+    matches.push_back(match);
+  }
+  {
+    AutocompleteMatch match;
+    match.destination_url = GURL("http://search-what-you-typed_url_1/");
+    match.relevance = 1220;
+    match.allowed_to_be_default_match = true;
+    match.type = AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED;
+    matches.push_back(match);
+  }
+  {
+    AutocompleteMatch match;
+    match.destination_url = GURL("http://history-title/");
+    match.relevance = 1200;
+    match.allowed_to_be_default_match = true;
+    match.type = AutocompleteMatchType::HISTORY_TITLE;
+    matches.push_back(match);
+  }
+  AutocompleteResult result;
+  result.AppendMatches(matches);
+  EXPECT_FALSE(result.TopMatchIsVerbatimAndHasNoConsecutiveVerbatimMatches());
+}
+
+TEST_F(AutocompleteResultTest, TopMatchIsNotVerbatim) {
+  ACMatches matches;
+  AutocompleteResult result;
+  result.AppendMatches(matches);
+
+  // Result set is empty.
+  EXPECT_FALSE(result.TopMatchIsVerbatimAndHasNoConsecutiveVerbatimMatches());
+
+  // Add a non-verbatim match to the result.
+  {
+    AutocompleteMatch match;
+    match.destination_url = GURL("http://history-title/");
+    match.relevance = 1200;
+    match.allowed_to_be_default_match = true;
+    match.type = AutocompleteMatchType::HISTORY_TITLE;
+    matches.push_back(match);
+  }
+  result.AppendMatches(matches);
+  EXPECT_FALSE(result.TopMatchIsVerbatimAndHasNoConsecutiveVerbatimMatches());
+}

[Mark P, 2013/12/12 23:13:07]

please add a test somewhere that
TopMatchIsVerbatimAndHasNoConsecutiveVerbatimMatches() is false for a single
match that is verbatim.

[kmadhusu, 2013/12/13 01:00:29]

I am sorry, can you explain why that function should return false for a single
match that is verbatim? (As per the function definition, it will return true).