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"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/autocomplete/autocomplete_input.h"
 #include "chrome/browser/autocomplete/autocomplete_match.h"
 #include "chrome/browser/autocomplete/autocomplete_provider.h"
 #include "chrome/browser/omnibox/omnibox_field_trial.h"
 #include "chrome/browser/search_engines/template_url_prepopulate_data.h"
 #include "chrome/browser/search_engines/template_url_service.h"
 #include "chrome/browser/search_engines/template_url_service_test_util.h"
 #include "chrome/common/autocomplete_match_type.h"
 #include "chrome/common/metrics/variations/variations_util.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/variations/entropy_provider.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace {
+
+// Creates an AutocompleteMatch using |destination_url| and |type| and appends
+// it to |matches|.
+void AddMatch(const std::string& destination_url, AutocompleteMatch::Type type,
+              ACMatches* matches) {

[Peter Kasting, 2013/12/17 23:47:34]

Nit: One arg per line

[kmadhusu, 2013/12/18 23:26:08]

Done.

+  ASSERT_TRUE(matches != NULL);
+  AutocompleteMatch* last_match =
+    !matches->empty() ? &((*matches)[matches->size() - 1]) : NULL;

[Peter Kasting, 2013/12/17 23:47:34]

Nit: Needlessly verbose.  ELiminate this line and do the following below:

  match.relevance = matches->empty() ? 1300 : (matches->back().relevance - 100);

[kmadhusu, 2013/12/18 23:26:08]

Done.

+  AutocompleteMatch match;
+  match.destination_url = GURL(destination_url);
+  match.relevance = last_match ? last_match->relevance - 100 : 1300;
+  match.allowed_to_be_default_match = true;
+  match.type = type;
+  matches->push_back(match);
+}
+
+}  // namespace
+
 class AutocompleteResultTest : public testing::Test  {
  public:
   struct TestData {
     // Used to build a url for the AutocompleteMatch. The URL becomes
     // "http://" + ('a' + |url_id|) (e.g. an ID of 2 yields "http://b").
     int url_id;
 
     // ID of the provider.
     int provider_id;
 
@@ skipping 276 matching lines @@
             result.match_at(1)->destination_url.spec());
   EXPECT_EQ(1200, result.match_at(1)->relevance);
   EXPECT_EQ("http://www.foo.com/",
             result.match_at(2)->destination_url.spec());
   EXPECT_EQ(900, result.match_at(2)->relevance);
 }
 
 TEST_F(AutocompleteResultTest, SortAndCullWithDemotionsByType) {
   // Add some matches.
   ACMatches matches;
-  {
-    AutocompleteMatch match;
-    match.destination_url = GURL("http://history-url/");
-    match.relevance = 1400;
-    match.allowed_to_be_default_match = true;
-    match.type = AutocompleteMatchType::HISTORY_URL;
-    matches.push_back(match);
-  }
-  {
-    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);
-  }
+  AddMatch("http://history-url/", AutocompleteMatchType::HISTORY_URL, &matches);
+  AddMatch("http://search-what-you-typed/",
+           AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, &matches);
+  AddMatch("http://history-title/", AutocompleteMatchType::HISTORY_TITLE,
+           &matches);
+
+  // Add a search history type match and demote its relevance score.
+  AddMatch("http://search-history/", AutocompleteMatchType::SEARCH_HISTORY,
+           &matches);
+  matches[matches.size() - 1].relevance = 500;

[Peter Kasting, 2013/12/17 23:47:34]

Nit: Use matches.back()

[kmadhusu, 2013/12/18 23:26:08]

Done.

 
   // Add a rule demoting history-url and killing history-title.
   {
     std::map<std::string, std::string> params;
     params[std::string(OmniboxFieldTrial::kDemoteByTypeRule) + ":3:*"] =
         "1:50,7:100,2:0";  // 3 == HOME_PAGE
     ASSERT_TRUE(chrome_variations::AssociateVariationParams(
         OmniboxFieldTrial::kBundledExperimentFieldTrialName, "A", params));
   }
   base::FieldTrialList::CreateFieldTrial(
@@ skipping 116 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) {

[Peter Kasting, 2013/12/17 23:47:34]

There are a ton of tests here, most of which start with very similar AddMatch()
calls.  Can we collapse most of these into one or two tests, which have an array
of details used to construct matches, use a for() loop to add them as necessary,
check the relevant function return values, then clear state and start over?

[kmadhusu, 2013/12/18 23:26:08]

Done.

+  // Add some matches.
+  ACMatches matches;
+  AddMatch("http://search-what-you-typed/",
+           AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, &matches);
+  AddMatch("http://history-title/", AutocompleteMatchType::HISTORY_TITLE,
+           &matches);
+  AddMatch("http://search-history/", AutocompleteMatchType::SEARCH_HISTORY,
+           &matches);
+
+  base::FieldTrialList::CreateFieldTrial("InstantExtended",
+                                         "Group1 hide_verbatim:1");
+  AutocompleteResult result;
+  result.AppendMatches(matches);
+  EXPECT_TRUE(result.ShouldHideTopMatch());
+}
+
+TEST_F(AutocompleteResultTest, DoNotHideTopMatch) {
+  ACMatches matches;
+  AddMatch("http://search-what-you-typed/",
+           AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, &matches);
+  AddMatch("http://url-what-you-typed/",
+           AutocompleteMatchType::URL_WHAT_YOU_TYPED, &matches);
+  AddMatch("http://history-title/", AutocompleteMatchType::HISTORY_TITLE,
+           &matches);
+  AddMatch("http://search-history/", AutocompleteMatchType::SEARCH_HISTORY,
+           &matches);
+
+  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;
+  AddMatch("http://search-history/", AutocompleteMatchType::SEARCH_HISTORY,
+           &matches);
+  AddMatch("http://search-what-you-typed/",
+           AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, &matches);
+  AddMatch("http://history-title/", AutocompleteMatchType::HISTORY_TITLE,
+           &matches);
+
+  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) {
+  // Add some matches. This test config is identical to ShouldHideTopMatch test
+  // except that the "hide_verbatim" flag is disabled in the field trials.
+  ACMatches matches;
+  AddMatch("http://search-what-you-typed/",
+           AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, &matches);
+  AddMatch("http://history-title/", AutocompleteMatchType::HISTORY_TITLE,
+           &matches);
+  AddMatch("http://search-history/", AutocompleteMatchType::SEARCH_HISTORY,
+           &matches);
+
+  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;
+  AddMatch("http://url-what-you-typed/",
+           AutocompleteMatchType::URL_WHAT_YOU_TYPED, &matches);
+  AddMatch("http://history-title/", AutocompleteMatchType::HISTORY_TITLE,
+           &matches);
+
+  AutocompleteResult result;
+  result.AppendMatches(matches);
+  EXPECT_TRUE(result.TopMatchIsVerbatimAndHasNoConsecutiveVerbatimMatches());
+}
+
+TEST_F(AutocompleteResultTest,
+       TopMatchIsVerbatimAndHasConsecutiveVerbatimMatches) {
+  ACMatches matches;
+  AddMatch("http://search-what-you-typed/",
+           AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, &matches);
+  AddMatch("http://url-what-you-typed/",
+           AutocompleteMatchType::URL_WHAT_YOU_TYPED, &matches);
+  AddMatch("http://history-title/", AutocompleteMatchType::HISTORY_TITLE,
+           &matches);
+
+  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.
+  AddMatch("http://history-title/", AutocompleteMatchType::HISTORY_TITLE,
+           &matches);
+
+  result.AppendMatches(matches);
+  EXPECT_FALSE(result.TopMatchIsVerbatimAndHasNoConsecutiveVerbatimMatches());
+}
+
+TEST_F(AutocompleteResultTest,
+       TopMatchIsVerbatimAndHasNoConsecutiveVerbatimMatches_SingleMatchFound) {
+  ACMatches matches;
+  AddMatch("http://url-what-you-typed/",
+           AutocompleteMatchType::URL_WHAT_YOU_TYPED, &matches);
+
+  AutocompleteResult result;
+  result.AppendMatches(matches);
+  EXPECT_TRUE(result.TopMatchIsVerbatimAndHasNoConsecutiveVerbatimMatches());
+}