Commit 0099c854 authored by Tommy C. Li's avatar Tommy C. Li Committed by Commit Bot

[omnibox] Rename ZeroSuggestRedirectToChrome

This CL is nearly a pure rename.

It renames the ZeroSuggestRedirectToChrome flags and parameters to
OnFocusSuggestionsCustomEndpoint.

I think the new name is more descriptive, as the old name implied to me
that the request was redirected locally to Chrome, when it was actually
redirected to a Chrome-team maintained custom endpoint.

Going forward, we may be using an endpoint that is NOT maintained by
Chrome-team, so this will become particularly inaccurate if we don't do
the rename.

This feature that's being renamed is currently enabled via field trial
for versions up to 76. This CL renames that feature, but the feature is
still enabled by default in the binary, so it should continue with the
server-side specified behavior.

In other words, only 76 binaries without this feature enabled by
default need the server-side config.

That being said, for good measure it may be good to land this in 77.

Bug: 966462
Change-Id: I4c9f1c1ae285f7d1be1a806b2f4c3b6bd8cd47ee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1633229
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: default avatarMark Pearson <mpearson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#665327}
parent 456ff15f
...@@ -89,7 +89,7 @@ std::string FormatRequestBodyExperimentalService(const std::string& current_url, ...@@ -89,7 +89,7 @@ std::string FormatRequestBodyExperimentalService(const std::string& current_url,
// stream_type = 1 corresponds to zero suggest suggestions. // stream_type = 1 corresponds to zero suggest suggestions.
request->SetInteger("stream_type", 1); request->SetInteger("stream_type", 1);
const int experiment_id = const int experiment_id =
OmniboxFieldTrial::GetZeroSuggestRedirectToChromeExperimentId(); OmniboxFieldTrial::GetOnFocusSuggestionsCustomEndpointExperimentId();
if (experiment_id >= 0) if (experiment_id >= 0)
request->SetInteger("experiment_id", experiment_id); request->SetInteger("experiment_id", experiment_id);
std::string result; std::string result;
...@@ -176,7 +176,8 @@ GURL ContextualSuggestionsService::ExperimentalContextualSuggestionsUrl( ...@@ -176,7 +176,8 @@ GURL ContextualSuggestionsService::ExperimentalContextualSuggestionsUrl(
return GURL(); return GURL();
} }
if (!base::FeatureList::IsEnabled(omnibox::kZeroSuggestRedirectToChrome)) { if (!base::FeatureList::IsEnabled(
omnibox::kOnFocusSuggestionsCustomEndpoint)) {
return GURL(); return GURL();
} }
...@@ -191,16 +192,16 @@ GURL ContextualSuggestionsService::ExperimentalContextualSuggestionsUrl( ...@@ -191,16 +192,16 @@ GURL ContextualSuggestionsService::ExperimentalContextualSuggestionsUrl(
} }
const std::string server_address_param = const std::string server_address_param =
OmniboxFieldTrial::GetZeroSuggestRedirectToChromeServerAddress(); OmniboxFieldTrial::GetOnFocusSuggestionsCustomEndpointURL();
GURL suggest_url(server_address_param.empty() GURL suggest_url(server_address_param.empty()
? kDefaultExperimentalServerAddress ? kDefaultExperimentalServerAddress
: server_address_param); : server_address_param);
// Check that the suggest URL for redirect to chrome field trial is valid. // Check that the custom endpoint URL is valid.
if (!suggest_url.is_valid()) { if (!suggest_url.is_valid()) {
return GURL(); return GURL();
} }
// Check that the suggest URL for redirect to chrome is HTTPS. // Check that the custom endpoint URL is HTTPS.
if (!suggest_url.SchemeIsCryptographic()) { if (!suggest_url.SchemeIsCryptographic()) {
return GURL(); return GURL();
} }
......
...@@ -31,7 +31,16 @@ class SharedURLLoaderFactory; ...@@ -31,7 +31,16 @@ class SharedURLLoaderFactory;
class SimpleURLLoader; class SimpleURLLoader;
} // namespace network } // namespace network
// A service to fetch suggestions from a remote endpoint given a URL. // A service to fetch suggestions from a remote endpoint. Usually, the remote
// endpoint is the default search provider's suggest service. However, the
// endpoint URL can be customized by field trial.
//
// This service is always sent the user's authentication state, so the
// suggestions always can be personalized. This service is also sometimes sent
// the user's current URL, so the suggestions are sometimes also contextual.
//
// TODO(tommycli): Probably we should just rename this RemoteSuggestionsService,
// as the suggestions are not always contextual.
class ContextualSuggestionsService : public KeyedService { class ContextualSuggestionsService : public KeyedService {
public: public:
// |identity_manager| may be null but only unauthenticated requests will // |identity_manager| may be null but only unauthenticated requests will
......
...@@ -49,7 +49,7 @@ class ContextualSuggestionsServiceTest : public testing::Test { ...@@ -49,7 +49,7 @@ class ContextualSuggestionsServiceTest : public testing::Test {
TEST_F(ContextualSuggestionsServiceTest, EnsureAttachCookies) { TEST_F(ContextualSuggestionsServiceTest, EnsureAttachCookies) {
base::test::ScopedFeatureList scoped_feature_list; base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndDisableFeature( scoped_feature_list.InitAndDisableFeature(
omnibox::kZeroSuggestRedirectToChrome); omnibox::kOnFocusSuggestionsCustomEndpoint);
network::ResourceRequest resource_request; network::ResourceRequest resource_request;
test_url_loader_factory_.SetInterceptor( test_url_loader_factory_.SetInterceptor(
......
...@@ -258,18 +258,18 @@ bool OmniboxFieldTrial::InZeroSuggestPersonalizedFieldTrial( ...@@ -258,18 +258,18 @@ bool OmniboxFieldTrial::InZeroSuggestPersonalizedFieldTrial(
} }
// static // static
int OmniboxFieldTrial::GetZeroSuggestRedirectToChromeExperimentId() { int OmniboxFieldTrial::GetOnFocusSuggestionsCustomEndpointExperimentId() {
return base::GetFieldTrialParamByFeatureAsInt( return base::GetFieldTrialParamByFeatureAsInt(
omnibox::kZeroSuggestRedirectToChrome, omnibox::kOnFocusSuggestionsCustomEndpoint,
kZeroSuggestRedirectToChromeExperimentIdParam, kOnFocusSuggestionsEndpointExperimentIdParam,
/*default_value=*/-1); /*default_value=*/-1);
} }
// static // static
std::string OmniboxFieldTrial::GetZeroSuggestRedirectToChromeServerAddress() { std::string OmniboxFieldTrial::GetOnFocusSuggestionsCustomEndpointURL() {
return base::GetFieldTrialParamValueByFeature( return base::GetFieldTrialParamValueByFeature(
omnibox::kZeroSuggestRedirectToChrome, omnibox::kOnFocusSuggestionsCustomEndpoint,
kZeroSuggestRedirectToChromeServerAddressParam); kOnFocusSuggestionsEndpointURLParam);
} }
bool OmniboxFieldTrial::ShortcutsScoringMaxRelevance( bool OmniboxFieldTrial::ShortcutsScoringMaxRelevance(
...@@ -806,10 +806,10 @@ const char OmniboxFieldTrial::kSimplifyHttpsIndicatorParameterBothToLock[] = ...@@ -806,10 +806,10 @@ const char OmniboxFieldTrial::kSimplifyHttpsIndicatorParameterBothToLock[] =
const char OmniboxFieldTrial::kSimplifyHttpsIndicatorParameterKeepSecureChip[] = const char OmniboxFieldTrial::kSimplifyHttpsIndicatorParameterKeepSecureChip[] =
"keep-secure-chip"; "keep-secure-chip";
const char OmniboxFieldTrial::kZeroSuggestRedirectToChromeExperimentIdParam[] = const char OmniboxFieldTrial::kOnFocusSuggestionsEndpointExperimentIdParam[] =
"ZeroSuggestRedirectToChromeExperimentID"; "CustomEndpointExperimentID";
const char OmniboxFieldTrial::kZeroSuggestRedirectToChromeServerAddressParam[] = const char OmniboxFieldTrial::kOnFocusSuggestionsEndpointURLParam[] =
"ZeroSuggestRedirectToChromeServerAddress"; "CustomEndpointURL";
// static // static
int OmniboxFieldTrial::kDefaultMinimumTimeBetweenSuggestQueriesMs = 100; int OmniboxFieldTrial::kDefaultMinimumTimeBetweenSuggestQueriesMs = 100;
......
...@@ -177,14 +177,14 @@ bool InZeroSuggestPersonalizedFieldTrial( ...@@ -177,14 +177,14 @@ bool InZeroSuggestPersonalizedFieldTrial(
metrics::OmniboxEventProto::PageClassification page_classification); metrics::OmniboxEventProto::PageClassification page_classification);
// --------------------------------------------------------- // ---------------------------------------------------------
// For the Zero Suggest Redirect to Chrome field trial. // For the On Focus Suggestions Custom Endpoint field trial.
// Returns the server-side experiment ID to use for contextual suggestions. // Returns the server-side experiment ID to use for contextual suggestions.
// Returns -1 if there is no associated experiment ID. // Returns -1 if there is no associated experiment ID.
int GetZeroSuggestRedirectToChromeExperimentId(); int GetOnFocusSuggestionsCustomEndpointExperimentId();
// Returns the server address associated with the current field trial. // Returns the server address associated with the current field trial.
std::string GetZeroSuggestRedirectToChromeServerAddress(); std::string GetOnFocusSuggestionsCustomEndpointURL();
// --------------------------------------------------------- // ---------------------------------------------------------
// For the ShortcutsScoringMaxRelevance experiment that's part of the // For the ShortcutsScoringMaxRelevance experiment that's part of the
...@@ -512,9 +512,9 @@ extern const char kSimplifyHttpsIndicatorParameterSecureToLock[]; ...@@ -512,9 +512,9 @@ extern const char kSimplifyHttpsIndicatorParameterSecureToLock[];
extern const char kSimplifyHttpsIndicatorParameterBothToLock[]; extern const char kSimplifyHttpsIndicatorParameterBothToLock[];
extern const char kSimplifyHttpsIndicatorParameterKeepSecureChip[]; extern const char kSimplifyHttpsIndicatorParameterKeepSecureChip[];
// Parameter names used by Zero Suggest Redirect to Chrome. // Parameter names used by On Focus Suggestions Custom Endpoint.
extern const char kZeroSuggestRedirectToChromeExperimentIdParam[]; extern const char kOnFocusSuggestionsEndpointExperimentIdParam[];
extern const char kZeroSuggestRedirectToChromeServerAddressParam[]; extern const char kOnFocusSuggestionsEndpointURLParam[];
// The amount of time to wait before sending a new suggest request after the // The amount of time to wait before sending a new suggest request after the
// previous one unless overridden by a field trial parameter. // previous one unless overridden by a field trial parameter.
......
...@@ -468,7 +468,7 @@ TEST_F(ZeroSuggestProviderTest, TestPsuggestZeroSuggestReceivedEmptyResults) { ...@@ -468,7 +468,7 @@ TEST_F(ZeroSuggestProviderTest, TestPsuggestZeroSuggestReceivedEmptyResults) {
prefs->GetString(omnibox::kZeroSuggestCachedResults)); prefs->GetString(omnibox::kZeroSuggestCachedResults));
} }
TEST_F(ZeroSuggestProviderTest, RedirectToChrome) { TEST_F(ZeroSuggestProviderTest, CustomEndpoint) {
CreateContextualSuggestFieldTrial(); CreateContextualSuggestFieldTrial();
// Coverage for the URL-specific page. (Regression test for a DCHECK). // Coverage for the URL-specific page. (Regression test for a DCHECK).
// This is exercising ContextualSuggestionsService::CreateExperimentalRequest, // This is exercising ContextualSuggestionsService::CreateExperimentalRequest,
...@@ -478,11 +478,10 @@ TEST_F(ZeroSuggestProviderTest, RedirectToChrome) { ...@@ -478,11 +478,10 @@ TEST_F(ZeroSuggestProviderTest, RedirectToChrome) {
// redirect to chrome mode on. // redirect to chrome mode on.
base::test::ScopedFeatureList features; base::test::ScopedFeatureList features;
std::map<std::string, std::string> params; std::map<std::string, std::string> params;
params[std::string( params[std::string(OmniboxFieldTrial::kOnFocusSuggestionsEndpointURLParam)] =
OmniboxFieldTrial::kZeroSuggestRedirectToChromeServerAddressParam)] =
"https://cuscochromeextension-pa.googleapis.com/v1/omniboxsuggestions"; "https://cuscochromeextension-pa.googleapis.com/v1/omniboxsuggestions";
features.InitAndEnableFeatureWithParameters( features.InitAndEnableFeatureWithParameters(
omnibox::kZeroSuggestRedirectToChrome, params); omnibox::kOnFocusSuggestionsCustomEndpoint, params);
EXPECT_CALL(*client_, IsAuthenticated()) EXPECT_CALL(*client_, IsAuthenticated())
.WillRepeatedly(testing::Return(true)); .WillRepeatedly(testing::Return(true));
......
...@@ -285,7 +285,8 @@ const base::Feature kOmniboxMaterialDesignWeatherIcons{ ...@@ -285,7 +285,8 @@ const base::Feature kOmniboxMaterialDesignWeatherIcons{
const base::Feature kOnFocusSuggestions{"OmniboxOnFocusSuggestions", const base::Feature kOnFocusSuggestions{"OmniboxOnFocusSuggestions",
base::FEATURE_ENABLED_BY_DEFAULT}; base::FEATURE_ENABLED_BY_DEFAULT};
// Feature used for the Zero Suggest Redirect to Chrome Field Trial. // Feature used to specify a custom endpoint URL for on-focus suggestions that
// are sourced via RPC.
// //
// This feature is *enabled* in order to *disable* all forms of suggestions // This feature is *enabled* in order to *disable* all forms of suggestions
// based on the URL on-focus (whether from "redirect to Chrome" or the // based on the URL on-focus (whether from "redirect to Chrome" or the
...@@ -296,8 +297,9 @@ const base::Feature kOnFocusSuggestions{"OmniboxOnFocusSuggestions", ...@@ -296,8 +297,9 @@ const base::Feature kOnFocusSuggestions{"OmniboxOnFocusSuggestions",
// If this feature were not enabled, Chrome would use the default suggest // If this feature were not enabled, Chrome would use the default suggest
// server for suggestions based on the current URL on focus. There is no // server for suggestions based on the current URL on focus. There is no
// code in Chrome to disable that, so that why we took this route. // code in Chrome to disable that, so that why we took this route.
const base::Feature kZeroSuggestRedirectToChrome{ const base::Feature kOnFocusSuggestionsCustomEndpoint{
"ZeroSuggestRedirectToChrome", base::FEATURE_ENABLED_BY_DEFAULT}; "OmniboxOnFocusSuggestionsCustomEndpoint",
base::FEATURE_ENABLED_BY_DEFAULT};
// Allow suggestions to be shown to the user on the New Tab Page upon focusing // Allow suggestions to be shown to the user on the New Tab Page upon focusing
// URL bar (the omnibox). // URL bar (the omnibox).
......
...@@ -51,7 +51,7 @@ extern const base::Feature kOmniboxMaterialDesignWeatherIcons; ...@@ -51,7 +51,7 @@ extern const base::Feature kOmniboxMaterialDesignWeatherIcons;
// On-Focus Suggestions a.k.a. ZeroSuggest. // On-Focus Suggestions a.k.a. ZeroSuggest.
extern const base::Feature kOnFocusSuggestions; extern const base::Feature kOnFocusSuggestions;
extern const base::Feature kZeroSuggestRedirectToChrome; extern const base::Feature kOnFocusSuggestionsCustomEndpoint;
extern const base::Feature kZeroSuggestionsOnNTP; extern const base::Feature kZeroSuggestionsOnNTP;
} // namespace omnibox } // namespace omnibox
......
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