[omnibox] Enables zero-prefix suggestions in Desktop NTP by default

- Enables zero-prefix suggestion in Desktop NTP (Omnibox and Realbox)
  and modifies the tests according to the new expectation.
- Removes already enabled features and related fieldtrial params from
  fieldtrial_testing_config.json

Bug: 1075724
Change-Id: I7e63dc5d6532fe8baf83fe358f47c00fa61729d0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2175138
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797904}

components/omnibox/browser/base_search_provider.cc

@@ -226,6 +226,16 @@ bool BaseSearchProvider::IsNTPPage(
          (classification == OEP::NTP_REALBOX);
 }
 
+// static
+bool BaseSearchProvider::IsSearchResultsPage(
+    metrics::OmniboxEventProto::PageClassification classification) {
+  using OEP = metrics::OmniboxEventProto;
+  return (classification ==
+          OEP::SEARCH_RESULT_PAGE_NO_SEARCH_TERM_REPLACEMENT) ||
+         (classification ==
+          OEP::SEARCH_RESULT_PAGE_DOING_SEARCH_TERM_REPLACEMENT);
+}
+
 void BaseSearchProvider::DeleteMatch(const AutocompleteMatch& match) {
   DCHECK(match.deletable);
   if (!match.GetAdditionalInfo(BaseSearchProvider::kDeletionUrlKey).empty()) {

components/omnibox/browser/base_search_provider.h

@@ -107,6 +107,9 @@ class BaseSearchProvider : public AutocompleteProvider {
   // Returns whether the provided classification indicates some sort of NTP.
   static bool IsNTPPage(
       metrics::OmniboxEventProto::PageClassification classification);
+  // Returns whether the provided classification indicates Search Results Page.
+  static bool IsSearchResultsPage(
+      metrics::OmniboxEventProto::PageClassification classification);
 
   // AutocompleteProvider:
   void DeleteMatch(const AutocompleteMatch& match) override;

components/omnibox/browser/local_history_zero_suggest_provider.cc

@@ -17,6 +17,7 @@
 #include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/trace_event/trace_event.h"
+#include "build/build_config.h"
 #include "components/google/core/common/google_util.h"
 #include "components/history/core/browser/history_database.h"
 #include "components/history/core/browser/history_service.h"
@@ -62,7 +63,12 @@ bool AllowLocalHistoryZeroSuggestSuggestions(const AutocompleteInput& input) {
 #else
   if (!base::FeatureList::IsEnabled(omnibox::kNewSearchFeatures))
     return false;
+#endif
 
+#if !defined(OS_IOS)  // Enabled by default on Desktop if not disabled by
+                      // kNewSearchFeatures.
+  return true;
+#else
   const auto current_page_classification = input.current_page_classification();
   // Reactive Zero-Prefix Suggestions (rZPS) and basically all remote ZPS on the
   // NTP are expected to be displayed alongside local history zero-prefix

components/omnibox/browser/local_history_zero_suggest_provider_unittest.cc

@@ -77,9 +77,11 @@ class LocalHistoryZeroSuggestProviderTest
     provider_ = base::WrapRefCounted(
         LocalHistoryZeroSuggestProvider::Create(client_.get(), this));
 
+#if defined(OS_IOS)  // Only needed for iOS.
     SetZeroSuggestVariant(
         metrics::OmniboxEventProto::NTP_REALBOX,
         LocalHistoryZeroSuggestProvider::kZeroSuggestLocalVariant);
+#endif
 
     // Add the fallback default search provider to the TemplateURLService so
     // that it gets a valid unique identifier. Make the newly added provider the
@@ -290,6 +292,18 @@ TEST_F(LocalHistoryZeroSuggestProviderTest, MAYBE_ZeroSuggestVariant) {
       {default_search_provider(), "hello world", "&foo=bar", 1},
   });
 
+  // Verify that local history zero-prefix suggestions are enabled by default
+  // on Desktop and Android NTP.
+  scoped_feature_list_ = std::make_unique<base::test::ScopedFeatureList>();
+  StartProviderAndWaitUntilDone();
+#if !defined(OS_IOS)  // Enabled by default on Desktop and Android NTP.
+  ExpectMatches({{"hello world", 500}});
+#else
+  ExpectMatches({});
+#endif
+
+  // Verify that local history zero-prefix suggestions are enabled on the NTP
+  // only.
   SetZeroSuggestVariant(
       metrics::OmniboxEventProto::OTHER,
       LocalHistoryZeroSuggestProvider::kZeroSuggestLocalVariant);
@@ -301,12 +315,14 @@ TEST_F(LocalHistoryZeroSuggestProviderTest, MAYBE_ZeroSuggestVariant) {
   SetZeroSuggestVariant(metrics::OmniboxEventProto::NTP_REALBOX,
                         /*zero_suggest_variant_value=*/"blah");
   StartProviderAndWaitUntilDone();
-#if defined(OS_ANDROID)  // Enabled by default.
+#if !defined(OS_IOS)  // Enabled by default on Desktop and Android NTP.
   ExpectMatches({{"hello world", 500}});
 #else
   ExpectMatches({});
 #endif
 
+  // Verify that reactive zero-prefix suggestions enable local history
+  // zero-prefix suggestions on the NTP.
   scoped_feature_list_ = std::make_unique<base::test::ScopedFeatureList>();
   scoped_feature_list_->InitAndEnableFeature(
       omnibox::kReactiveZeroSuggestionsOnNTPRealbox);
@@ -317,7 +333,7 @@ TEST_F(LocalHistoryZeroSuggestProviderTest, MAYBE_ZeroSuggestVariant) {
   scoped_feature_list_->InitAndEnableFeature(
       omnibox::kReactiveZeroSuggestionsOnNTPOmnibox);
   StartProviderAndWaitUntilDone();
-#if defined(OS_ANDROID)  // Enabled by default.
+#if !defined(OS_IOS)  // Enabled by default on Desktop and Android NTP.
   ExpectMatches({{"hello world", 500}});
 #else
   ExpectMatches({});
@@ -332,18 +348,20 @@ TEST_F(LocalHistoryZeroSuggestProviderTest, MAYBE_ZeroSuggestVariant) {
   // Make sure disabling omnibox::kNewSearchFeatures disables zero suggest.
   scoped_feature_list_ = std::make_unique<base::test::ScopedFeatureList>();
   scoped_feature_list_->InitWithFeatures(
-      {omnibox::kReactiveZeroSuggestionsOnNTPOmnibox},
+      {omnibox::kReactiveZeroSuggestionsOnNTPOmnibox},  // Enables the provider.
       {omnibox::kNewSearchFeatures});
   StartProviderAndWaitUntilDone(
       /*text=*/"", OmniboxFocusType::ON_FOCUS,
       /*page_classification=*/
       metrics::OmniboxEventProto::INSTANT_NTP_WITH_OMNIBOX_AS_STARTING_FOCUS);
-#if defined(OS_ANDROID)  // Enabled by default.
+#if defined(OS_ANDROID)  // Enabled by default on Android NTP.
   ExpectMatches({{"hello world", 500}});
 #else
   ExpectMatches({});
 #endif
 
+  // Verify that configuring the ZeroSuggestVariant param with "local" for the
+  // NTP, enables local history zero-prefix suggestions for that context.
   SetZeroSuggestVariant(
       metrics::OmniboxEventProto::NTP_REALBOX,
       LocalHistoryZeroSuggestProvider::kZeroSuggestLocalVariant);

components/omnibox/browser/zero_suggest_provider.cc

@@ -752,18 +752,15 @@ ZeroSuggestProvider::ResultType ZeroSuggestProvider::TypeOfResultToRun(
   if (base::Contains(field_trial_variants, kMostVisitedVariant))
     return MOST_VISITED;
 
-#if defined(OS_ANDROID)
-  // For Android NTP, default to REMOTE_NO_URL, if it's allowed.
+#if !defined(OS_IOS)
+  // For Desktop and Android, default to REMOTE_NO_URL on the NTP, if allowed.
   if (IsNTPPage(current_page_classification_) && remote_no_url_allowed)
     return REMOTE_NO_URL;
 #endif
 
 #if defined(OS_ANDROID) || defined(OS_IOS)
-  // Then, for Android and iOS, default to MOST_VISITED except on the SERP.
-  if (current_page_classification_ !=
-          OmniboxEventProto::SEARCH_RESULT_PAGE_NO_SEARCH_TERM_REPLACEMENT &&
-      current_page_classification_ !=
-          OmniboxEventProto::SEARCH_RESULT_PAGE_DOING_SEARCH_TERM_REPLACEMENT) {
+  // For Android and iOS, default to MOST_VISITED everywhere except on the SERP.
+  if (!IsSearchResultsPage(current_page_classification_)) {
     return MOST_VISITED;
   }
 #endif

components/omnibox/browser/zero_suggest_provider_unittest.cc

@@ -111,6 +111,8 @@ class FakeAutocompleteProviderClient : public MockAutocompleteProviderClient {
 
   PrefService* GetPrefs() override { return &pref_service_; }
 
+  bool IsPersonalizedUrlDataCollectionActive() const override { return true; }
+
   void Classify(
       const base::string16& text,
       bool prefer_keyword,
@@ -277,24 +279,59 @@ TEST_F(ZeroSuggestProviderTest, TypeOfResultToRun) {
   GURL current_url = GURL("https://example.com/");
   GURL suggest_url = GetSuggestURL(metrics::OmniboxEventProto::OTHER);
 
-  EXPECT_CALL(*client_, IsPersonalizedUrlDataCollectionActive())
-      .WillRepeatedly(testing::Return(true));
-
-  // Verify the unconfigured state returns platorm-specific defaults.
-  auto ExpectPlatformSpecificDefaultZeroSuggestBehavior = [&]() {
-#if defined(OS_IOS) || defined(OS_ANDROID)
-    EXPECT_EQ(ZeroSuggestProvider::ResultType::MOST_VISITED,
-              provider_->TypeOfResultToRun(current_url, suggest_url));
-#else
-    EXPECT_EQ(ZeroSuggestProvider::ResultType::NONE,
-              provider_->TypeOfResultToRun(current_url, suggest_url));
+  // Verifies the unconfigured state. Returns platorm-specific defaults.
+  auto ExpectPlatformSpecificDefaultZeroSuggestBehavior =
+      [&](const bool remote_no_url_allowed) {
+        const auto current_page_classification =
+            provider_->current_page_classification_;
+        const auto result_type =
+            provider_->TypeOfResultToRun(current_url, suggest_url);
+#if !defined(OS_ANDROID) && !defined(OS_IOS)  // Desktop
+        EXPECT_EQ(BaseSearchProvider::IsNTPPage(current_page_classification) &&
+                          remote_no_url_allowed
+                      ? ZeroSuggestProvider::ResultType::REMOTE_NO_URL
+                      : ZeroSuggestProvider::ResultType::NONE,
+                  result_type);
+#elif !defined(OS_IOS)  // Android
+        EXPECT_EQ(BaseSearchProvider::IsNTPPage(current_page_classification) &&
+                          remote_no_url_allowed
+                      ? ZeroSuggestProvider::ResultType::REMOTE_NO_URL
+                      : !BaseSearchProvider::IsSearchResultsPage(
+                            current_page_classification)
+                            ? ZeroSuggestProvider::ResultType::MOST_VISITED
+                            : ZeroSuggestProvider::ResultType::NONE,
+                  result_type);
+#else                   // iOS
+        EXPECT_EQ(!BaseSearchProvider::IsSearchResultsPage(
+                      current_page_classification)
+                      ? ZeroSuggestProvider::ResultType::MOST_VISITED
+                      : ZeroSuggestProvider::ResultType::NONE,
+                  result_type);
 #endif
-  };
-  ExpectPlatformSpecificDefaultZeroSuggestBehavior();
+      };
+  ExpectPlatformSpecificDefaultZeroSuggestBehavior(
+      /*remote_no_url_allowed=*/false);
+
+  // Verify the platorm-specific defaults for the NTP.
+  provider_->SetPageClassificationForTesting(metrics::OmniboxEventProto::NTP);
+  suggest_url = GetSuggestURL(metrics::OmniboxEventProto::NTP);
+  ExpectPlatformSpecificDefaultZeroSuggestBehavior(
+      /*remote_no_url_allowed=*/false);
 
   // Verify RemoteNoUrl works when the user is signed in.
   EXPECT_CALL(*client_, IsAuthenticated())
       .WillRepeatedly(testing::Return(true));
+
+  // Verify the platorm-specific defaults for the NTP.
+  provider_->SetPageClassificationForTesting(metrics::OmniboxEventProto::NTP);
+  suggest_url = GetSuggestURL(metrics::OmniboxEventProto::NTP);
+  ExpectPlatformSpecificDefaultZeroSuggestBehavior(
+      /*remote_no_url_allowed=*/true);
+
+  // Restore to non-NTP page classification.
+  provider_->SetPageClassificationForTesting(metrics::OmniboxEventProto::OTHER);
+  suggest_url = GetSuggestURL(metrics::OmniboxEventProto::OTHER);
+
   CreateRemoteNoUrlFieldTrial();
   EXPECT_EQ(ZeroSuggestProvider::ResultType::REMOTE_NO_URL,
             provider_->TypeOfResultToRun(current_url, suggest_url));
@@ -302,7 +339,8 @@ TEST_F(ZeroSuggestProviderTest, TypeOfResultToRun) {
   // But if the user has signed out, fall back to platform-specific defaults.
   EXPECT_CALL(*client_, IsAuthenticated())
       .WillRepeatedly(testing::Return(false));
-  ExpectPlatformSpecificDefaultZeroSuggestBehavior();
+  ExpectPlatformSpecificDefaultZeroSuggestBehavior(
+      /*remote_no_url_allowed=*/false);
 
   // Restore authentication state, but now set a non-Google default search
   // engine. Verify that we still disallow remote suggestions.
@@ -316,7 +354,8 @@ TEST_F(ZeroSuggestProviderTest, TypeOfResultToRun) {
   auto* other_search_provider =
       turl_model->Add(std::make_unique<TemplateURL>(data));
   turl_model->SetUserSelectedDefaultSearchProvider(other_search_provider);
-  ExpectPlatformSpecificDefaultZeroSuggestBehavior();
+  ExpectPlatformSpecificDefaultZeroSuggestBehavior(
+      /*remote_no_url_allowed=*/false);
 
   // Restore Google as the default search provider.
   turl_model->SetUserSelectedDefaultSearchProvider(
@@ -344,13 +383,13 @@ TEST_F(ZeroSuggestProviderTest, TypeOfResultToRun) {
            ZeroSuggestProvider::kMostVisitedVariant},
           {base::StringPrintf("%s:%d:*",
                               OmniboxFieldTrial::kZeroSuggestVariantRule,
-                              metrics::OmniboxEventProto::NTP),
+                              metrics::OmniboxEventProto::BLANK),
            ZeroSuggestProvider::kNoneVariant},
       });
   provider_->SetPageClassificationForTesting(metrics::OmniboxEventProto::OTHER);
   EXPECT_EQ(ZeroSuggestProvider::ResultType::MOST_VISITED,
             provider_->TypeOfResultToRun(current_url, suggest_url));
-  provider_->SetPageClassificationForTesting(metrics::OmniboxEventProto::NTP);
+  provider_->SetPageClassificationForTesting(metrics::OmniboxEventProto::BLANK);
   EXPECT_EQ(
       ZeroSuggestProvider::ResultType::NONE,
       provider_->TypeOfResultToRun(GURL("chrome://newtab/"), suggest_url));

testing/variations/fieldtrial_testing_config.json

@@ -4863,21 +4863,17 @@
                     "name": "DesktopExperiments",
                     "params": {
                         "OmniboxMaxURLMatches": "7",
-                        "UIMaxAutocompleteMatches": "8",
-                        "ZeroSuggestVariant:15:*": "RemoteNoUrl,Local",
-                        "ZeroSuggestVariant:1:*": "RemoteNoUrl,Local",
-                        "ZeroSuggestVariant:7:*": "RemoteNoUrl,Local"
+                        "UIMaxAutocompleteMatches": "8"
                     },
                     "enable_features": [
                         "NtpRealbox",
                         "OmniboxDisplayTitleForCurrentUrl",
                         "OmniboxDocumentProvider",
                         "OmniboxMaxURLMatches",
-                        "OmniboxOnFocusSuggestions",
                         "OmniboxRemoveSuggestionsFromClipboard",
                         "OmniboxRichEntitySuggestions",
                         "OmniboxUIExperimentMaxAutocompleteMatches",
-                        "OmniboxZeroSuggestionsOnNTP",
+                        "QueryInOmnibox",
                         "SearchSuggestChips"
                     ]
                 }