Commit 31e27a63 authored by Tommy Li's avatar Tommy Li Committed by Commit Bot

[omnibox] Allow enabling on-clobber contextual web by itself

After this CL, it's possible to enable on-clobber contextual web
ZeroSuggest without being forced to also enable on-focus contextual web.

Previously, this was not possible, and you HAD to enable on-focus
contextual web ZeroSuggest in order to make on-clobber work.

In other words:

Before this CL:
  To enable on-focus only:
    - kOnFocusSuggestionsContextualWeb
  To enable on-clobber only:
    - NOT POSSIBLE
  To enable both:
    - kOnFocusSuggestionsContextualWeb
    - kClobberTriggersContextualWebZeroSuggest

After this CL:
  To enable on-focus only:
    - kOnFocusSuggestionsContextualWeb
  To enable on-clobber only:
    - kClobberTriggersContextualWebZeroSuggest
  To enable both:
    - kOnFocusSuggestionsContextualWeb
    - kClobberTriggersContextualWebZeroSuggest

Bug: 1106096
Change-Id: Ia75bacc868549013463bf84afb55d28054d0beed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2364138Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799976}
parent b6136a85
...@@ -720,13 +720,21 @@ ZeroSuggestProvider::ResultType ZeroSuggestProvider::TypeOfResultToRun( ...@@ -720,13 +720,21 @@ ZeroSuggestProvider::ResultType ZeroSuggestProvider::TypeOfResultToRun(
return REMOTE_NO_URL; return REMOTE_NO_URL;
// Contextual Open Web - (same client side behavior for multiple variants). // Contextual Open Web - (same client side behavior for multiple variants).
bool contextual_web_suggestions_enabled =
base::FeatureList::IsEnabled(omnibox::kOnFocusSuggestionsContextualWeb) ||
base::FeatureList::IsEnabled(
omnibox::kOnFocusSuggestionsContextualWebOnContent);
if (current_page_classification == OmniboxEventProto::OTHER && if (current_page_classification == OmniboxEventProto::OTHER &&
can_send_current_url && contextual_web_suggestions_enabled) { can_send_current_url) {
return REMOTE_SEND_URL; if (input.focus_type() == OmniboxFocusType::ON_FOCUS &&
(base::FeatureList::IsEnabled(
omnibox::kOnFocusSuggestionsContextualWeb) ||
base::FeatureList::IsEnabled(
omnibox::kOnFocusSuggestionsContextualWebOnContent))) {
return REMOTE_SEND_URL;
}
if (input.focus_type() == OmniboxFocusType::DELETED_PERMANENT_TEXT &&
base::FeatureList::IsEnabled(
omnibox::kClobberTriggersContextualWebZeroSuggest)) {
return REMOTE_SEND_URL;
}
} }
// Reactive Zero-Prefix Suggestions (rZPS) on NTP cases. // Reactive Zero-Prefix Suggestions (rZPS) on NTP cases.
......
...@@ -87,6 +87,8 @@ class ZeroSuggestProvider : public BaseSearchProvider { ...@@ -87,6 +87,8 @@ class ZeroSuggestProvider : public BaseSearchProvider {
FRIEND_TEST_ALL_PREFIXES(ZeroSuggestProviderTest, FRIEND_TEST_ALL_PREFIXES(ZeroSuggestProviderTest,
AllowZeroSuggestSuggestions); AllowZeroSuggestSuggestions);
FRIEND_TEST_ALL_PREFIXES(ZeroSuggestProviderTest, TypeOfResultToRun); FRIEND_TEST_ALL_PREFIXES(ZeroSuggestProviderTest, TypeOfResultToRun);
FRIEND_TEST_ALL_PREFIXES(ZeroSuggestProviderTest,
TypeOfResultToRunForContextualWeb);
FRIEND_TEST_ALL_PREFIXES(ZeroSuggestProviderTest, FRIEND_TEST_ALL_PREFIXES(ZeroSuggestProviderTest,
TestStartWillStopForSomeInput); TestStartWillStopForSomeInput);
ZeroSuggestProvider(AutocompleteProviderClient* client, ZeroSuggestProvider(AutocompleteProviderClient* client,
......
...@@ -260,6 +260,7 @@ TEST_F(ZeroSuggestProviderTest, AllowZeroSuggestSuggestions) { ...@@ -260,6 +260,7 @@ TEST_F(ZeroSuggestProviderTest, AllowZeroSuggestSuggestions) {
} }
} }
// TODO(tommycli): Break up this test into smaller ones.
TEST_F(ZeroSuggestProviderTest, TypeOfResultToRun) { TEST_F(ZeroSuggestProviderTest, TypeOfResultToRun) {
// Verifies the unconfigured state. Returns platorm-specific defaults. // Verifies the unconfigured state. Returns platorm-specific defaults.
// TODO(tommycli): The remote_no_url_allowed idiom seems kind of confusing, // TODO(tommycli): The remote_no_url_allowed idiom seems kind of confusing,
...@@ -399,6 +400,79 @@ TEST_F(ZeroSuggestProviderTest, TypeOfResultToRun) { ...@@ -399,6 +400,79 @@ TEST_F(ZeroSuggestProviderTest, TypeOfResultToRun) {
} }
} }
TEST_F(ZeroSuggestProviderTest, TypeOfResultToRunForContextualWeb) {
std::string input_url = "https://example.com/";
GURL suggest_url = GetSuggestURL(metrics::OmniboxEventProto::OTHER);
AutocompleteInput on_focus_input(base::ASCIIToUTF16(input_url),
metrics::OmniboxEventProto::OTHER,
TestSchemeClassifier());
on_focus_input.set_current_url(GURL(input_url));
on_focus_input.set_focus_type(OmniboxFocusType::ON_FOCUS);
AutocompleteInput on_clobber_input(base::string16(),
metrics::OmniboxEventProto::OTHER,
TestSchemeClassifier());
on_clobber_input.set_current_url(GURL(input_url));
on_clobber_input.set_focus_type(OmniboxFocusType::DELETED_PERMANENT_TEXT);
#if defined(OS_ANDROID) || defined(OS_IOS)
const ZeroSuggestProvider::ResultType kDefaultContextualWebResultType =
ZeroSuggestProvider::ResultType::MOST_VISITED;
#else
const ZeroSuggestProvider::ResultType kDefaultContextualWebResultType =
ZeroSuggestProvider::ResultType::NONE;
#endif
EXPECT_EQ(kDefaultContextualWebResultType,
ZeroSuggestProvider::TypeOfResultToRun(
client_.get(), on_focus_input, suggest_url));
EXPECT_EQ(kDefaultContextualWebResultType,
ZeroSuggestProvider::TypeOfResultToRun(
client_.get(), on_clobber_input, suggest_url));
// Enable on-focus only.
{
base::test::ScopedFeatureList features;
features.InitAndEnableFeature(omnibox::kOnFocusSuggestionsContextualWeb);
EXPECT_EQ(ZeroSuggestProvider::ResultType::REMOTE_SEND_URL,
ZeroSuggestProvider::TypeOfResultToRun(
client_.get(), on_focus_input, suggest_url));
EXPECT_EQ(kDefaultContextualWebResultType,
ZeroSuggestProvider::TypeOfResultToRun(
client_.get(), on_clobber_input, suggest_url));
}
// Enable on-clobber only.
{
base::test::ScopedFeatureList features;
features.InitAndEnableFeature(
omnibox::kClobberTriggersContextualWebZeroSuggest);
EXPECT_EQ(kDefaultContextualWebResultType,
ZeroSuggestProvider::TypeOfResultToRun(
client_.get(), on_focus_input, suggest_url));
EXPECT_EQ(ZeroSuggestProvider::ResultType::REMOTE_SEND_URL,
ZeroSuggestProvider::TypeOfResultToRun(
client_.get(), on_clobber_input, suggest_url));
}
// Enable on-focus and on-clobber.
{
base::test::ScopedFeatureList features;
features.InitWithFeatures(
{omnibox::kOnFocusSuggestionsContextualWeb,
omnibox::kClobberTriggersContextualWebZeroSuggest},
{});
EXPECT_EQ(ZeroSuggestProvider::ResultType::REMOTE_SEND_URL,
ZeroSuggestProvider::TypeOfResultToRun(
client_.get(), on_focus_input, suggest_url));
EXPECT_EQ(ZeroSuggestProvider::ResultType::REMOTE_SEND_URL,
ZeroSuggestProvider::TypeOfResultToRun(
client_.get(), on_clobber_input, suggest_url));
}
}
TEST_F(ZeroSuggestProviderTest, TestDoesNotReturnMatchesForPrefix) { TEST_F(ZeroSuggestProviderTest, TestDoesNotReturnMatchesForPrefix) {
CreateRemoteNoUrlFieldTrial(); CreateRemoteNoUrlFieldTrial();
......
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