Commit 52f76712 authored by sfiera's avatar sfiera Committed by Commit bot

Inform server of dismissed articles.

It seems that the server currently has logic to not send back excluded
IDs, but lacks the logic needed to request more. Still, that's something
that can be improved in the server later.

There should be tests for NTPSnippetsService to verify the new behavior,
but that's not possible until it uses the new server.

BUG=633613

Review-Url: https://codereview.chromium.org/2274953002
Cr-Commit-Position: refs/heads/master@{#414428}
parent 798a8b94
...@@ -70,6 +70,8 @@ const char kPersonalizationBothString[] = "both"; // the default value ...@@ -70,6 +70,8 @@ const char kPersonalizationBothString[] = "both"; // the default value
const char kHostRestrictionOnString[] = "on"; // the default value const char kHostRestrictionOnString[] = "on"; // the default value
const char kHostRestrictionOffString[] = "off"; const char kHostRestrictionOffString[] = "off";
const int kMaxExcludedIds = 100;
std::string FetchResultToString(NTPSnippetsFetcher::FetchResult result) { std::string FetchResultToString(NTPSnippetsFetcher::FetchResult result) {
switch (result) { switch (result) {
case NTPSnippetsFetcher::FetchResult::SUCCESS: case NTPSnippetsFetcher::FetchResult::SUCCESS:
...@@ -216,6 +218,7 @@ void NTPSnippetsFetcher::SetCallback( ...@@ -216,6 +218,7 @@ void NTPSnippetsFetcher::SetCallback(
void NTPSnippetsFetcher::FetchSnippetsFromHosts( void NTPSnippetsFetcher::FetchSnippetsFromHosts(
const std::set<std::string>& hosts, const std::set<std::string>& hosts,
const std::string& language_code, const std::string& language_code,
const std::set<std::string>& excluded_ids,
int count, int count,
bool interactive_request) { bool interactive_request) {
if (!request_throttler_.DemandQuotaForRequest(interactive_request)) if (!request_throttler_.DemandQuotaForRequest(interactive_request))
...@@ -223,6 +226,7 @@ void NTPSnippetsFetcher::FetchSnippetsFromHosts( ...@@ -223,6 +226,7 @@ void NTPSnippetsFetcher::FetchSnippetsFromHosts(
hosts_ = hosts; hosts_ = hosts;
fetch_start_time_ = tick_clock_->NowTicks(); fetch_start_time_ = tick_clock_->NowTicks();
excluded_ids_ = excluded_ids;
if (UsesHostRestrictions() && hosts_.empty()) { if (UsesHostRestrictions() && hosts_.empty()) {
FetchFinished(OptionalSnippets(), FetchResult::EMPTY_HOSTS, FetchFinished(OptionalSnippets(), FetchResult::EMPTY_HOSTS,
...@@ -317,12 +321,21 @@ std::string NTPSnippetsFetcher::RequestParams::BuildRequest() { ...@@ -317,12 +321,21 @@ std::string NTPSnippetsFetcher::RequestParams::BuildRequest() {
if (!user_locale.empty()) { if (!user_locale.empty()) {
request->SetString("uiLanguage", user_locale); request->SetString("uiLanguage", user_locale);
} }
auto regular_hosts = base::MakeUnique<base::ListValue>(); auto regular_hosts = base::MakeUnique<base::ListValue>();
for (const auto& host : host_restricts) { for (const auto& host : host_restricts) {
regular_hosts->AppendString(host); regular_hosts->AppendString(host);
} }
request->Set("regularlyVisitedHostNames", std::move(regular_hosts)); request->Set("regularlyVisitedHostNames", std::move(regular_hosts));
auto excluded = base::MakeUnique<base::ListValue>();
for (const auto& id : excluded_ids) {
excluded->AppendString(id);
if (excluded->GetSize() >= kMaxExcludedIds)
break;
}
request->Set("excludedSuggestionIds", std::move(excluded));
// TODO(sfiera): support authentication and personalization // TODO(sfiera): support authentication and personalization
// TODO(sfiera): support count_to_fetch // TODO(sfiera): support count_to_fetch
break; break;
...@@ -392,6 +405,7 @@ void NTPSnippetsFetcher::FetchSnippetsNonAuthenticated() { ...@@ -392,6 +405,7 @@ void NTPSnippetsFetcher::FetchSnippetsNonAuthenticated() {
params.fetch_api = fetch_api_; params.fetch_api = fetch_api_;
params.host_restricts = params.host_restricts =
UsesHostRestrictions() ? hosts_ : std::set<std::string>(); UsesHostRestrictions() ? hosts_ : std::set<std::string>();
params.excluded_ids = excluded_ids_;
params.count_to_fetch = count_to_fetch_; params.count_to_fetch = count_to_fetch_;
FetchSnippetsImpl(url, std::string(), params.BuildRequest()); FetchSnippetsImpl(url, std::string(), params.BuildRequest());
} }
...@@ -407,6 +421,7 @@ void NTPSnippetsFetcher::FetchSnippetsAuthenticated( ...@@ -407,6 +421,7 @@ void NTPSnippetsFetcher::FetchSnippetsAuthenticated(
params.user_locale = locale_; params.user_locale = locale_;
params.host_restricts = params.host_restricts =
UsesHostRestrictions() ? hosts_ : std::set<std::string>(); UsesHostRestrictions() ? hosts_ : std::set<std::string>();
params.excluded_ids = excluded_ids_;
params.count_to_fetch = count_to_fetch_; params.count_to_fetch = count_to_fetch_;
// TODO(jkrcal, treib): Add unit-tests for authenticated fetches. // TODO(jkrcal, treib): Add unit-tests for authenticated fetches.
FetchSnippetsImpl(fetch_url_, FetchSnippetsImpl(fetch_url_,
......
...@@ -90,7 +90,11 @@ class NTPSnippetsFetcher : public OAuth2TokenService::Consumer, ...@@ -90,7 +90,11 @@ class NTPSnippetsFetcher : public OAuth2TokenService::Consumer,
void SetCallback(const SnippetsAvailableCallback& callback); void SetCallback(const SnippetsAvailableCallback& callback);
// Fetches snippets from the server. |hosts| restricts the results to a set of // Fetches snippets from the server. |hosts| restricts the results to a set of
// hosts, e.g. "www.google.com". An empty host set produces an error. // hosts, e.g. "www.google.com". If host restrictions are enabled, an empty
// host set produces an error without issuing a fetch.
//
// |excluded_ids| will be reported to the server; the server should not return
// suggestions with those IDs.
// //
// If an ongoing fetch exists, it will be cancelled and a new one started, // If an ongoing fetch exists, it will be cancelled and a new one started,
// without triggering an additional callback (i.e. not noticeable by // without triggering an additional callback (i.e. not noticeable by
...@@ -100,6 +104,7 @@ class NTPSnippetsFetcher : public OAuth2TokenService::Consumer, ...@@ -100,6 +104,7 @@ class NTPSnippetsFetcher : public OAuth2TokenService::Consumer,
// |interactive_request| is set to true (use only for user-initiated fetches). // |interactive_request| is set to true (use only for user-initiated fetches).
void FetchSnippetsFromHosts(const std::set<std::string>& hosts, void FetchSnippetsFromHosts(const std::set<std::string>& hosts,
const std::string& language_code, const std::string& language_code,
const std::set<std::string>& excluded_ids,
int count, int count,
bool interactive_request); bool interactive_request);
...@@ -134,6 +139,7 @@ class NTPSnippetsFetcher : public OAuth2TokenService::Consumer, ...@@ -134,6 +139,7 @@ class NTPSnippetsFetcher : public OAuth2TokenService::Consumer,
private: private:
FRIEND_TEST_ALL_PREFIXES(NTPSnippetsFetcherTest, BuildRequestAuthenticated); FRIEND_TEST_ALL_PREFIXES(NTPSnippetsFetcherTest, BuildRequestAuthenticated);
FRIEND_TEST_ALL_PREFIXES(NTPSnippetsFetcherTest, BuildRequestUnauthenticated); FRIEND_TEST_ALL_PREFIXES(NTPSnippetsFetcherTest, BuildRequestUnauthenticated);
FRIEND_TEST_ALL_PREFIXES(NTPSnippetsFetcherTest, BuildRequestExcludedIds);
enum FetchAPI { enum FetchAPI {
CHROME_READER_API, CHROME_READER_API,
...@@ -146,6 +152,7 @@ class NTPSnippetsFetcher : public OAuth2TokenService::Consumer, ...@@ -146,6 +152,7 @@ class NTPSnippetsFetcher : public OAuth2TokenService::Consumer,
bool only_return_personalized_results; bool only_return_personalized_results;
std::string user_locale; std::string user_locale;
std::set<std::string> host_restricts; std::set<std::string> host_restricts;
std::set<std::string> excluded_ids;
int count_to_fetch; int count_to_fetch;
RequestParams(); RequestParams();
...@@ -201,6 +208,9 @@ class NTPSnippetsFetcher : public OAuth2TokenService::Consumer, ...@@ -201,6 +208,9 @@ class NTPSnippetsFetcher : public OAuth2TokenService::Consumer,
// Hosts to restrict the snippets to. // Hosts to restrict the snippets to.
std::set<std::string> hosts_; std::set<std::string> hosts_;
// Snippets to exclude from the results.
std::set<std::string> excluded_ids_;
// Count of snippets to fetch. // Count of snippets to fetch.
int count_to_fetch_; int count_to_fetch_;
......
...@@ -265,8 +265,13 @@ void NTPSnippetsService::FetchSnippetsFromHosts( ...@@ -265,8 +265,13 @@ void NTPSnippetsService::FetchSnippetsFromHosts(
if (snippets_.empty()) if (snippets_.empty())
UpdateCategoryStatus(CategoryStatus::AVAILABLE_LOADING); UpdateCategoryStatus(CategoryStatus::AVAILABLE_LOADING);
snippets_fetcher_->FetchSnippetsFromHosts( std::set<std::string> excluded_ids;
hosts, application_language_code_, kMaxSnippetCount, interactive_request); for (const auto& snippet : dismissed_snippets_) {
excluded_ids.insert(snippet->id());
}
snippets_fetcher_->FetchSnippetsFromHosts(hosts, application_language_code_,
excluded_ids, kMaxSnippetCount,
interactive_request);
} }
void NTPSnippetsService::RescheduleFetching() { void NTPSnippetsService::RescheduleFetching() {
......
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