Allow the history URL provider to handle input of type QUERY. This helps in the case where the user types something that on its own isn't navigable, but might be the prefix of something else navigable.

While there are no tests for this directly, another change of mine to treat more inputs with explicit schemes as queries (e.g. "http:/") relies on this, and does unittest for it.

In order to fit the new relevance scores into the table, I went through and simplified the relevance scoring so that generally providers used the same scores for more input types.  The effects of this should be barely noticeable (it affects the ranking of past search queries that are old against results from the secondary search provider), and it simplifies the code noticeably.

This also fixes a "bug" that the NavSuggest results were incremented backwards, but since we only score one of these right now there's no visible effect.

BUG=none
TEST=none
Review URL: http://codereview.chromium.org/291005

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

7 files changed, 97 insertions, 157 deletions

diff --git a/chrome/browser/autocomplete/autocomplete.h b/chrome/browser/autocomplete/autocomplete.h
index 5134f8f..0416328 100644
--- a/chrome/browser/autocomplete/autocomplete.h
+++ b/chrome/browser/autocomplete/autocomplete.h
@@ -63,8 +63,8 @@
 // --------------------------------------------------------------------|-----
 // Keyword (non-substituting or in keyword UI mode, exact match)       | 1500
 // HistoryURL (exact or inline autocomplete match)                     | 1400
-// HistoryURL (what you typed)                                         | 1300
-// Search Primary Provider (what you typed)                            | 1200
+// HistoryURL (what you typed)                                         | 1200
+// Search Primary Provider (what you typed)                            | 1150
 // Keyword (substituting, exact match)                                 | 1100
 // Search Primary Provider (past query in history)                     | 1050--
 // HistoryContents (any match in title of starred page)                | 1000++
@@ -100,16 +100,18 @@
 // QUERY input type:
 // --------------------------------------------------------------------|-----
 // Keyword (non-substituting or in keyword UI mode, exact match)       | 1500
-// Keyword (substituting, exact match)                                 | 1400
+// Keyword (substituting, exact match)                                 | 1450
+// HistoryURL (exact or inline autocomplete match)                     | 1400
 // Search Primary Provider (what you typed)                            | 1300
-// Search Primary Provider (past query in history)                     | 1250--
-// HistoryContents (any match in title of starred page)                | 1200++
-// Search Primary Provider (navigational suggestion)                   | 1000++
-// HistoryContents (any match in title of nonstarred page)             |  900++
-// Search Primary Provider (suggestion)                                |  800++
-// HistoryContents (any match in body of starred page)                 |  750++
-// HistoryContents (any match in body of nonstarred page)              |  700++
-// Keyword (inexact match)                                             |  650
+// Search Primary Provider (past query in history)                     | 1050--
+// HistoryContents (any match in title of starred page)                | 1000++
+// HistoryURL (inexact match)                                          |  900++
+// Search Primary Provider (navigational suggestion)                   |  800++
+// HistoryContents (any match in title of nonstarred page)             |  700++
+// Search Primary Provider (suggestion)                                |  600++
+// HistoryContents (any match in body of starred page)                 |  550++
+// HistoryContents (any match in body of nonstarred page)              |  500++
+// Keyword (inexact match)                                             |  450
 // Search Secondary Provider (what you typed)                          |  250
 // Search Secondary Provider (past query in history)                   |  200--
 // Search Secondary Provider (navigational suggestion)                 |  150++
@@ -117,14 +119,14 @@
 //
 // FORCED_QUERY input type:
 // --------------------------------------------------------------------|-----
-// Search Primary Provider (what you typed)                            | 1500
-// Search Primary Provider (past query in history)                     | 1250--
-// HistoryContents (any match in title of starred page)                | 1200++
-// Search Primary Provider (navigational suggestion)                   | 1000++
-// HistoryContents (any match in title of nonstarred page)             |  900++
-// Search Primary Provider (suggestion)                                |  800++
-// HistoryContents (any match in body of starred page)                 |  750++
-// HistoryContents (any match in body of nonstarred page)              |  700++
+// Search Primary Provider (what you typed)                            | 1300
+// Search Primary Provider (past query in history)                     | 1050--
+// HistoryContents (any match in title of starred page)                | 1000++
+// Search Primary Provider (navigational suggestion)                   |  800++
+// HistoryContents (any match in title of nonstarred page)             |  700++
+// Search Primary Provider (suggestion)                                |  600++
+// HistoryContents (any match in body of starred page)                 |  550++
+// HistoryContents (any match in body of nonstarred page)              |  500++
 //
 // (A search keyword is a keyword with a replacement string; a bookmark keyword
 // is a keyword with no replacement string, that is, a shortcut for a URL.)
@@ -157,6 +159,10 @@ typedef std::vector<AutocompleteProvider*> ACProviders;
 // The user input for an autocomplete query.  Allows copying.
 class AutocompleteInput {
  public:
+  // Note that the type below may be misleading.  For example, "http:/" alone
+  // cannot be opened as a URL, so it is marked as a QUERY; yet the user
+  // probably intends to type more and have it eventually become a URL, so we
+  // need to make sure we still run it through inline autocomplete.
   enum Type {
     INVALID,        // Empty input
     UNKNOWN,        // Valid input whose type cannot be determined
diff --git a/chrome/browser/autocomplete/history_contents_provider.cc b/chrome/browser/autocomplete/history_contents_provider.cc
index 558911c..d4d264e 100644
--- a/chrome/browser/autocomplete/history_contents_provider.cc
+++ b/chrome/browser/autocomplete/history_contents_provider.cc
@@ -250,31 +250,11 @@ void HistoryContentsProvider::ClassifyDescription(
 int HistoryContentsProvider::CalculateRelevance(
     const history::URLResult& result) {
   const bool in_title = MatchInTitle(result);
-  const bool is_starred =
-      (profile_->GetBookmarkModel() &&
-       profile_->GetBookmarkModel()->IsBookmarked(result.url()));
-
-  switch (input_type_) {
-    case AutocompleteInput::UNKNOWN:
-    case AutocompleteInput::REQUESTED_URL:
-      if (is_starred) {
-        return in_title ?
-            (1000 + star_title_count_++) : (550 + star_contents_count_++);
-      }
-      return in_title ? (700 + title_count_++) : (500 + contents_count_++);
-
-    case AutocompleteInput::QUERY:
-    case AutocompleteInput::FORCED_QUERY:
-      if (is_starred) {
-        return in_title ?
-            (1200 + star_title_count_++) : (750 + star_contents_count_++);
-      }
-      return in_title ? (900 + title_count_++) : (700 + contents_count_++);
-
-    default:
-      NOTREACHED();
-      return 0;
-  }
+  if (!profile_->GetBookmarkModel() ||
+      !profile_->GetBookmarkModel()->IsBookmarked(result.url()))
+    return in_title ? (700 + title_count_++) : (500 + contents_count_++);
+  return in_title ?
+      (1000 + star_title_count_++) : (550 + star_contents_count_++);
 }
 
 void HistoryContentsProvider::QueryBookmarks(const AutocompleteInput& input) {
diff --git a/chrome/browser/autocomplete/history_url_provider.cc b/chrome/browser/autocomplete/history_url_provider.cc
index d333ec5..3d394e6 100644
--- a/chrome/browser/autocomplete/history_url_provider.cc
+++ b/chrome/browser/autocomplete/history_url_provider.cc
@@ -126,7 +126,8 @@ void HistoryURLProvider::DoAutocomplete(history::HistoryBackend* backend,
   // Create a What You Typed match, which we'll need below.
   bool have_what_you_typed_match =
       params->input.canonicalized_url().is_valid() &&
-      (params->input.type() != AutocompleteInput::UNKNOWN);
+      (params->input.type() != AutocompleteInput::UNKNOWN) &&
+      (params->input.type() != AutocompleteInput::QUERY);
   AutocompleteMatch what_you_typed_match(SuggestExactInput(params->input,
                                                            params->trim_http));
 
@@ -172,7 +173,7 @@ void HistoryURLProvider::DoAutocomplete(history::HistoryBackend* backend,
   if (what_you_typed_match.is_history_what_you_typed_match &&
       FixupExactSuggestion(db, params->input, &what_you_typed_match,
                            &history_matches)) {
-    // Got an exact match for the user's input.  Treat is as the best match
+    // Got an exact match for the user's input.  Treat it as the best match
     // regardless of the input type.
     exact_suggestion = 1;
     params->matches.push_back(what_you_typed_match);
@@ -180,7 +181,7 @@ void HistoryURLProvider::DoAutocomplete(history::HistoryBackend* backend,
       history_matches.empty() ||
       !PromoteMatchForInlineAutocomplete(params, history_matches.front())) {
     // Failed to promote any URLs for inline autocompletion.  Use the What You
-    // Typed match, if we have it and the input wasn't UNKNOWN.
+    // Typed match, if we have it and the input looked like a URL.
     first_match = 0;
     if (have_what_you_typed_match)
       params->matches.push_back(what_you_typed_match);
@@ -464,7 +465,7 @@ int HistoryURLProvider::CalculateRelevance(AutocompleteInput::Type input_type,
       return 1400;
 
     case WHAT_YOU_TYPED:
-      return (input_type == AutocompleteInput::REQUESTED_URL) ? 1300 : 1200;
+      return 1200;
 
     default:
       return 900 + static_cast<int>(match_number);
@@ -592,9 +593,8 @@ void HistoryURLProvider::RunAutocompletePasses(
     bool fixup_input_and_run_pass_1) {
   matches_.clear();
 
-  if ((input.type() != AutocompleteInput::UNKNOWN) &&
-      (input.type() != AutocompleteInput::REQUESTED_URL) &&
-      (input.type() != AutocompleteInput::URL))
+  if ((input.type() == AutocompleteInput::INVALID) ||
+      (input.type() == AutocompleteInput::FORCED_QUERY))
     return;
 
   // Create a match for exactly what the user typed.  This will only be used as
@@ -602,7 +602,10 @@ void HistoryURLProvider::RunAutocompletePasses(
   // we'll run this again in DoAutocomplete() and use that result instead.
   const bool trim_http = !url_util::FindAndCompareScheme(
       WideToUTF8(input.text()), chrome::kHttpScheme, NULL);
-  if (input.canonicalized_url().is_valid())
+  // Don't do this for queries -- while we can sometimes mark up a match for
+  // this, it's not what the user wants, and just adds noise.
+  if ((input.type() != AutocompleteInput::QUERY) &&
+      input.canonicalized_url().is_valid())
     matches_.push_back(SuggestExactInput(input, trim_http));
 
   // We'll need the history service to run both passes, so try to obtain it.
diff --git a/chrome/browser/autocomplete/history_url_provider_unittest.cc b/chrome/browser/autocomplete/history_url_provider_unittest.cc
index 7730f5a..d06c543 100644
--- a/chrome/browser/autocomplete/history_url_provider_unittest.cc
+++ b/chrome/browser/autocomplete/history_url_provider_unittest.cc
@@ -208,11 +208,11 @@ TEST_F(HistoryURLProviderTest, PromoteShorterURLs) {
     "http://news.google.com/",
     "http://news.google.com/?ned=us&topic=n",
   };
-  RunTest(L"news", std::wstring(), true, expected_combine,
-          arraysize(expected_combine));
+  ASSERT_NO_FATAL_FAILURE(RunTest(L"news", std::wstring(), true,
+      expected_combine, arraysize(expected_combine)));
   // The title should also have gotten set properly on the host for the
   // synthesized one, since it was also in the results.
-  EXPECT_EQ(std::wstring(L"Google News"), matches_[0].description);
+  EXPECT_EQ(std::wstring(L"Google News"), matches_.front().description);
 
   // Test that short URL matching works correctly as the user types more
   // (several tests):
@@ -345,8 +345,9 @@ TEST_F(HistoryURLProviderTest, Fixup) {
   // after "file:", not just after "file://".
   const std::wstring input_1(L"file:");
   const std::string fixup_1[] = {"file:///C:/foo.txt"};
-  RunTest(input_1, std::wstring(), false, fixup_1, arraysize(fixup_1));
-  EXPECT_EQ(input_1.length(), matches_[0].inline_autocomplete_offset);
+  ASSERT_NO_FATAL_FAILURE(RunTest(input_1, std::wstring(), false, fixup_1,
+                                  arraysize(fixup_1)));
+  EXPECT_EQ(input_1.length(), matches_.front().inline_autocomplete_offset);
 
   // Fixing up "http:/" should result in an inline autocomplete offset of just
   // after "http:/", not just after "http:".
@@ -356,8 +357,9 @@ TEST_F(HistoryURLProviderTest, Fixup) {
     "http://bogussite.com/b",
     "http://bogussite.com/c",
   };
-  RunTest(input_2, std::wstring(), false, fixup_2, arraysize(fixup_2));
-  EXPECT_EQ(input_2.length(), matches_[0].inline_autocomplete_offset);
+  ASSERT_NO_FATAL_FAILURE(RunTest(input_2, std::wstring(), false, fixup_2,
+                                  arraysize(fixup_2)));
+  EXPECT_EQ(input_2.length(), matches_.front().inline_autocomplete_offset);
 
   // Adding a TLD to a small number like "56" should result in "www.56.com"
   // rather than "0.0.0.56.com".
diff --git a/chrome/browser/autocomplete/keyword_provider.cc b/chrome/browser/autocomplete/keyword_provider.cc
index 1185390..68f59ff 100644
--- a/chrome/browser/autocomplete/keyword_provider.cc
+++ b/chrome/browser/autocomplete/keyword_provider.cc
@@ -234,24 +234,11 @@ void KeywordProvider::FillInURLAndContents(
 int KeywordProvider::CalculateRelevance(AutocompleteInput::Type type,
                                         bool complete,
                                         bool no_query_text_needed) {
-  if (complete && no_query_text_needed)
+  if (!complete)
+    return (type == AutocompleteInput::URL) ? 700 : 450;
+  if (no_query_text_needed)
     return 1500;
-
-  switch (type) {
-    case AutocompleteInput::UNKNOWN:
-    case AutocompleteInput::REQUESTED_URL:
-      return complete ? 1100 : 450;
-
-    case AutocompleteInput::URL:
-      return complete ? 1100 : 700;
-
-    case AutocompleteInput::QUERY:
-      return complete ? 1400 : 650;
-
-    default:
-      NOTREACHED();
-      return 0;
-  }
+  return (type == AutocompleteInput::QUERY) ? 1450 : 1100;
 }
 
 AutocompleteMatch KeywordProvider::CreateAutocompleteMatch(
diff --git a/chrome/browser/autocomplete/search_provider.cc b/chrome/browser/autocomplete/search_provider.cc
index e024280..65b5fd7 100644
--- a/chrome/browser/autocomplete/search_provider.cc
+++ b/chrome/browser/autocomplete/search_provider.cc
@@ -521,10 +521,11 @@ void SearchProvider::AddNavigationResultsToMatches(
     // TODO(kochi): http://b/1170574  We add only one results for navigational
     // suggestions. If we can get more useful information about the score,
     // consider adding more results.
-    matches_.push_back(
-        NavigationToMatch(navigation_results.front(),
-                          CalculateRelevanceForNavigation(0, is_keyword),
-                          is_keyword));
+    const size_t num_results = is_keyword ?
+        keyword_navigation_results_.size() : default_navigation_results_.size();
+    matches_.push_back(NavigationToMatch(navigation_results.front(),
+        CalculateRelevanceForNavigation(num_results, 0, is_keyword),
+        is_keyword));
   }
 }
 
@@ -547,7 +548,7 @@ void SearchProvider::AddSuggestResultsToMap(
     MatchMap* map) {
   for (size_t i = 0; i < suggest_results.size(); ++i) {
     AddMatchToMap(suggest_results[i],
-                  CalculateRelevanceForSuggestion(suggest_results, i,
+                  CalculateRelevanceForSuggestion(suggest_results.size(), i,
                                                   is_keyword),
                   AutocompleteMatch::SEARCH_SUGGEST,
                   static_cast<int>(i), is_keyword, map);
@@ -555,21 +556,20 @@ void SearchProvider::AddSuggestResultsToMap(
 }
 
 int SearchProvider::CalculateRelevanceForWhatYouTyped() const {
+  if (providers_.valid_keyword_provider())
+    return 250;
+
   switch (input_.type()) {
     case AutocompleteInput::UNKNOWN:
-      return providers_.valid_keyword_provider() ? 250 : 1300;
+    case AutocompleteInput::QUERY:
+    case AutocompleteInput::FORCED_QUERY:
+      return 1300;
 
     case AutocompleteInput::REQUESTED_URL:
-      return providers_.valid_keyword_provider() ? 250 : 1200;
+      return 1150;
 
     case AutocompleteInput::URL:
-      return providers_.valid_keyword_provider() ? 250 : 850;
-
-    case AutocompleteInput::QUERY:
-      return providers_.valid_keyword_provider() ? 250 : 1300;
-
-    case AutocompleteInput::FORCED_QUERY:
-      return providers_.valid_keyword_provider() ? 250 : 1500;
+      return 850;
 
     default:
       NOTREACHED();
@@ -589,73 +589,34 @@ int SearchProvider::CalculateRelevanceForHistory(const Time& time,
   // Don't let scores go below 0.  Negative relevance scores are meaningful in
   // a different way.
   int base_score;
-  bool is_primary = providers_.is_primary_provider(is_keyword);
-  switch (input_.type()) {
-    case AutocompleteInput::UNKNOWN:
-    case AutocompleteInput::REQUESTED_URL:
-      base_score = is_primary ? 1050 : 200;
-      break;
-
-    case AutocompleteInput::URL:
-      base_score = is_primary ? 750 : 200;
-      break;
-
-    case AutocompleteInput::QUERY:
-    case AutocompleteInput::FORCED_QUERY:
-      base_score = is_primary ? 1250 : 200;
-      break;
-
-    default:
-      NOTREACHED();
-      base_score = 0;
-      break;
-  }
+  if (!providers_.is_primary_provider(is_keyword))
+    base_score = 200;
+  else
+    base_score = (input_.type() == AutocompleteInput::URL) ? 750 : 1050;
   return std::max(0, base_score - score_discount);
 }
 
-int SearchProvider::CalculateRelevanceForSuggestion(
-    const SuggestResults& suggest_results,
-    size_t suggestion_number,
-    bool is_keyword) const {
-  DCHECK(suggestion_number < suggest_results.size());
-  bool is_primary = providers_.is_primary_provider(is_keyword);
-  const int suggestion_value =
-      static_cast<int>(suggest_results.size() - 1 - suggestion_number);
-  switch (input_.type()) {
-    case AutocompleteInput::UNKNOWN:
-    case AutocompleteInput::REQUESTED_URL:
-      return suggestion_value + (is_primary ? 600 : 100);
-
-    case AutocompleteInput::URL:
-      return suggestion_value + (is_primary ? 300 : 100);
-
-    case AutocompleteInput::QUERY:
-    case AutocompleteInput::FORCED_QUERY:
-      return suggestion_value + (is_primary ? 800 : 100);
-
-    default:
-      NOTREACHED();
-      return 0;
-  }
+int SearchProvider::CalculateRelevanceForSuggestion(size_t num_results,
+                                                    size_t result_number,
+                                                    bool is_keyword) const {
+  DCHECK(result_number < num_results);
+  int base_score;
+  if (!providers_.is_primary_provider(is_keyword))
+    base_score = 100;
+  else
+    base_score = (input_.type() == AutocompleteInput::URL) ? 300 : 600;
+  return base_score +
+      static_cast<int>(num_results - 1 - result_number);
 }
 
-int SearchProvider::CalculateRelevanceForNavigation(
-    size_t suggestion_number,
-    bool is_keyword) const {
-  DCHECK(
-      (is_keyword && suggestion_number < keyword_navigation_results_.size()) ||
-      (!is_keyword && suggestion_number < default_navigation_results_.size()));
+int SearchProvider::CalculateRelevanceForNavigation(size_t num_results,
+                                                    size_t result_number,
+                                                    bool is_keyword) const {
+  DCHECK(result_number < num_results);
   // TODO(kochi): http://b/784900  Use relevance score from the NavSuggest
   // server if possible.
-  bool is_primary = providers_.is_primary_provider(is_keyword);
-  switch (input_.type()) {
-    case AutocompleteInput::QUERY:
-    case AutocompleteInput::FORCED_QUERY:
-      return static_cast<int>(suggestion_number) + (is_primary ? 1000 : 150);
-
-    default:
-      return static_cast<int>(suggestion_number) + (is_primary ? 800 : 150);
-  }
+  return (providers_.is_primary_provider(is_keyword) ? 800 : 150) +
+      static_cast<int>(num_results - 1 - result_number);
 }
 
 void SearchProvider::AddMatchToMap(const std::wstring& query_string,
diff --git a/chrome/browser/autocomplete/search_provider.h b/chrome/browser/autocomplete/search_provider.h
index 9b0ef38..f333c4f 100644
--- a/chrome/browser/autocomplete/search_provider.h
+++ b/chrome/browser/autocomplete/search_provider.h
@@ -236,15 +236,16 @@ class SearchProvider : public AutocompleteProvider,
   // if the search is from the keyword provider.
   int CalculateRelevanceForHistory(const base::Time& time,
                                    bool is_keyword) const;
-  // |suggestion_value| is the index of the suggestion in |suggest_results| that
-  // was returned from the server; the best suggestion is suggestion number 0.
-  // |is_keyword| is true if the search is from the keyword provider.
-  int CalculateRelevanceForSuggestion(const SuggestResults& suggest_results,
-                                      size_t suggestion_number,
+  // |result_number| is the index of the suggestion in the result set from the
+  // server; the best suggestion is suggestion number 0.  |is_keyword| is true
+  // if the search is from the keyword provider.
+  int CalculateRelevanceForSuggestion(size_t num_results,
+                                      size_t result_number,
                                       bool is_keyword) const;
-  // |suggestion_value| is same as above. |is_keyword| is true if the navigation
+  // |result_number| is same as above. |is_keyword| is true if the navigation
   // result was suggested by the keyword provider.
-  int CalculateRelevanceForNavigation(size_t suggestion_value,
+  int CalculateRelevanceForNavigation(size_t num_results,
+                                      size_t result_number,
                                       bool is_keyword) const;
 
   // Creates an AutocompleteMatch for "Search <engine> for |query_string|" with