Initial implementation of dedupping search provider's URLs.

chrome/browser/autocomplete/autocomplete_result_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 "chrome/browser/autocomplete/autocomplete_result.h"
 
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/autocomplete/autocomplete_input.h"
 #include "chrome/browser/autocomplete/autocomplete_match.h"
 #include "chrome/browser/autocomplete/autocomplete_provider.h"
+#include "chrome/test/base/testing_profile.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 class AutocompleteResultTest : public testing::Test  {
  public:
   struct TestData {
     // Used to build a url for the AutocompleteMatch. The URL becomes
     // "http://" + ('a' + |url_id|) (e.g. an ID of 2 yields "http://b").
     int url_id;
 
     // ID of the provider.
     int provider_id;
 
     // Relevance score.
     int relevance;
   };
 
-  AutocompleteResultTest() {}
+  AutocompleteResultTest() : profile_(new TestingProfile()) {}
 
   // Configures |match| from |data|.
   static void PopulateAutocompleteMatch(const TestData& data,
                                         AutocompleteMatch* match);
 
   // Adds |count| AutocompleteMatches to |matches|.
   static void PopulateAutocompleteMatches(const TestData* data,
                                           size_t count,
                                           ACMatches* matches);
 
   // Asserts that |result| has |expected_count| matches matching |expected|.
   void AssertResultMatches(const AutocompleteResult& result,
                            const TestData* expected,
                            size_t expected_count);
 
   // Creates an AutocompleteResult from |last| and |current|. The two are
   // merged by |CopyOldMatches| and compared by |AssertResultMatches|.
   void RunCopyOldMatchesTest(const TestData* last, size_t last_size,
                              const TestData* current, size_t current_size,
                              const TestData* expected, size_t expected_size);
 
+ protected:
+  scoped_ptr<TestingProfile> profile_;

[Peter Kasting, 2012/10/26 22:13:49]

Nit: Does this need to be heap-allocated?  Seems like just a TestingProfile
member would work.

[Bart N., 2012/10/26 23:36:55]

Yup. Done.

+
  private:
   DISALLOW_COPY_AND_ASSIGN(AutocompleteResultTest);
 };
 
 // static
 void AutocompleteResultTest::PopulateAutocompleteMatch(
     const TestData& data,
     AutocompleteMatch* match) {
   match->provider = reinterpret_cast<AutocompleteProvider*>(data.provider_id);
   match->fill_into_edit = base::IntToString16(data.url_id);
@@ skipping 34 matching lines @@
     const TestData* last, size_t last_size,
     const TestData* current, size_t current_size,
     const TestData* expected, size_t expected_size) {
   AutocompleteInput input(ASCIIToUTF16("a"), string16(), false, false, false,
                           AutocompleteInput::ALL_MATCHES);
 
   ACMatches last_matches;
   PopulateAutocompleteMatches(last, last_size, &last_matches);
   AutocompleteResult last_result;
   last_result.AppendMatches(last_matches);
-  last_result.SortAndCull(input);
+  last_result.SortAndCull(input, profile_.get());
 
   ACMatches current_matches;
   PopulateAutocompleteMatches(current, current_size, &current_matches);
   AutocompleteResult current_result;
   current_result.AppendMatches(current_matches);
-  current_result.SortAndCull(input);
-  current_result.CopyOldMatches(input, last_result);
+  current_result.SortAndCull(input, profile_.get());
+  current_result.CopyOldMatches(input, last_result, profile_.get());
 
   AssertResultMatches(current_result, expected, expected_size);
 }
 
 // Assertion testing for AutocompleteResult::Swap.
 TEST_F(AutocompleteResultTest, Swap) {
   AutocompleteResult r1;
   AutocompleteResult r2;
 
   // Swap with empty shouldn't do anything interesting.
   r1.Swap(&r2);
   EXPECT_EQ(r1.end(), r1.default_match());
   EXPECT_EQ(r2.end(), r2.default_match());
 
   // Swap with a single match.
   ACMatches matches;
   AutocompleteMatch match;
   AutocompleteInput input(ASCIIToUTF16("a"), string16(), false, false, false,
                           AutocompleteInput::ALL_MATCHES);
   matches.push_back(match);
   r1.AppendMatches(matches);
-  r1.SortAndCull(input);
+  r1.SortAndCull(input, profile_.get());
   EXPECT_EQ(r1.begin(), r1.default_match());
   EXPECT_EQ("http://a/", r1.alternate_nav_url().spec());
   r1.Swap(&r2);
   EXPECT_TRUE(r1.empty());
   EXPECT_EQ(r1.end(), r1.default_match());
   EXPECT_TRUE(r1.alternate_nav_url().is_empty());
   ASSERT_FALSE(r2.empty());
   EXPECT_EQ(r2.begin(), r2.default_match());
   EXPECT_EQ("http://a/", r2.alternate_nav_url().spec());
 }
@@ skipping 58 matching lines @@
   ACMatches matches;
   PopulateAutocompleteMatches(data, arraysize(data), &matches);
   matches[1].destination_url = GURL();
   matches[3].destination_url = GURL();
   matches[4].destination_url = GURL();
 
   AutocompleteResult result;
   result.AppendMatches(matches);
   AutocompleteInput input(string16(), string16(), false, false, false,
                           AutocompleteInput::ALL_MATCHES);
-  result.SortAndCull(input);
+  result.SortAndCull(input, profile_.get());
 
   // Of the two results with the same non-empty destination URL, the
   // lower-relevance one should be dropped.  All of the results with empty URLs
   // should be kept.
   ASSERT_EQ(4U, result.size());
   EXPECT_TRUE(result.match_at(0)->destination_url.is_empty());
   EXPECT_EQ(1300, result.match_at(0)->relevance);
   EXPECT_TRUE(result.match_at(1)->destination_url.is_empty());
   EXPECT_EQ(1200, result.match_at(1)->relevance);
   EXPECT_TRUE(result.match_at(2)->destination_url.is_empty());
   EXPECT_EQ(1100, result.match_at(2)->relevance);
   EXPECT_EQ("http://b/", result.match_at(3)->destination_url.spec());
   EXPECT_EQ(1000, result.match_at(3)->relevance);
 }
+
+TEST_F(AutocompleteResultTest, SortAndCullDuplicateSearchURLs) {
+  TestData data[] = {
+    { 0, 0, 1300 },
+    { 1, 0, 1200 },
+    { 2, 0, 1100 },
+    { 3, 0, 1000 },
+    { 4, 1, 900 },
+  };
+
+  ACMatches matches;
+  PopulateAutocompleteMatches(data, arraysize(data), &matches);
+  matches[0].destination_url = GURL("http://www.google.com/search?q=foo");
+  matches[1].destination_url = GURL("http://www.google.com/search?q=foo2");
+  matches[2].destination_url = GURL("http://www.google.com/search?q=foo&oq=f");
+  matches[3].destination_url = GURL("http://www.google.com/search?q=foo&aqs=0");
+  matches[4].destination_url = GURL("http://www.google.com/");

[Peter Kasting, 2012/10/26 22:13:49]

This seems like it relies on the test data having a TemplateURL for google.com. 
I'm not sure that's a good assumption to make.

Perhaps this test should manually add a TemplateURL for some fake search engine,
then use that.

[Bart N., 2012/10/26 23:36:55]

Yes - I'll update it in the followup CL.

+
+  AutocompleteResult result;
+  result.AppendMatches(matches);
+  AutocompleteInput input(string16(), string16(), false, false, false,
+                          AutocompleteInput::ALL_MATCHES);
+  result.SortAndCull(input, profile_.get());
+
+  // We expect the 3rd and 4th results to be removed.
+  ASSERT_EQ(3U, result.size());
+  EXPECT_EQ("http://www.google.com/search?q=foo",
+            result.match_at(0)->destination_url.spec());
+  EXPECT_EQ(1300, result.match_at(0)->relevance);
+  EXPECT_EQ("http://www.google.com/search?q=foo2",
+            result.match_at(1)->destination_url.spec());
+  EXPECT_EQ(1200, result.match_at(1)->relevance);
+  EXPECT_EQ("http://www.google.com/",
+            result.match_at(2)->destination_url.spec());
+  EXPECT_EQ(900, result.match_at(2)->relevance);
+}