Optimize shortcuts provider

components/omnibox/browser/shortcut_match.h

Index: components/omnibox/browser/shortcut_match.h
diff --git a/components/omnibox/browser/shortcut_match.h b/components/omnibox/browser/shortcut_match.h
new file mode 100644
index 0000000000000000000000000000000000000000..b6534d27c1782dc67d9a55a457884de75c39eafd
--- /dev/null
+++ b/components/omnibox/browser/shortcut_match.h
@@ -0,0 +1,37 @@
+// Copyright (c) 2016 The Chromium Authors. All rights reserved.

[Peter Kasting, 2016/04/12 00:55:08]

Nit: Wrong copyright header (no (c))

[Alexander Yashkin, 2016/04/12 09:09:20]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_OMNIBOX_BROWSER_SHORTCUT_MATCH_H_
+#define COMPONENTS_OMNIBOX_BROWSER_SHORTCUT_MATCH_H_
+
+#include <vector>
+
+#include "base/strings/string16.h"
+#include "components/omnibox/browser/shortcuts_backend.h"
+#include "url/gurl.h"
+
+// Structure that describes match from shortcut database. It holds only
+// information necessary for shortcut deduplication and relevance sort.
+// It is converted to AutocompleteMatch at final stage of shortcuts provider
+// processing.

[Peter Kasting, 2016/04/12 00:55:08]

Nit: How about:

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.

[Alexander Yashkin, 2016/04/12 09:09:20]

Done, thanks.

+struct ShortcutMatch {
+  ShortcutMatch(int relevance,
+                const GURL& stripped_destination_url,
+                const ShortcutsDatabase::Shortcut* shortcut);
+  static bool DestinationsEqual(const ShortcutMatch& elem1,

[Peter Kasting, 2016/04/12 00:55:08]

Why are these next two methods static on this class instead of just being, say,
local to shortcuts_provider.cc?

[Alexander Yashkin, 2016/04/12 09:09:20]

I agree that DedupShortcutMatchesByDestination is somewhat foreign to
ShortcutMatch interface.
Yet DestinationsEqual seems to me like in proper place.
I'll move DedupShortcutMatchesByDestination to ShortcutsProvider. 
I can not make it local there because i want to use it in tests.

+                                const ShortcutMatch& elem2);
+  static void DedupShortcutMatchesByDestination(
+      metrics::OmniboxEventProto::PageClassification page_classification,
+      std::vector<ShortcutMatch>* matches);
+
+  int relevance;
+  GURL stripped_destination_url;
+  const ShortcutsDatabase::Shortcut* shortcut;
+  base::string16 contents;
+  AutocompleteMatch::Type type;
+};
+
+typedef std::vector<ShortcutMatch> ShortcutMatches;
+
+#endif  // COMPONENTS_YANDEX_OMNIBOX_BROWSER_SHORTCUT_MATCH_H_