Commit 189b653b authored by Moe Ahmadi's avatar Moe Ahmadi Committed by Commit Bot

[omnibox] Request remote zero-prefix suggestions for signed out users

This is gated by omnibox::kOmniboxTrendingZeroPrefixSuggestionsOnNTP

With the change the platform-specific defaults for signed-out users will
change as follows:

- Desktop: no request -> remote suggestions.
- Android: Most Visited -> remote suggestions.
- iOS: N/A. Continues to request Most Visited.

Bug: 1122669
Change-Id: I5b226c7d5d56912a93f3d6ac4b1e295e7c50e07b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2388157Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803916}
parent 87445d03
...@@ -113,6 +113,7 @@ void LogOmniboxZeroSuggestRequest( ...@@ -113,6 +113,7 @@ void LogOmniboxZeroSuggestRequest(
void LogOmniboxRemoteNoUrlEligibilityOnNTP( void LogOmniboxRemoteNoUrlEligibilityOnNTP(
OmniboxEventProto::PageClassification page_class, OmniboxEventProto::PageClassification page_class,
bool log_for_profile_open, bool log_for_profile_open,
bool check_authentication_state,
AutocompleteProviderClient* client) { AutocompleteProviderClient* client) {
ZeroSuggestEligibilityForRemoteNoURL value = ZeroSuggestEligibilityForRemoteNoURL value =
ZeroSuggestEligibilityForRemoteNoURL::kEligible; ZeroSuggestEligibilityForRemoteNoURL::kEligible;
...@@ -131,7 +132,7 @@ void LogOmniboxRemoteNoUrlEligibilityOnNTP( ...@@ -131,7 +132,7 @@ void LogOmniboxRemoteNoUrlEligibilityOnNTP(
} else if (!client->SearchSuggestEnabled()) { } else if (!client->SearchSuggestEnabled()) {
value = value =
ZeroSuggestEligibilityForRemoteNoURL::kIneligibleSuggestionsDisabled; ZeroSuggestEligibilityForRemoteNoURL::kIneligibleSuggestionsDisabled;
} else if (!client->IsAuthenticated()) { } else if (check_authentication_state && !client->IsAuthenticated()) {
value = value =
ZeroSuggestEligibilityForRemoteNoURL::kIneligibleUserNotAuthenticated; ZeroSuggestEligibilityForRemoteNoURL::kIneligibleUserNotAuthenticated;
} else if (service == nullptr || provider == nullptr || } else if (service == nullptr || provider == nullptr ||
...@@ -160,17 +161,21 @@ constexpr char kOmniboxZeroSuggestEligibleHistogramName[] = ...@@ -160,17 +161,21 @@ constexpr char kOmniboxZeroSuggestEligibleHistogramName[] =
"Omnibox.ZeroSuggest.Eligible.OnFocusV2"; "Omnibox.ZeroSuggest.Eligible.OnFocusV2";
// Remote suggestions are allowed only if the user is signed-in and has Google // Remote suggestions are allowed only if the user is signed-in and has Google
// set up as their default search engine. This only applies to // set up as their default search engine. The authentication state check is done
// kRemoteNoUrlVariant since most of these checks are done in // not for privacy reasons but to prevent signed-out users from querying the
// BaseSearchProvider::CanSendURL (with the exception of the authentication // server which does not have any suggestions for them. This check is skipped if
// state) which applies to kRemoteSendUrlVariant. // |check_authentication_state| is false.
// This function only applies to kRemoteNoUrlVariant. For kRemoteSendUrlVariant,
// most of these checks with the exception of the authentication state are done
// in BaseSearchProvider::CanSendURL().
bool RemoteNoUrlSuggestionsAreAllowed( bool RemoteNoUrlSuggestionsAreAllowed(
AutocompleteProviderClient* client, AutocompleteProviderClient* client,
const TemplateURLService* template_url_service) { const TemplateURLService* template_url_service,
bool check_authentication_state) {
if (!client->SearchSuggestEnabled()) if (!client->SearchSuggestEnabled())
return false; return false;
if (!client->IsAuthenticated()) if (check_authentication_state && !client->IsAuthenticated())
return false; return false;
if (template_url_service == nullptr) if (template_url_service == nullptr)
...@@ -214,8 +219,10 @@ void ZeroSuggestProvider::Start(const AutocompleteInput& input, ...@@ -214,8 +219,10 @@ void ZeroSuggestProvider::Start(const AutocompleteInput& input,
if (input.focus_type() != OmniboxFocusType::DEFAULT && if (input.focus_type() != OmniboxFocusType::DEFAULT &&
IsNTPPage(current_page_classification_)) { IsNTPPage(current_page_classification_)) {
bool check_authentication_state = !base::FeatureList::IsEnabled(
omnibox::kOmniboxTrendingZeroPrefixSuggestionsOnNTP);
LogOmniboxRemoteNoUrlEligibilityOnNTP(current_page_classification_, false, LogOmniboxRemoteNoUrlEligibilityOnNTP(current_page_classification_, false,
client()); check_authentication_state, client());
} }
if (!AllowZeroSuggestSuggestions(input)) { if (!AllowZeroSuggestSuggestions(input)) {
...@@ -359,9 +366,11 @@ ZeroSuggestProvider::ZeroSuggestProvider( ...@@ -359,9 +366,11 @@ ZeroSuggestProvider::ZeroSuggestProvider(
template_url_service->search_terms_data(), client, template_url_service->search_terms_data(), client,
false)); false));
bool check_authentication_state = !base::FeatureList::IsEnabled(
omnibox::kOmniboxTrendingZeroPrefixSuggestionsOnNTP);
LogOmniboxRemoteNoUrlEligibilityOnNTP( LogOmniboxRemoteNoUrlEligibilityOnNTP(
OmniboxEventProto::INSTANT_NTP_WITH_OMNIBOX_AS_STARTING_FOCUS, true, OmniboxEventProto::INSTANT_NTP_WITH_OMNIBOX_AS_STARTING_FOCUS, true,
client); check_authentication_state, client);
} }
} }
...@@ -738,8 +747,10 @@ ZeroSuggestProvider::ResultType ZeroSuggestProvider::TypeOfResultToRun( ...@@ -738,8 +747,10 @@ ZeroSuggestProvider::ResultType ZeroSuggestProvider::TypeOfResultToRun(
} }
// Reactive Zero-Prefix Suggestions (rZPS) on NTP cases. // Reactive Zero-Prefix Suggestions (rZPS) on NTP cases.
bool remote_no_url_allowed = bool check_authentication_state = !base::FeatureList::IsEnabled(
RemoteNoUrlSuggestionsAreAllowed(client, template_url_service); omnibox::kOmniboxTrendingZeroPrefixSuggestionsOnNTP);
bool remote_no_url_allowed = RemoteNoUrlSuggestionsAreAllowed(
client, template_url_service, check_authentication_state);
if (remote_no_url_allowed) { if (remote_no_url_allowed) {
// NTP Omnibox. // NTP Omnibox.
if ((current_page_classification == OmniboxEventProto::NTP || if ((current_page_classification == OmniboxEventProto::NTP ||
......
...@@ -334,6 +334,14 @@ TEST_F(ZeroSuggestProviderTest, TypeOfResultToRun) { ...@@ -334,6 +334,14 @@ TEST_F(ZeroSuggestProviderTest, TypeOfResultToRun) {
other_input, other_input,
/*remote_no_url_allowed=*/false); /*remote_no_url_allowed=*/false);
// Unless we allow remote suggestions for signed-out users.
scoped_feature_list_ = std::make_unique<base::test::ScopedFeatureList>();
scoped_feature_list_->InitAndEnableFeature(
omnibox::kOmniboxTrendingZeroPrefixSuggestionsOnNTP);
ExpectPlatformSpecificDefaultZeroSuggestBehavior(
other_input,
/*remote_no_url_allowed=*/true);
// Restore authentication state, but now set a non-Google default search // Restore authentication state, but now set a non-Google default search
// engine. Verify that we still disallow remote suggestions. // engine. Verify that we still disallow remote suggestions.
EXPECT_CALL(*client_, IsAuthenticated()) EXPECT_CALL(*client_, IsAuthenticated())
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment