[omnibox] ZeroSuggestProvider::TypeOfResultToRun w/ AutocompleteInput
This CL refactors ZeroSuggestProvider::TypeOfResultToRun to take an
AutocompleteInput as a parameter instead of just the current page URL.
We need this because differentiating the on-focus vs. on-clobber
ZeroSuggest modes requires access to AutocompleteInput::focus_type().
This CL also makes the method 'static', for three reasons:
1. The function is more easily testable when it only depends on its
parameters, rather than any state in the member variables.
2. The member variables store data that's also in the |input|
parameter, and I didn't want to introduce ambiguity as to which
is used.
3. We plan to split MostVisited into its own provider, and we will
want to share some logic from TypeOfResultToRun. Making this
method static is a step towards that future.
For reviewability, this CL actually has no functional changes, and
just moves things around.
The followup CL will actually add the on-clobber vs. on-focus logic to
TypeOfResultToRun.
Bug: 1106096
Change-Id: I69c8f2f08a182796be9ce1c66611fe6788a6158a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2360562
Reviewed-by: Tomasz Wiszkowski <[email protected]>
Commit-Queue: Tommy Li <[email protected]>
Cr-Commit-Position: refs/heads/master@{#799394}
diff --git a/components/omnibox/browser/zero_suggest_provider.cc b/components/omnibox/browser/zero_suggest_provider.cc
index 1215cbd..22930fa8 100644
--- a/components/omnibox/browser/zero_suggest_provider.cc
+++ b/components/omnibox/browser/zero_suggest_provider.cc
@@ -241,7 +241,7 @@
if (!suggest_url.is_valid())
return;
- result_type_running_ = TypeOfResultToRun(input.current_url(), suggest_url);
+ result_type_running_ = TypeOfResultToRun(client(), input, suggest_url);
if (result_type_running_ == NONE)
return;
@@ -674,26 +674,34 @@
}
}
+// static
ZeroSuggestProvider::ResultType ZeroSuggestProvider::TypeOfResultToRun(
- const GURL& current_url,
+ AutocompleteProviderClient* client,
+ const AutocompleteInput& input,
const GURL& suggest_url) {
+ DCHECK(client);
// Check if the URL can be sent in any suggest request.
const TemplateURLService* template_url_service =
- client()->GetTemplateURLService();
+ client->GetTemplateURLService();
DCHECK(template_url_service);
const TemplateURL* default_provider =
template_url_service->GetDefaultSearchProvider();
+
+ GURL current_url = input.current_url();
+ metrics::OmniboxEventProto::PageClassification current_page_classification =
+ input.current_page_classification();
+
const bool can_send_current_url = CanSendURL(
- current_url, suggest_url, default_provider, current_page_classification_,
- template_url_service->search_terms_data(), client(), false);
+ current_url, suggest_url, default_provider, current_page_classification,
+ template_url_service->search_terms_data(), client, false);
// Collect metrics on eligibility.
GURL arbitrary_insecure_url(kArbitraryInsecureUrlString);
ZeroSuggestEligibility eligibility = ZeroSuggestEligibility::ELIGIBLE;
if (!can_send_current_url) {
const bool can_send_ordinary_url =
CanSendURL(arbitrary_insecure_url, suggest_url, default_provider,
- current_page_classification_,
- template_url_service->search_terms_data(), client(), false);
+ current_page_classification,
+ template_url_service->search_terms_data(), client, false);
eligibility = can_send_ordinary_url
? ZeroSuggestEligibility::URL_INELIGIBLE
: ZeroSuggestEligibility::GENERALLY_INELIGIBLE;
@@ -703,12 +711,12 @@
static_cast<int>(ZeroSuggestEligibility::ELIGIBLE_MAX_VALUE));
const auto field_trial_variants =
- OmniboxFieldTrial::GetZeroSuggestVariants(current_page_classification_);
+ OmniboxFieldTrial::GetZeroSuggestVariants(current_page_classification);
if (base::Contains(field_trial_variants, kNoneVariant))
return NONE;
- if (current_page_classification_ == OmniboxEventProto::CHROMEOS_APP_LIST)
+ if (current_page_classification == OmniboxEventProto::CHROMEOS_APP_LIST)
return REMOTE_NO_URL;
// Contextual Open Web - (same client side behavior for multiple variants).
@@ -716,25 +724,25 @@
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) {
return REMOTE_SEND_URL;
}
// Reactive Zero-Prefix Suggestions (rZPS) on NTP cases.
bool remote_no_url_allowed =
- RemoteNoUrlSuggestionsAreAllowed(client(), template_url_service);
+ RemoteNoUrlSuggestionsAreAllowed(client, template_url_service);
if (remote_no_url_allowed) {
// NTP Omnibox.
- if ((current_page_classification_ == OmniboxEventProto::NTP ||
- current_page_classification_ ==
+ if ((current_page_classification == OmniboxEventProto::NTP ||
+ current_page_classification ==
OmniboxEventProto::INSTANT_NTP_WITH_OMNIBOX_AS_STARTING_FOCUS) &&
base::FeatureList::IsEnabled(
omnibox::kReactiveZeroSuggestionsOnNTPOmnibox)) {
return REMOTE_NO_URL;
}
// NTP Realbox.
- if (current_page_classification_ == OmniboxEventProto::NTP_REALBOX &&
+ if (current_page_classification == OmniboxEventProto::NTP_REALBOX &&
base::FeatureList::IsEnabled(
omnibox::kReactiveZeroSuggestionsOnNTPRealbox)) {
return REMOTE_NO_URL;
@@ -755,13 +763,13 @@
#if !defined(OS_IOS)
// For Desktop and Android, default to REMOTE_NO_URL on the NTP, if allowed.
- if (IsNTPPage(current_page_classification_) && remote_no_url_allowed)
+ if (IsNTPPage(current_page_classification) && remote_no_url_allowed)
return REMOTE_NO_URL;
#endif
#if defined(OS_ANDROID) || defined(OS_IOS)
// For Android and iOS, default to MOST_VISITED everywhere except on the SERP.
- if (!IsSearchResultsPage(current_page_classification_)) {
+ if (!IsSearchResultsPage(current_page_classification)) {
return MOST_VISITED;
}
#endif
