Omnibox: Make HUP Scoring Saner in Prevent-Inline Mode

In particular, make HistoryURL provider scoring
basically the same within regular mode and
prevent-inline-autocomplete mode (under the constraint
that we always need a valid default match first).
For what it's worth, the current code demotes (from
~1410) all non-URL-what-you-typed URLs (non-UWYT)
to 900 in prevent-inline-autocomplete mode.  If
there is a UWYT, though, these results will be
clustered / given a free ride to appear right
below it, scoring ~1200.  After this change, all
non-UWYT results will score the same in
prevent-inline-autocomplete and normal modes.
We instead rely on the allowed_to_be_default_match
bit to reorder results as necessary.

To implement this feature, we need knowledge of
whether the what-you-typed match is reasonable.
This change re-adds a variable
have_what_you_typed_match to HistoryURLParams.  This
variable was previously added in
https://codereview.chromium.org/347963002 for
a field trial and then later removed in
https://codereview.chromium.org/879053002

With this variable, the change is straightforward.
Don't change the scoring in prevent-inline-autocomplete
mode.  Instead, simply make sure the what-you-type
match gets added to the list in prevent-inline-mode.
We add it early on the list (using the Promote()
mechanism) so that it'll always appear in the final
set of suggestions.

Tested using about:omnibox

BUG=421772

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

Cr-Commit-Position: refs/heads/master@{#322528}

chrome/browser/autocomplete/history_url_provider.cc

@@ -568,7 +568,7 @@ void HistoryURLProvider::Start(const AutocompleteInput& input,
   if (url_db) {
     DoAutocomplete(NULL, url_db, params.get());
     matches_.clear();
-    PromoteMatchIfNecessary(*params);
+    PromoteMatchesIfNecessary(*params);
     // NOTE: We don't reset |params| here since at least the |promote_type|
     // field on it will be read by the second pass -- see comments in
     // DoAutocomplete().
@@ -766,7 +766,7 @@ void HistoryURLProvider::DoAutocomplete(history::HistoryBackend* backend,
   // searching has been disabled by policy. In the cases where we've parsed as
   // UNKNOWN, we'll still show an accidental search infobar if need be.
   VisitClassifier classifier(this, params->input, db);
-  bool have_what_you_typed_match =
+  params->have_what_you_typed_match =
       (params->input.type() != metrics::OmniboxInputType::QUERY) &&
       ((params->input.type() != metrics::OmniboxInputType::UNKNOWN) ||
        (classifier.type() == VisitClassifier::UNVISITED_INTRANET) ||
@@ -774,7 +774,7 @@ void HistoryURLProvider::DoAutocomplete(history::HistoryBackend* backend,
        (AutocompleteInput::NumNonHostComponents(params->input.parts()) > 0) ||
        !params->default_search_provider);
   const bool have_shorter_suggestion_suitable_for_inline_autocomplete =
-      PromoteOrCreateShorterSuggestion(db, have_what_you_typed_match, params);
+      PromoteOrCreateShorterSuggestion(db, params);
 
   // Check whether what the user typed appears in history.
   const bool can_check_history_for_exact_match =
@@ -802,13 +802,24 @@ void HistoryURLProvider::DoAutocomplete(history::HistoryBackend* backend,
   // this input, so we can promote that as the best match.
   if (params->exact_suggestion_is_in_history) {
     params->promote_type = HistoryURLProviderParams::WHAT_YOU_TYPED_MATCH;
-  } else if (!params->prevent_inline_autocomplete && !params->matches.empty() &&
-      (have_shorter_suggestion_suitable_for_inline_autocomplete ||
-       CanPromoteMatchForInlineAutocomplete(params->matches[0]))) {
+  } else if (!params->matches.empty() &&
+             (have_shorter_suggestion_suitable_for_inline_autocomplete ||
+              CanPromoteMatchForInlineAutocomplete(params->matches[0]))) {
+    // Note that we promote this inline-autocompleted match even when
+    // params->prevent_inline_autocomplete is true.  This is safe because in
+    // this case the match will be marked as "not allowed to be default", and
+    // a non-inlined match that is "allowed to be default" will be reordered
+    // above it by the controller/AutocompleteResult.  We ensure there is such
+    // a match in two ways:
+    //   * If params->have_what_you_typed_match is true, we force the
+    //     what-you-typed match to be added in this case.  See comments in
+    //     PromoteMatchesIfNecessary().
+    //   * Otherwise, we should have some sort of QUERY or UNKNOWN input that
+    //     the SearchProvider will provide a defaultable WYT match for.
     params->promote_type = HistoryURLProviderParams::FRONT_HISTORY_MATCH;
   } else {
     // Failed to promote any URLs.  Use the What You Typed match, if we have it.
-    params->promote_type = have_what_you_typed_match ?
+    params->promote_type = params->have_what_you_typed_match ?
         HistoryURLProviderParams::WHAT_YOU_TYPED_MATCH :
         HistoryURLProviderParams::NEITHER;
   }
@@ -826,15 +837,33 @@ void HistoryURLProvider::DoAutocomplete(history::HistoryBackend* backend,
   }
 }
 
-void HistoryURLProvider::PromoteMatchIfNecessary(
+void HistoryURLProvider::PromoteMatchesIfNecessary(
     const HistoryURLProviderParams& params) {
   if (params.promote_type == HistoryURLProviderParams::NEITHER)
     return;
-  matches_.push_back(
-      (params.promote_type == HistoryURLProviderParams::WHAT_YOU_TYPED_MATCH) ?
-      params.what_you_typed_match :
-      HistoryMatchToACMatch(params, 0, INLINE_AUTOCOMPLETE,
-                            CalculateRelevance(INLINE_AUTOCOMPLETE, 0)));
+  if (params.promote_type == HistoryURLProviderParams::FRONT_HISTORY_MATCH) {
+    matches_.push_back(
+        HistoryMatchToACMatch(params, 0, INLINE_AUTOCOMPLETE,
+                              CalculateRelevance(INLINE_AUTOCOMPLETE, 0)));
+  }
+  // There are two cases where we need to add the what-you-typed-match:
+  //   * If params.promote_type is WHAT_YOU_TYPED_MATCH, we're being explicitly
+  //     directed to.
+  //   * If params.have_what_you_typed_match is true, then params.promote_type
+  //     can't be NEITHER (see code near the end of DoAutocomplete()), so if
+  //     it's not WHAT_YOU_TYPED_MATCH, it must be FRONT_HISTORY_MATCH, and
+  //     we'll have promoted the history match above.  If
+  //     params.prevent_inline_autocomplete is also true, then this match
+  //     will be marked "not allowed to be default", and we need to add the
+  //     what-you-typed match to ensure there's a legal default match for the
+  //     controller/AutocompleteResult to promote.  (If
+  //     params.have_what_you_typed_match is false, the SearchProvider should
+  //     take care of adding this defaultable match.)
+  if ((params.promote_type == HistoryURLProviderParams::WHAT_YOU_TYPED_MATCH) ||
+      (params.prevent_inline_autocomplete &&
+       params.have_what_you_typed_match)) {
+    matches_.push_back(params.what_you_typed_match);
+  }
 }
 
 void HistoryURLProvider::QueryComplete(
@@ -855,7 +884,7 @@ void HistoryURLProvider::QueryComplete(
   // match in it, whereas |params->matches| will be empty.
   if (!params->failed) {
     matches_.clear();
-    PromoteMatchIfNecessary(*params);
+    PromoteMatchesIfNecessary(*params);
 
     // Determine relevance of highest scoring match, if any.
     int relevance = matches_.empty() ?
@@ -956,7 +985,6 @@ bool HistoryURLProvider::CanFindIntranetURL(
 
 bool HistoryURLProvider::PromoteOrCreateShorterSuggestion(
     history::URLDatabase* db,
-    bool have_what_you_typed_match,
     HistoryURLProviderParams* params) {
   if (params->matches.empty())
     return false;  // No matches, nothing to do.
@@ -966,7 +994,7 @@ bool HistoryURLProvider::PromoteOrCreateShorterSuggestion(
   // the same" as any "what you typed" match.
   const history::HistoryMatch& match = params->matches[0];
   GURL search_base = ConvertToHostOnly(match, params->input.text());
-  bool can_add_search_base_to_matches = !have_what_you_typed_match;
+  bool can_add_search_base_to_matches = !params->have_what_you_typed_match;
   if (search_base.is_empty()) {
     // Search from what the user typed when we couldn't reduce the best match
     // to a host.  Careful: use a substring of |match| here, rather than the

chrome/browser/autocomplete/history_url_provider.h

@@ -157,6 +157,11 @@ struct HistoryURLProviderParams {
   // this to.  See comments in DoAutocomplete().
   PromoteType promote_type;
 
+  // True if |what_you_typed_match| is eligible for display.  If this is true,
+  // PromoteMatchesIfNecessary() may choose to place |what_you_typed_match| on
+  // |matches_| even when |promote_type| is not WHAT_YOU_TYPED_MATCH.
+  bool have_what_you_typed_match;
+
   // Languages we should pass to gfx::GetCleanStringFromUrl.
   std::string languages;
 
@@ -246,10 +251,11 @@ class HistoryURLProvider : public HistoryProvider {
                       history::URLDatabase* db,
                       HistoryURLProviderParams* params);
 
-  // May promote either the what you typed match or first history match in
-  // params->matches to the front of |matches_|, depending on the value of
-  // params->promote_type.
-  void PromoteMatchIfNecessary(const HistoryURLProviderParams& params);
+  // May promote the what you typed match, the first history match in
+  // params->matches, or both to the front of |matches_|, depending on the
+  // values of params->promote_type, params->have_what_you_typed_match, and
+  // params->prevent_inline_autocomplete.
+  void PromoteMatchesIfNecessary(const HistoryURLProviderParams& params);
 
   // Dispatches the results to the autocomplete controller. Called on the
   // main thread by ExecuteWithDB when the results are available.
@@ -281,7 +287,6 @@ class HistoryURLProvider : public HistoryProvider {
   // willing to inline autocomplete.
   bool PromoteOrCreateShorterSuggestion(
       history::URLDatabase* db,
-      bool have_what_you_typed_match,
       HistoryURLProviderParams* params);
 
   // Removes results that have been rarely typed or visited, and not any time

chrome/browser/autocomplete/history_url_provider_unittest.cc

@@ -413,23 +413,40 @@ TEST_F(HistoryURLProviderTest, PromoteShorterURLs) {
   // shorter URL that's "good enough".  The host doesn't match the user input
   // and so should not appear.
   const UrlAndLegalDefault short_3[] = {
-    { "http://foo.com/d", true },
     { "http://foo.com/dir/another/", false },
+    { "http://foo.com/d", true },
     { "http://foo.com/dir/another/again/myfile.html", false },
     { "http://foo.com/dir/", false }
   };
   RunTest(ASCIIToUTF16("foo.com/d"), std::string(), true, short_3,
           arraysize(short_3));
+  // If prevent_inline_autocomplete is false, we won't bother creating the
+  // URL-what-you-typed match because we have promoted inline autocompletions.
+  const UrlAndLegalDefault short_3_allow_inline[] = {
+    { "http://foo.com/dir/another/", true },
+    { "http://foo.com/dir/another/again/myfile.html", true },
+    { "http://foo.com/dir/", true }
+  };
+  RunTest(ASCIIToUTF16("foo.com/d"), std::string(), false, short_3_allow_inline,
+          arraysize(short_3_allow_inline));
 
   // We shouldn't promote shorter URLs than the best if they're not good
   // enough.
   const UrlAndLegalDefault short_4[] = {
-    { "http://foo.com/dir/another/a", true },
     { "http://foo.com/dir/another/again/myfile.html", false },
+    { "http://foo.com/dir/another/a", true },
     { "http://foo.com/dir/another/again/", false }
   };
   RunTest(ASCIIToUTF16("foo.com/dir/another/a"), std::string(), true, short_4,
           arraysize(short_4));
+  // If prevent_inline_autocomplete is false, we won't bother creating the
+  // URL-what-you-typed match because we have promoted inline autocompletions.
+  const UrlAndLegalDefault short_4_allow_inline[] = {
+    { "http://foo.com/dir/another/again/myfile.html", true },
+    { "http://foo.com/dir/another/again/", true }
+  };
+  RunTest(ASCIIToUTF16("foo.com/dir/another/a"), std::string(), false,
+          short_4_allow_inline, arraysize(short_4_allow_inline));
 
   // Exact matches should always be best no matter how much more another match
   // has been typed.
@@ -485,11 +502,21 @@ TEST_F(HistoryURLProviderTest, CullRedirects) {
   // "what you typed" result, plus this one.
   const base::string16 typing(ASCIIToUTF16("http://redirects/"));
   const UrlAndLegalDefault expected_results[] = {
-    { base::UTF16ToUTF8(typing), true },
-    { test_cases[0].url, false }
+    { test_cases[0].url, false },
+    { base::UTF16ToUTF8(typing), true }
   };
   RunTest(typing, std::string(), true, expected_results,
           arraysize(expected_results));
+
+  // If prevent_inline_autocomplete is false, we won't bother creating the
+  // URL-what-you-typed match because we have promoted inline autocompletions.
+  // The result set should instead consist of a single URL representing the
+  // whole set of redirects.
+  const UrlAndLegalDefault expected_results_allow_inlining[] = {
+    { test_cases[0].url, true }
+  };
+  RunTest(typing, std::string(), false, expected_results_allow_inlining,
+          arraysize(expected_results_allow_inlining));
 }
 
 TEST_F(HistoryURLProviderTestNoSearchProvider, WhatYouTypedNoSearchProvider) {