Skip to content

GitLab

T
tangled
Commit 621811e4 authored by Kyle Milka's avatar Kyle Milka Committed by Commit Bot
Browse files
Options

Reland "Reland "[NTP] Cap search suggestion impressions"" 

This is a reland of 5d52425f

Initialize the int memebers of SearchSuggestData to 0. The uninitialized
values were causing separate failures than the previously flaky test.

Original change's description:
> Reland "[NTP] Cap search suggestion impressions"
>
> This is a reland of 41e54be1
>
> One of the checks is converting the times from ms->s. In practice
> seconds is plenty precise but in the test 800ms ended up being 0,
> so increase it to 1000ms (similar to ImpressionCountResetsAfterTimeout
> which was not flaky).
>
> Original change's description:
> > [NTP] Cap search suggestion impressions
> >
> > The parameters for capping suggestions impressions are provided as part
> > of the update proto. Read and update the local pref on each fetch. Use
> > these prefs to determine if the impression cap has been reach or if
> > fetching is frozen due to an empty response.
> >
> > Bug: 904565
> > Change-Id: Ib5539a99f18e9da2ac1223ecc7aff65cb909bca8
> > Reviewed-on: https://chromium-review.googlesource.com/c/1395188
> > Commit-Queue: Kyle Milka <kmilka@chromium.org>
> > Reviewed-by: Kristi Park <kristipark@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#624994}
>
> Bug: 904565
> Change-Id: I424cb51d2a9c1738c0e5168f58ef52b73ed4b223
> Reviewed-on: https://chromium-review.googlesource.com/c/1431472
> Reviewed-by: Kristi Park <kristipark@chromium.org>
> Commit-Queue: Kyle Milka <kmilka@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#625437}

Bug: 904565
Change-Id: I01e2c868d36ce1ab5d7ffd91b86e40d8573f121e
Reviewed-on: https://chromium-review.googlesource.com/c/1434081Reviewed-by: default avatarKristi Park <kristipark@chromium.org>
Commit-Queue: Kyle Milka <kmilka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#625786}
parent 18deda86
Hide whitespace changes
Inline Side-by-side
Showing with 410 additions and 31 deletions
chrome/browser/search/local_ntp_source.cc
View file @ 621811e4
......@@ -1281,7 +1281,7 @@ void LocalNtpSource::MaybeServeSearchSuggestions(
}
SearchSuggestData suggest_data = *data;
search_suggest_service_->ClearSearchSuggestData();
search_suggest_service_->SuggestionsDisplayed();
scoped_refptr<base::RefCountedString> result;
std::string js;
base::JSONWriter::Write(*ConvertSearchSuggestDataToDict(suggest_data), &js);
......
chrome/browser/search/search_suggest/search_suggest_data.cc
View file @ 621811e4
......@@ -4,7 +4,10 @@
#include "chrome/browser/search/search_suggest/search_suggest_data.h"
SearchSuggestData::SearchSuggestData() = default;
SearchSuggestData::SearchSuggestData()
: impression_cap_expire_time_ms(0),
request_freeze_time_ms(0),
max_impressions(0) {}
SearchSuggestData::SearchSuggestData(const SearchSuggestData&) = default;
SearchSuggestData::SearchSuggestData(SearchSuggestData&&) = default;
SearchSuggestData::~SearchSuggestData() = default;
......@@ -15,7 +18,11 @@ SearchSuggestData& SearchSuggestData::operator=(SearchSuggestData&&) = default;
bool operator==(const SearchSuggestData& lhs, const SearchSuggestData& rhs) {
return lhs.suggestions_html == rhs.suggestions_html &&
lhs.end_of_body_script == rhs.end_of_body_script;
lhs.end_of_body_script == rhs.end_of_body_script &&
lhs.impression_cap_expire_time_ms ==
rhs.impression_cap_expire_time_ms &&
lhs.request_freeze_time_ms == rhs.request_freeze_time_ms &&
lhs.max_impressions == rhs.max_impressions;
}
bool operator!=(const SearchSuggestData& lhs, const SearchSuggestData& rhs) {
......
chrome/browser/search/search_suggest/search_suggest_data.h
View file @ 621811e4
......@@ -24,6 +24,11 @@ struct SearchSuggestData {
// Javascript for search suggestion that should be appended at the end of the
// New Tab Page <body>.
std::string end_of_body_script;
// Parameters that control impression capping and freezing.
int impression_cap_expire_time_ms;
int request_freeze_time_ms;
int max_impressions;
};
bool operator==(const SearchSuggestData& lhs, const SearchSuggestData& rhs);
......
chrome/browser/search/search_suggest/search_suggest_loader.h
View file @ 621811e4
......@@ -26,8 +26,12 @@ class SearchSuggestLoader {
// A fatal error occurred, such as the server responding with an error code
// or with invalid data. Any previously cached response should be cleared.
FATAL_ERROR,
// The user has opted out of seeing search suggestions on the NTP.
OPTED_OUT
// The user has opted out of seeing search suggestions on the NTP
OPTED_OUT,
// The limit for number of impressions was hit.
IMPRESSION_CAP,
// Received an empty response so requests are temporarily frozen.
REQUESTS_FROZEN
};
using SearchSuggestionsCallback =
base::OnceCallback<void(Status,
......
chrome/browser/search/search_suggest/search_suggest_loader_impl.cc
View file @ 621811e4
......@@ -75,6 +75,32 @@ base::Optional<SearchSuggestData> JsonToSearchSuggestionData(
result.end_of_body_script = end_of_body_script;
int impression_cap_expire_time_ms;
if (!query_suggestions->GetInteger("impression_cap_expire_time_ms",
&impression_cap_expire_time_ms)) {
DLOG(WARNING) << "Parse error: no impression_cap_expire_time_ms";
return base::nullopt;
}
result.impression_cap_expire_time_ms = impression_cap_expire_time_ms;
int request_freeze_time_ms;
if (!query_suggestions->GetInteger("request_freeze_time_ms",
&request_freeze_time_ms)) {
DLOG(WARNING) << "Parse error: no request_freeze_time_ms";
return base::nullopt;
}
result.request_freeze_time_ms = request_freeze_time_ms;
int max_impressions;
if (!query_suggestions->GetInteger("max_impressions", &max_impressions)) {
DLOG(WARNING) << "Parse error: no max_impressions";
return base::nullopt;
}
result.max_impressions = max_impressions;
return result;
}
......
chrome/browser/search/search_suggest/search_suggest_loader_impl_unittest.cc
View file @ 621811e4
......@@ -39,7 +39,9 @@ namespace {
const char kApplicationLocale[] = "us";
const char kMinimalValidResponse[] = R"json({"update": { "query_suggestions": {
"query_suggestions_with_html": "", "script": ""
"query_suggestions_with_html": "", "script": "",
"impression_cap_expire_time_ms": 0, "request_freeze_time_ms": 0,
"max_impressions": 0
}}})json";
// Required to instantiate a GoogleUrlTracker in UNIT_TEST_MODE.
......@@ -170,9 +172,11 @@ TEST_F(SearchSuggestLoaderImplTest, HandlesResponsePreamble) {
}
TEST_F(SearchSuggestLoaderImplTest, ParsesFullResponse) {
SetUpResponseWithData(
R"json({"update": { "query_suggestions": {"query_suggestions_with_html" :
"<div></div>", "script" : "<script></script>"}}})json");
SetUpResponseWithData(R"json({"update": { "query_suggestions": {
"query_suggestions_with_html": "<div></div>",
"script": "<script></script>", "impression_cap_expire_time_ms": 1,
"request_freeze_time_ms": 2, "max_impressions": 3
}}})json");
base::MockCallback<SearchSuggestLoader::SearchSuggestionsCallback> callback;
std::string blacklist;
......@@ -185,8 +189,11 @@ TEST_F(SearchSuggestLoaderImplTest, ParsesFullResponse) {
loop.Run();
ASSERT_TRUE(data.has_value());
EXPECT_THAT(data->suggestions_html, Eq("<div></div>"));
EXPECT_THAT(data->end_of_body_script, Eq("<script></script>"));
EXPECT_EQ("<div></div>", data->suggestions_html);
EXPECT_EQ("<script></script>", data->end_of_body_script);
EXPECT_EQ(1, data->impression_cap_expire_time_ms);
EXPECT_EQ(2, data->request_freeze_time_ms);
EXPECT_EQ(3, data->max_impressions);
}
TEST_F(SearchSuggestLoaderImplTest, CoalescesMultipleRequests) {
......
chrome/browser/search/search_suggest/search_suggest_service.cc
View file @ 621811e4
......@@ -13,6 +13,34 @@
#include "components/prefs/scoped_user_pref_update.h"
#include "services/identity/public/cpp/identity_manager.h"
namespace {
const char kFirstShownTimeMs[] = "first_shown_time_ms";
const char kImpressionCapExpireTimeMs[] = "impression_cap_expire_time_ms";
const char kImpressionsCount[] = "impressions_count";
const char kIsRequestFrozen[] = "is_request_frozen";
const char kMaxImpressions[] = "max_impressions";
const char kRequestFreezeTimeMs[] = "request_freeze_time_ms";
const char kRequestFrozenTimeMs[] = "request_frozen_time_ms";
// Default value for max_impressions specified by the VASCO team.
const int kDefaultMaxImpressions = 4;
std::unique_ptr<base::DictionaryValue> ImpressionDictDefaults() {
std::unique_ptr<base::DictionaryValue> defaults =
std::make_unique<base::DictionaryValue>();
defaults->SetInteger(kFirstShownTimeMs, 0);
defaults->SetInteger(kImpressionCapExpireTimeMs, 0);
defaults->SetInteger(kImpressionsCount, 0);
defaults->SetBoolean(kIsRequestFrozen, false);
defaults->SetInteger(kMaxImpressions, kDefaultMaxImpressions);
defaults->SetInteger(kRequestFreezeTimeMs, 0);
defaults->SetInteger(kRequestFrozenTimeMs, 0);
return defaults;
}
} // namespace
class SearchSuggestService::SigninObserver
: public identity::IdentityManager::Observer {
public:
......@@ -72,6 +100,12 @@ void SearchSuggestService::Refresh() {
} else if (pref_service_->GetBoolean(prefs::kNtpSearchSuggestionsOptOut)) {
SearchSuggestDataLoaded(SearchSuggestLoader::Status::OPTED_OUT,
base::nullopt);
} else if (RequestsFrozen()) {
SearchSuggestDataLoaded(SearchSuggestLoader::Status::REQUESTS_FROZEN,
base::nullopt);
} else if (ImpressionCapReached()) {
SearchSuggestDataLoaded(SearchSuggestLoader::Status::IMPRESSION_CAP,
base::nullopt);
} else {
const std::string blacklist = GetBlacklistAsString();
loader_->Load(blacklist,
......@@ -102,10 +136,22 @@ void SearchSuggestService::SearchSuggestDataLoaded(
// In case of transient errors, keep our cached data (if any), but still
// notify observers of the finished load (attempt).
if (status != SearchSuggestLoader::Status::TRANSIENT_ERROR) {
// TODO(crbug/904565): Verify that cached data is also cleared when the
// impression cap is reached. Including the response from the request made
// on the same load that the cap was hit.
search_suggest_data_ = data;
DictionaryPrefUpdate update(pref_service_,
prefs::kNtpSearchSuggestionsImpressions);
if (data.has_value()) {
base::DictionaryValue* dict = update.Get();
dict->SetInteger(kMaxImpressions, data->max_impressions);
dict->SetInteger(kImpressionCapExpireTimeMs,
data->impression_cap_expire_time_ms);
dict->SetInteger(kRequestFreezeTimeMs, data->request_freeze_time_ms);
} else if (status == SearchSuggestLoader::Status::FATAL_ERROR) {
base::DictionaryValue* dict = update.Get();
dict->SetBoolean(kIsRequestFrozen, true);
dict->SetInteger(kRequestFrozenTimeMs, base::Time::Now().ToTimeT());
}
}
NotifyObservers();
}
......@@ -116,6 +162,61 @@ void SearchSuggestService::NotifyObservers() {
}
}
bool SearchSuggestService::ImpressionCapReached() {
const base::DictionaryValue* dict =
pref_service_->GetDictionary(prefs::kNtpSearchSuggestionsImpressions);
int first_shown_time_ms = 0;
int impression_cap_expire_time_ms = 0;
int impression_count = 0;
int max_impressions = 0;
dict->GetInteger(kFirstShownTimeMs, &first_shown_time_ms);
dict->GetInteger(kImpressionCapExpireTimeMs, &impression_cap_expire_time_ms);
dict->GetInteger(kImpressionsCount, &impression_count);
dict->GetInteger(kMaxImpressions, &max_impressions);
int64_t time_delta =
base::TimeDelta(base::Time::Now() -
base::Time::FromTimeT(first_shown_time_ms))
.InMilliseconds();
if (time_delta > impression_cap_expire_time_ms) {
impression_count = 0;
DictionaryPrefUpdate update(pref_service_,
prefs::kNtpSearchSuggestionsImpressions);
update.Get()->SetInteger(kImpressionsCount, impression_count);
}
return impression_count >= max_impressions;
}
bool SearchSuggestService::RequestsFrozen() {
const base::DictionaryValue* dict =
pref_service_->GetDictionary(prefs::kNtpSearchSuggestionsImpressions);
bool is_request_frozen = false;
int request_freeze_time_ms = 0;
int request_frozen_time_ms = 0;
dict->GetBoolean(kIsRequestFrozen, &is_request_frozen);
dict->GetInteger(kRequestFrozenTimeMs, &request_frozen_time_ms);
dict->GetInteger(kRequestFreezeTimeMs, &request_freeze_time_ms);
int64_t time_delta =
base::TimeDelta(base::Time::Now() -
base::Time::FromTimeT(request_frozen_time_ms))
.InMilliseconds();
if (is_request_frozen) {
if (time_delta < request_freeze_time_ms) {
return true;
} else {
DictionaryPrefUpdate update(pref_service_,
prefs::kNtpSearchSuggestionsImpressions);
update.Get()->SetBoolean(kIsRequestFrozen, false);
}
}
return false;
}
void SearchSuggestService::BlacklistSearchSuggestion(int task_version,
long task_id) {
std::string task_version_id =
......@@ -178,6 +279,23 @@ std::string SearchSuggestService::GetBlacklistAsString() {
return blacklist_as_string;
}
void SearchSuggestService::SuggestionsDisplayed() {
search_suggest_data_ = base::nullopt;
DictionaryPrefUpdate update(pref_service_,
prefs::kNtpSearchSuggestionsImpressions);
base::DictionaryValue* dict = update.Get();
int impression_count = 0;
dict->GetInteger(kImpressionsCount, &impression_count);
dict->SetInteger(kImpressionsCount, impression_count + 1);
// When suggestions are displayed for the first time record the timestamp.
if (impression_count == 0) {
dict->SetInteger(kFirstShownTimeMs, base::Time::Now().ToTimeT());
}
}
void SearchSuggestService::OptOutOfSearchSuggestions() {
pref_service_->SetBoolean(prefs::kNtpSearchSuggestionsOptOut, true);
......@@ -187,5 +305,7 @@ void SearchSuggestService::OptOutOfSearchSuggestions() {
// static
void SearchSuggestService::RegisterProfilePrefs(PrefRegistrySimple* registry) {
registry->RegisterDictionaryPref(prefs::kNtpSearchSuggestionsBlacklist);
registry->RegisterDictionaryPref(prefs::kNtpSearchSuggestionsImpressions,
ImpressionDictDefaults());
registry->RegisterBooleanPref(prefs::kNtpSearchSuggestionsOptOut, false);
}
chrome/browser/search/search_suggest/search_suggest_service.h
View file @ 621811e4
......@@ -38,11 +38,11 @@ class SearchSuggestService : public KeyedService {
return search_suggest_data_;
}
// Clears any currently cached search suggest data.
void ClearSearchSuggestData() { search_suggest_data_ = base::nullopt; }
// Requests an asynchronous refresh from the network. After the update
// completes, OnSearchSuggestDataUpdated will be called on the observers.
// Determines if a request for search suggestions should be made. If a request
// should not be made immediately call SearchSuggestDataLoaded with the
// reason. Otherwise requests an asynchronous refresh from the network. After
// the update completes, regardless of success, OnSearchSuggestDataUpdated
// will be called on the observers.
void Refresh();
// Add/remove observers. All observers must unregister themselves before the
......@@ -82,16 +82,37 @@ class SearchSuggestService : public KeyedService {
// "task_id1:hash1,hash2,hash3;task_id2;task_id3:hash1,hash2".
std::string GetBlacklistAsString();
// Called when suggestions are displayed on the NTP, clears the cached data
// and updates timestamps and impression counts.
void SuggestionsDisplayed();
private:
class SigninObserver;
void SigninStatusChanged();
// Called when a Refresh() is requested. If |status|==OK, |data| will contain
// the fetched data. Otherwise |data| will be nullopt and |status| will
// indicate if the request failed or the reason it was not sent.
//
// If the |status|==FATAL_ERROR freeze future requests until the request
// freeze interval has elapsed.
void SearchSuggestDataLoaded(SearchSuggestLoader::Status status,
const base::Optional<SearchSuggestData>& data);
void NotifyObservers();
// Returns true if the number of impressions has reached the maxmium allowed
// for the impression interval (e.g. 4 impressions / 12 hours), and false
// otherwise.
bool ImpressionCapReached();
// Returns true if requests are frozen and request freeze time interval has
// not elapsed, false otherwise.
//
// Requests are frozen on any request that results in a FATAL_ERROR.
bool RequestsFrozen();
std::unique_ptr<SearchSuggestLoader> loader_;
std::unique_ptr<SigninObserver> signin_observer_;
......
chrome/browser/search/search_suggest/search_suggest_service_unittest.cc
View file @ 621811e4
......@@ -12,6 +12,9 @@
#include "base/test/scoped_task_environment.h"
#include "chrome/browser/search/search_suggest/search_suggest_data.h"
#include "chrome/browser/search/search_suggest/search_suggest_loader.h"
#include "chrome/common/pref_names.h"
#include "components/signin/core/browser/account_tracker_service.h"
#include "components/signin/core/browser/test_signin_client.h"
#include "components/sync_preferences/testing_pref_service_syncable.h"
#include "services/identity/public/cpp/identity_test_environment.h"
#include "services/identity/public/cpp/identity_test_utils.h"
......@@ -61,6 +64,9 @@ class SearchSuggestServiceTest : public testing::Test {
FakeSearchSuggestLoader* loader() { return loader_; }
SearchSuggestService* service() { return service_.get(); }
sync_preferences::TestingPrefServiceSyncable* pref_service() {
return &pref_service_;
}
void SignIn() {
AccountInfo account_info =
......@@ -72,6 +78,26 @@ class SearchSuggestServiceTest : public testing::Test {
identity_env_.SetCookieAccounts({});
}
// Returns a default data object for testing, initializes the impression
// parameters to values where they won't prevent fetching.
SearchSuggestData TestSearchSuggestData() {
SearchSuggestData data;
data.suggestions_html = "<div></div>";
data.impression_cap_expire_time_ms = 60000;
data.request_freeze_time_ms = 60000;
data.max_impressions = 10;
return data;
}
void RunFor(base::TimeDelta time_period) {
base::RunLoop run_loop;
base::CancelableCallback<void()> callback(run_loop.QuitWhenIdleClosure());
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE, callback.callback(), time_period);
run_loop.Run();
callback.Cancel();
}
private:
base::test::ScopedTaskEnvironment task_environment_;
......@@ -103,8 +129,7 @@ TEST_F(SearchSuggestServiceTest, RefreshesOnSignedInRequest) {
EXPECT_THAT(loader()->GetCallbackCount(), Eq(1u));
// Fulfill it.
SearchSuggestData data;
data.suggestions_html = "<div></div>";
SearchSuggestData data = TestSearchSuggestData();
loader()->RespondToAllCallbacks(SearchSuggestLoader::Status::OK, data);
EXPECT_THAT(service()->search_suggest_data(), Eq(data));
......@@ -116,8 +141,7 @@ TEST_F(SearchSuggestServiceTest, RefreshesOnSignedInRequest) {
EXPECT_THAT(service()->search_suggest_data(), Eq(data));
// Fulfill the second request.
SearchSuggestData other_data;
other_data.suggestions_html = "<div>Different!</div>";
SearchSuggestData other_data = TestSearchSuggestData();
loader()->RespondToAllCallbacks(SearchSuggestLoader::Status::OK, other_data);
EXPECT_THAT(service()->search_suggest_data(), Eq(other_data));
}
......@@ -128,8 +152,7 @@ TEST_F(SearchSuggestServiceTest, KeepsCacheOnTransientError) {
// Load some data.
service()->Refresh();
SearchSuggestData data;
data.suggestions_html = "<div></div>";
SearchSuggestData data = TestSearchSuggestData();
loader()->RespondToAllCallbacks(SearchSuggestLoader::Status::OK, data);
ASSERT_THAT(service()->search_suggest_data(), Eq(data));
......@@ -147,8 +170,7 @@ TEST_F(SearchSuggestServiceTest, ClearsCacheOnFatalError) {
// Load some data.
service()->Refresh();
SearchSuggestData data;
data.suggestions_html = "<div></div>";
SearchSuggestData data = TestSearchSuggestData();
loader()->RespondToAllCallbacks(SearchSuggestLoader::Status::OK, data);
ASSERT_THAT(service()->search_suggest_data(), Eq(data));
......@@ -166,8 +188,7 @@ TEST_F(SearchSuggestServiceTest, ResetsOnSignOut) {
// Load some data.
service()->Refresh();
SearchSuggestData data;
data.suggestions_html = "<div></div>";
SearchSuggestData data = TestSearchSuggestData();
loader()->RespondToAllCallbacks(SearchSuggestLoader::Status::OK, data);
ASSERT_THAT(service()->search_suggest_data(), Eq(data));
......@@ -216,8 +237,7 @@ TEST_F(SearchSuggestServiceTest, BlacklistClearsCachedDataAndIssuesRequest) {
EXPECT_THAT(loader()->GetCallbackCount(), Eq(1u));
// Fulfill it.
SearchSuggestData data;
data.suggestions_html = "<div></div>";
SearchSuggestData data = TestSearchSuggestData();
loader()->RespondToAllCallbacks(SearchSuggestLoader::Status::OK, data);
EXPECT_THAT(service()->search_suggest_data(), Eq(data));
......@@ -245,3 +265,169 @@ TEST_F(SearchSuggestServiceTest, OptOutPreventsRequests) {
EXPECT_THAT(loader()->GetCallbackCount(), Eq(0u));
EXPECT_THAT(service()->search_suggest_data(), Eq(base::nullopt));
}
TEST_F(SearchSuggestServiceTest, UpdateImpressionCapParameters) {
ASSERT_EQ(base::nullopt, service()->search_suggest_data());
SignIn();
// Request a refresh. That should arrive at the loader.
service()->Refresh();
EXPECT_EQ(1u, loader()->GetCallbackCount());
// Fulfill it.
SearchSuggestData data = TestSearchSuggestData();
loader()->RespondToAllCallbacks(SearchSuggestLoader::Status::OK, data);
EXPECT_EQ(data, service()->search_suggest_data());
// Request another refresh.
service()->Refresh();
EXPECT_EQ(1u, loader()->GetCallbackCount());
// For now, the old data should still be there.
EXPECT_EQ(data, service()->search_suggest_data());
// Fulfill the second request.
SearchSuggestData other_data;
other_data.suggestions_html = "<div>different</div>";
other_data.impression_cap_expire_time_ms = 1234;
other_data.request_freeze_time_ms = 4321;
other_data.max_impressions = 456;
loader()->RespondToAllCallbacks(SearchSuggestLoader::Status::OK, other_data);
EXPECT_EQ(other_data, service()->search_suggest_data());
// Ensure the pref parses successfully.
const base::DictionaryValue* dict =
pref_service()->GetDictionary(prefs::kNtpSearchSuggestionsImpressions);
int impression_cap_expire_time_ms = 0;
ASSERT_TRUE(dict->GetInteger("impression_cap_expire_time_ms",
&impression_cap_expire_time_ms));
int request_freeze_time_ms = 0;
ASSERT_TRUE(
dict->GetInteger("request_freeze_time_ms", &request_freeze_time_ms));
int max_impressions = 0;
ASSERT_TRUE(dict->GetInteger("max_impressions", &max_impressions));
EXPECT_EQ(1234, impression_cap_expire_time_ms);
EXPECT_EQ(4321, request_freeze_time_ms);
EXPECT_EQ(456, max_impressions);
}
TEST_F(SearchSuggestServiceTest, DontRequestWhenImpressionCapped) {
ASSERT_EQ(base::nullopt, service()->search_suggest_data());
SignIn();
const base::DictionaryValue* dict =
pref_service()->GetDictionary(prefs::kNtpSearchSuggestionsImpressions);
int impressions_count = 0;
ASSERT_TRUE(dict->GetInteger("impressions_count", &impressions_count));
EXPECT_EQ(0, impressions_count);
// Request a refresh. That should arrive at the loader.
service()->Refresh();
EXPECT_EQ(1u, loader()->GetCallbackCount());
// Fulfill it.
SearchSuggestData data = TestSearchSuggestData();
data.max_impressions = 2;
loader()->RespondToAllCallbacks(SearchSuggestLoader::Status::OK, data);
EXPECT_EQ(data, service()->search_suggest_data());
service()->SuggestionsDisplayed();
dict = pref_service()->GetDictionary(prefs::kNtpSearchSuggestionsImpressions);
ASSERT_TRUE(dict->GetInteger("impressions_count", &impressions_count));
EXPECT_EQ(1, impressions_count);
// Request another refresh.
service()->Refresh();
EXPECT_EQ(1u, loader()->GetCallbackCount());
loader()->RespondToAllCallbacks(SearchSuggestLoader::Status::OK, data);
EXPECT_EQ(data, service()->search_suggest_data());
service()->SuggestionsDisplayed();
dict = pref_service()->GetDictionary(prefs::kNtpSearchSuggestionsImpressions);
ASSERT_TRUE(dict->GetInteger("impressions_count", &impressions_count));
EXPECT_EQ(2, impressions_count);
// Should not make another request as we've reached the cap
service()->Refresh();
EXPECT_EQ(0u, loader()->GetCallbackCount());
}
TEST_F(SearchSuggestServiceTest, ImpressionCountResetsAfterTimeout) {
ASSERT_EQ(base::nullopt, service()->search_suggest_data());
SignIn();
const base::DictionaryValue* dict =
pref_service()->GetDictionary(prefs::kNtpSearchSuggestionsImpressions);
int impressions_count = 0;
ASSERT_TRUE(dict->GetInteger("impressions_count", &impressions_count));
EXPECT_EQ(0, impressions_count);
// Request a refresh. That should arrive at the loader.
service()->Refresh();
EXPECT_EQ(1u, loader()->GetCallbackCount());
// Fulfill it.
SearchSuggestData data = TestSearchSuggestData();
data.max_impressions = 1;
data.impression_cap_expire_time_ms = 1000;
loader()->RespondToAllCallbacks(SearchSuggestLoader::Status::OK, data);
EXPECT_EQ(data, service()->search_suggest_data());
service()->SuggestionsDisplayed();
dict = pref_service()->GetDictionary(prefs::kNtpSearchSuggestionsImpressions);
ASSERT_TRUE(dict->GetInteger("impressions_count", &impressions_count));
EXPECT_EQ(1, impressions_count);
// The impression cap has been reached.
service()->Refresh();
EXPECT_EQ(base::nullopt, service()->search_suggest_data());
RunFor(base::TimeDelta::FromMilliseconds(1000));
// The impression cap timeout has expired.
service()->Refresh();
EXPECT_EQ(1u, loader()->GetCallbackCount());
loader()->RespondToAllCallbacks(SearchSuggestLoader::Status::OK, data);
EXPECT_EQ(data, service()->search_suggest_data());
}
TEST_F(SearchSuggestServiceTest, RequestsFreezeOnEmptyResponse) {
ASSERT_EQ(base::nullopt, service()->search_suggest_data());
SignIn();
// Request a refresh. That should arrive at the loader.
service()->Refresh();
EXPECT_EQ(1u, loader()->GetCallbackCount());
// Fulfill it.
SearchSuggestData data = TestSearchSuggestData();
data.request_freeze_time_ms = 1000;
loader()->RespondToAllCallbacks(SearchSuggestLoader::Status::OK, data);
EXPECT_EQ(data, service()->search_suggest_data());
// Request a refresh. That should arrive at the loader.
service()->Refresh();
EXPECT_EQ(1u, loader()->GetCallbackCount());
loader()->RespondToAllCallbacks(SearchSuggestLoader::Status::FATAL_ERROR,
base::nullopt);
const base::DictionaryValue* dict =
pref_service()->GetDictionary(prefs::kNtpSearchSuggestionsImpressions);
bool is_request_frozen;
ASSERT_TRUE(dict->GetBoolean("is_request_frozen", &is_request_frozen));
EXPECT_TRUE(is_request_frozen);
// No request should be made since they are frozen.
service()->Refresh();
EXPECT_EQ(base::nullopt, service()->search_suggest_data());
RunFor(base::TimeDelta::FromMilliseconds(1000));
// The freeze timeout has expired.
service()->Refresh();
EXPECT_EQ(1u, loader()->GetCallbackCount());
loader()->RespondToAllCallbacks(SearchSuggestLoader::Status::OK, data);
EXPECT_EQ(data, service()->search_suggest_data());
}
chrome/common/pref_names.cc
View file @ 621811e4
......@@ -1544,6 +1544,8 @@ const char kNtpCustomBackgroundLocalToDevice[] =
// Data associated with search suggestions that appear on the NTP.
const char kNtpSearchSuggestionsBlacklist[] =
"ntp.search_suggestions_blacklist";
const char kNtpSearchSuggestionsImpressions[] =
"ntp.search_suggestions_impressions";
const char kNtpSearchSuggestionsOptOut[] = "ntp.search_suggestions_opt_out";
#endif // defined(OS_ANDROID)
......
chrome/common/pref_names.h
View file @ 621811e4
......@@ -537,6 +537,7 @@ extern const char kContentSuggestionsNotificationsSentCount[];
extern const char kNtpCustomBackgroundDict[];
extern const char kNtpCustomBackgroundLocalToDevice[];
extern const char kNtpSearchSuggestionsBlacklist[];
extern const char kNtpSearchSuggestionsImpressions[];
extern const char kNtpSearchSuggestionsOptOut[];
#endif // defined(OS_ANDROID)
extern const char kNtpShownPage[];
......
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