Optimize shortcuts provider

components/omnibox/browser/shortcuts_provider.h

 // 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.
 
 #ifndef COMPONENTS_OMNIBOX_BROWSER_SHORTCUTS_PROVIDER_H_
 #define COMPONENTS_OMNIBOX_BROWSER_SHORTCUTS_PROVIDER_H_
 
 #include <map>
 #include <set>
 #include <string>
@@ skipping 13 matching lines @@
                           public ShortcutsBackend::ShortcutsBackendObserver {
  public:
   explicit ShortcutsProvider(AutocompleteProviderClient* client);
 
   // Performs the autocompletion synchronously. Since no asynch completion is
   // performed |minimal_changes| is ignored.
   void Start(const AutocompleteInput& input, bool minimal_changes) override;
 
   void DeleteMatch(const AutocompleteMatch& match) override;
 
+  // ShortcutMatch holds sufficient information about a single match from the
+  // shortcut database to allow for destination deduping and relevance sorting.
+  // After those stages the top matches are converted to the more heavyweight
+  // AutocompleteMatch struct.  Avoiding constructing the larger struct for
+  // every such match can save significant time when there are many shortcut
+  // matches to process.
+  struct ShortcutMatch {

[Peter Kasting, 2016/04/14 23:52:33]

This struct doesn't need to access any members of ShortcutsProvider, so I would
not make it a member struct.  I would keep it as a toplevel struct, probably
with its own (very short) header and .cc file since you want to be able to sue
it in the unittest.  (The other option would be to change the unittests to test
a larger piece of functionality than just SortAndDedupMatches(), e.g.
GetMatches(), which would not take ShortcutMatches directly as input, and then
make this struct local to the .cc file here.)

[Alexander Yashkin, 2016/04/15 09:14:15]

I moved ShortcutMatch structure and SortAndDedupMatches function to
shortcuts_provider.cc anonimous namespace.
I removed unittest for  SortAndDedupMatches entirely because I believe its
functionality is already tested
by existing ShortcutsProvider tests.

+    ShortcutMatch(int relevance,
+                  const GURL& stripped_destination_url,
+                  const ShortcutsDatabase::Shortcut* shortcut);
+    int relevance;
+    GURL stripped_destination_url;
+    const ShortcutsDatabase::Shortcut* shortcut;
+    base::string16 contents;
+    AutocompleteMatch::Type type;
+  };
+
  private:
   friend class ClassifyTest;
   friend class ShortcutsProviderExtensionTest;
   friend class ShortcutsProviderTest;
   FRIEND_TEST_ALL_PREFIXES(ShortcutsProviderTest, CalculateScore);
+  FRIEND_TEST_ALL_PREFIXES(ShortcutsProviderTest, SortAndDedupMatches);

[Peter Kasting, 2016/04/14 23:52:34]

Nit: If this is only needed because of the call to SortAndDedupMatches(), I'd
rather make that function public and eliminate this.

[Alexander Yashkin, 2016/04/15 09:14:15]

Removed.

 
   typedef std::multimap<base::char16, base::string16> WordMap;
 
   ~ShortcutsProvider() override;
 
-  // ShortcutsBackendObserver:
-  void OnShortcutsLoaded() override;
-
-  // Performs the autocomplete matching and scoring.
-  void GetMatches(const AutocompleteInput& input);
-
-  // Returns an AutocompleteMatch corresponding to |shortcut|. Assigns it
-  // |relevance| score in the process, and highlights the description and
-  // contents against |input|, which should be the lower-cased version of
-  // the user's input. |input| and |fixed_up_input_text| are used to decide
-  // what can be inlined.
-  AutocompleteMatch ShortcutToACMatch(
-      const ShortcutsDatabase::Shortcut& shortcut,
-      int relevance,
-      const AutocompleteInput& input,
-      const base::string16& fixed_up_input_text);
-
   // Returns a map mapping characters to groups of words from |text| that start
   // with those characters, ordered lexicographically descending so that longer
   // words appear before their prefixes (if any) within a particular
   // equal_range().
   static WordMap CreateWordMapForString(const base::string16& text);
 
   // Given |text| and a corresponding base set of classifications
   // |original_class|, adds ACMatchClassification::MATCH markers for all
   // instances of the words from |find_words| within |text| and returns the
   // resulting classifications.  (|find_text| is provided as the original string
@@ skipping 12 matching lines @@
   // |find_words| should be as constructed by CreateWordMapForString(find_text).
   //
   // |find_text| (and thus |find_words|) are expected to be lowercase.  |text|
   // will be lowercased in this function.
   static ACMatchClassifications ClassifyAllMatchesInString(
       const base::string16& find_text,
       const WordMap& find_words,
       const base::string16& text,
       const ACMatchClassifications& original_class);
 
+  // Sorts |matches| by destination, taking into account demotions based on
+  // |page_classification| when resolving ties about which of several
+  // duplicates to keep.  The matches are also deduplicated.
+  static void SortAndDedupMatches(
+      metrics::OmniboxEventProto::PageClassification page_classification,
+      std::vector<ShortcutMatch>* matches);
+
+  // ShortcutsBackendObserver:
+  void OnShortcutsLoaded() override;
+
+  // Performs the autocomplete matching and scoring.
+  void GetMatches(const AutocompleteInput& input);
+
+  // Returns an AutocompleteMatch corresponding to |shortcut_match|. Assigns it
+  // |relevance| score in the process, and highlights the description and
+  // contents against |input|, which should be the lower-cased version of
+  // the user's input. |input| and |fixed_up_input_text| are used to decide
+  // what can be inlined.
+  AutocompleteMatch ShortcutMatchToACMatch(
+      const ShortcutMatch& shortcut_match,
+      const AutocompleteInput& input,
+      const base::string16& fixed_up_input_text,
+      const base::string16 term_string,
+      const WordMap& terms_map);
+
   // Returns iterator to first item in |shortcuts_map_| matching |keyword|.
   // Returns shortcuts_map_.end() if there are no matches.
   ShortcutsBackend::ShortcutMap::const_iterator FindFirstMatch(
       const base::string16& keyword,
       ShortcutsBackend* backend);
 
   int CalculateScore(const base::string16& terms,
                      const ShortcutsDatabase::Shortcut& shortcut,
                      int max_relevance);
 
   // The default max relevance unless overridden by a field trial.
   static const int kShortcutsProviderDefaultMaxRelevance;
 
   AutocompleteProviderClient* client_;
   std::string languages_;
   bool initialized_;
 };
 
 #endif  // COMPONENTS_OMNIBOX_BROWSER_SHORTCUTS_PROVIDER_H_