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 60 matching lines @@
   void AssertResultMatches(const AutocompleteResult& result,
                            const TestData* expected,
                            size_t expected_count);
 
   // Creates an AutocompleteResult from |last| and |current|. The two are
   // merged by |CopyOldMatches| and compared by |AssertResultMatches|.
   void RunCopyOldMatchesTest(const TestData* last, size_t last_size,
                              const TestData* current, size_t current_size,
                              const TestData* expected, size_t expected_size);
 
+  // Creates an AutocompleteMatch using |destination_url| and |type| and appends

[Mark P, 2013/12/13 01:10:55]

There's no reason for this to be in the class.  It can be in an anonymous
namespace in this file.

[kmadhusu, 2013/12/13 17:59:17]

Done.

+  // it to |matches|.
+  void AddMatch(const GURL& destination_url, AutocompleteMatch::Type type,
+                ACMatches* matches);
+
  protected:
   TemplateURLServiceTestUtil test_util_;
 
  private:
   scoped_ptr<base::FieldTrialList> field_trial_list_;
 
   DISALLOW_COPY_AND_ASSIGN(AutocompleteResultTest);
 };
 
 // static
@@ skipping 54 matching lines @@
   ACMatches current_matches;
   PopulateAutocompleteMatches(current, current_size, &current_matches);
   AutocompleteResult current_result;
   current_result.AppendMatches(current_matches);
   current_result.SortAndCull(input, test_util_.profile());
   current_result.CopyOldMatches(input, last_result, test_util_.profile());
 
   AssertResultMatches(current_result, expected, expected_size);
 }
 
+void AutocompleteResultTest::AddMatch(const GURL& destination_url,
+                                      AutocompleteMatch::Type type,
+                                      ACMatches* matches) {
+  ASSERT_TRUE(matches != NULL);
+  AutocompleteMatch* previous_match =

[Mark P, 2013/12/13 01:10:55]

nit: last_match please.

[kmadhusu, 2013/12/13 17:59:17]

Done.

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

[Mark P, 2013/12/13 01:10:55]

nit: space before 1

[kmadhusu, 2013/12/13 17:59:17]

Done.

+  AutocompleteMatch match;
+  match.destination_url = destination_url;
+  match.relevance =
+      previous_match ? previous_match->relevance - 100 : 1300;
+  match.allowed_to_be_default_match = true;
+  match.type = type;
+  matches->push_back(match);
+}
+
 // Assertion testing for AutocompleteResult::Swap.
 TEST_F(AutocompleteResultTest, Swap) {
   AutocompleteResult r1;
   AutocompleteResult r2;
 
   // Swap with empty shouldn't do anything interesting.
   r1.Swap(&r2);
   EXPECT_EQ(r1.end(), r1.default_match());
   EXPECT_EQ(r2.end(), r2.default_match());
 
@@ skipping 146 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(GURL("http://history-url/"),
+           AutocompleteMatchType::HISTORY_URL,

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

For better readability, I have added the arguments in its own line.

[Mark P, 2013/12/13 01:10:55]

Please consider putting them all one line if they fit on one line.  Note that
you get rid of the GURL() call everywhere by passing a string to the AddMatch
function and constructing the GURL in that function.

[kmadhusu, 2013/12/13 17:59:17]

Done.

+           &matches);
+  AddMatch(GURL("http://search-what-you-typed/"),
+           AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED,
+           &matches);
+  AddMatch(GURL("http://history-title/"),
+           AutocompleteMatchType::HISTORY_TITLE,
+           &matches);
+
+  // Add a search history type match and demote its relevance score.
+  AddMatch(GURL("http://search-history/"),
+           AutocompleteMatchType::SEARCH_HISTORY,
+           &matches);
+  matches[matches.size()-1].relevance = 500;

[Mark P, 2013/12/13 01:10:55]

nit: spaces around binary operators (-)

[kmadhusu, 2013/12/13 17:59:17]

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) {
+  // Add some matches.
+  ACMatches matches;
+  AddMatch(GURL("http://search-what-you-typed/"),
+           AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED,
+           &matches);
+  AddMatch(GURL("http://history-title/"),
+           AutocompleteMatchType::HISTORY_TITLE,
+           &matches);
+  AddMatch(GURL("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(GURL("http://search-what-you-typed/"),
+           AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED,
+           &matches);
+  AddMatch(GURL("http://url-what-you-typed/"),
+           AutocompleteMatchType::URL_WHAT_YOU_TYPED,
+           &matches);
+  AddMatch(GURL("http://history-title/"),
+           AutocompleteMatchType::HISTORY_TITLE,
+           &matches);
+  AddMatch(GURL("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(GURL("http://search-history/"),
+           AutocompleteMatchType::SEARCH_HISTORY,
+           &matches);
+  AddMatch(GURL("http://search-what-you-typed/"),
+           AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED,
+           &matches);
+  AddMatch(GURL("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) {

[Mark P, 2013/12/13 01:10:55]

Please comment here that this is an identical config.

[kmadhusu, 2013/12/13 17:59:17]

Done.

+  // Add some matches.
+  ACMatches matches;
+  AddMatch(GURL("http://search-what-you-typed/"),
+           AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED,
+           &matches);
+  AddMatch(GURL("http://history-title/"),
+           AutocompleteMatchType::HISTORY_TITLE,
+           &matches);
+  AddMatch(GURL("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(GURL("http://url-what-you-typed/"),
+           AutocompleteMatchType::URL_WHAT_YOU_TYPED,
+           &matches);
+  AddMatch(GURL("http://history-title/"),
+           AutocompleteMatchType::HISTORY_TITLE,
+           &matches);
+
+  AutocompleteResult result;
+  result.AppendMatches(matches);
+  EXPECT_TRUE(result.TopMatchIsVerbatimAndHasNoConsecutiveVerbatimMatches());
+}
+
+TEST_F(AutocompleteResultTest,
+       TopMatchIsVerbatimAndHasConsecutiveVerbatimMatches) {
+  ACMatches matches;
+  AddMatch(GURL("http://url-what-you-typed/"),
+           AutocompleteMatchType::URL_WHAT_YOU_TYPED,
+           &matches);
+  AddMatch(GURL("http://url-what-you-typed-1/"),
+           AutocompleteMatchType::URL_WHAT_YOU_TYPED,

[Mark P, 2013/12/13 01:10:55]

There can be only one URL_WHAT_YOU_TYPED.  This setup doesn't make any sense. 
Please do a SEARCH_WHAT_YOU_TYPED then URL_WHAT_YOU_TYPED.

[kmadhusu, 2013/12/13 17:59:17]

Done.

+           &matches);
+  AddMatch(GURL("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(GURL("http://history-title/"),
+           AutocompleteMatchType::HISTORY_TITLE,
+           &matches);
+
+  result.AppendMatches(matches);
+  EXPECT_FALSE(result.TopMatchIsVerbatimAndHasNoConsecutiveVerbatimMatches());
+}
+
+TEST_F(AutocompleteResultTest,
+       TopMatchIsVerbatimAndHasNoConsecutiveVerbatimMatches_SingleMatchFound) {
+  ACMatches matches;
+  AddMatch(GURL("http://url-what-you-typed/"),
+           AutocompleteMatchType::URL_WHAT_YOU_TYPED,
+           &matches);
+
+  AutocompleteResult result;
+  result.AppendMatches(matches);
+  EXPECT_TRUE(result.TopMatchIsVerbatimAndHasNoConsecutiveVerbatimMatches());
+}