Omnibox: Create Keyword Verbatim Result in Search Provider

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 c37da20e228f36040e821a071aecc4df86b70668..47060eea701bd8569dee7865dbd553619a047946 100644
--- a/chrome/browser/autocomplete/keyword_provider.cc
+++ b/chrome/browser/autocomplete/keyword_provider.cc
@@ -286,51 +286,62 @@ void KeywordProvider::Start(const AutocompleteInput& input,
   // front of our vector.
   if (keyword_matches.front() == keyword) {
     const TemplateURL* template_url(model->GetTemplateURLForKeyword(keyword));

[Bart N., 2013/01/29 18:50:42]

That's a weird way of initializing. Why no simply " = " as above and elsewhere?

[Mark P, 2013/01/30 19:46:21]

Done.

-    // 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 (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();
-      }
+    const bool is_extension_keyword =
+        profile_ && template_url->IsExtensionKeyword();
+    if (remaining_input.empty() || is_extension_keyword) {

[Bart N., 2013/01/29 18:50:42]

This is getting ugly (many nested ifs and not to mention size of this method
which definitely exceeds recommended 25 limit).

Can you simplify it by negating the condition and returning, i.e.:
if (!remaining_input.empty()  && !is_extension_keyword)
  return;

[Mark P, 2013/01/30 19:46:21]

Done.

+      // We 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., 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.
+
+      // 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));
 
-      extensions::ApplyDefaultSuggestionForExtensionKeyword(
-          profile_, template_url,
-          remaining_input,
-          &matches_[0]);
+      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();
+        }
 
-      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);
+        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) {
+          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.
+          if (have_listeners)
+            done_ = false;
         }
-      } else if (input.matches_requested() == 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.
-        if (have_listeners)
-          done_ = false;
       }
     }
-  } else {
+  } else {  // no exact keyword match
     if (keyword_matches.size() > kMaxMatches) {
       keyword_matches.erase(keyword_matches.begin() + kMaxMatches,
                             keyword_matches.end());