Omnibox: Create Keyword Verbatim Result in Search Provider

chrome/browser/autocomplete/search_provider.cc

 // Copyright 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/search_provider.h"
 
 #include <algorithm>
 #include <cmath>
 
 #include "base/callback.h"
@@ skipping 765 matching lines @@
 }
 
 void SearchProvider::ConvertResultsToAutocompleteMatches() {
   // Convert all the results to matches and add them to a map, so we can keep
   // the most relevant match for each result.
   MatchMap map;
   const Time no_time;
   int did_not_accept_keyword_suggestion = keyword_suggest_results_.empty() ?
       TemplateURLRef::NO_SUGGESTIONS_AVAILABLE :
       TemplateURLRef::NO_SUGGESTION_CHOSEN;
-  // Keyword what you typed results are handled by the KeywordProvider.
 
   int verbatim_relevance = GetVerbatimRelevance();
   int did_not_accept_default_suggestion = default_suggest_results_.empty() ?
       TemplateURLRef::NO_SUGGESTIONS_AVAILABLE :
       TemplateURLRef::NO_SUGGESTION_CHOSEN;
   if (verbatim_relevance > 0) {
     AddMatchToMap(input_.text(), input_.text(), verbatim_relevance,
                   AutocompleteMatch::SEARCH_WHAT_YOU_TYPED,
                   did_not_accept_default_suggestion, false, &map);
   }
-  const size_t what_you_typed_size = map.size();
+  if (!keyword_input_text_.empty()) {
+    const TemplateURL* keyword_url = providers_.GetKeywordProviderURL();
+    // We only create the verbatim search query match for a keyword
+    // if it's not an extension keyword.  Extension keywords are handled
+    // in KeywordProvider::Start().  (Extensions are complicated...)
+    if (keyword_url && !keyword_url->IsExtensionKeyword()) {
+      AddMatchToMap(keyword_input_text_, keyword_input_text_,
+                    CalculateRelevanceForKeywordVerbatim(
+                        input_.type(), input_.prefer_keyword()),
+                    AutocompleteMatch::SEARCH_OTHER_ENGINE,
+                    did_not_accept_keyword_suggestion, true, &map);
+    }
+  }
+  const size_t verbatim_matches_size = map.size();
   if (!default_provider_suggestion_.text.empty() &&
       default_provider_suggestion_.type == INSTANT_SUGGESTION_SEARCH &&
       !input_.prevent_inline_autocomplete())
     AddMatchToMap(input_.text() + default_provider_suggestion_.text,
                   input_.text(), verbatim_relevance + 1,
                   AutocompleteMatch::SEARCH_SUGGEST,
                   did_not_accept_default_suggestion, false, &map);
 
   AddHistoryResultsToMap(keyword_history_results_, true,
                          did_not_accept_keyword_suggestion, &map);
@@ skipping 15 matching lines @@
     // |verbatim_relevance| here.
     matches_.push_back(NavigationToMatch(
         NavigationResult(GURL(UTF16ToUTF8(default_provider_suggestion_.text)),
                          string16(),
                          kNonURLVerbatimRelevance + 1),
         false));
   }
   AddNavigationResultsToMatches(keyword_navigation_results_, true);
   AddNavigationResultsToMatches(default_navigation_results_, false);
 
-  // Allow an additional match for "what you typed" if it's present.
-  const size_t max_total_matches = kMaxMatches + what_you_typed_size;
+  // Allow additional match(es) for verbatim results if present.
+  const size_t max_total_matches = kMaxMatches + verbatim_matches_size;
   std::partial_sort(matches_.begin(),
       matches_.begin() + std::min(max_total_matches, matches_.size()),
       matches_.end(), &AutocompleteMatch::MoreRelevant);
 
   // If the top match is effectively 'verbatim' but exceeds the calculated
   // verbatim relevance, and REQUESTED_URL |input_| has a |desired_tld|
   // (for example ".com" when the CTRL key is pressed for REQUESTED_URL input),
   // promote a URL_WHAT_YOU_TYPED match to the top. Otherwise, these matches can
   // stomp the HistoryURLProvider's similar transient URL_WHAT_YOU_TYPED match,
   // and CTRL+ENTER will invoke the search instead of the expected navigation.
@@ skipping 13 matching lines @@
 }
 
 bool SearchProvider::IsTopMatchScoreTooLow() const {
   return matches_.front().relevance < CalculateRelevanceForVerbatim();
 }
 
 bool SearchProvider::IsTopMatchHighRankSearchForURL() const {
   return input_.type() == AutocompleteInput::URL &&
          matches_.front().relevance > CalculateRelevanceForVerbatim() &&
          (matches_.front().type == AutocompleteMatch::SEARCH_SUGGEST ||
-          matches_.front().type == AutocompleteMatch::SEARCH_WHAT_YOU_TYPED);
+          matches_.front().type == AutocompleteMatch::SEARCH_WHAT_YOU_TYPED ||
+          matches_.front().type == AutocompleteMatch::SEARCH_OTHER_ENGINE);
 }
 
 bool SearchProvider::IsTopMatchNotInlinable() const {
   return matches_.front().type != AutocompleteMatch::SEARCH_WHAT_YOU_TYPED &&
          matches_.front().type != AutocompleteMatch::URL_WHAT_YOU_TYPED &&
+         matches_.front().type != AutocompleteMatch::SEARCH_OTHER_ENGINE &&

[msw, 2013/01/31 00:38:16]

I realize the only SEARCH_OTHER_ENGINE match at the moment is verbatim, and I
like how simple it is to check right now, but this may change in the future. You
should either ensure the SEARCH_OTHER_ENGINE match is a keyword verbatim, or add
a comment regarding this weakness in the check.

[Mark P, 2013/01/31 23:09:27]

I like this idea:
How do you suggest I do that?  It's not obvious to me that you do the analogous
thing and verify SEARCH_WHAT_YOU_TYPED is the default engine verbatim.

Some ideas:
In the code to create the keyword verbatim match, DCHECK that input.text() =
keyword + (some amount of spaces) + remaining_text?
Simply rename SEARCH_OTHER_ENGINE globally to what it actually means:
KEYWORD_SEARCH_WHAT_YOU_TYPED?
Assert in tests that there is only one one SEARCH_OTHER_ENGINE result?  (This
already happens implicitly because we explicitly check the type of each result,
and we EXPECT only one SEARCH_OTHER_ENGINE result.)

[msw, 2013/02/01 20:52:37]

Hmm, that might be a decent DCHECK to add there, regardless of this, but it may
be overkill to check that here right now.
It seems like you'd need to split SEARCH_OTHER_ENGINE into perhaps
SEARCH_KEYWORD and KEYWORD_WHAT_YOU_TYPED. If you do a mass-renaming, perhaps
replace *_WHAT_YOU_TYPED with *_VERBATIM :) 
That also sounds like a good idea for SearchProviderTest.

[Mark P, 2013/02/02 00:24:35]

Agreed that writing the DCHECK is overkill (lots of whitespace possibilities to
handle).  Instead, I'll make sure the tests handle whitespace.
Oooh, good point.  I don't want to do anything that'll require changing the UMA
logs data.  Pass.
Added more tests.

[msw, 2013/02/02 02:27:52]

It might not be too tough if you use .EndsWith, etc. Unless the trailing
whitespace issues would be problematic here too.
Too bad, that sounded nice.
I'd still very much like to see a stronger check or a mention somewhere (even a
comment/TODO here) that this assumes the SEARCH_OTHER_ENGINE result is a keyword
verbatim match, even though it's not checked.

[Mark P, 2013/02/04 19:46:09]

I tried to write a test that would work (verify the SEARCH_OTHER_ENGINE result
is effectively verbatim) but given the different ways the same keyword can be
expressed (bing.com vs www.bing.com versus http://bing.com etc.) I found it too
difficult without a ton of code. :(
Added comments here and also where the verbatim match is created.  Hopefully
these comments will be enough (given the lack of an explicit test).

[msw, 2013/02/04 22:51:39]

Please try using URLPrefix::BestURLPrefix.

[Mark P, 2013/02/05 00:54:36]

I was hoping you'd let me get away without this check. 

The ugly mass is now there in UpdateMatches().

          matches_.front().inline_autocomplete_offset == string16::npos &&
          matches_.front().fill_into_edit != input_.text();
 }
 
 void SearchProvider::UpdateMatches() {
   ConvertResultsToAutocompleteMatches();
 
   // Check constraints that may be violated by suggested relevances.
   if (!matches_.empty() &&
       (has_suggested_relevance_ || verbatim_relevance_ >= 0)) {
@@ skipping 11 matching lines @@
       // Disregard the suggested search and verbatim relevances if the input
       // type is URL and the top match is a highly-ranked search suggestion.
       // For example, prevent a search for "foo.com" from outranking another
       // provider's navigation for "foo.com" or "foo.com/url_from_history".
       ApplyCalculatedSuggestRelevance(&keyword_suggest_results_, true);
       ApplyCalculatedSuggestRelevance(&default_suggest_results_, false);
       verbatim_relevance_ = -1;
       ConvertResultsToAutocompleteMatches();
     }
     if (IsTopMatchNotInlinable()) {
-      // Disregard suggested relevances if the top match is not SWYT, inlinable,
-      // or URL_WHAT_YOU_TYPED (which may be top match regardless of inlining).
-      // For example, input "foo" should not invoke a search for "bar", which
-      // would happen if the "bar" search match outranked all other matches.
+      // Disregard suggested relevances if the top match is not a verbatim
+      // match, inlinable, or URL_WHAT_YOU_TYPED (which may be top match
+      // regardless of inlining).  For example, input "foo" should not
+      // invoke a search for "bar", which would happen if the "bar" search
+      // match outranked all other matches.
       ApplyCalculatedRelevance();
       ConvertResultsToAutocompleteMatches();
     }
     DCHECK(!IsTopMatchScoreTooLow());
     DCHECK(!IsTopMatchHighRankSearchForURL());
     DCHECK(!IsTopMatchNotInlinable());
   }
 
   UpdateStarredStateOfMatches();
   UpdateDone();
@@ skipping 164 matching lines @@
 
     case AutocompleteInput::URL:
       return 850;
 
     default:
       NOTREACHED();
       return 0;
   }
 }
 
+// static
+int SearchProvider::CalculateRelevanceForKeywordVerbatim(
+    AutocompleteInput::Type type,
+    bool prefer_keyword) {
+  if (prefer_keyword)
+    return 1500;
+  return type == AutocompleteInput::QUERY ? 1450 : 1100;
+}
+
 int SearchProvider::CalculateRelevanceForHistory(
     const Time& time,
     bool is_keyword,
     bool prevent_inline_autocomplete) const {
   // The relevance of past searches falls off over time. There are two distinct
   // equations used. If the first equation is used (searches to the primary
   // provider that we want to inline autocomplete), the score starts at 1399 and
   // falls to 1300. If the second equation is used the relevance of a search 15
   // minutes ago is discounted 50 points, while the relevance of a search two
   // weeks ago is discounted 450 points.
@@ skipping 198 matching lines @@
 }
 
 void SearchProvider::UpdateDone() {
   // We're done when the timer isn't running, there are no suggest queries
   // pending, and we're not waiting on instant.
   done_ = (!timer_.IsRunning() && (suggest_results_pending_ == 0) &&
            (instant_finalized_ ||
             (!chrome::BrowserInstantController::IsInstantEnabled(profile_) &&
              !chrome::search::IsInstantExtendedAPIEnabled(profile_))));
 }