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

Reland "[omnibox] Rename the ZeroSuggestProvider::ResultType values"

This is a reland of a9d56928

Original change's description:
> [omnibox] Rename the ZeroSuggestProvider::ResultType values
> 
> This CL renames the ZeroSuggestProvider::ResultType values to be
> more descriptive, and adds more commentary.
> 
> They used to imply that we asked the default search provider for
> zero-suggestions, which was sometimes misleading, since that's not
> always true.
> 
> Bug: 963173
> Change-Id: I85a55cb57749441d2adfdb53642247fd055b7d9a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1631903
> Commit-Queue: Tommy Li <tommycli@chromium.org>
> Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#665330}

Bug: 963173
Change-Id: I206a4fd36189e2400bfd754ac4f0fba4ab076075
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1640160
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#666125}
parent c014c4b7
...@@ -180,9 +180,8 @@ void ZeroSuggestProvider::Start(const AutocompleteInput& input, ...@@ -180,9 +180,8 @@ void ZeroSuggestProvider::Start(const AutocompleteInput& input,
return; return;
} }
const std::string current_url = result_type_running_ == DEFAULT_SERP_FOR_URL const std::string current_url =
? current_query_ result_type_running_ == REMOTE_SEND_URL ? current_query_ : std::string();
: std::string();
// Create a request for suggestions, routing completion to // Create a request for suggestions, routing completion to
// OnContextualSuggestionsLoaderAvailable. // OnContextualSuggestionsLoaderAvailable.
client() client()
...@@ -350,7 +349,7 @@ bool ZeroSuggestProvider::UpdateResults(const std::string& json_data) { ...@@ -350,7 +349,7 @@ bool ZeroSuggestProvider::UpdateResults(const std::string& json_data) {
// When running the personalized service, we want to store suggestion // When running the personalized service, we want to store suggestion
// responses if non-empty. // responses if non-empty.
if (result_type_running_ == DEFAULT_SERP && !json_data.empty()) { if (result_type_running_ == REMOTE_NO_URL && !json_data.empty()) {
client()->GetPrefs()->SetString(omnibox::kZeroSuggestCachedResults, client()->GetPrefs()->SetString(omnibox::kZeroSuggestCachedResults,
json_data); json_data);
...@@ -564,7 +563,7 @@ bool ZeroSuggestProvider::AllowZeroSuggestSuggestions( ...@@ -564,7 +563,7 @@ bool ZeroSuggestProvider::AllowZeroSuggestSuggestions(
} }
void ZeroSuggestProvider::MaybeUseCachedSuggestions() { void ZeroSuggestProvider::MaybeUseCachedSuggestions() {
if (result_type_running_ != DEFAULT_SERP) if (result_type_running_ != REMOTE_NO_URL)
return; return;
std::string json_data = std::string json_data =
...@@ -590,7 +589,6 @@ ZeroSuggestProvider::ResultType ZeroSuggestProvider::TypeOfResultToRun( ...@@ -590,7 +589,6 @@ ZeroSuggestProvider::ResultType ZeroSuggestProvider::TypeOfResultToRun(
const bool can_send_current_url = CanSendURL( const bool can_send_current_url = CanSendURL(
current_url, suggest_url, default_provider, current_page_classification_, current_url, suggest_url, default_provider, current_page_classification_,
template_url_service->search_terms_data(), client()); template_url_service->search_terms_data(), client());
// Collect metrics on eligibility. // Collect metrics on eligibility.
GURL arbitrary_insecure_url(kArbitraryInsecureUrlString); GURL arbitrary_insecure_url(kArbitraryInsecureUrlString);
ZeroSuggestEligibility eligibility = ZeroSuggestEligibility::ELIGIBLE; ZeroSuggestEligibility eligibility = ZeroSuggestEligibility::ELIGIBLE;
...@@ -614,7 +612,7 @@ ZeroSuggestProvider::ResultType ZeroSuggestProvider::TypeOfResultToRun( ...@@ -614,7 +612,7 @@ ZeroSuggestProvider::ResultType ZeroSuggestProvider::TypeOfResultToRun(
if (current_page_classification_ == if (current_page_classification_ ==
metrics::OmniboxEventProto::CHROMEOS_APP_LIST) { metrics::OmniboxEventProto::CHROMEOS_APP_LIST) {
return DEFAULT_SERP; return REMOTE_NO_URL;
} }
if (OmniboxFieldTrial::InZeroSuggestPersonalizedFieldTrial( if (OmniboxFieldTrial::InZeroSuggestPersonalizedFieldTrial(
...@@ -623,7 +621,7 @@ ZeroSuggestProvider::ResultType ZeroSuggestProvider::TypeOfResultToRun( ...@@ -623,7 +621,7 @@ ZeroSuggestProvider::ResultType ZeroSuggestProvider::TypeOfResultToRun(
client()->GetPrefs(), client()->IsAuthenticated(), client()->GetPrefs(), client()->IsAuthenticated(),
template_url_service) template_url_service)
? MOST_VISITED ? MOST_VISITED
: DEFAULT_SERP; : REMOTE_NO_URL;
} }
if (OmniboxFieldTrial::InZeroSuggestMostVisitedWithoutSerpFieldTrial( if (OmniboxFieldTrial::InZeroSuggestMostVisitedWithoutSerpFieldTrial(
...@@ -642,7 +640,7 @@ ZeroSuggestProvider::ResultType ZeroSuggestProvider::TypeOfResultToRun( ...@@ -642,7 +640,7 @@ ZeroSuggestProvider::ResultType ZeroSuggestProvider::TypeOfResultToRun(
if (can_send_current_url && if (can_send_current_url &&
OmniboxFieldTrial::InZeroSuggestRemoteSendURLFieldTrial( OmniboxFieldTrial::InZeroSuggestRemoteSendURLFieldTrial(
current_page_classification_)) { current_page_classification_)) {
return DEFAULT_SERP_FOR_URL; return REMOTE_SEND_URL;
} }
return NONE; return NONE;
......
...@@ -64,6 +64,7 @@ class ZeroSuggestProvider : public BaseSearchProvider { ...@@ -64,6 +64,7 @@ class ZeroSuggestProvider : public BaseSearchProvider {
void ResetSession() override; void ResetSession() override;
private: private:
FRIEND_TEST_ALL_PREFIXES(ZeroSuggestProviderTest, TypeOfResultToRun);
FRIEND_TEST_ALL_PREFIXES(ZeroSuggestProviderTest, FRIEND_TEST_ALL_PREFIXES(ZeroSuggestProviderTest,
TestStartWillStopForSomeInput); TestStartWillStopForSomeInput);
ZeroSuggestProvider(AutocompleteProviderClient* client, ZeroSuggestProvider(AutocompleteProviderClient* client,
...@@ -76,12 +77,19 @@ class ZeroSuggestProvider : public BaseSearchProvider { ...@@ -76,12 +77,19 @@ class ZeroSuggestProvider : public BaseSearchProvider {
// at any time. // at any time.
enum ResultType { enum ResultType {
NONE, NONE,
DEFAULT_SERP, // The default search provider is queried for
// zero-suggest suggestions. // A remote endpoint (usually the default search provider) is queried for
DEFAULT_SERP_FOR_URL, // The default search provider is queried for // suggestions. The endpoint is sent the user's authentication state, but
// zero-suggest suggestions that are specific // not sent the current URL.
// to the visited URL. REMOTE_NO_URL,
MOST_VISITED
// A remote endpoint (usually the default search provider) is queried for
// suggestions. The endpoint is sent the user's authentication state and
// the current URL.
REMOTE_SEND_URL,
// Gets the most visited sites from local history.
MOST_VISITED,
}; };
// BaseSearchProvider: // BaseSearchProvider:
...@@ -177,7 +185,8 @@ class ZeroSuggestProvider : public BaseSearchProvider { ...@@ -177,7 +185,8 @@ class ZeroSuggestProvider : public BaseSearchProvider {
// The type of page the user is viewing (a search results page doing search // The type of page the user is viewing (a search results page doing search
// term replacement, an arbitrary URL, etc.). // term replacement, an arbitrary URL, etc.).
metrics::OmniboxEventProto::PageClassification current_page_classification_; metrics::OmniboxEventProto::PageClassification current_page_classification_ =
metrics::OmniboxEventProto::INVALID_SPEC;
// Copy of OmniboxEditModel::permanent_text_. // Copy of OmniboxEditModel::permanent_text_.
base::string16 permanent_text_; base::string16 permanent_text_;
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "base/test/scoped_task_environment.h" #include "base/test/scoped_task_environment.h"
#include "build/build_config.h"
#include "components/history/core/browser/top_sites.h" #include "components/history/core/browser/top_sites.h"
#include "components/omnibox/browser/autocomplete_provider_listener.h" #include "components/omnibox/browser/autocomplete_provider_listener.h"
#include "components/omnibox/browser/mock_autocomplete_provider_client.h" #include "components/omnibox/browser/mock_autocomplete_provider_client.h"
...@@ -151,7 +152,6 @@ class ZeroSuggestProviderTest : public testing::Test, ...@@ -151,7 +152,6 @@ class ZeroSuggestProviderTest : public testing::Test,
void CreatePersonalizedFieldTrial(); void CreatePersonalizedFieldTrial();
void CreateMostVisitedFieldTrial(); void CreateMostVisitedFieldTrial();
void CreateContextualSuggestFieldTrial();
void SetZeroSuggestVariantForAllContexts(const std::string& variant); void SetZeroSuggestVariantForAllContexts(const std::string& variant);
base::test::ScopedTaskEnvironment scoped_task_environment_; base::test::ScopedTaskEnvironment scoped_task_environment_;
...@@ -205,6 +205,36 @@ void ZeroSuggestProviderTest::SetZeroSuggestVariantForAllContexts( ...@@ -205,6 +205,36 @@ void ZeroSuggestProviderTest::SetZeroSuggestVariantForAllContexts(
variant}}); variant}});
} }
TEST_F(ZeroSuggestProviderTest, TypeOfResultToRun) {
GURL current_url = GURL("https://example.com/");
GURL suggest_url = GURL("https://www.google.com/complete/?q={searchTerms}");
EXPECT_CALL(*client_, IsAuthenticated())
.WillRepeatedly(testing::Return(true));
EXPECT_CALL(*client_, IsPersonalizedUrlDataCollectionActive())
.WillRepeatedly(testing::Return(true));
#if defined(OS_IOS) || defined(OS_ANDROID)
EXPECT_EQ(ZeroSuggestProvider::ResultType::MOST_VISITED,
provider_->TypeOfResultToRun(current_url, suggest_url));
#else
EXPECT_EQ(ZeroSuggestProvider::ResultType::NONE,
provider_->TypeOfResultToRun(current_url, suggest_url));
#endif
SetZeroSuggestVariantForAllContexts("RemoteSendURL");
EXPECT_EQ(ZeroSuggestProvider::ResultType::REMOTE_SEND_URL,
provider_->TypeOfResultToRun(current_url, suggest_url));
CreatePersonalizedFieldTrial();
EXPECT_EQ(ZeroSuggestProvider::ResultType::REMOTE_NO_URL,
provider_->TypeOfResultToRun(current_url, suggest_url));
CreateMostVisitedFieldTrial();
EXPECT_EQ(ZeroSuggestProvider::ResultType::MOST_VISITED,
provider_->TypeOfResultToRun(current_url, suggest_url));
}
TEST_F(ZeroSuggestProviderTest, TestDoesNotReturnMatchesForPrefix) { TEST_F(ZeroSuggestProviderTest, TestDoesNotReturnMatchesForPrefix) {
CreatePersonalizedFieldTrial(); CreatePersonalizedFieldTrial();
......
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