Skip to content

Commit

Permalink
[Merge 110][ZPS] Enable cross-session persistence for ZPS on SRP/Web.
Browse files Browse the repository at this point in the history 
As a follow-up to a recent CL that enabled cross-session persistence for
ZPS responses on NTP that are stored in the in-memory cache, this CL
enables cross-session persistence of ZPS responses on SRP/Web. In
particular, this change will minimize any cold-start penalty that could
occur on SRP/Web pages when the user initiates a new browser session.

(cherry picked from commit bd01394)

Bug: 1351224
Change-Id: I42bef6a06bdd6bbaa222fd7aee7a4e76e7c6a189
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4149642
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Commit-Queue: Khalid Peer <khalidpeer@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1091624}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4165682
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5481@{#287}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
Khalid Peer authored and Chromium LUCI CQ committed Jan 13, 2023
1 parent d0a6518 commit 62bab07
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 10 deletions.
33 changes: 32 additions & 1 deletion components/omnibox/browser/zero_suggest_cache_service.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@

#include "components/omnibox/browser/zero_suggest_cache_service.h"

#include "base/feature_list.h"
#include "base/metrics/histogram_functions.h"
#include "base/trace_event/memory_usage_estimator.h"
#include "base/values.h"
#include "components/omnibox/browser/autocomplete_input.h"
#include "components/omnibox/browser/autocomplete_provider_client.h"
#include "components/omnibox/browser/omnibox_prefs.h"
#include "components/omnibox/browser/search_suggestion_parser.h"
#include "components/omnibox/common/omnibox_features.h"
#include "components/prefs/pref_service.h"

using CacheEntry = ZeroSuggestCacheService::CacheEntry;
Expand All @@ -20,18 +23,45 @@ ZeroSuggestCacheService::ZeroSuggestCacheService(PrefService* prefs,
DCHECK_GT(cache_size, 0u);

if (prefs_) {
if (!base::FeatureList::IsEnabled(omnibox::kZeroSuggestInMemoryCaching)) {
return;
}

// Load cached ZPS response for NTP from prior session via prefs.
ntp_entry_.response_json =
omnibox::GetUserPreferenceForZeroSuggestCachedResponse(prefs_,
/*page_url=*/"");

// Load cached ZPS responses for SRP/Web from prior session via prefs.
const auto& prefs_dict =
prefs->GetDict(omnibox::kZeroSuggestCachedResultsWithURL);
for (auto it = prefs_dict.begin(); it != prefs_dict.end(); ++it) {
const auto& page_url = it->first;
const auto& response_json = (it->second).GetString();
StoreZeroSuggestResponse(page_url, CacheEntry(response_json));
}
}
}

ZeroSuggestCacheService::~ZeroSuggestCacheService() {
if (prefs_) {
if (!base::FeatureList::IsEnabled(omnibox::kZeroSuggestInMemoryCaching)) {
return;
}

// Dump cached ZPS response for NTP to prefs.
omnibox::SetUserPreferenceForZeroSuggestCachedResponse(
prefs_, /*page_url=*/"", /*response=*/ntp_entry_.response_json);

// Dump cached ZPS responses for SRP/Web to prefs.
base::Value::Dict prefs_dict;
for (const auto& item : cache_) {
const auto& page_url = item.first;
const auto& response_json = item.second.response_json;
prefs_dict.Set(page_url, response_json);
}
prefs_->SetDict(omnibox::kZeroSuggestCachedResultsWithURL,
std::move(prefs_dict));
}
}

Expand Down Expand Up @@ -69,11 +99,12 @@ void ZeroSuggestCacheService::StoreZeroSuggestResponse(
}

void ZeroSuggestCacheService::ClearCache() {
ntp_entry_.response_json.clear();
cache_.Clear();
}

bool ZeroSuggestCacheService::IsCacheEmpty() const {
return cache_.empty();
return ntp_entry_.response_json.empty() && cache_.empty();
}

void ZeroSuggestCacheService::AddObserver(Observer* observer) {
Expand Down
68 changes: 59 additions & 9 deletions components/omnibox/browser/zero_suggest_cache_service_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@
#include <string>

#include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
#include "base/values.h"
#include "components/omnibox/browser/omnibox_prefs.h"
#include "components/omnibox/browser/zero_suggest_provider.h"
#include "components/omnibox/common/omnibox_features.h"
#include "components/prefs/testing_pref_service.h"
#include "testing/gtest/include/gtest/gtest.h"

Expand Down Expand Up @@ -51,6 +54,10 @@ class ZeroSuggestCacheServiceTest : public testing::Test {
void SetUp() override {
prefs_ = std::make_unique<TestingPrefServiceSimple>();
ZeroSuggestProvider::RegisterProfilePrefs(prefs_->registry());

scoped_feature_list_.InitWithFeatures(
/*enabled_features=*/{omnibox::kZeroSuggestInMemoryCaching},
/*disabled_features=*/{});
}

void TearDown() override { prefs_.reset(); }
Expand All @@ -59,6 +66,7 @@ class ZeroSuggestCacheServiceTest : public testing::Test {

private:
std::unique_ptr<TestingPrefServiceSimple> prefs_;
base::test::ScopedFeatureList scoped_feature_list_;
};

TEST_F(ZeroSuggestCacheServiceTest, CacheStartsEmpty) {
Expand Down Expand Up @@ -231,43 +239,85 @@ TEST_F(ZeroSuggestCacheServiceTest, ReadResponseUpdatesRecency) {
}

TEST_F(ZeroSuggestCacheServiceTest, ClearCacheResultsInEmptyCache) {
ZeroSuggestCacheService cache_svc(GetPrefs(), 1);
TestCacheEntry ntp_entry = {"", "foo"};
TestCacheEntry srp_entry = {"https://www.google.com/search?q=bar", "bar"};
TestCacheEntry web_entry = {"https://www.example.com", "eggs"};

cache_svc.StoreZeroSuggestResponse("https://www.google.com",
CacheEntry("foo"));
ZeroSuggestCacheService cache_svc(GetPrefs(), 3);

EXPECT_FALSE(cache_svc.IsCacheEmpty());
cache_svc.StoreZeroSuggestResponse(ntp_entry.url,
CacheEntry(ntp_entry.response));
cache_svc.StoreZeroSuggestResponse(srp_entry.url,
CacheEntry(srp_entry.response));
cache_svc.StoreZeroSuggestResponse(web_entry.url,
CacheEntry(web_entry.response));

EXPECT_FALSE(
cache_svc.ReadZeroSuggestResponse(ntp_entry.url).response_json.empty());
EXPECT_FALSE(
cache_svc.ReadZeroSuggestResponse(srp_entry.url).response_json.empty());
EXPECT_FALSE(
cache_svc.ReadZeroSuggestResponse(web_entry.url).response_json.empty());

cache_svc.ClearCache();

EXPECT_TRUE(cache_svc.IsCacheEmpty());
EXPECT_TRUE(
cache_svc.ReadZeroSuggestResponse(ntp_entry.url).response_json.empty());
EXPECT_TRUE(
cache_svc.ReadZeroSuggestResponse(srp_entry.url).response_json.empty());
EXPECT_TRUE(
cache_svc.ReadZeroSuggestResponse(web_entry.url).response_json.empty());
}

TEST_F(ZeroSuggestCacheServiceTest, CacheLoadsFromPrefsOnStartup) {
TestCacheEntry ntp_entry = {"", "foo"};
TestCacheEntry srp_entry = {"https://www.google.com/search?q=bar", "bar"};
TestCacheEntry web_entry = {"https://www.example.com", "eggs"};

base::Value::Dict prefs_dict;
prefs_dict.Set(ntp_entry.url, ntp_entry.response);
prefs_dict.Set(srp_entry.url, srp_entry.response);
prefs_dict.Set(web_entry.url, web_entry.response);

PrefService* prefs = GetPrefs();
omnibox::SetUserPreferenceForZeroSuggestCachedResponse(prefs, ntp_entry.url,
ntp_entry.response);
ZeroSuggestCacheService cache_svc(prefs, 1);
prefs->SetDict(omnibox::kZeroSuggestCachedResultsWithURL,
std::move(prefs_dict));

ZeroSuggestCacheService cache_svc(prefs, 3);

EXPECT_EQ(cache_svc.ReadZeroSuggestResponse(ntp_entry.url).response_json,
ntp_entry.response);
EXPECT_EQ(cache_svc.ReadZeroSuggestResponse(srp_entry.url).response_json,
srp_entry.response);
EXPECT_EQ(cache_svc.ReadZeroSuggestResponse(web_entry.url).response_json,
web_entry.response);
}

TEST_F(ZeroSuggestCacheServiceTest, CacheDumpsToPrefsOnShutdown) {
TestCacheEntry ntp_entry = {"", "foo"};
TestCacheEntry srp_entry = {"https://www.google.com/search?q=bar", "bar"};
TestCacheEntry web_entry = {"https://www.example.com", "eggs"};

PrefService* prefs = GetPrefs();
{
ZeroSuggestCacheService cache_svc(prefs, 1);
ZeroSuggestCacheService cache_svc(prefs, 3);
cache_svc.StoreZeroSuggestResponse(ntp_entry.url,
CacheEntry(ntp_entry.response));
cache_svc.StoreZeroSuggestResponse(srp_entry.url,
CacheEntry(srp_entry.response));
cache_svc.StoreZeroSuggestResponse(web_entry.url,
CacheEntry(web_entry.response));
}

EXPECT_EQ(omnibox::GetUserPreferenceForZeroSuggestCachedResponse(
prefs, ntp_entry.url),
ntp_entry.response);
EXPECT_EQ(omnibox::GetUserPreferenceForZeroSuggestCachedResponse(
prefs, srp_entry.url),
srp_entry.response);
EXPECT_EQ(omnibox::GetUserPreferenceForZeroSuggestCachedResponse(
prefs, web_entry.url),
web_entry.response);
}

TEST_F(ZeroSuggestCacheServiceTest, CacheWorksGivenNullPrefService) {
Expand Down

0 comments on commit 62bab07

Please sign in to comment.