[Omnibox] Adds feature to disable remote ZPS caching
Fixed: 1136654
Change-Id: I55f7cb4ca34332a33d81421a9a21475279e6fa08
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2463335
Reviewed-by: Tomasz Wiszkowski <[email protected]>
Reviewed-by: Tommy Li <[email protected]>
Commit-Queue: Moe Ahmadi <[email protected]>
Cr-Commit-Position: refs/heads/master@{#816202}
(cherry picked from commit d9379587734c44089bb8be4278fa5e6a7de12fa5)
[email protected]
Change-Id: Ibe4ce9d810e8667ef7d096efeb3d21135e254e85
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2472441
Reviewed-by: Moe Ahmadi <[email protected]>
Reviewed-by: Tommy Li <[email protected]>
Commit-Queue: Moe Ahmadi <[email protected]>
Cr-Commit-Position: refs/branch-heads/4280@{#397}
Cr-Branched-From: ea420fb963f9658c9969b6513c56b8f47efa1a2a-refs/heads/master@{#812852}
diff --git a/components/omnibox/browser/zero_suggest_provider.cc b/components/omnibox/browser/zero_suggest_provider.cc
index 4e689bc1..694ef2f 100644
--- a/components/omnibox/browser/zero_suggest_provider.cc
+++ b/components/omnibox/browser/zero_suggest_provider.cc
@@ -366,7 +366,8 @@
// When running the REMOTE_NO_URL variant, we want to store suggestion
// responses if non-empty.
- if (result_type_running_ == REMOTE_NO_URL && !json_data.empty()) {
+ if (base::FeatureList::IsEnabled(omnibox::kOmniboxZeroSuggestCaching) &&
+ result_type_running_ == REMOTE_NO_URL && !json_data.empty()) {
client()->GetPrefs()->SetString(omnibox::kZeroSuggestCachedResults,
json_data);
@@ -614,8 +615,10 @@
}
void ZeroSuggestProvider::MaybeUseCachedSuggestions() {
- if (result_type_running_ != REMOTE_NO_URL)
+ if (!base::FeatureList::IsEnabled(omnibox::kOmniboxZeroSuggestCaching) ||
+ result_type_running_ != REMOTE_NO_URL) {
return;
+ }
std::string json_data =
client()->GetPrefs()->GetString(omnibox::kZeroSuggestCachedResults);
diff --git a/components/omnibox/browser/zero_suggest_provider_unittest.cc b/components/omnibox/browser/zero_suggest_provider_unittest.cc
index f8f0f5b..427c5c8 100644
--- a/components/omnibox/browser/zero_suggest_provider_unittest.cc
+++ b/components/omnibox/browser/zero_suggest_provider_unittest.cc
@@ -656,8 +656,10 @@
EXPECT_EQ(3U, provider_->matches().size()); // 3 results, no verbatim match
#endif
- EXPECT_EQ(json_response,
- prefs->GetString(omnibox::kZeroSuggestCachedResults));
+ if (base::FeatureList::IsEnabled(omnibox::kOmniboxZeroSuggestCaching)) {
+ EXPECT_EQ(json_response,
+ prefs->GetString(omnibox::kZeroSuggestCachedResults));
+ }
}
TEST_F(ZeroSuggestProviderTest, TestPsuggestZeroSuggestHasCachedResults) {
@@ -681,18 +683,20 @@
provider_->Start(input, false);
- // Expect that matches get populated synchronously out of the cache.
+ if (base::FeatureList::IsEnabled(omnibox::kOmniboxZeroSuggestCaching)) {
+ // Expect that matches get populated synchronously out of the cache.
#if defined(OS_ANDROID) || defined(OS_IOS)
- ASSERT_EQ(4U, provider_->matches().size()); // 3 results + verbatim
- EXPECT_EQ(base::ASCIIToUTF16("search1"), provider_->matches()[1].contents);
- EXPECT_EQ(base::ASCIIToUTF16("search2"), provider_->matches()[2].contents);
- EXPECT_EQ(base::ASCIIToUTF16("search3"), provider_->matches()[3].contents);
+ ASSERT_EQ(4U, provider_->matches().size()); // 3 results + verbatim
+ EXPECT_EQ(base::ASCIIToUTF16("search1"), provider_->matches()[1].contents);
+ EXPECT_EQ(base::ASCIIToUTF16("search2"), provider_->matches()[2].contents);
+ EXPECT_EQ(base::ASCIIToUTF16("search3"), provider_->matches()[3].contents);
#else
- ASSERT_EQ(3U, provider_->matches().size()); // 3 results, no verbatim match
- EXPECT_EQ(base::ASCIIToUTF16("search1"), provider_->matches()[0].contents);
- EXPECT_EQ(base::ASCIIToUTF16("search2"), provider_->matches()[1].contents);
- EXPECT_EQ(base::ASCIIToUTF16("search3"), provider_->matches()[2].contents);
+ ASSERT_EQ(3U, provider_->matches().size()); // 3 results, no verbatim match
+ EXPECT_EQ(base::ASCIIToUTF16("search1"), provider_->matches()[0].contents);
+ EXPECT_EQ(base::ASCIIToUTF16("search2"), provider_->matches()[1].contents);
+ EXPECT_EQ(base::ASCIIToUTF16("search3"), provider_->matches()[2].contents);
#endif
+ }
GURL suggest_url = GetSuggestURL(metrics::OmniboxEventProto::OTHER);
EXPECT_TRUE(test_loader_factory()->IsPending(suggest_url.spec()));
@@ -703,22 +707,37 @@
base::RunLoop().RunUntilIdle();
- // Expect the same results after the response has been handled.
+ if (base::FeatureList::IsEnabled(omnibox::kOmniboxZeroSuggestCaching)) {
+ // Expect the same results after the response has been handled.
#if defined(OS_ANDROID) || defined(OS_IOS)
- ASSERT_EQ(4U, provider_->matches().size()); // 3 results + verbatim
- EXPECT_EQ(base::ASCIIToUTF16("search1"), provider_->matches()[1].contents);
- EXPECT_EQ(base::ASCIIToUTF16("search2"), provider_->matches()[2].contents);
- EXPECT_EQ(base::ASCIIToUTF16("search3"), provider_->matches()[3].contents);
+ ASSERT_EQ(4U, provider_->matches().size()); // 3 results + verbatim
+ EXPECT_EQ(base::ASCIIToUTF16("search1"), provider_->matches()[1].contents);
+ EXPECT_EQ(base::ASCIIToUTF16("search2"), provider_->matches()[2].contents);
+ EXPECT_EQ(base::ASCIIToUTF16("search3"), provider_->matches()[3].contents);
#else
- ASSERT_EQ(3U, provider_->matches().size()); // 3 results, no verbatim match
- EXPECT_EQ(base::ASCIIToUTF16("search1"), provider_->matches()[0].contents);
- EXPECT_EQ(base::ASCIIToUTF16("search2"), provider_->matches()[1].contents);
- EXPECT_EQ(base::ASCIIToUTF16("search3"), provider_->matches()[2].contents);
+ ASSERT_EQ(3U, provider_->matches().size()); // 3 results, no verbatim match
+ EXPECT_EQ(base::ASCIIToUTF16("search1"), provider_->matches()[0].contents);
+ EXPECT_EQ(base::ASCIIToUTF16("search2"), provider_->matches()[1].contents);
+ EXPECT_EQ(base::ASCIIToUTF16("search3"), provider_->matches()[2].contents);
#endif
// Expect the new results have been stored.
EXPECT_EQ(json_response2,
prefs->GetString(omnibox::kZeroSuggestCachedResults));
+ } else {
+ // Expect fresh results after the response has been handled.
+#if defined(OS_ANDROID) || defined(OS_IOS)
+ ASSERT_EQ(4U, provider_->matches().size()); // 3 results + verbatim
+ EXPECT_EQ(base::ASCIIToUTF16("search4"), provider_->matches()[1].contents);
+ EXPECT_EQ(base::ASCIIToUTF16("search5"), provider_->matches()[2].contents);
+ EXPECT_EQ(base::ASCIIToUTF16("search6"), provider_->matches()[3].contents);
+#else
+ ASSERT_EQ(3U, provider_->matches().size()); // 3 results, no verbatim match
+ EXPECT_EQ(base::ASCIIToUTF16("search4"), provider_->matches()[0].contents);
+ EXPECT_EQ(base::ASCIIToUTF16("search5"), provider_->matches()[1].contents);
+ EXPECT_EQ(base::ASCIIToUTF16("search6"), provider_->matches()[2].contents);
+#endif
+ }
}
TEST_F(ZeroSuggestProviderTest, TestPsuggestZeroSuggestReceivedEmptyResults) {
@@ -742,18 +761,22 @@
provider_->Start(input, false);
- // Expect that matches get populated synchronously out of the cache.
+ if (base::FeatureList::IsEnabled(omnibox::kOmniboxZeroSuggestCaching)) {
+ // Expect that matches get populated synchronously out of the cache.
#if defined(OS_ANDROID) || defined(OS_IOS)
- ASSERT_EQ(4U, provider_->matches().size()); // 3 results + verbatim
- EXPECT_EQ(base::ASCIIToUTF16("search1"), provider_->matches()[1].contents);
- EXPECT_EQ(base::ASCIIToUTF16("search2"), provider_->matches()[2].contents);
- EXPECT_EQ(base::ASCIIToUTF16("search3"), provider_->matches()[3].contents);
+ ASSERT_EQ(4U, provider_->matches().size()); // 3 results + verbatim
+ EXPECT_EQ(base::ASCIIToUTF16("search1"), provider_->matches()[1].contents);
+ EXPECT_EQ(base::ASCIIToUTF16("search2"), provider_->matches()[2].contents);
+ EXPECT_EQ(base::ASCIIToUTF16("search3"), provider_->matches()[3].contents);
#else
- ASSERT_EQ(3U, provider_->matches().size()); // 3 results, no verbatim match
- EXPECT_EQ(base::ASCIIToUTF16("search1"), provider_->matches()[0].contents);
- EXPECT_EQ(base::ASCIIToUTF16("search2"), provider_->matches()[1].contents);
- EXPECT_EQ(base::ASCIIToUTF16("search3"), provider_->matches()[2].contents);
+ ASSERT_EQ(3U, provider_->matches().size()); // 3 results, no verbatim match
+ EXPECT_EQ(base::ASCIIToUTF16("search1"), provider_->matches()[0].contents);
+ EXPECT_EQ(base::ASCIIToUTF16("search2"), provider_->matches()[1].contents);
+ EXPECT_EQ(base::ASCIIToUTF16("search3"), provider_->matches()[2].contents);
#endif
+ } else {
+ ASSERT_TRUE(provider_->matches().empty());
+ }
GURL suggest_url = GetSuggestURL(metrics::OmniboxEventProto::OTHER);
EXPECT_TRUE(test_loader_factory()->IsPending(suggest_url.spec()));
@@ -765,7 +788,9 @@
// Expect that the matches have been cleared.
ASSERT_TRUE(provider_->matches().empty());
- // Expect the new results have been stored.
- EXPECT_EQ(empty_response,
- prefs->GetString(omnibox::kZeroSuggestCachedResults));
+ if (base::FeatureList::IsEnabled(omnibox::kOmniboxZeroSuggestCaching)) {
+ // Expect the new results have been stored.
+ EXPECT_EQ(empty_response,
+ prefs->GetString(omnibox::kZeroSuggestCachedResults));
+ }
}
diff --git a/components/omnibox/common/omnibox_features.cc b/components/omnibox/common/omnibox_features.cc
index 65fb56a6..75d2747 100644
--- a/components/omnibox/common/omnibox_features.cc
+++ b/components/omnibox/common/omnibox_features.cc
@@ -239,6 +239,11 @@
const base::Feature kOnFocusSuggestions{"OmniboxOnFocusSuggestions",
base::FEATURE_ENABLED_BY_DEFAULT};
+// Used to enable/disable caching for remote zero-prefix suggestions. Caching is
+// enabled by default. We will be experimenting with DISABLING this behavior.
+const base::Feature kOmniboxZeroSuggestCaching{
+ "OmniboxZeroSuggestCaching", base::FEATURE_ENABLED_BY_DEFAULT};
+
// Enables on-focus suggestions on the Open Web, that are contextual to the
// current URL. Will only work if user is signed-in and syncing, or is
// otherwise eligible to send the current page URL to the suggest server.
diff --git a/components/omnibox/common/omnibox_features.h b/components/omnibox/common/omnibox_features.h
index 370bd18..e6c440b 100644
--- a/components/omnibox/common/omnibox_features.h
+++ b/components/omnibox/common/omnibox_features.h
@@ -63,6 +63,7 @@
extern const base::Feature kOmniboxLocalZeroSuggestFrecencyRanking;
extern const base::Feature kOmniboxTrendingZeroPrefixSuggestionsOnNTP;
extern const base::Feature kOnFocusSuggestions;
+extern const base::Feature kOmniboxZeroSuggestCaching;
extern const base::Feature kOnFocusSuggestionsContextualWeb;
extern const base::Feature kOnFocusSuggestionsContextualWebOnContent;
extern const base::Feature kReactiveZeroSuggestionsOnNTPOmnibox;
