Don't demote top match for certain match types.

BUG=

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@232979 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 131 insertions, 2 deletions

diff --git a/chrome/browser/autocomplete/autocomplete_result.cc b/chrome/browser/autocomplete/autocomplete_result.cc
index 0c01b39..7078f55 100644
--- a/chrome/browser/autocomplete/autocomplete_result.cc
+++ b/chrome/browser/autocomplete/autocomplete_result.cc
@@ -154,10 +154,22 @@ void AutocompleteResult::SortAndCull(const AutocompleteInput& input,
                              &AutocompleteMatch::DestinationsEqual),
                  matches_.end());
 
+  // Find the top match before possibly applying demotions.
+  if (!matches_.empty())
+    std::partial_sort(matches_.begin(), matches_.begin() +  1, matches_.end(),
+                      &AutocompleteMatch::MoreRelevant);
+  // Don't demote the top match if applicable.
+  OmniboxFieldTrial::UndemotableTopMatchTypes undemotable_top_types =
+      OmniboxFieldTrial::GetUndemotableTopTypes(
+          input.current_page_classification());
+  const bool preserve_top_match = !matches_.empty() &&
+      (undemotable_top_types.count(matches_.begin()->type) != 0);
+
   // Sort and trim to the most relevant kMaxMatches matches.
   size_t max_num_matches = std::min(kMaxMatches, matches_.size());
   CompareWithDemoteByType comparing_object(input.current_page_classification());
-  std::sort(matches_.begin(), matches_.end(), comparing_object);
+  std::sort(matches_.begin() + (preserve_top_match ? 1 : 0), matches_.end(),
+            comparing_object);
   if (!matches_.empty() && !matches_.begin()->allowed_to_be_default_match &&
       OmniboxFieldTrial::ReorderForLegalDefaultMatch(
           input.current_page_classification())) {
@@ -316,6 +328,8 @@ void AutocompleteResult::AddMatch(
   DCHECK_EQ(AutocompleteMatch::SanitizeString(match.contents), match.contents);
   DCHECK_EQ(AutocompleteMatch::SanitizeString(match.description),
             match.description);
+  // GetUndemotableTopTypes() is not used here because it's done in
+  // SortAndCull(), and we depend on SortAndCull() to be called afterwards.
   CompareWithDemoteByType comparing_object(page_classification);
   ACMatches::iterator insertion_point =
       std::upper_bound(begin(), end(), match, comparing_object);
diff --git a/chrome/browser/autocomplete/autocomplete_result_unittest.cc b/chrome/browser/autocomplete/autocomplete_result_unittest.cc
index babf1ae..ab152fb 100644
--- a/chrome/browser/autocomplete/autocomplete_result_unittest.cc
+++ b/chrome/browser/autocomplete/autocomplete_result_unittest.cc
@@ -377,6 +377,56 @@ TEST_F(AutocompleteResultTest, SortAndCullWithDemotionsByType) {
             result.match_at(2)->destination_url.spec());
 }
 
+TEST_F(AutocompleteResultTest, SortAndCullWithUndemotableTypes) {
+  // Add some matches.
+  ACMatches matches(3);
+  matches[0].destination_url = GURL("http://top-history-url/");
+  matches[0].relevance = 1400;
+  matches[0].allowed_to_be_default_match = true;
+  matches[0].type = AutocompleteMatchType::HISTORY_URL;
+  matches[1].destination_url = GURL("http://history-url2/");
+  matches[1].relevance = 1300;
+  matches[1].allowed_to_be_default_match = true;
+  matches[1].type = AutocompleteMatchType::HISTORY_URL;
+  matches[2].destination_url = GURL("http://search-what-you-typed/");
+  matches[2].relevance = 1200;
+  matches[2].allowed_to_be_default_match = true;
+  matches[2].type = AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED;
+
+  // Add a rule demoting history-url, but don't demote the top match.
+  {
+    std::map<std::string, std::string> params;
+    // 3 == HOME_PAGE
+    params[std::string(OmniboxFieldTrial::kDemoteByTypeRule) + ":3:*"] =
+        "1:50";
+    params[std::string(OmniboxFieldTrial::kUndemotableTopTypeRule) + ":3:*"] =
+        "1,5";
+    ASSERT_TRUE(chrome_variations::AssociateVariationParams(
+        OmniboxFieldTrial::kBundledExperimentFieldTrialName, "B", params));
+  }
+  base::FieldTrialList::CreateFieldTrial(
+      OmniboxFieldTrial::kBundledExperimentFieldTrialName, "B");
+
+  AutocompleteResult result;
+  result.AppendMatches(matches);
+  AutocompleteInput input(string16(), string16::npos, string16(), GURL(),
+                          AutocompleteInput::HOME_PAGE, false, false, false,
+                          AutocompleteInput::ALL_MATCHES);
+  result.SortAndCull(input, test_util_.profile());
+
+  // Check the new ordering.  The first history-url result should not be
+  // demoted, but the second result should be.
+  // We cannot check relevance scores because the matches are sorted by
+  // demoted relevance but the actual relevance scores are not modified.
+  ASSERT_EQ(3u, result.size());
+  EXPECT_EQ("http://top-history-url/",
+            result.match_at(0)->destination_url.spec());
+  EXPECT_EQ("http://search-what-you-typed/",
+            result.match_at(1)->destination_url.spec());
+  EXPECT_EQ("http://history-url2/",
+            result.match_at(2)->destination_url.spec());
+}
+
 TEST_F(AutocompleteResultTest, SortAndCullReorderForDefaultMatch) {
   TestData data[] = {
     { 0, 0, 1300 },
diff --git a/chrome/browser/omnibox/omnibox_field_trial.cc b/chrome/browser/omnibox/omnibox_field_trial.cc
index e2c7144..6b9510b 100644
--- a/chrome/browser/omnibox/omnibox_field_trial.cc
+++ b/chrome/browser/omnibox/omnibox_field_trial.cc
@@ -254,7 +254,7 @@ void OmniboxFieldTrial::GetDemotionsByType(
     for (base::StringPairs::const_iterator it = kv_pairs.begin();
          it != kv_pairs.end(); ++it) {
       // This is a best-effort conversion; we trust the hand-crafted parameters
-      // downloaded from the server to be perfect.  There's no need for handle
+      // downloaded from the server to be perfect.  There's no need to handle
       // errors smartly.
       int k, v;
       base::StringToInt(it->first, &k);
@@ -265,6 +265,32 @@ void OmniboxFieldTrial::GetDemotionsByType(
   }
 }
 
+OmniboxFieldTrial::UndemotableTopMatchTypes
+OmniboxFieldTrial::GetUndemotableTopTypes(
+    AutocompleteInput::PageClassification current_page_classification) {
+  UndemotableTopMatchTypes undemotable_types;
+  const std::string types_rule =
+      OmniboxFieldTrial::GetValueForRuleInContext(
+          kUndemotableTopTypeRule,
+          current_page_classification);
+  // The value of the UndemotableTopTypes rule is a comma-separated list of
+  // AutocompleteMatchType::Type enums represented as an integer. The
+  // DemoteByType rule does not apply to the top match if the type of the top
+  // match is in this list.
+  std::vector<std::string> types;
+  base::SplitString(types_rule, ',', &types);
+  for (std::vector<std::string>::const_iterator it = types.begin();
+       it != types.end(); ++it) {
+    // This is a best-effort conversion; we trust the hand-crafted parameters
+    // downloaded from the server to be perfect.  There's no need to handle
+    // errors smartly.
+    int t;
+    base::StringToInt(*it, &t);
+    undemotable_types.insert(static_cast<AutocompleteMatchType::Type>(t));
+  }
+  return undemotable_types;
+}
+
 bool OmniboxFieldTrial::ReorderForLegalDefaultMatch(
     AutocompleteInput::PageClassification current_page_classification) {
   return OmniboxFieldTrial::GetValueForRuleInContext(
@@ -278,6 +304,7 @@ const char OmniboxFieldTrial::kShortcutsScoringMaxRelevanceRule[] =
     "ShortcutsScoringMaxRelevance";
 const char OmniboxFieldTrial::kSearchHistoryRule[] = "SearchHistory";
 const char OmniboxFieldTrial::kDemoteByTypeRule[] = "DemoteByType";
+const char OmniboxFieldTrial::kUndemotableTopTypeRule[] = "UndemotableTopTypes";
 const char OmniboxFieldTrial::kReorderForLegalDefaultMatchRule[] =
     "ReorderForLegalDefaultMatch";
 const char OmniboxFieldTrial::kReorderForLegalDefaultMatchRuleEnabled[] =
diff --git a/chrome/browser/omnibox/omnibox_field_trial.h b/chrome/browser/omnibox/omnibox_field_trial.h
index 50b8701..1c9f80e 100644
--- a/chrome/browser/omnibox/omnibox_field_trial.h
+++ b/chrome/browser/omnibox/omnibox_field_trial.h
@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_OMNIBOX_OMNIBOX_FIELD_TRIAL_H_
 
 #include <map>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -22,6 +23,9 @@ class OmniboxFieldTrial {
   // given number.  Omitted types are assumed to have multipliers of 1.0.
   typedef std::map<AutocompleteMatchType::Type, float> DemotionMultipliers;
 
+  // A set of types that should not be demoted when they are the top match.
+  typedef std::set<AutocompleteMatchType::Type> UndemotableTopMatchTypes;
+
   // Creates the static field trial groups.
   // *** MUST NOT BE CALLED MORE THAN ONCE. ***
   static void ActivateStaticTrials();
@@ -148,6 +152,11 @@ class OmniboxFieldTrial {
       AutocompleteInput::PageClassification current_page_classification,
       DemotionMultipliers* demotions_by_type);
 
+  // Get the set of types that should not be demoted if they are the top
+  // match.
+  static UndemotableTopMatchTypes GetUndemotableTopTypes(
+      AutocompleteInput::PageClassification current_page_classification);
+
   // ---------------------------------------------------------
   // For the ReorderForLegalDefaultMatch experiment that's part of the
   // bundled omnibox field trial.
@@ -168,6 +177,7 @@ class OmniboxFieldTrial {
   static const char kShortcutsScoringMaxRelevanceRule[];
   static const char kSearchHistoryRule[];
   static const char kDemoteByTypeRule[];
+  static const char kUndemotableTopTypeRule[];
   static const char kReorderForLegalDefaultMatchRule[];
   // Rule values.
   static const char kReorderForLegalDefaultMatchRuleEnabled[];
diff --git a/chrome/browser/omnibox/omnibox_field_trial_unittest.cc b/chrome/browser/omnibox/omnibox_field_trial_unittest.cc
index 82522b6..edf222b 100644
--- a/chrome/browser/omnibox/omnibox_field_trial_unittest.cc
+++ b/chrome/browser/omnibox/omnibox_field_trial_unittest.cc
@@ -188,6 +188,34 @@ TEST_F(OmniboxFieldTrialTest, GetDemotionsByTypeWithFallback) {
   VerifyDemotion(demotions_by_type, AutocompleteMatchType::HISTORY_URL, 0.25);
 }
 
+TEST_F(OmniboxFieldTrialTest, GetUndemotableTopTypes) {
+  {
+    std::map<std::string, std::string> params;
+    const std::string rule(OmniboxFieldTrial::kUndemotableTopTypeRule);
+    params[rule + ":1:*"] = "1,3";
+    params[rule + ":3:*"] = "5";
+    params[rule + ":*:*"] = "2";
+    ASSERT_TRUE(chrome_variations::AssociateVariationParams(
+        OmniboxFieldTrial::kBundledExperimentFieldTrialName, "A", params));
+  }
+  base::FieldTrialList::CreateFieldTrial(
+      OmniboxFieldTrial::kBundledExperimentFieldTrialName, "A");
+  OmniboxFieldTrial::UndemotableTopMatchTypes undemotable_types;
+  undemotable_types = OmniboxFieldTrial::GetUndemotableTopTypes(
+      AutocompleteInput::NEW_TAB_PAGE);
+  ASSERT_EQ(2u, undemotable_types.size());
+  ASSERT_EQ(1u, undemotable_types.count(AutocompleteMatchType::HISTORY_URL));
+  ASSERT_EQ(1u, undemotable_types.count(AutocompleteMatchType::HISTORY_BODY));
+  undemotable_types = OmniboxFieldTrial::GetUndemotableTopTypes(
+      AutocompleteInput::HOME_PAGE);
+  ASSERT_EQ(1u, undemotable_types.size());
+  ASSERT_EQ(1u, undemotable_types.count(AutocompleteMatchType::NAVSUGGEST));
+  undemotable_types = OmniboxFieldTrial::GetUndemotableTopTypes(
+      AutocompleteInput::BLANK);
+  ASSERT_EQ(1u, undemotable_types.size());
+  ASSERT_EQ(1u, undemotable_types.count(AutocompleteMatchType::HISTORY_TITLE));
+}
+
 TEST_F(OmniboxFieldTrialTest, GetValueForRuleInContext) {
   // This test starts with Instant Extended off (the default state), then
   // enables Instant Extended and tests again on the same rules.