Omnibox - Search Provider - Cleanup Keyword Mode's Legal Matches

chrome/browser/autocomplete/base_search_provider.cc

Index: chrome/browser/autocomplete/base_search_provider.cc
diff --git a/chrome/browser/autocomplete/base_search_provider.cc b/chrome/browser/autocomplete/base_search_provider.cc
index f93318df111d1886b0a8b6954c0a82f352d7acaf..e9388355b31021d0b93d60d55f8805a57e5b8df4 100644
--- a/chrome/browser/autocomplete/base_search_provider.cc
+++ b/chrome/browser/autocomplete/base_search_provider.cc
@@ -122,7 +122,8 @@ AutocompleteMatch BaseSearchProvider::CreateSearchSuggestion(
     const TemplateURL* template_url,
     const SearchTermsData& search_terms_data) {
   return CreateSearchSuggestion(
-      NULL, AutocompleteInput(), SearchSuggestionParser::SuggestResult(
+      NULL, AutocompleteInput(), from_keyword_provider,

[msw, 2014/08/15 18:32:00]

You're passing |from_keyword_provider| as |in_keyword_mode|, which seems wrong
(saying that we're in keyword mode iff the suggestion is from a keyword
provider, etc.). Please fix this, clarify the parameter names, or clarify the
reasoning for such behavior with a comment.

[Mark P, 2014/08/15 18:47:07]

I hoped the warning by this function's declaration in the header would be
enough.
// Returns a simpler AutocompleteMatch suitable for persistence like in
// ShortcutsDatabase.
// NOTE: Use with care. Most likely you want the other CreateSearchSuggestion
// with protected access.

Put a similar comment in the code.

[msw, 2014/08/15 19:06:06]

I saw the warning in the header, it's not bad, but I like your additional
specificity about it's use of default values; thanks.

+      SearchSuggestionParser::SuggestResult(
           suggestion, type, suggestion, base::string16(), base::string16(),
           base::string16(), base::string16(), std::string(), std::string(),
           from_keyword_provider, 0, false, false, base::string16()),
@@ -215,6 +216,7 @@ void BaseSearchProvider::SetDeletionURL(const std::string& deletion_url,
 AutocompleteMatch BaseSearchProvider::CreateSearchSuggestion(
     AutocompleteProvider* autocomplete_provider,
     const AutocompleteInput& input,
+    const bool in_keyword_mode,
     const SearchSuggestionParser::SuggestResult& suggestion,
     const TemplateURL* template_url,
     const SearchTermsData& search_terms_data,
@@ -247,6 +249,7 @@ AutocompleteMatch BaseSearchProvider::CreateSearchSuggestion(
 
   // suggestion.match_contents() should have already been collapsed.
   match.allowed_to_be_default_match =
+      (!in_keyword_mode || suggestion.from_keyword_provider()) &&
       (base::CollapseWhitespace(input.text(), false) ==
        suggestion.match_contents());
 
@@ -258,6 +261,7 @@ AutocompleteMatch BaseSearchProvider::CreateSearchSuggestion(
   if (suggestion.from_keyword_provider())
     match.fill_into_edit.append(match.keyword + base::char16(' '));
   if (!input.prevent_inline_autocomplete() &&
+      (!in_keyword_mode || suggestion.from_keyword_provider()) &&
       StartsWith(suggestion.suggestion(), input.text(), false)) {
     match.inline_autocompletion =
         suggestion.suggestion().substr(input.text().length());
@@ -408,9 +412,10 @@ void BaseSearchProvider::AddMatchToMap(
     const std::string& metadata,
     int accepted_suggestion,
     bool mark_as_deletable,
+    bool in_keyword_mode,
     MatchMap* map) {
   AutocompleteMatch match = CreateSearchSuggestion(
-      this, GetInput(result.from_keyword_provider()), result,
+      this, GetInput(result.from_keyword_provider()), in_keyword_mode, result,
       GetTemplateURL(result.from_keyword_provider()),
       UIThreadSearchTermsData(profile_), accepted_suggestion,
       ShouldAppendExtraParams(result));