Omnibox: Create Keyword Verbatim Result in Search Provider

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/string16.h"
@@ skipping 267 matching lines @@
   }
   if (keyword_matches.empty())
     return;
   std::sort(keyword_matches.begin(), keyword_matches.end(), CompareQuality());
 
   // Limit to one exact or three inexact matches, and mark them up for display
   // in the autocomplete popup.
   // Any exact match is going to be the highest quality match, and thus at the
   // front of our vector.
   if (keyword_matches.front() == keyword) {
-    const TemplateURL* template_url(model->GetTemplateURLForKeyword(keyword));
+    const TemplateURL* template_url = model->GetTemplateURLForKeyword(keyword);
+    const bool is_extension_keyword =
+        profile_ && template_url->IsExtensionKeyword();

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

Is |profile_| actually required for extension keywords?
(or was that a separate condition in the old check below?)

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

That was a condition in the original if test so I thought it was worth keeping.

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

I see that, but I asked if that was a separate condition. The question is, how
does the presence of |profile_| factor into your new check on line 297? If it's
unrelated, just keep it as a separate check in the conditional at line 306.

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

It's unrelated.  Moved as you suggested.

+
+    // We only create an exact match if remaining_input is empty or

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

optional nits: |remaining_input| here and below and I prefer passive voice.

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

Done.

+    // if this is an extension keyword; if remaining_input is a non-empty
+    // non-extension keyword (i.e., we have a regular keyword that
+    // supports replacement and has extra text following it), then we
+    // let SearchProvider create 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|.
     matches_.push_back(CreateAutocompleteMatch(model, keyword, input,
                                                keyword.length(),
                                                remaining_input, -1));
 
-    if (profile_ && template_url->IsExtensionKeyword()) {
+    if (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();
       }
 
       extensions::ApplyDefaultSuggestionForExtensionKeyword(
           profile_, template_url,
           remaining_input,
           &matches_[0]);
 
       if (minimal_changes &&
           (input.matches_requested() != AutocompleteInput::BEST_MATCH)) {
         // If the input hasn't significantly changed, we can just use the
         // suggestions from last time. We need to readjust the relevance to
         // ensure it is less than the main match's relevance.
         for (size_t i = 0; i < extension_suggest_matches_.size(); ++i) {
           matches_.push_back(extension_suggest_matches_[i]);
           matches_.back().relevance = matches_[0].relevance - (i + 1);
         }
-      } else if (input.matches_requested() == AutocompleteInput::ALL_MATCHES) {
+      } else if (input.matches_requested() ==

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

optional nit: don't split this line

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

Done.

+                 AutocompleteInput::ALL_MATCHES) {
         extension_suggest_last_input_ = input;
         extension_suggest_matches_.clear();
 
         bool have_listeners =
           extensions::ExtensionOmniboxEventRouter::OnInputChanged(
               profile_, template_url->GetExtensionId(),
               UTF16ToUTF8(remaining_input), current_input_id_);
 
         // We only have to wait for suggest results if there are actually
         // extensions listening for input changes.
@@ skipping 85 matching lines @@
           remaining_input.length(), match->contents.length(),
           ACMatchClassification::NONE, &match->contents_class);
     } else {
       // See comments on an identical NOTREACHED() in search_provider.cc.
       NOTREACHED();
     }
   }
 }
 
 // static
 int KeywordProvider::CalculateRelevance(AutocompleteInput::Type type,

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

optional nit: add a decl comment that these scores should stay in sync with
SearchProvider::CalculateRelevanceForKeywordVerbatim.

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

It doesn't have to stay in sync.  See the last paragraph in the changelist
description.

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

It still seems odd if they diverged, but I don't really understand why they
should or shouldn't. Perhaps just mention the relationship?

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

Done.

                                         bool complete,
                                         bool supports_replacement,
                                         bool prefer_keyword,
                                         bool allow_exact_keyword_match) {
   if (!complete)
     return (type == AutocompleteInput::URL) ? 700 : 450;
   if (!supports_replacement || (allow_exact_keyword_match && prefer_keyword))
     return 1500;
   return (allow_exact_keyword_match && (type == AutocompleteInput::QUERY)) ?
       1450 : 1100;
@@ skipping 151 matching lines @@
 }
 
 void KeywordProvider::MaybeEndExtensionKeywordMode() {
   if (!current_keyword_extension_id_.empty()) {
     extensions::ExtensionOmniboxEventRouter::OnInputCancelled(
         profile_, current_keyword_extension_id_);
 
     current_keyword_extension_id_.clear();
   }
 }