Omnibox: Make the Controller Reorder Matches for Inlining

chrome/browser/autocomplete/keyword_provider.cc

Index: chrome/browser/autocomplete/keyword_provider.cc
diff --git a/chrome/browser/autocomplete/keyword_provider.cc b/chrome/browser/autocomplete/keyword_provider.cc
index dd2959e9eb41a0921a1f294c8847d3eeadf919ec..43b60280da5a877de23feec3df0b6bd38cc9dcd9 100644
--- a/chrome/browser/autocomplete/keyword_provider.cc
+++ b/chrome/browser/autocomplete/keyword_provider.cc
@@ -217,7 +217,7 @@ AutocompleteMatch KeywordProvider::CreateAutocompleteMatch(
     const AutocompleteInput& input) {
   return CreateAutocompleteMatch(
       GetTemplateURLService()->GetTemplateURLForKeyword(keyword), input,
-      keyword.length(), SplitReplacementStringFromInput(text, true), 0);
+      keyword.length(), SplitReplacementStringFromInput(text, true), true, 0);

[msw, 2013/07/23 21:55:33]

It's not obvious why this file has true or false sprinkled in to make some
matches able to be default and some not; can you clarify?

[Mark P, 2013/07/26 16:48:13]

Sure.  I'll clarify in this comment; you can decide if we need to add comments
in the code.

(1)
KeywordProvider::CreateAutocompleteMatch()
does
// Creates a fully marked-up AutocompleteMatch for a specific keyword.
according to the .h
This is only called by the omnibox_controller.
Suggesting a keyword should be inlineable.

(2) on line 322
is where we create an exact match for a specific keyword but with no remaining
text.
This should be inlineable too.  It's an exact keyword suggestion.

(3) on line 368
is where we create inexact matches for keywords.  (The user types the beginning
of a keyword.)
These could reasonably be considered inlineable but they are never scores higher
than 700 according to CalculateRelevance() so in practice were never inlineable.
 I'm marking them as such (not inlineable).  I agree the one is confusing; I
added a comment there.

(4) on line 578
See comment by 569-574.  These are suggestions from extensions.  They're given
scores less than the extension's exact what-you-typed match so these are never
expected/allowed to be the default match.

[msw, 2013/07/26 20:04:49]

I think your comments are insightful enough that they belong in the code, the
behavior isn't entirely obvious without them, but then again, I haven't worked
with KeywordProvider much.

[Mark P, 2013/07/26 23:55:24]

Added comments in those other three areas as requested.

 }
 
 void KeywordProvider::Start(const AutocompleteInput& input,
@@ -319,7 +319,7 @@ void KeywordProvider::Start(const AutocompleteInput& input,
     // TODO(pkasting): We should probably check that if the user explicitly
     // typed a scheme, that scheme matches the one in |template_url|.
     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) {
@@ -365,7 +365,7 @@ void KeywordProvider::Start(const AutocompleteInput& input,
     for (TemplateURLService::TemplateURLVector::const_iterator i(
          matches.begin()); i != matches.end(); ++i) {
       matches_.push_back(CreateAutocompleteMatch(
-          *i, input, keyword.length(), remaining_input, -1));
+          *i, input, keyword.length(), remaining_input, false, -1));
     }
   }
 }
@@ -420,6 +420,7 @@ AutocompleteMatch KeywordProvider::CreateAutocompleteMatch(
     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 =
@@ -442,6 +443,7 @@ AutocompleteMatch KeywordProvider::CreateAutocompleteMatch(
   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' ');
@@ -573,7 +575,7 @@ void KeywordProvider::Observe(int type,
             input.prefer_keyword(), input.allow_exact_keyword_match());
         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));