Optimize shortcuts provider

components/omnibox/browser/shortcuts_provider.cc

Index: components/omnibox/browser/shortcuts_provider.cc
diff --git a/components/omnibox/browser/shortcuts_provider.cc b/components/omnibox/browser/shortcuts_provider.cc
index 22ee0c71543b97dc00a86e9acc4582fa098805e2..83692427b354fad34f52cba659807e645653aaf5 100644
--- a/components/omnibox/browser/shortcuts_provider.cc
+++ b/components/omnibox/browser/shortcuts_provider.cc
@@ -28,7 +28,9 @@
 #include "components/omnibox/browser/autocomplete_provider_client.h"
 #include "components/omnibox/browser/autocomplete_result.h"
 #include "components/omnibox/browser/history_provider.h"
+#include "components/omnibox/browser/match_compare.h"
 #include "components/omnibox/browser/omnibox_field_trial.h"
+#include "components/omnibox/browser/shortcut_match.h"
 #include "components/omnibox/browser/url_prefix.h"
 #include "components/prefs/pref_service.h"
 #include "components/url_formatter/url_fixer.h"
@@ -138,6 +140,8 @@ void ShortcutsProvider::GetMatches(const AutocompleteInput& input) {
     max_relevance = kShortcutsProviderDefaultMaxRelevance;
   TemplateURLService* template_url_service = client_->GetTemplateURLService();
   const base::string16 fixed_up_input(FixupUserInput(input).second);
+
+  ShortcutMatches shortcut_matches;
   for (ShortcutsBackend::ShortcutMap::const_iterator it =
            FindFirstMatch(term_string, backend.get());
        it != backend->shortcuts_map().end() &&
@@ -147,10 +151,16 @@ void ShortcutsProvider::GetMatches(const AutocompleteInput& input) {
     // Don't return shortcuts with zero relevance.
     int relevance = CalculateScore(term_string, it->second, max_relevance);
     if (relevance) {
-      matches_.push_back(
-          ShortcutToACMatch(it->second, relevance, input, fixed_up_input));
-      matches_.back().ComputeStrippedDestinationURL(
-          input, client_->GetAcceptLanguages(), template_url_service);
+      const ShortcutsDatabase::Shortcut& shortcut = it->second;
+      GURL stripped_destination_url(AutocompleteMatch::GURLToStrippedGURL(
+          shortcut.match_core.destination_url,
+          input,
+          languages_,
+          template_url_service,
+          shortcut.match_core.keyword));
+      shortcut_matches.push_back(ShortcutMatch(relevance,
+                                               stripped_destination_url,
+                                               &(it->second)));

[Peter Kasting, 2016/04/12 23:29:50]

Nit: Shorter:

      shortcut_matches.push_back(
          ShortcutMatch(relevance, stripped_destination_url, &(it->second)));

[Alexander Yashkin, 2016/04/13 09:29:36]

Done.

     }
   }
   // Remove duplicates.  This is important because it's common to have multiple
@@ -164,37 +174,46 @@ void ShortcutsProvider::GetMatches(const AutocompleteInput& input) {
   // |duplicate_matches| field--duplicates don't need to be preserved in the
   // matches because they are only used for deletions, and this provider
   // deletes matches based on the URL.
-  AutocompleteResult::DedupMatchesByDestination(
-      input.current_page_classification(), false, &matches_);
+  DedupShortcutMatchesByDestination(input.current_page_classification(),
+                                    &shortcut_matches);

[Peter Kasting, 2016/04/12 23:29:50]

After removing the call to AutocompleteResult::DedupMatchesByDestination(), that
function's lone non-test caller passes |true| for the second argument, so that
function should be simplified to eliminate the second arg (assuming it's always
true) and the unittest location that calls it updated to not rely on the |false|
behavior.

[Alexander Yashkin, 2016/04/13 09:29:36]

Done.

   // Find best matches.
   std::partial_sort(
-      matches_.begin(),
-      matches_.begin() +
-          std::min(AutocompleteProvider::kMaxMatches, matches_.size()),
-      matches_.end(), &AutocompleteMatch::MoreRelevant);
-  if (matches_.size() > AutocompleteProvider::kMaxMatches) {
-    matches_.erase(matches_.begin() + AutocompleteProvider::kMaxMatches,
-                   matches_.end());
+      shortcut_matches.begin(),
+      shortcut_matches.begin() +
+          std::min(AutocompleteProvider::kMaxMatches, shortcut_matches.size()),
+      shortcut_matches.end(),
+      ShortcutMatch::MoreRelevant);
+  if (shortcut_matches.size() > AutocompleteProvider::kMaxMatches) {
+    shortcut_matches.erase(
+        shortcut_matches.begin() + AutocompleteProvider::kMaxMatches,
+        shortcut_matches.end());
   }
-  // Guarantee that all scores are decreasing (but do not assign any scores
-  // below 1).
-  for (ACMatches::iterator it = matches_.begin(); it != matches_.end(); ++it) {
-    max_relevance = std::min(max_relevance, it->relevance);
-    it->relevance = max_relevance;
+  // Create and initialize autocomplete matches from shortcut matches.
+  // Also guarantee that all relevance scores are decreasing
+  // (but do not assign any scores below 1).
+  WordMap terms_map(CreateWordMapForString(term_string));
+  matches_.reserve(shortcut_matches.size());
+  for (ShortcutMatch& match : shortcut_matches) {
+    max_relevance = std::min(max_relevance, match.relevance);
+    match.relevance = max_relevance;
     if (max_relevance > 1)
       --max_relevance;
+    matches_.push_back(ShortcutMatchToACMatch(match, input, fixed_up_input,
+                                              term_string, terms_map));
   }
 }
 
-AutocompleteMatch ShortcutsProvider::ShortcutToACMatch(
-    const ShortcutsDatabase::Shortcut& shortcut,
-    int relevance,
+AutocompleteMatch ShortcutsProvider::ShortcutMatchToACMatch(
+    const ShortcutMatch& shortcut_match,
     const AutocompleteInput& input,
-    const base::string16& fixed_up_input_text) {
+    const base::string16& fixed_up_input_text,
+    const base::string16 term_string,
+    const WordMap& terms_map) {
   DCHECK(!input.text().empty());
+  const ShortcutsDatabase::Shortcut& shortcut = *shortcut_match.shortcut;
   AutocompleteMatch match;
   match.provider = this;
-  match.relevance = relevance;
+  match.relevance = shortcut_match.relevance;
   match.deletable = true;
   match.fill_into_edit = shortcut.match_core.fill_into_edit;
   match.destination_url = shortcut.match_core.destination_url;
@@ -208,6 +227,7 @@ AutocompleteMatch ShortcutsProvider::ShortcutToACMatch(
   match.transition = ui::PageTransitionFromInt(shortcut.match_core.transition);
   match.type = static_cast<AutocompleteMatch::Type>(shortcut.match_core.type);
   match.keyword = shortcut.match_core.keyword;
+  match.stripped_destination_url = shortcut_match.stripped_destination_url;
   match.RecordAdditionalInfo("number of hits", shortcut.number_of_hits);
   match.RecordAdditionalInfo("last access time", shortcut.last_access_time);
   match.RecordAdditionalInfo("original input text",
@@ -247,13 +267,11 @@ AutocompleteMatch ShortcutsProvider::ShortcutToACMatch(
     }
   }
   match.EnsureUWYTIsAllowedToBeDefault(input,
-                                       client_->GetAcceptLanguages(),
+                                       languages_,
                                        client_->GetTemplateURLService());
 
   // Try to mark pieces of the contents and description as matches if they
   // appear in |input.text()|.
-  const base::string16 term_string = base::i18n::ToLower(input.text());
-  WordMap terms_map(CreateWordMapForString(term_string));
   if (!terms_map.empty()) {
     match.contents_class = ClassifyAllMatchesInString(
         term_string, terms_map, match.contents, match.contents_class);
@@ -423,3 +441,19 @@ int ShortcutsProvider::CalculateScore(
   return static_cast<int>((base_score / exp(decay_exponent / decay_divisor)) +
                           0.5);
 }
+
+// static
+void ShortcutsProvider::DedupShortcutMatchesByDestination(
+    OmniboxEventProto::PageClassification page_classification,
+    std::vector<ShortcutMatch>* matches) {
+  DestinationSort<ShortcutMatch> destination_sort(page_classification);

[Peter Kasting, 2016/04/12 23:29:50]

Nit: Seems like we could just inline this into the sort() call.

[Alexander Yashkin, 2016/04/13 09:29:36]

Done.

+  // Sort matches such that duplicate matches are consecutive.
+  std::sort(matches->begin(), matches->end(), destination_sort);
+
+  // Erase duplicate matches.
+  matches->erase(
+      std::unique(matches->begin(),
+                  matches->end(),
+                  &ShortcutMatch::DestinationsEqual),
+                  matches->end());
+}