[omnibox] Demotes URLs in the NTP Realbox.

Change-Id: I9ea3144f0eb659c8b4bd64be364d056267e6d606
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2099103
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Reviewed-by: manuk hovanesian <manukh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749360}

components/omnibox/browser/autocomplete_result.cc

@@ -523,12 +523,13 @@ ACMatches::iterator AutocompleteResult::FindTopMatch(
   // expects to see a commonly visited URL as the default match, the URL is not
   // suppressed by type demotion.
   // However, we don't care about this URL behavior when the user is using the
-  // fakebox, which is intended to work more like a search-only box. Unless the
-  // user's input is a URL in which case we still want to ensure they can get a
-  // URL as the default match.
+  // fakebox/realbox, which is intended to work more like a search-only box.
+  // Unless the user's input is a URL in which case we still want to ensure they
+  // can get a URL as the default match.
   if ((input.current_page_classification() !=
-           OmniboxEventProto::INSTANT_NTP_WITH_FAKEBOX_AS_STARTING_FOCUS ||
-       input.type() == metrics::OmniboxInputType::URL)) {
+           OmniboxEventProto::INSTANT_NTP_WITH_FAKEBOX_AS_STARTING_FOCUS &&
+       input.current_page_classification() != OmniboxEventProto::NTP_REALBOX) ||
+      input.type() == metrics::OmniboxInputType::URL) {
     auto best = matches->end();
     for (auto it = matches->begin(); it != matches->end(); ++it) {
       if (it->allowed_to_be_default_match && !it->IsSubMatch() &&

components/omnibox/browser/autocomplete_result_unittest.cc

@@ -145,6 +145,13 @@ class AutocompleteResultTest : public testing::Test {
                                  const TestData* expected,
                                  size_t expected_size);
 
+  void SortMatchesAndVerfiyOrder(
+      const std::string& input_text,
+      OmniboxEventProto::PageClassification page_classification,
+      const ACMatches& matches,
+      const std::vector<size_t>& expected_order,
+      const AutocompleteMatchTestData data[]);
+
   // Returns a (mock) AutocompleteProvider of given |provider_id|.
   MockAutocompleteProvider* GetProvider(int provider_id) {
     EXPECT_LT(provider_id, static_cast<int>(mock_provider_list_.size()));
@@ -231,6 +238,25 @@ void AutocompleteResultTest::RunTransferOldMatchesTest(const TestData* last,
   AssertResultMatches(current_result, expected, expected_size);
 }
 
+void AutocompleteResultTest::SortMatchesAndVerfiyOrder(
+    const std::string& input_text,
+    OmniboxEventProto::PageClassification page_classification,
+    const ACMatches& matches,
+    const std::vector<size_t>& expected_order,
+    const AutocompleteMatchTestData data[]) {
+  AutocompleteInput input(base::ASCIIToUTF16(input_text), page_classification,
+                          TestSchemeClassifier());
+  AutocompleteResult result;
+  result.AppendMatches(input, matches);
+  result.SortAndCull(input, template_url_service_.get());
+
+  ASSERT_EQ(expected_order.size(), result.size());
+  for (size_t i = 0; i < expected_order.size(); ++i) {
+    EXPECT_EQ(data[expected_order[i]].destination_url,
+              result.match_at(i)->destination_url.spec());
+  }
+}
+
 // Assertion testing for AutocompleteResult::Swap.
 TEST_F(AutocompleteResultTest, Swap) {
   AutocompleteResult r1;
@@ -1040,75 +1066,37 @@ TEST_F(AutocompleteResultTest, DemoteByType) {
   base::FieldTrialList::CreateFieldTrial(
       OmniboxFieldTrial::kBundledExperimentFieldTrialName, "A");
 
-  {
-    AutocompleteInput input(base::ASCIIToUTF16("a"),
-                            OmniboxEventProto::HOME_PAGE,
-                            TestSchemeClassifier());
-    AutocompleteResult result;
-    result.AppendMatches(input, matches);
-    result.SortAndCull(input, template_url_service_.get());
-
-    // Because we want to ensure the highest naturally scoring
-    // allowed-to-be default suggestion is the default, make sure history-title
-    // is the default match despite demotion.
-    // Make sure history-URL is the last match due to the logic which groups
-    // searches and URLs together.
-    size_t expected_order[] = {1, 2, 3, 0};
-
-    ASSERT_EQ(base::size(expected_order), result.size());
-    for (size_t i = 0; i < base::size(expected_order); ++i) {
-      EXPECT_EQ(data[expected_order[i]].destination_url,
-                result.match_at(i)->destination_url.spec());
-    }
-  }
-
-  {
-    // However, in the fakebox, we do want to use the demoted score when
-    // selecting the default match because we generally only expect it to be
-    // used for queries and we demote URLs strongly. So here we re-sort with a
-    // page classification of fake-box, and make sure history-title is now
-    // demoted.
-    AutocompleteInput input(
-        base::ASCIIToUTF16("a"),
-        OmniboxEventProto::INSTANT_NTP_WITH_FAKEBOX_AS_STARTING_FOCUS,
-        TestSchemeClassifier());
-    AutocompleteResult result;
-    result.AppendMatches(input, matches);
-    result.SortAndCull(input, template_url_service_.get());
-
-    // Make sure history-URL is the last match due to the logic which groups
-    // searches and URLs together.
-    size_t expected_order[] = {3, 2, 0, 1};
-
-    ASSERT_EQ(base::size(expected_order), result.size());
-    for (size_t i = 0; i < base::size(expected_order); ++i) {
-      EXPECT_EQ(data[expected_order[i]].destination_url,
-                result.match_at(i)->destination_url.spec());
-    }
-  }
-
-  {
-    // Unless, the user's input looks like a URL, in which case we want to use
-    // the natural scoring again to make sure the user gets a URL if they're
-    // clearly trying to navigate. So here we re-sort with a page classification
-    // of fake-box and an input that's a URL, and make sure history-title is
-    // once again the default match.
-    AutocompleteInput input(
-        base::ASCIIToUTF16("www.example.com"),
-        OmniboxEventProto::INSTANT_NTP_WITH_FAKEBOX_AS_STARTING_FOCUS,
-        TestSchemeClassifier());
-    AutocompleteResult result;
-    result.AppendMatches(input, matches);
-    result.SortAndCull(input, template_url_service_.get());
-
-    size_t expected_order[] = {1, 2, 3, 0};
-
-    ASSERT_EQ(base::size(expected_order), result.size());
-    for (size_t i = 0; i < base::size(expected_order); ++i) {
-      EXPECT_EQ(data[expected_order[i]].destination_url,
-                result.match_at(i)->destination_url.spec());
-    }
-  }
+  // Because we want to ensure the highest naturally scoring
+  // allowed-to-be default suggestion is the default, make sure history-title
+  // is the default match despite demotion.
+  // Make sure history-URL is the last match due to the logic which groups
+  // searches and URLs together.
+  SortMatchesAndVerfiyOrder("a", OmniboxEventProto::HOME_PAGE, matches,
+                            {1, 2, 3, 0}, data);
+
+  // However, in the fakebox/realbox, we do want to use the demoted score when
+  // selecting the default match because we generally only expect it to be
+  // used for queries and we demote URLs strongly. So here we re-sort with a
+  // page classification of fakebox/realbox, and make sure history-title is now
+  // demoted. We also make sure history-URL is the last match due to the logic
+  // which groups searches and URLs together.
+  SortMatchesAndVerfiyOrder(
+      "a", OmniboxEventProto::INSTANT_NTP_WITH_FAKEBOX_AS_STARTING_FOCUS,
+      matches, {3, 2, 0, 1}, data);
+  SortMatchesAndVerfiyOrder("a", OmniboxEventProto::NTP_REALBOX, matches,
+                            {3, 2, 0, 1}, data);
+
+  // Unless, the user's input looks like a URL, in which case we want to use
+  // the natural scoring again to make sure the user gets a URL if they're
+  // clearly trying to navigate. So here we re-sort with a page classification
+  // of fakebox/realbox and an input that's a URL, and make sure history-title
+  // is once again the default match.
+  SortMatchesAndVerfiyOrder(
+      "www.example.com",
+      OmniboxEventProto::INSTANT_NTP_WITH_FAKEBOX_AS_STARTING_FOCUS, matches,
+      {1, 2, 3, 0}, data);
+  SortMatchesAndVerfiyOrder("www.example.com", OmniboxEventProto::NTP_REALBOX,
+                            matches, {1, 2, 3, 0}, data);
 }
 
 TEST_F(AutocompleteResultTest, SortAndCullReorderForDefaultMatch) {

components/omnibox/browser/omnibox_field_trial.cc

@@ -311,8 +311,10 @@ void OmniboxFieldTrial::GetDemotionsByType(
       demotion_rule = "1:61,2:61,3:61,4:61,16:61,24:61";
 #endif
     if (current_page_classification ==
-        OmniboxEventProto::INSTANT_NTP_WITH_FAKEBOX_AS_STARTING_FOCUS)
+            OmniboxEventProto::INSTANT_NTP_WITH_FAKEBOX_AS_STARTING_FOCUS ||
+        current_page_classification == OmniboxEventProto::NTP_REALBOX) {
       demotion_rule = "1:10,2:10,3:10,4:10,5:10,16:10,17:10,24:10";
+    }
   }
 
   // The value of the DemoteByType rule is a comma-separated list of