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));

[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));
+    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.
 
-    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();
-      }
+      // 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());
     }
     for (std::vector<string16>::const_iterator i(keyword_matches.begin());
          i != keyword_matches.end(); ++i) {
       matches_.push_back(CreateAutocompleteMatch(model, *i,
                                                  input, keyword.length(),
                                                  remaining_input, -1));
     }
@@ skipping 242 matching lines @@
 }
 
 void KeywordProvider::MaybeEndExtensionKeywordMode() {
   if (!current_keyword_extension_id_.empty()) {
     extensions::ExtensionOmniboxEventRouter::OnInputCancelled(
         profile_, current_keyword_extension_id_);
 
     current_keyword_extension_id_.clear();
   }
 }