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

chrome/browser/autocomplete/autocomplete_result_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/autocomplete/autocomplete_result.h"
 
 #include "base/memory/scoped_ptr.h"
 #include "base/metrics/field_trial.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
@@ skipping 468 matching lines @@
                             AutocompleteInput::HOME_PAGE, false, false, false,
                             AutocompleteInput::ALL_MATCHES);
     result.SortAndCull(input, test_util_.profile());
     ASSERT_EQ(4U, result.size());
     EXPECT_EQ("http://c/", result.match_at(0)->destination_url.spec());
     EXPECT_EQ("http://a/", result.match_at(1)->destination_url.spec());
     EXPECT_EQ("http://b/", result.match_at(2)->destination_url.spec());
     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).