Optimize shortcuts provider

components/omnibox/browser/shortcut_match_unittest.cc

+// Copyright (c) 2016 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 "components/omnibox/browser/shortcut_match.h"
+
+#include <algorithm>
+#include <string>
+
+#include "base/strings/utf_string_conversions.h"
+#include "components/omnibox/browser/shortcuts_backend.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class ShortcutMatchTest : public testing::Test {
+ protected:
+  ShortcutMatch CreateShortcutMatch(int relevance,
+                                    const GURL& stripped_destination_url,
+                                    const std::string& contents);
+
+  const base::string16& GetMatchContents(const ShortcutMatch& match);
+
+  std::vector<std::unique_ptr<ShortcutsDatabase::Shortcut>> db_shortcuts_;
+};
+
+ShortcutMatch ShortcutMatchTest::CreateShortcutMatch(
+    int relevance,
+    const GURL& stripped_destination_url,
+    const std::string& contents) {
+  // Create ShortcutsDatabase::Shortcut object and intialize its contents
+  // field only. Store it in db_shortcuts vector.

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

Why do we need a vector to hold one object?  And why does ShortcutMatchTest need
to own the object at all -- why can't this just construct a ShortcutMatch and
return it by value?

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

Redone, removed vector of ShortcutMatches. It remained from my previous attempt,
where i used Shortcut pointer to access contents string.
Now i store contents string in ShortcutMatch object so where is no need to hold
ShortcutsDatabase::Shortcut at all.

Also moved this test to shortcuts_provider_unittest.cc

+  db_shortcuts_.emplace_back(new ShortcutsDatabase::Shortcut());
+  db_shortcuts_.back()->match_core.contents = base::ASCIIToUTF16(contents);
+  return ShortcutMatch(relevance, stripped_destination_url,
+      db_shortcuts_.back().get());
+}
+
+const base::string16& ShortcutMatchTest::GetMatchContents(
+    const ShortcutMatch& match) {
+  return match.shortcut->match_core.contents;
+}
+
+TEST_F(ShortcutMatchTest, 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"));
+
+  ShortcutMatch::DedupShortcutMatchesByDestination(
+      metrics::OmniboxEventProto::INVALID_SPEC, &matches);
+  // Expect deduplicated matches to be sorted by destination.
+  // Matches with higher relevance are left from duplicates.

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

Nit: How about:

...The remaining match out of each set of duplicates should be the one with the
highest relevance.

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

Done.

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