Optimize shortcuts provider

components/omnibox/browser/shortcuts_provider_unittest.cc

Index: components/omnibox/browser/shortcuts_provider_unittest.cc
diff --git a/components/omnibox/browser/shortcuts_provider_unittest.cc b/components/omnibox/browser/shortcuts_provider_unittest.cc
index 00947f962f62b8efa9230ceb67f07bf0a46f5dcd..df6091d2773eea4c2a65afd4f9878fe6c3df40f9 100644
--- a/components/omnibox/browser/shortcuts_provider_unittest.cc
+++ b/components/omnibox/browser/shortcuts_provider_unittest.cc
@@ -31,6 +31,7 @@
 #include "components/omnibox/browser/autocomplete_result.h"
 #include "components/omnibox/browser/in_memory_url_index.h"
 #include "components/omnibox/browser/mock_autocomplete_provider_client.h"
+#include "components/omnibox/browser/shortcut_match.h"
 #include "components/omnibox/browser/shortcuts_backend.h"
 #include "components/omnibox/browser/shortcuts_provider_test_util.h"
 #include "components/omnibox/browser/test_scheme_classifier.h"
@@ -221,6 +222,20 @@ class FakeAutocompleteProviderClient
   DISALLOW_COPY_AND_ASSIGN(FakeAutocompleteProviderClient);
 };
 
+ShortcutMatch CreateShortcutMatch(int relevance,
+                                  const GURL& stripped_destination_url,
+                                  const std::string& contents) {
+  // Create fake ShortcutsDatabase::Shortcut object and intialize its contents
+  // and type field only.
+  ShortcutsDatabase::Shortcut::MatchCore fake_match_core(
+      base::string16(), GURL(), base::UTF8ToUTF16(contents), std::string(),
+      base::string16(), std::string(), 0, 0, base::string16());
+    // Object used only for ShortcutMatch initialization.

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

Nit: Wrong indenting

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

Done.

+  ShortcutsDatabase::Shortcut fake_shortcut("", base::string16(),

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

Nit: "" -> std::string

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

Done.

+      fake_match_core, base::Time(), 0);
+  return ShortcutMatch(relevance, stripped_destination_url, &fake_shortcut);

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

This leaves a pointer-to-garbage in the ShortcutMatch when this function
returns.  That's asking for someone later to add code that reads that field.

The easiest way to avoid this is to eliminate this helper, move the fake_XXX
objects to the caller side, and convert the CreateShortcutMatch() calls to
simply calling the ShortcutMatch() constructor directly.

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

Done.

+}
+
 }  // namespace
 
 // ClassifyTest ---------------------------------------------------------------
@@ -750,3 +765,35 @@ TEST_F(ShortcutsProviderTest, DoesNotProvideOnFocus) {
   provider_->Start(input, false);
   EXPECT_TRUE(provider_->matches().empty());
 }
+
+TEST_F(ShortcutsProviderTest, DedupShortcutMatchesByDestination) {
+  ShortcutMatches matches;
+  matches.push_back(
+      CreateShortcutMatch(42, GURL("http://www.abc.com"), "123"));
+  matches.push_back(
+      CreateShortcutMatch(43, GURL("http://www.abcd.com"), "123"));
+  matches.push_back(
+      CreateShortcutMatch(42, GURL("http://www.abcde.com"), "123"));
+  matches.push_back(
+      CreateShortcutMatch(41, GURL("http://www.abcd.com"), "DEADBEEF"));
+  matches.push_back(
+      CreateShortcutMatch(42, GURL("http://www.abcde.com"), "234"));
+  matches.push_back(
+      CreateShortcutMatch(42, GURL("http://www.abc.com"), "123"));
+
+  ShortcutsProvider::DedupShortcutMatchesByDestination(
+      metrics::OmniboxEventProto::INVALID_SPEC, &matches);
+  // Expect deduplicated matches to be sorted by destination.
+  // The remaining match out of each set of duplicates should be the one with
+  // the highest relevance.
+  EXPECT_EQ(matches.size(), 3U);

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

Nit: (expected, actual) (all lines)

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

Done.

+  EXPECT_EQ(matches[0].stripped_destination_url, GURL("http://www.abc.com"));
+  EXPECT_EQ(matches[0].relevance, 42);
+  EXPECT_EQ(matches[0].contents, base::ASCIIToUTF16("123"));
+  EXPECT_EQ(matches[1].stripped_destination_url, GURL("http://www.abcd.com"));
+  EXPECT_EQ(matches[1].relevance, 43);
+  EXPECT_EQ(matches[1].contents, base::ASCIIToUTF16("123"));
+  EXPECT_EQ(matches[2].stripped_destination_url, GURL("http://www.abcde.com"));
+  EXPECT_EQ(matches[2].relevance, 42);
+  EXPECT_EQ(matches[2].contents, base::ASCIIToUTF16("123"));
+}