Misc. cleanup:

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/strings/string16.h"
@@ skipping 68 matching lines @@
 // Helper functor for Start(), for sorting keyword matches by quality.
 class CompareQuality {
  public:
   // A keyword is of higher quality when a greater fraction of it has been
   // typed, that is, when it is shorter.
   //
   // TODO(pkasting): http://b/740691 Most recent and most frequent keywords are
   // probably better rankings than the fraction of the keyword typed.  We should
   // always put any exact matches first no matter what, since the code in
   // Start() assumes this (and it makes sense).
-  bool operator()(const string16& keyword1,
-                  const string16& keyword2) const {
-    return keyword1.length() < keyword2.length();
+  bool operator()(const TemplateURL* t_url1, const TemplateURL* t_url2) const {
+    return t_url1->keyword().length() < t_url2->keyword().length();
   }
 };
 
 // We need our input IDs to be unique across all profiles, so we keep a global
 // UID that each provider uses.
 static int global_input_uid_;
 
 }  // namespace
 
 // static
@@ skipping 107 matching lines @@
       return string16();
   }
 
   return keyword;
 }
 
 AutocompleteMatch KeywordProvider::CreateAutocompleteMatch(
     const string16& text,
     const string16& keyword,
     const AutocompleteInput& input) {
-  return CreateAutocompleteMatch(GetTemplateURLService(), keyword, input,
-      keyword.size(), SplitReplacementStringFromInput(text, true), 0);
+  return CreateAutocompleteMatch(
+      GetTemplateURLService()->GetTemplateURLForKeyword(keyword), input,
+      keyword.length(), SplitReplacementStringFromInput(text, true), 0);
 }
 
 void KeywordProvider::Start(const AutocompleteInput& input,
                             bool minimal_changes) {
   // This object ensures we end keyword mode if we exit the function without
   // toggling keyword mode to on.
   ScopedEndExtensionKeywordMode keyword_mode_toggle(this);
 
   matches_.clear();
 
@@ skipping 15 matching lines @@
   // whatever we do here!
   //
   // TODO(pkasting): http://b/1112681 If someday we remember usage frequency for
   // keywords, we might suggest keywords that haven't even been partially typed,
   // if the user uses them enough and isn't obviously typing something else.  In
   // this case we'd consider all input here to be query input.
   string16 keyword, remaining_input;
   if (!ExtractKeywordFromInput(input, &keyword, &remaining_input))
     return;
 
-  TemplateURLService* model = GetTemplateURLService();
-
   // Get the best matches for this keyword.
   //
   // NOTE: We could cache the previous keywords and reuse them here in the
   // |minimal_changes| case, but since we'd still have to recalculate their
   // relevances and we can just recreate the results synchronously anyway, we
   // don't bother.
   //
   // TODO(pkasting): http://b/893701 We should remember the user's use of a
   // search query both from the autocomplete popup and from web pages
   // themselves.
-  std::vector<string16> keyword_matches;
-  model->FindMatchingKeywords(keyword,
-                              !remaining_input.empty(),
-                              &keyword_matches);
+  TemplateURLService::TemplateURLVector matches;
+  GetTemplateURLService()->FindMatchingKeywords(
+      keyword, !remaining_input.empty(), &matches);
 
-  for (std::vector<string16>::iterator i(keyword_matches.begin());
-       i != keyword_matches.end(); ) {
-    const TemplateURL* template_url = model->GetTemplateURLForKeyword(*i);
+  for (TemplateURLService::TemplateURLVector::iterator i(matches.begin());
+       i != matches.end(); ) {
+    const TemplateURL* template_url = *i;
 
     // Prune any extension keywords that are disallowed in incognito mode (if
     // we're incognito), or disabled.
     if (profile_ && template_url->IsExtensionKeyword()) {
       ExtensionService* service = extensions::ExtensionSystem::Get(profile_)->
           extension_service();
-      const extensions::Extension* extension = service->GetExtensionById(
-          template_url->GetExtensionId(), false);
+      const extensions::Extension* extension =
+          service->GetExtensionById(template_url->GetExtensionId(), false);
       bool enabled =
           extension && (!profile_->IsOffTheRecord() ||
                         service->IsIncognitoEnabled(extension->id()));
       if (!enabled) {
-        i = keyword_matches.erase(i);
+        i = matches.erase(i);
         continue;
       }
     }
 
     // Prune any substituting keywords if there is no substitution.
     if (template_url->SupportsReplacement() && remaining_input.empty() &&
         !input.allow_exact_keyword_match()) {
-      i = keyword_matches.erase(i);
+      i = matches.erase(i);
       continue;
     }
 
     ++i;
   }
-  if (keyword_matches.empty())
+  if (matches.empty())
     return;
-  std::sort(keyword_matches.begin(), keyword_matches.end(), CompareQuality());
+  std::sort(matches.begin(), 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);
+  if (matches.front()->keyword() == keyword) {
+    const TemplateURL* template_url = matches.front();
     const bool is_extension_keyword = template_url->IsExtensionKeyword();
 
     // 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., a regular keyword that
     // supports replacement and that has extra text following it),
     // then SearchProvider creates 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));
+    matches_.push_back(CreateAutocompleteMatch(
+        template_url, input, keyword.length(), remaining_input, -1));
 
     if (profile_ && 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();
       }
 
@@ skipping 20 matching lines @@
               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 (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));
+    if (matches.size() > kMaxMatches)
+      matches.erase(matches.begin() + kMaxMatches, matches.end());
+    for (TemplateURLService::TemplateURLVector::const_iterator i(
+         matches.begin()); i != matches.end(); ++i) {
+      matches_.push_back(CreateAutocompleteMatch(
+          *i, input, keyword.length(), remaining_input, -1));
     }
   }
 }
 
 void KeywordProvider::Stop(bool clear_cached_results) {
   done_ = true;
   MaybeEndExtensionKeywordMode();
 }
 
 KeywordProvider::~KeywordProvider() {}
 
 // static
 bool KeywordProvider::ExtractKeywordFromInput(const AutocompleteInput& input,
                                               string16* keyword,
                                               string16* remaining_input) {
   if ((input.type() == AutocompleteInput::INVALID) ||
       (input.type() == AutocompleteInput::FORCED_QUERY))
     return false;
 
   string16 trimmed_input;
   TrimWhitespace(input.text(), TRIM_TRAILING, &trimmed_input);
   *keyword = TemplateURLService::CleanUserInputKeyword(
       SplitKeywordFromInput(trimmed_input, true, remaining_input));
   return !keyword->empty();
 }
 
 // static
-void KeywordProvider::FillInURLAndContents(
-    const string16& remaining_input,
-    const TemplateURL* element,
-    AutocompleteMatch* match) {
+int KeywordProvider::CalculateRelevance(AutocompleteInput::Type type,
+                                        bool complete,
+                                        bool supports_replacement,
+                                        bool prefer_keyword,
+                                        bool allow_exact_keyword_match) {
+  // This function is responsible for scoring suggestions of keywords
+  // themselves and the suggestion of the verbatim query on an
+  // extension keyword.  SearchProvider::CalculateRelevanceForKeywordVerbatim()
+  // scores verbatim query suggestions for non-extension keywords.
+  // These two functions are currently in sync, but there's no reason
+  // we couldn't decide in the future to score verbatim matches
+  // differently for extension and non-extension keywords.  If you
+  // make such a change, however, you should update this comment to
+  // describe it, so it's clear why the functions diverge.
+  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;
+}
+
+// static
+void KeywordProvider::FillInURLAndContents(const string16& remaining_input,
+                                           const TemplateURL* element,
+                                           AutocompleteMatch* match) {
   DCHECK(!element->short_name().empty());
   const TemplateURLRef& element_ref = element->url_ref();
   DCHECK(element_ref.IsValid());
   int message_id = element->IsExtensionKeyword() ?
       IDS_EXTENSION_KEYWORD_COMMAND : IDS_KEYWORD_SEARCH;
   if (remaining_input.empty()) {
     // Allow extension keyword providers to accept empty string input. This is
     // useful to allow extensions to do something in the case where no input is
     // entered.
     if (element_ref.SupportsReplacement() && !element->IsExtensionKeyword()) {
@@ skipping 18 matching lines @@
     // input, but we rely on later canonicalization functions to do more
     // fixup to make the URL valid if necessary.
     DCHECK(element_ref.SupportsReplacement());
     match->destination_url = GURL(element_ref.ReplaceSearchTerms(
         TemplateURLRef::SearchTermsArgs(remaining_input)));
     std::vector<size_t> content_param_offsets;
     match->contents.assign(l10n_util::GetStringFUTF16(message_id,
                                                       element->short_name(),
                                                       remaining_input,
                                                       &content_param_offsets));
-    if (content_param_offsets.size() == 2) {
-      AutocompleteMatch::ClassifyLocationInString(content_param_offsets[1],
-          remaining_input.length(), match->contents.length(),
-          ACMatchClassification::NONE, &match->contents_class);
-    } else {
-      // See comments on an identical NOTREACHED() in search_provider.cc.
-      NOTREACHED();
-    }
+    DCHECK_EQ(2U, content_param_offsets.size());
+    AutocompleteMatch::ClassifyLocationInString(content_param_offsets[1],
+        remaining_input.length(), match->contents.length(),
+        ACMatchClassification::NONE, &match->contents_class);
   }
 }
 
-// static
-int KeywordProvider::CalculateRelevance(AutocompleteInput::Type type,
-                                        bool complete,
-                                        bool supports_replacement,
-                                        bool prefer_keyword,
-                                        bool allow_exact_keyword_match) {
-  // This function is responsible for scoring suggestions of keywords
-  // themselves and the suggestion of the verbatim query on an
-  // extension keyword.  SearchProvider::CalculateRelevanceForKeywordVerbatim()
-  // scores verbatim query suggestions for non-extension keywords.
-  // These two functions are currently in sync, but there's no reason
-  // we couldn't decide in the future to score verbatim matches
-  // differently for extension and non-extension keywords.  If you
-  // make such a change, however, you should update this comment to
-  // describe it, so it's clear why the functions diverge.
-  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;
-}
-
 AutocompleteMatch KeywordProvider::CreateAutocompleteMatch(
-    TemplateURLService* model,
-    const string16& keyword,
+    const TemplateURL* template_url,
     const AutocompleteInput& input,
     size_t prefix_length,
     const string16& remaining_input,
     int relevance) {
-  DCHECK(model);
-  // Get keyword data from data store.
-  TemplateURL* element = model->GetTemplateURLForKeyword(keyword);
-  DCHECK(element);
-  const bool supports_replacement = element->url_ref().SupportsReplacement();
+  DCHECK(template_url);
+  const bool supports_replacement =
+      template_url->url_ref().SupportsReplacement();
 

[Jered, 2013/06/28 21:57:28]

Consider const string16& keyword = template_url()->keyword() here since it's
used a bunch.

[Peter Kasting, 2013/06/28 22:16:02]

Done.

   // Create an edit entry of "[keyword] [remaining input]".  This is helpful
   // even when [remaining input] is empty, as the user can select the popup
   // choice and immediately begin typing in query input.
-  const bool keyword_complete = (prefix_length == keyword.length());
+  const bool keyword_complete =
+      (prefix_length == template_url->keyword().length());
   if (relevance < 0) {
     relevance =
         CalculateRelevance(input.type(), keyword_complete,
                            // When the user wants keyword matches to take
                            // preference, score them highly regardless of
                            // whether the input provides query text.
                            supports_replacement, input.prefer_keyword(),
                            input.allow_exact_keyword_match());
   }
   AutocompleteMatch match(this, relevance, false,
       supports_replacement ? AutocompleteMatchType::SEARCH_OTHER_ENGINE :
                              AutocompleteMatchType::HISTORY_KEYWORD);
-  match.fill_into_edit.assign(keyword);
+  match.fill_into_edit = template_url->keyword();
   if (!remaining_input.empty() || !keyword_complete || supports_replacement)
     match.fill_into_edit.push_back(L' ');
   match.fill_into_edit.append(remaining_input);
   // If we wanted to set |result.inline_autocomplete_offset| correctly, we'd
   // need CleanUserInputKeyword() to return the amount of adjustment it's made
   // to the user's input.  Because right now inexact keyword matches can't score
   // more highly than a "what you typed" match from one of the other providers,
   // we just don't bother to do this, and leave inline autocompletion off.
   match.inline_autocomplete_offset = string16::npos;
 
   // Create destination URL and popup entry content by substituting user input
   // into keyword templates.
-  FillInURLAndContents(remaining_input, element, &match);
+  FillInURLAndContents(remaining_input, template_url, &match);
 
-  match.keyword = keyword;
+  match.keyword = template_url->keyword();
   match.transition = content::PAGE_TRANSITION_KEYWORD;
 
   return match;
 }
 
 void KeywordProvider::Observe(int type,
                               const content::NotificationSource& source,
                               const content::NotificationDetails& details) {
   TemplateURLService* model = GetTemplateURLService();
   const AutocompleteInput& input = extension_suggest_last_input_;
@@ skipping 44 matching lines @@
         const omnibox_api::SuggestResult& suggestion =
             *suggestions.suggest_results[i];
         // We want to order these suggestions in descending order, so start with
         // the relevance of the first result (added synchronously in Start()),
         // and subtract 1 for each subsequent suggestion from the extension.
         // We know that |complete| is true, because we wouldn't get results from
         // the extension unless the full keyword had been typed.
         int first_relevance = CalculateRelevance(input.type(), true, true,
             input.prefer_keyword(), input.allow_exact_keyword_match());
         extension_suggest_matches_.push_back(CreateAutocompleteMatch(
-            model, keyword, input, keyword.length(),
+            model->GetTemplateURLForKeyword(keyword), input, keyword.length(),

[Jered, 2013/06/28 21:57:28]

Move this outside the loop.

[Peter Kasting, 2013/06/28 22:16:02]

Done.  Nice catch.

             UTF8ToUTF16(suggestion.content), first_relevance - (i + 1)));
 
         AutocompleteMatch* match = &extension_suggest_matches_.back();
         match->contents.assign(UTF8ToUTF16(suggestion.description));
         match->contents_class =
             extensions::StyleTypesToACMatchClassifications(suggestion);
         match->description.clear();
         match->description_class.clear();
       }
 
@@ skipping 30 matching lines @@
 }
 
 void KeywordProvider::MaybeEndExtensionKeywordMode() {
   if (!current_keyword_extension_id_.empty()) {
     extensions::ExtensionOmniboxEventRouter::OnInputCancelled(
         profile_, current_keyword_extension_id_);
 
     current_keyword_extension_id_.clear();
   }
 }