[ZPS] Allows local history zps on all zero-suggest eligible surfaces

- Cleans up ZeroSuggestProvider::TypeOfResultToRun by simplifying the function interface and explicitly checking the input type. - Uses ZeroSuggestProvider::TypeOfResultToRun to enable local history zero-suggest in every context the remote zero-suggest is enabled behind a feature flag. - Adds tests. - adds an about:flags entry for the new feature on Desktop and Android. Bug: 1340079 Change-Id: Ic175e320a019678ab2a2475ab57b0aa3727d9ec0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3786813 Commit-Queue: Moe Ahmadi <mahmadi@chromium.org> Reviewed-by: Tomasz Wiszkowski <ender@google.com> Cr-Commit-Position: refs/heads/main@{#1032305}
chromium · Aug 6, 2022 · 01028f2 · 01028f2
1 parent 4b2721c
commit 01028f2
Show file tree

Hide file tree

Showing 12 changed files with 128 additions and 25 deletions.
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
@@ -5173,6 +5173,11 @@ const FeatureEntry kFeatureEntries[] = {
      kOsAndroid, FEATURE_VALUE_TYPE(omnibox::kMostVisitedTilesTitleWrapAround)},
 #endif  // BUILDFLAG(IS_ANDROID)
 
+    {"omnibox-local-history-zero-suggest-beyond-ntp",
+     flag_descriptions::kOmniboxLocalHistoryZeroSuggestBeyondNTPName,
+     flag_descriptions::kOmniboxLocalHistoryZeroSuggestBeyondNTPDescription,
+     kOsAll, FEATURE_VALUE_TYPE(omnibox::kLocalHistoryZeroSuggestBeyondNTP)},
+
     {"omnibox-on-focus-suggestions-contextual-web",
      flag_descriptions::kOmniboxFocusTriggersContextualWebZeroSuggestName,
      flag_descriptions::

diff --git a/chrome/browser/flag-metadata.json b/chrome/browser/flag-metadata.json
@@ -4733,6 +4733,11 @@
     "owners": ["christianxu", "stkhapugin", "chrome-omnibox-team@google.com" ],
     "expiry_milestone": 110
   },
+  {
+    "name": "omnibox-local-history-zero-suggest-beyond-ntp",
+    "owners": [ "mahmadi", "ender", "stkhapugin", "chrome-omnibox-team@google.com" ],
+    "expiry_milestone": 110
+  },
   {
     "name": "omnibox-max-url-matches",
     "owners": [ "orinj", "chrome-omnibox-team@google.com" ],

diff --git a/chrome/browser/flag_descriptions.cc b/chrome/browser/flag_descriptions.cc
@@ -1973,6 +1973,13 @@ const char kOmniboxSiteSearchStarterPackName[] =
 const char kOmniboxSiteSearchStarterPackDescription[] =
     "Enables @history, @bookmarks, and @tabs scopes in Omnibox Site "
     "Search/Keyword Mode";
+
+const char kOmniboxLocalHistoryZeroSuggestBeyondNTPName[] =
+    "Allow local history zero-prefix suggestions beyond NTP";
+const char kOmniboxLocalHistoryZeroSuggestBeyondNTPDescription[] =
+    "Enables local history zero-prefix suggestions in every context in which "
+    "the remote zero-prefix suggestions are enabled.";
+
 const char kOmniboxFocusTriggersSRPZeroSuggestName[] =
     "Allow Omnibox contextual web on-focus suggestions on the SRP";
 const char kOmniboxFocusTriggersSRPZeroSuggestDescription[] =

diff --git a/chrome/browser/flag_descriptions.h b/chrome/browser/flag_descriptions.h
@@ -1103,6 +1103,9 @@ extern const char kOmniboxRemoveSuggestionHeaderCapitalizationDescription[];
 extern const char kOmniboxRichAutocompletionPromisingName[];
 extern const char kOmniboxRichAutocompletionPromisingDescription[];
 
+extern const char kOmniboxLocalHistoryZeroSuggestBeyondNTPName[];
+extern const char kOmniboxLocalHistoryZeroSuggestBeyondNTPDescription[];
+
 extern const char kOmniboxFocusTriggersSRPZeroSuggestName[];
 extern const char kOmniboxFocusTriggersSRPZeroSuggestDescription[];
 

diff --git a/components/omnibox/browser/local_history_zero_suggest_provider.cc b/components/omnibox/browser/local_history_zero_suggest_provider.cc
@@ -34,6 +34,7 @@
 #include "components/omnibox/browser/base_search_provider.h"
 #include "components/omnibox/browser/omnibox_field_trial.h"
 #include "components/omnibox/browser/omnibox_prefs.h"
+#include "components/omnibox/browser/zero_suggest_provider.h"
 #include "components/omnibox/common/omnibox_features.h"
 #include "components/search_engines/omnibox_focus_type.h"
 #include "components/search_engines/template_url_service.h"
@@ -58,21 +59,13 @@ std::u16string GetSearchTermsFromURL(const GURL& url,
 // Invoked early, confirms all the conditions for zero suggestions are met.
 bool AllowLocalHistoryZeroSuggestSuggestions(AutocompleteProviderClient* client,
                                              const AutocompleteInput& input) {
-  // Allow local history query suggestions only when the user is not in an
-  // off-the-record context.
+  // Allow local history zero-suggest only when the user is not in incognito
+  // mode.
   if (client->IsOffTheRecord())
     return false;
 
-  // Allow local history query suggestions only when the omnibox is empty and is
-  // focused from the NTP.
-  if (input.focus_type() == OmniboxFocusType::DEFAULT ||
-      input.type() != OmniboxInputType::EMPTY ||
-      !BaseSearchProvider::IsNTPPage(input.current_page_classification())) {
-    return false;
-  }
-
-  // Allow local history query suggestions only when the user has set up Google
-  // as their default search engine.
+  // Allow local history zero-suggest only when the user has set up Google as
+  // their default search engine.
   TemplateURLService* template_url_service = client->GetTemplateURLService();
   if (!template_url_service ||
       !template_url_service->GetDefaultSearchProvider() ||
@@ -81,7 +74,18 @@ bool AllowLocalHistoryZeroSuggestSuggestions(AutocompleteProviderClient* client,
     return false;
   }
 
-  return true;
+  if (base::FeatureList::IsEnabled(
+          omnibox::kLocalHistoryZeroSuggestBeyondNTP)) {
+    // Allow local history zero-suggest where remote zero-suggest is eligible.
+    return ZeroSuggestProvider::ResultTypeToRun(client, input) !=
+           ZeroSuggestProvider::ResultType::kNone;
+  }
+
+  // Allow local history query suggestions only when the omnibox is empty and is
+  // focused from the NTP.
+  return input.focus_type() == OmniboxFocusType::ON_FOCUS &&
+         input.type() == OmniboxInputType::EMPTY &&
+         BaseSearchProvider::IsNTPPage(input.current_page_classification());
 }
 
 void RecordDBMetrics(const base::TimeTicks db_query_time,

diff --git a/components/omnibox/browser/local_history_zero_suggest_provider_unittest.cc b/components/omnibox/browser/local_history_zero_suggest_provider_unittest.cc
@@ -37,6 +37,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 using base::Time;
+using metrics::OmniboxEventProto;
 using OmniboxFieldTrial::GetLocalHistoryZeroSuggestAgeThreshold;
 using OmniboxFieldTrial::kLocalHistoryZeroSuggestRelevanceScore;
 
@@ -136,7 +137,8 @@ class LocalHistoryZeroSuggestProviderTest
   // Creates an input using the provided information and queries the provider.
   void StartProviderAndWaitUntilDone(const std::string& text,
                                      OmniboxFocusType focus_type,
-                                     PageClassification page_classification);
+                                     PageClassification page_classification,
+                                     const std::string& current_url);
 
   // Verifies that provider matches are as expected.
   void ExpectMatches(const std::vector<TestMatchData>& match_data_list);
@@ -198,11 +200,12 @@ void LocalHistoryZeroSuggestProviderTest::WaitForHistoryService() {
 void LocalHistoryZeroSuggestProviderTest::StartProviderAndWaitUntilDone(
     const std::string& text = "",
     OmniboxFocusType focus_type = OmniboxFocusType::ON_FOCUS,
-    PageClassification page_classification =
-        metrics::OmniboxEventProto::NTP_REALBOX) {
+    PageClassification page_classification = OmniboxEventProto::NTP_REALBOX,
+    const std::string& current_url = "") {
   AutocompleteInput input(base::ASCIIToUTF16(text), page_classification,
                           TestSchemeClassifier());
   input.set_focus_type(focus_type);
+  input.set_current_url(GURL(current_url));
   provider_->Start(input, false);
   if (!provider_->done()) {
     provider_run_loop_ = std::make_unique<base::RunLoop>();
@@ -310,19 +313,72 @@ TEST_P(LocalHistoryZeroSuggestProviderTest, Incognito) {
       {{"hello world", kLocalHistoryZeroSuggestRelevanceScore.Get()}});
 }
 
-// Tests that suggestions are returned only if FeatureFlags is configured
-// to return local history suggestions in the NTP.
-TEST_P(LocalHistoryZeroSuggestProviderTest, FeatureFlags) {
+// Tests that suggestions are allowed in the eligibile entry points.
+TEST_P(LocalHistoryZeroSuggestProviderTest, EntryPoint) {
   LoadURLs({
       {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();
-  ExpectMatches(
-      {{"hello world", kLocalHistoryZeroSuggestRelevanceScore.Get()}});
+  {
+    // Disable local history zero-prefix suggestions beyond NTP.
+    base::test::ScopedFeatureList features;
+    features.InitAndDisableFeature(omnibox::kLocalHistoryZeroSuggestBeyondNTP);
+    StartProviderAndWaitUntilDone();
+
+    // Local history zero-prefix suggestions are enabled by default.
+    ExpectMatches(
+        {{"hello world", kLocalHistoryZeroSuggestRelevanceScore.Get()}});
+  }
+  {
+    // Enable on-focus for SRP.
+    // Disable local history zero-prefix suggestions beyond NTP.
+    base::test::ScopedFeatureList features;
+    features.InitWithFeatures(
+        /*enabled_features=*/{omnibox::kFocusTriggersSRPZeroSuggest},
+        /*disabled_features=*/{omnibox::kLocalHistoryZeroSuggestBeyondNTP});
+    StartProviderAndWaitUntilDone(
+        /*text=*/"https://example.com/", OmniboxFocusType::ON_FOCUS,
+        OmniboxEventProto::SEARCH_RESULT_PAGE_NO_SEARCH_TERM_REPLACEMENT,
+        /*current_url=*/"https://example.com/");
+
+    // Local history zero-prefix suggestions are disabled for on-focus SRP.
+    ExpectMatches({});
+  }
+  {
+    // Enable on-focus for SRP.
+    // Enable local history zero-prefix suggestions beyond NTP.
+    base::test::ScopedFeatureList features;
+    features.InitWithFeatures(
+        /*enabled_features=*/
+        {
+            omnibox::kFocusTriggersSRPZeroSuggest,
+            omnibox::kLocalHistoryZeroSuggestBeyondNTP,
+        },
+        /*disabled_features=*/{});
+    StartProviderAndWaitUntilDone(
+        /*text=*/"https://example.com/", OmniboxFocusType::ON_FOCUS,
+        OmniboxEventProto::SEARCH_RESULT_PAGE_NO_SEARCH_TERM_REPLACEMENT,
+        /*current_url=*/"https://example.com/");
+
+    // Local history zero-prefix suggestions are enabled for on-focus SRP.
+    ExpectMatches(
+        {{"hello world", kLocalHistoryZeroSuggestRelevanceScore.Get()}});
+  }
+  {
+    // Disable on-focus for SRP.
+    // Enable local history zero-prefix suggestions beyond NTP.
+    base::test::ScopedFeatureList features;
+    features.InitWithFeatures(
+        /*enabled_features=*/{omnibox::kLocalHistoryZeroSuggestBeyondNTP},
+        /*disabled_features=*/{omnibox::kFocusTriggersSRPZeroSuggest});
+    StartProviderAndWaitUntilDone(
+        /*text=*/"https://example.com/", OmniboxFocusType::ON_FOCUS,
+        OmniboxEventProto::SEARCH_RESULT_PAGE_NO_SEARCH_TERM_REPLACEMENT,
+        /*current_url=*/"https://example.com/");
+
+    // Local history zero-prefix suggestions are disabled for on-focus SRP.
+    ExpectMatches({});
+  }
 }
 
 // Tests that search terms are extracted from the default search provider's

diff --git a/components/omnibox/common/omnibox_features.cc b/components/omnibox/common/omnibox_features.cc
@@ -132,6 +132,11 @@ const base::Feature kFocusTriggersSRPZeroSuggest{
 extern const base::Feature kLocalHistorySuggestRevamp{
     "LocalHistorySuggestRevamp", base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Enables local history zero-prefix suggestions in every context in which the
+// remote zero-prefix suggestions are enabled.
+const base::Feature kLocalHistoryZeroSuggestBeyondNTP{
+    "LocalHistoryZeroSuggestBeyondNTP", base::FEATURE_DISABLED_BY_DEFAULT};
+
 // Used to adjust the age threshold since the last visit in order to consider a
 // normalized keyword search term as a zero-prefix suggestion. If disabled, the
 // default value of 60 days for Desktop and 7 days for Android and iOS is used.

diff --git a/components/omnibox/common/omnibox_features.h b/components/omnibox/common/omnibox_features.h
@@ -42,6 +42,7 @@ extern const base::Feature kClobberTriggersSRPZeroSuggest;
 extern const base::Feature kFocusTriggersContextualWebZeroSuggest;
 extern const base::Feature kFocusTriggersSRPZeroSuggest;
 extern const base::Feature kLocalHistorySuggestRevamp;
+extern const base::Feature kLocalHistoryZeroSuggestBeyondNTP;
 extern const base::Feature kOmniboxLocalZeroSuggestAgeThreshold;
 extern const base::Feature kZeroSuggestOnNTPForSignedOutUsers;
 extern const base::Feature kZeroSuggestPrefetching;

diff --git a/ios/chrome/browser/flags/about_flags.mm b/ios/chrome/browser/flags/about_flags.mm
@@ -605,6 +605,11 @@
          omnibox::kUIExperimentMaxAutocompleteMatches,
          kOmniboxUIMaxAutocompleteMatchesVariations,
          "OmniboxUIMaxAutocompleteVariations")},
+    {"omnibox-local-history-zero-suggest-beyond-ntp",
+     flag_descriptions::kOmniboxLocalHistoryZeroSuggestBeyondNTPName,
+     flag_descriptions::kOmniboxLocalHistoryZeroSuggestBeyondNTPDescription,
+     flags_ui::kOsIos,
+     FEATURE_VALUE_TYPE(omnibox::kLocalHistoryZeroSuggestBeyondNTP)},
     {"omnibox-max-zps-matches", flag_descriptions::kOmniboxMaxZPSMatchesName,
      flag_descriptions::kOmniboxMaxZPSMatchesDescription, flags_ui::kOsIos,
      FEATURE_WITH_PARAMS_VALUE_TYPE(omnibox::kMaxZeroSuggestMatches,

diff --git a/ios/chrome/browser/flags/ios_chrome_flag_descriptions.cc b/ios/chrome/browser/flags/ios_chrome_flag_descriptions.cc
@@ -545,6 +545,12 @@ const char kOmniboxPasteButtonDescription[] =
     "Add a paste button when showing clipboard suggestions in the omnibox. iOS "
     "16 and above.";
 
+const char kOmniboxLocalHistoryZeroSuggestBeyondNTPName[] =
+    "Allow local history zero-prefix suggestions beyond NTP";
+const char kOmniboxLocalHistoryZeroSuggestBeyondNTPDescription[] =
+    "Enables local history zero-prefix suggestions in every context in which "
+    "the remote zero-prefix suggestions are enabled.";
+
 const char kOmniboxZeroSuggestPrefetchingName[] =
     "Omnibox Zero Prefix Suggestion Prefetching on NTP";
 const char kOmniboxZeroSuggestPrefetchingDescription[] =

diff --git a/ios/chrome/browser/flags/ios_chrome_flag_descriptions.h b/ios/chrome/browser/flags/ios_chrome_flag_descriptions.h
@@ -501,6 +501,10 @@ extern const char kOmniboxNewImplementationDescription[];
 extern const char kOmniboxPasteButtonName[];
 extern const char kOmniboxPasteButtonDescription[];
 
+// Title and description for local history zero-prefix suggestions beyond NTP.
+extern const char kOmniboxLocalHistoryZeroSuggestBeyondNTPName[];
+extern const char kOmniboxLocalHistoryZeroSuggestBeyondNTPDescription[];
+
 // Title and description for the zero-suggest prefetching on the New Tab Page.
 extern const char kOmniboxZeroSuggestPrefetchingName[];
 extern const char kOmniboxZeroSuggestPrefetchingDescription[];

diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
@@ -61160,6 +61160,7 @@ from previous Chrome versions.
   <int value="1398044738" label="EchePhoneHubPermissionsOnboarding:enabled"/>
   <int value="1398049903" label="WebUIDarkMode:disabled"/>
   <int value="1398333721" label="ForceEnableDevicesPage:disabled"/>
+  <int value="1399386367" label="LocalHistoryZeroSuggestBeyondNTP:disabled"/>
   <int value="1399950951" label="AutofillTokenPrefixMatching:disabled"/>
   <int value="1403195370" label="ArcCupsApi:enabled"/>
   <int value="1403792475" label="ShareMenu:enabled"/>
@@ -62160,6 +62161,7 @@ from previous Chrome versions.
   <int value="2051403297" label="ShowBluetoothDeviceBattery:disabled"/>
   <int value="2051886966" label="PwaInstallUseBottomSheet:enabled"/>
   <int value="2052194225" label="ChromeOSSharingHub:disabled"/>
+  <int value="2055184887" label="LocalHistoryZeroSuggestBeyondNTP:enabled"/>
   <int value="2056572020" label="EnableUsernameCorrection:disabled"/>
   <int value="2058148069" label="UseMessagesStagingUrl:enabled"/>
   <int value="2058283872" label="CCTModuleCache:disabled"/>