Optimize shortcuts provider

components/omnibox/browser/shortcuts_provider_unittest.cc

 // 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.
 
 #include "components/omnibox/browser/shortcuts_provider.h"
 
 #include <math.h>
 #include <stddef.h>
 
 #include <algorithm>
@@ skipping 13 matching lines @@
 #include "components/history/core/browser/history_service.h"
 #include "components/history/core/browser/url_database.h"
 #include "components/history/core/test/history_service_test_util.h"
 #include "components/metrics/proto/omnibox_event.pb.h"
 #include "components/omnibox/browser/autocomplete_input.h"
 #include "components/omnibox/browser/autocomplete_match.h"
 #include "components/omnibox/browser/autocomplete_provider.h"
 #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"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using base::ASCIIToUTF16;
 using ExpectedURLs = std::vector<ExpectedURLAndAllowedToBeDefault>;
 
 namespace {
 
@@ skipping 170 matching lines @@
  private:
   base::Thread db_thread_;
   base::SequencedWorkerPoolOwner pool_owner_;
   base::ScopedTempDir history_dir_;
   scoped_ptr<history::HistoryService> history_service_;
   scoped_refptr<ShortcutsBackend> shortcuts_backend_;
 
   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 ---------------------------------------------------------------
 
 // Helper class to make running tests of ClassifyAllMatchesInString() more
 // convenient.
 class ClassifyTest {
  public:
   ClassifyTest(const base::string16& text, ACMatchClassifications matches);
   ~ClassifyTest();
@@ skipping 509 matching lines @@
 }
 
 TEST_F(ShortcutsProviderTest, DoesNotProvideOnFocus) {
   AutocompleteInput input(ASCIIToUTF16("about:o"), base::string16::npos,
                           std::string(), GURL(),
                           metrics::OmniboxEventProto::INVALID_SPEC, false,
                           false, true, true, true, TestSchemeClassifier());
   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"));
+}