Omnibox: Make the Controller Reorder Matches for Inlining

chrome/browser/autocomplete/keyword_provider.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/keyword_provider.h"
 
 #include <algorithm>
 #include <vector>
 
 #include "base/strings/string16.h"
@@ skipping 197 matching lines @@
       return string16();
   }
 
   return keyword;
 }
 
 AutocompleteMatch KeywordProvider::CreateAutocompleteMatch(
     const string16& text,
     const string16& keyword,
     const AutocompleteInput& input) {
+  // This enclosing function is only called by omnibox_controller.cc to create
+  // a what-you-typed match on the keyword input.  It's okay to allow this
+  // match to be the default match.
   return CreateAutocompleteMatch(
       GetTemplateURLService()->GetTemplateURLForKeyword(keyword), input,
-      keyword.length(), SplitReplacementStringFromInput(text, true), 0);
+      keyword.length(), SplitReplacementStringFromInput(text, true), true, 0);
 }
 
 void KeywordProvider::Start(const AutocompleteInput& input,
                             bool minimal_changes) {
   // This object ensures we end keyword mode if we exit the function without
   // toggling keyword mode to on.
   ScopedEndExtensionKeywordMode keyword_mode_toggle(this);
 
   matches_.clear();
 
@@ skipping 80 matching lines @@
     // Only create an exact match if |remaining_input| is empty or if
     // this is an extension keyword.  If |remaining_input| is a
     // non-empty non-extension keyword (i.e., a regular keyword that
     // supports replacement and that has extra text following it),
     // then SearchProvider creates the exact (a.k.a. verbatim) match.
     if (!remaining_input.empty() && !is_extension_keyword)
       return;
 
     // TODO(pkasting): We should probably check that if the user explicitly
     // typed a scheme, that scheme matches the one in |template_url|.
+
+    // When creating an exact match (either for the keyword itself, no
+    // remaining query or an extension keyword, possibly with remaining
+    // input), allow the match to be the default match.
     matches_.push_back(CreateAutocompleteMatch(
-        template_url, input, keyword.length(), remaining_input, -1));
+        template_url, input, keyword.length(), remaining_input, true, -1));
 
     if (profile_ && is_extension_keyword) {
       if (input.matches_requested() == AutocompleteInput::ALL_MATCHES) {
         if (template_url->GetExtensionId() != current_keyword_extension_id_)
           MaybeEndExtensionKeywordMode();
         if (current_keyword_extension_id_.empty())
           EnterExtensionKeywordMode(template_url->GetExtensionId());
         keyword_mode_toggle.StayInKeywordMode();
       }
 
@@ skipping 24 matching lines @@
         // extensions listening for input changes.
         if (have_listeners)
           done_ = false;
       }
     }
   } else {
     if (matches.size() > kMaxMatches)
       matches.erase(matches.begin() + kMaxMatches, matches.end());
     for (TemplateURLService::TemplateURLVector::const_iterator i(
          matches.begin()); i != matches.end(); ++i) {
+      // These inexact (prefix) keyword matches probably could be allowed
+      // to be the default match but in practice they're always outranked
+      // by a verbatim match (a query from SearchProvider from non-URL-like
+      // inputs or a URL-what-you-typed from HistoryURLProvider for URL-like
+      // inputs).  We set allowed_to_be_default_match to false to mirror
+      // this implicit consequence of the relevance scoring code.

[Peter Kasting, 2013/08/06 22:56:16]

How could these be allowed to be default?  Wouldn't we need to inline
autocomplete them to do so?  And then we still wouldn't have a navigable match
because there would be no search terms.  Right?  Shouldn't all that be the
rationale for why these shouldn't be default?

[Mark P, 2013/08/07 00:44:31]

Good points.  (I misunderstand something about how destinations worked when you
typed an exact URL.)

Removed the comment.  I think the code is clear without a comment.

       matches_.push_back(CreateAutocompleteMatch(
-          *i, input, keyword.length(), remaining_input, -1));
+          *i, input, keyword.length(), remaining_input, false, -1));
     }
   }
 }
 
 void KeywordProvider::Stop(bool clear_cached_results) {
   done_ = true;
   MaybeEndExtensionKeywordMode();
 }
 
 KeywordProvider::~KeywordProvider() {}
@@ skipping 34 matching lines @@
     return 1500;
   return (allow_exact_keyword_match && (type == AutocompleteInput::QUERY)) ?
       1450 : 1100;
 }
 
 AutocompleteMatch KeywordProvider::CreateAutocompleteMatch(
     const TemplateURL* template_url,
     const AutocompleteInput& input,
     size_t prefix_length,
     const string16& remaining_input,
+    bool allowed_to_be_default_match,
     int relevance) {
   DCHECK(template_url);
   const bool supports_replacement =
       template_url->url_ref().SupportsReplacement();
 
   // Create an edit entry of "[keyword] [remaining input]".  This is helpful
   // even when [remaining input] is empty, as the user can select the popup
   // choice and immediately begin typing in query input.
   const string16& keyword = template_url->keyword();
   const bool keyword_complete = (prefix_length == keyword.length());
   if (relevance < 0) {
     relevance =
         CalculateRelevance(input.type(), keyword_complete,
                            // When the user wants keyword matches to take
                            // preference, score them highly regardless of
                            // whether the input provides query text.
                            supports_replacement, input.prefer_keyword(),
                            input.allow_exact_keyword_match());
   }
   AutocompleteMatch match(this, relevance, false,
       supports_replacement ? AutocompleteMatchType::SEARCH_OTHER_ENGINE :
                              AutocompleteMatchType::HISTORY_KEYWORD);
+  match.allowed_to_be_default_match = allowed_to_be_default_match;
   match.fill_into_edit = keyword;
   if (!remaining_input.empty() || !keyword_complete || supports_replacement)
     match.fill_into_edit.push_back(L' ');
   match.fill_into_edit.append(remaining_input);
   // If we wanted to set |result.inline_autocompletion| correctly, we'd need
   // CleanUserInputKeyword() to return the amount of adjustment it's made to
   // the user's input.  Because right now inexact keyword matches can't score
   // more highly than a "what you typed" match from one of the other providers,
   // we just don't bother to do this, and leave inline autocompletion off.
 
@@ skipping 105 matching lines @@
           model->GetTemplateURLForKeyword(keyword);
 
       // TODO(mpcomplete): consider clamping the number of suggestions to
       // AutocompleteProvider::kMaxMatches.
       for (size_t i = 0; i < suggestions.suggest_results.size(); ++i) {
         const omnibox_api::SuggestResult& suggestion =
             *suggestions.suggest_results[i];
         // We want to order these suggestions in descending order, so start with
         // the relevance of the first result (added synchronously in Start()),
         // and subtract 1 for each subsequent suggestion from the extension.
-        // We know that |complete| is true, because we wouldn't get results from
-        // the extension unless the full keyword had been typed.
+        // We recompute the first match's relevance; we know that |complete|
+        // is true, because we wouldn't get results from the extension unless
+        // the full keyword had been typed.
         int first_relevance = CalculateRelevance(input.type(), true, true,
             input.prefer_keyword(), input.allow_exact_keyword_match());
+        // Since these matches score less than the first match, they should
+        // never become the default match.  Mark them as such.

[Peter Kasting, 2013/08/06 22:56:16]

It seems like the real reason we want these to not be default is because they're
async.  Scoring less than the first match was the original way of ensuring that,
so it's more parallel to (rather than causative of) marking "can't be default"
here.

[Mark P, 2013/08/07 00:44:31]

Rewrote comment with that in mind.

         extension_suggest_matches_.push_back(CreateAutocompleteMatch(
             template_url, input, keyword.length(),
-            UTF8ToUTF16(suggestion.content), first_relevance - (i + 1)));
+            UTF8ToUTF16(suggestion.content), false, first_relevance - (i + 1)));
 
         AutocompleteMatch* match = &extension_suggest_matches_.back();
         match->contents.assign(UTF8ToUTF16(suggestion.description));
         match->contents_class =
             extensions::StyleTypesToACMatchClassifications(suggestion);
         match->description.clear();
         match->description_class.clear();
       }
 
       done_ = true;
@@ skipping 29 matching lines @@
 }
 
 void KeywordProvider::MaybeEndExtensionKeywordMode() {
   if (!current_keyword_extension_id_.empty()) {
     extensions::ExtensionOmniboxEventRouter::OnInputCancelled(
         profile_, current_keyword_extension_id_);
 
     current_keyword_extension_id_.clear();
   }
 }