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_;
+    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_)

[H Fung, 2013/08/06 22:03:17]

Nit: the if is not needed?

[Mark P, 2013/08/06 22:18:06]

I will ignore this comment because
https://codereview.chromium.org/22031002/
will land before this change, and that change has a better structure for this
field trial state stuff.

+    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());
+  }
+}