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

[omnibox] Gate DEFAULT_SERP_FOR_URL by page classification

Currently, ZeroSuggestProvider uses DEFAULT_SERP_FOR_URL by default.

After this CL, ZeroSuggestProvider will use NONE as the default.

And there will be a ZeroSuggestVariant value of "RemoteSendURL" that
can be queried via:
OmniboxFieldTrial::InZeroSuggestRemoteSendURLFieldTrial()

The objective of this is twofold:

 1) Enable us to switch put certain page classifications into the
    DEFAULT_SERP_FOR_URL mode without putting every page classification
    into this. This is needed for On Focus Query
    Refinement.

 2) Disables ZeroSuggestProvider by default for all page
    classifications not in this field trial. This will allow us to
    get rid of the fake ZeroSuggestRedirectToChrome URL method of
    disabling ZeroSuggestProvider, which is not obvious.

    This is also fairly important for On Focus Query Refinement, as
    then we will want to specify a real endpoint URL, not a fake one
    to disable the feature.

Bug: 963173
Change-Id: I61ce4bbf2f228b477bdf82c581b4cedd88fa546b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1632920Reviewed-by: default avatarMark Pearson <mpearson@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#665959}
parent a617d4c0
...@@ -257,6 +257,13 @@ bool OmniboxFieldTrial::InZeroSuggestPersonalizedFieldTrial( ...@@ -257,6 +257,13 @@ bool OmniboxFieldTrial::InZeroSuggestPersonalizedFieldTrial(
page_classification) == "Personalized"; page_classification) == "Personalized";
} }
bool OmniboxFieldTrial::InZeroSuggestRemoteSendURLFieldTrial(
metrics::OmniboxEventProto::PageClassification page_classification) {
return internal::GetValueForRuleInContextByFeature(
omnibox::kOnFocusSuggestions, kZeroSuggestVariantRule,
page_classification) == "RemoteSendURL";
}
// static // static
int OmniboxFieldTrial::GetOnFocusSuggestionsCustomEndpointExperimentId() { int OmniboxFieldTrial::GetOnFocusSuggestionsCustomEndpointExperimentId() {
return base::GetFieldTrialParamByFeatureAsInt( return base::GetFieldTrialParamByFeatureAsInt(
......
...@@ -176,6 +176,11 @@ bool InZeroSuggestMostVisitedWithoutSerpFieldTrial( ...@@ -176,6 +176,11 @@ bool InZeroSuggestMostVisitedWithoutSerpFieldTrial(
bool InZeroSuggestPersonalizedFieldTrial( bool InZeroSuggestPersonalizedFieldTrial(
metrics::OmniboxEventProto::PageClassification page_classification); metrics::OmniboxEventProto::PageClassification page_classification);
// Returns whether the user is in a ZeroSuggest field trial where a remote
// contextual suggestions endpoint is queried and sent the current URL.
bool InZeroSuggestRemoteSendURLFieldTrial(
metrics::OmniboxEventProto::PageClassification page_classification);
// --------------------------------------------------------- // ---------------------------------------------------------
// For the On Focus Suggestions Custom Endpoint field trial. // For the On Focus Suggestions Custom Endpoint field trial.
......
...@@ -639,5 +639,11 @@ ZeroSuggestProvider::ResultType ZeroSuggestProvider::TypeOfResultToRun( ...@@ -639,5 +639,11 @@ ZeroSuggestProvider::ResultType ZeroSuggestProvider::TypeOfResultToRun(
return MOST_VISITED; return MOST_VISITED;
} }
return can_send_current_url ? DEFAULT_SERP_FOR_URL : NONE; if (can_send_current_url &&
OmniboxFieldTrial::InZeroSuggestRemoteSendURLFieldTrial(
current_page_classification_)) {
return DEFAULT_SERP_FOR_URL;
}
return NONE;
} }
...@@ -155,7 +155,7 @@ class ZeroSuggestProviderTest : public testing::Test, ...@@ -155,7 +155,7 @@ class ZeroSuggestProviderTest : public testing::Test,
void SetZeroSuggestVariantForAllContexts(const std::string& variant); void SetZeroSuggestVariantForAllContexts(const std::string& variant);
base::test::ScopedTaskEnvironment scoped_task_environment_; base::test::ScopedTaskEnvironment scoped_task_environment_;
base::test::ScopedFeatureList scoped_feature_list_; std::unique_ptr<base::test::ScopedFeatureList> scoped_feature_list_;
std::unique_ptr<FakeAutocompleteProviderClient> client_; std::unique_ptr<FakeAutocompleteProviderClient> client_;
scoped_refptr<ZeroSuggestProvider> provider_; scoped_refptr<ZeroSuggestProvider> provider_;
...@@ -196,13 +196,10 @@ void ZeroSuggestProviderTest::CreateMostVisitedFieldTrial() { ...@@ -196,13 +196,10 @@ void ZeroSuggestProviderTest::CreateMostVisitedFieldTrial() {
SetZeroSuggestVariantForAllContexts("MostVisitedWithoutSERP"); SetZeroSuggestVariantForAllContexts("MostVisitedWithoutSERP");
} }
void ZeroSuggestProviderTest::CreateContextualSuggestFieldTrial() {
SetZeroSuggestVariantForAllContexts("ContextualSuggestions");
}
void ZeroSuggestProviderTest::SetZeroSuggestVariantForAllContexts( void ZeroSuggestProviderTest::SetZeroSuggestVariantForAllContexts(
const std::string& variant) { const std::string& variant) {
scoped_feature_list_.InitAndEnableFeatureWithParameters( scoped_feature_list_ = std::make_unique<base::test::ScopedFeatureList>();
scoped_feature_list_->InitAndEnableFeatureWithParameters(
omnibox::kOnFocusSuggestions, omnibox::kOnFocusSuggestions,
{{std::string(OmniboxFieldTrial::kZeroSuggestVariantRule) + ":*:*", {{std::string(OmniboxFieldTrial::kZeroSuggestVariantRule) + ":*:*",
variant}}); variant}});
...@@ -469,7 +466,6 @@ TEST_F(ZeroSuggestProviderTest, TestPsuggestZeroSuggestReceivedEmptyResults) { ...@@ -469,7 +466,6 @@ TEST_F(ZeroSuggestProviderTest, TestPsuggestZeroSuggestReceivedEmptyResults) {
} }
TEST_F(ZeroSuggestProviderTest, CustomEndpoint) { TEST_F(ZeroSuggestProviderTest, CustomEndpoint) {
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,
// and to do that, ZeroSuggestProvider needs to be looking for // and to do that, ZeroSuggestProvider needs to be looking for
...@@ -477,11 +473,21 @@ TEST_F(ZeroSuggestProviderTest, CustomEndpoint) { ...@@ -477,11 +473,21 @@ TEST_F(ZeroSuggestProviderTest, CustomEndpoint) {
// experiments off, IsPersonalizedUrlDataCollectionActive true), and the // experiments off, IsPersonalizedUrlDataCollectionActive true), and the
// 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; features.InitWithFeaturesAndParameters(
params[std::string(OmniboxFieldTrial::kOnFocusSuggestionsEndpointURLParam)] = {
"https://cuscochromeextension-pa.googleapis.com/v1/omniboxsuggestions"; {
features.InitAndEnableFeatureWithParameters( omnibox::kOnFocusSuggestions,
omnibox::kOnFocusSuggestionsCustomEndpoint, params); {{std::string(OmniboxFieldTrial::kZeroSuggestVariantRule) +
":*:*",
"RemoteSendURL"}},
},
{
omnibox::kOnFocusSuggestionsCustomEndpoint,
{{OmniboxFieldTrial::kOnFocusSuggestionsEndpointURLParam,
"https://valid-but-fake-endpoint.com/fakepath"}},
},
},
{});
EXPECT_CALL(*client_, IsAuthenticated()) EXPECT_CALL(*client_, IsAuthenticated())
.WillRepeatedly(testing::Return(true)); .WillRepeatedly(testing::Return(true));
...@@ -496,6 +502,6 @@ TEST_F(ZeroSuggestProviderTest, CustomEndpoint) { ...@@ -496,6 +502,6 @@ TEST_F(ZeroSuggestProviderTest, CustomEndpoint) {
input.set_from_omnibox_focus(true); input.set_from_omnibox_focus(true);
provider_->Start(input, false); provider_->Start(input, false);
EXPECT_TRUE(test_loader_factory()->IsPending( EXPECT_TRUE(test_loader_factory()->IsPending(
"https://cuscochromeextension-pa.googleapis.com/v1/omniboxsuggestions")); "https://valid-but-fake-endpoint.com/fakepath"));
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
} }
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