Omnibox: Make the Controller Reorder Matches for Inlining

chrome/browser/autocomplete/autocomplete_result_unittest.cc

Index: chrome/browser/autocomplete/autocomplete_result_unittest.cc
diff --git a/chrome/browser/autocomplete/autocomplete_result_unittest.cc b/chrome/browser/autocomplete/autocomplete_result_unittest.cc
index 3038c27a017dd8d6ca05996518140d6486edf7cd..b99b2fd5dbb5bb9488591cf030d28760d88bfdb6 100644
--- a/chrome/browser/autocomplete/autocomplete_result_unittest.cc
+++ b/chrome/browser/autocomplete/autocomplete_result_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/autocomplete/autocomplete_result.h"
 
+#include "base/metrics/field_trial.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
@@ -13,6 +14,8 @@
 #include "chrome/browser/search_engines/template_url_prepopulate_data.h"
 #include "chrome/browser/search_engines/template_url_service.h"
 #include "chrome/browser/search_engines/template_url_service_test_util.h"
+#include "chrome/common/metrics/entropy_provider.h"
+#include "chrome/common/metrics/variations/variations_util.h"
 #include "chrome/test/base/testing_profile.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -45,6 +48,17 @@ class AutocompleteResultTest : public testing::Test  {
     test_util_.TearDown();
   }
 
+  static void SetUpTestCase() {
+    ResetFieldTrialList();
+  }
+
+  static void TearDownTestCase() {
+    delete field_trial_list_;

[Peter Kasting, 2013/08/06 22:56:16]

Nit: Use scoped_ptr<> (see other CL you have in flight)

[Mark P, 2013/08/07 00:44:31]

I assume the other changelist will land first so didn't bother to update this
changelist with the improvements made on that one.  (I'd figure it out when
resolving.)  But given that both you and Harry complained, I copied the changes
that are landing for the field trial stuff on that changelist to this one
(basically in effect resolving before that change lands).

+    field_trial_list_ = NULL;
+  }
+
+  static void ResetFieldTrialList();
+
   // Configures |match| from |data|.
   static void PopulateAutocompleteMatch(const TestData& data,
                                         AutocompleteMatch* match);
@@ -69,10 +83,25 @@ class AutocompleteResultTest : public testing::Test  {
   TemplateURLServiceTestUtil test_util_;
 
  private:
+  static base::FieldTrialList* field_trial_list_;
+
   DISALLOW_COPY_AND_ASSIGN(AutocompleteResultTest);
 };
 
 // static
+base::FieldTrialList* AutocompleteResultTest::field_trial_list_ = NULL;
+
+// static
+void AutocompleteResultTest::ResetFieldTrialList() {
+  // It's important to delete the old pointer first which sets
+  // FieldTrialList::global_ to NULL.
+  if (field_trial_list_)
+    delete field_trial_list_;
+  field_trial_list_ = new base::FieldTrialList(
+      new metrics::SHA1EntropyProvider("foo"));
+}
+
+// static
 void AutocompleteResultTest::PopulateAutocompleteMatch(
     const TestData& data,
     AutocompleteMatch* match) {
@@ -81,6 +110,7 @@ void AutocompleteResultTest::PopulateAutocompleteMatch(
   std::string url_id(1, data.url_id + 'a');
   match->destination_url = GURL("http://" + url_id);
   match->relevance = data.relevance;
+  match->allowed_to_be_default_match = true;
 }
 
 // static
@@ -148,6 +178,7 @@ TEST_F(AutocompleteResultTest, Swap) {
   // Swap with a single match.
   ACMatches matches;
   AutocompleteMatch match;
+  match.allowed_to_be_default_match = true;
   AutocompleteInput input(ASCIIToUTF16("a"), string16::npos, string16(), GURL(),
                           AutocompleteInput::INVALID_SPEC, false, false, false,
                           AutocompleteInput::ALL_MATCHES);
@@ -292,3 +323,62 @@ TEST_F(AutocompleteResultTest, SortAndCullDuplicateSearchURLs) {
             result.match_at(2)->destination_url.spec());
   EXPECT_EQ(900, result.match_at(2)->relevance);
 }
+
+TEST_F(AutocompleteResultTest, SortAndCullReorderForDefaultMatch) {
+  TestData data[] = {
+    { 0, 0, 1300 },
+    { 1, 0, 1200 },
+    { 2, 0, 1100 },
+    { 3, 0, 1000 }
+  };
+
+  // Must be the same as kBundledExperimentFieldTrialName
+  // defined in omnibox_field_trial.cc.
+  const std::string kTrialName = "OmniboxBundledExperimentV1";
+  // Must be the same as kReorderForLegalDefaultMatchRule defined in
+  // omnibox_field_trial.cc.
+  const std::string kRuleName = "ReorderForLegalDefaultMatch";
+  // Must be the same as the constant used in
+  // OmniboxFieldTrial::ReorderForLegalDefaultMatch().
+  const std::string kReorderEnabled = "ReorderForLegalDefaultMatch";
+  std::map<std::string, std::string> params;
+  // Enable reorder for omnibox inputs on the user's homepage.
+  params[kRuleName + ":3"] = kReorderEnabled;
+  ASSERT_TRUE(chrome_variations::AssociateVariationParams(
+      kTrialName, "A", params));
+  base::FieldTrialList::CreateFieldTrial(kTrialName, "A");
+
+  {
+    // Check that reorder doesn't do anything if the top result
+    // is already a legal default match (which is the default from
+    // PopulateAutocompleteMatches()).
+    ACMatches matches;
+    PopulateAutocompleteMatches(data, arraysize(data), &matches);
+    AutocompleteResult result;
+    result.AppendMatches(matches);
+    AutocompleteInput input(string16(), string16::npos, string16(), GURL(),
+                            AutocompleteInput::HOMEPAGE, false, false, false,
+                            AutocompleteInput::ALL_MATCHES);
+    result.SortAndCull(input, test_util_.profile());
+    AssertResultMatches(result, data, 4);
+  }
+
+  {
+    // Check that reorder swaps up a result appropriately.
+    ACMatches matches;
+    PopulateAutocompleteMatches(data, arraysize(data), &matches);
+    matches[0].allowed_to_be_default_match = false;
+    matches[1].allowed_to_be_default_match = false;
+    AutocompleteResult result;
+    result.AppendMatches(matches);
+    AutocompleteInput input(string16(), string16::npos, string16(), GURL(),
+                            AutocompleteInput::HOMEPAGE, false, false, false,
+                            AutocompleteInput::ALL_MATCHES);
+    result.SortAndCull(input, test_util_.profile());
+    ASSERT_EQ(4U, result.size());
+    EXPECT_EQ("http://c/", result.match_at(0)->destination_url.spec());
+    EXPECT_EQ("http://a/", result.match_at(1)->destination_url.spec());
+    EXPECT_EQ("http://b/", result.match_at(2)->destination_url.spec());
+    EXPECT_EQ("http://d/", result.match_at(3)->destination_url.spec());
+  }
+}