Skip to content

GitLab

T
tangled
Commit 7acc981d authored by Tim Schumann's avatar Tim Schumann Committed by Commit Bot
Browse files
Options

Remove left-over ChromeReader integration code. 

This is about the very first integration where Chrome would directly talk
to a ChromeReader frontend and interpret ChromeReader's JSON protocol.

Change-Id: I1e53e64eacf879a05704bbe04b960a24fc1bba36
Reviewed-on: https://chromium-review.googlesource.com/659859
Commit-Queue: Tim Schumann <tschumann@chromium.org>
Reviewed-by: default avatarvitaliii <vitaliii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501243}
parent 71b12ba1
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 27 additions and 530 deletions
components/ntp_snippets/remote/proto/ntp_snippets.proto
View file @ 7acc981d
......@@ -22,7 +22,7 @@ message SnippetProto {
optional int64 publish_date = 5;
optional int64 expiry_date = 6;
optional float score = 7;
repeated SnippetSourceProto sources = 8;
optional SnippetSourceProto source = 8;
optional bool dismissed = 9;
optional int32 remote_category_id = 10;
// The time when the snippet was fetched from the server.
......
components/ntp_snippets/remote/remote_suggestion.cc
View file @ 7acc981d
......@@ -16,39 +16,6 @@
namespace {
struct SnippetSource {
SnippetSource(const GURL& url,
const std::string& publisher_name,
const GURL& amp_url)
: url(url), publisher_name(publisher_name), amp_url(amp_url) {}
GURL url;
std::string publisher_name;
GURL amp_url;
};
const SnippetSource& FindBestSource(const std::vector<SnippetSource>& sources) {
// The same article can be hosted by multiple sources, e.g. nytimes.com,
// cnn.com, etc. We need to parse the list of sources for this article and
// find the best match. In order of preference:
// 1) A source that has URL, publisher name, AMP URL
// 2) A source that has URL, publisher name
// 3) A source that has URL and AMP URL, or URL only (since we won't show
// the snippet to users if the article does not have a publisher name, it
// doesn't matter whether the snippet has the AMP URL or not)
int best_source_index = 0;
for (size_t i = 0; i < sources.size(); ++i) {
const SnippetSource& source = sources[i];
if (!source.publisher_name.empty()) {
best_source_index = i;
if (!source.amp_url.is_empty()) {
// This is the best possible source, stop looking.
break;
}
}
}
return sources[best_source_index];
}
// dict.Get() specialization for base::Time values
bool GetTimeValue(const base::DictionaryValue& dict,
const std::string& key,
......@@ -81,8 +48,6 @@ static_assert(
kArticlesRemoteId,
"kArticlesRemoteId has a wrong value?!");
const int kChromeReaderDefaultExpiryTimeMins = 3 * 24 * 60;
RemoteSuggestion::RemoteSuggestion(const std::vector<std::string>& ids,
int remote_category_id)
: ids_(ids),
......@@ -95,128 +60,6 @@ RemoteSuggestion::RemoteSuggestion(const std::vector<std::string>& ids,
RemoteSuggestion::~RemoteSuggestion() = default;
// static
std::unique_ptr<RemoteSuggestion>
RemoteSuggestion::CreateFromChromeReaderDictionary(
const base::DictionaryValue& dict,
const base::Time& fetch_date) {
const base::DictionaryValue* content = nullptr;
if (!dict.GetDictionary("contentInfo", &content)) {
return nullptr;
}
// Need at least a primary id.
std::string primary_id;
if (!content->GetString("url", &primary_id) || primary_id.empty()) {
return nullptr;
}
const base::ListValue* corpus_infos_list = nullptr;
if (!content->GetList("sourceCorpusInfo", &corpus_infos_list)) {
DLOG(WARNING) << "No sources found for article " << primary_id;
return nullptr;
}
std::vector<std::string> ids(1, primary_id);
std::vector<SnippetSource> sources;
for (const auto& value : *corpus_infos_list) {
const base::DictionaryValue* dict_value = nullptr;
if (!value.GetAsDictionary(&dict_value)) {
DLOG(WARNING) << "Invalid source info for article " << primary_id;
continue;
}
std::string corpus_id_str;
GURL corpus_id;
if (dict_value->GetString("corpusId", &corpus_id_str)) {
corpus_id = GURL(corpus_id_str);
}
if (!corpus_id.is_valid()) {
// We must at least have a valid source URL.
DLOG(WARNING) << "Invalid article url " << corpus_id_str;
continue;
}
const base::DictionaryValue* publisher_data = nullptr;
std::string site_title;
if (dict_value->GetDictionary("publisherData", &publisher_data)) {
if (!publisher_data->GetString("sourceName", &site_title)) {
// It's possible but not desirable to have no publisher data.
DLOG(WARNING) << "No publisher name for article " << corpus_id_str;
}
} else {
DLOG(WARNING) << "No publisher data for article " << corpus_id_str;
}
std::string amp_url_str;
GURL amp_url;
// Expected to not have AMP url sometimes.
if (dict_value->GetString("ampUrl", &amp_url_str)) {
amp_url = GURL(amp_url_str);
DLOG_IF(WARNING, !amp_url.is_valid())
<< "Invalid AMP url " << amp_url_str;
}
sources.emplace_back(corpus_id, site_title,
amp_url.is_valid() ? amp_url : GURL());
// We use the raw string so that we can compare it against other primary
// IDs. Parsing the ID as a URL might add a trailing slash (and we don't do
// this for the primary ID).
ids.push_back(corpus_id_str);
}
if (sources.empty()) {
DLOG(WARNING) << "No sources found for article " << primary_id;
return nullptr;
}
std::unique_ptr<RemoteSuggestion> snippet(
new RemoteSuggestion(ids, kArticlesRemoteId));
snippet->fetch_date_ = fetch_date;
std::string title;
if (content->GetString("title", &title)) {
snippet->title_ = title;
}
std::string salient_image_url;
if (content->GetString("thumbnailUrl", &salient_image_url)) {
snippet->salient_image_url_ = GURL(salient_image_url);
}
std::string snippet_str;
if (content->GetString("snippet", &snippet_str)) {
snippet->snippet_ = snippet_str;
}
// The creation and expiry timestamps are uint64s which are stored as strings.
std::string creation_timestamp_str;
if (content->GetString("creationTimestampSec", &creation_timestamp_str)) {
snippet->publish_date_ = TimeFromJsonString(creation_timestamp_str);
}
std::string expiry_timestamp_str;
if (content->GetString("expiryTimestampSec", &expiry_timestamp_str)) {
snippet->expiry_date_ = TimeFromJsonString(expiry_timestamp_str);
}
// If publish and/or expiry date are missing, fill in reasonable defaults.
if (snippet->publish_date_.is_null()) {
snippet->publish_date_ = base::Time::Now();
}
if (snippet->expiry_date_.is_null()) {
snippet->expiry_date_ =
snippet->publish_date() +
base::TimeDelta::FromMinutes(kChromeReaderDefaultExpiryTimeMins);
}
const SnippetSource& source = FindBestSource(sources);
snippet->url_ = source.url;
snippet->publisher_name_ = source.publisher_name;
snippet->amp_url_ = source.amp_url;
double score;
if (dict.GetDouble("score", &score)) {
snippet->score_ = score;
}
return snippet;
}
// static
std::unique_ptr<RemoteSuggestion>
RemoteSuggestion::CreateFromContentSuggestionsDictionary(
......@@ -338,33 +181,25 @@ std::unique_ptr<RemoteSuggestion> RemoteSuggestion::CreateFromProto(
snippet->score_ = proto.score();
snippet->is_dismissed_ = proto.dismissed();
std::vector<SnippetSource> sources;
for (int i = 0; i < proto.sources_size(); ++i) {
const SnippetSourceProto& source_proto = proto.sources(i);
GURL url(source_proto.url());
if (!url.is_valid()) {
// We must at least have a valid source URL.
DLOG(WARNING) << "Invalid article url " << source_proto.url();
continue;
}
GURL amp_url;
if (source_proto.has_amp_url()) {
amp_url = GURL(source_proto.amp_url());
DLOG_IF(WARNING, !amp_url.is_valid())
<< "Invalid AMP URL " << source_proto.amp_url();
}
sources.emplace_back(url, source_proto.publisher_name(), amp_url);
if (!proto.has_source()) {
DLOG(WARNING) << "No source found for article " << snippet->id();
return nullptr;
}
if (sources.empty()) {
DLOG(WARNING) << "No sources found for article " << snippet->id();
GURL url(proto.source().url());
if (!url.is_valid()) {
// We must at least have a valid source URL.
DLOG(WARNING) << "Invalid article url " << proto.source().url();
return nullptr;
}
const SnippetSource& source = FindBestSource(sources);
snippet->url_ = source.url;
snippet->publisher_name_ = source.publisher_name;
snippet->amp_url_ = source.amp_url;
GURL amp_url;
if (proto.source().has_amp_url()) {
amp_url = GURL(proto.source().amp_url());
DLOG_IF(WARNING, !amp_url.is_valid())
<< "Invalid AMP URL " << proto.source().amp_url();
}
snippet->url_ = url;
snippet->publisher_name_ = proto.source().publisher_name();
snippet->amp_url_ = amp_url;
if (proto.has_fetch_date()) {
snippet->fetch_date_ = base::Time::FromInternalValue(proto.fetch_date());
......@@ -404,7 +239,7 @@ SnippetProto RemoteSuggestion::ToProto() const {
result.set_dismissed(is_dismissed_);
result.set_remote_category_id(remote_category_id_);
SnippetSourceProto* source_proto = result.add_sources();
SnippetSourceProto* source_proto = result.mutable_source();
source_proto->set_url(url_.spec());
if (!publisher_name_.empty()) {
source_proto->set_publisher_name(publisher_name_);
......@@ -457,24 +292,6 @@ ContentSuggestion RemoteSuggestion::ToContentSuggestion(
return suggestion;
}
// static
base::Time RemoteSuggestion::TimeFromJsonString(
const std::string& timestamp_str) {
int64_t timestamp;
if (!base::StringToInt64(timestamp_str, &timestamp)) {
// Even if there's an error in the conversion, some garbage data may still
// be written to the output var, so reset it.
DLOG(WARNING) << "Invalid json timestamp: " << timestamp_str;
timestamp = 0;
}
return base::Time::UnixEpoch() + base::TimeDelta::FromSeconds(timestamp);
}
// static
std::string RemoteSuggestion::TimeToJsonString(const base::Time& time) {
return base::Int64ToString((time - base::Time::UnixEpoch()).InSeconds());
}
// static
std::unique_ptr<RemoteSuggestion> RemoteSuggestion::MakeUnique(
const std::vector<std::string>& ids,
......
components/ntp_snippets/remote/remote_suggestion.h
View file @ 7acc981d
......@@ -23,7 +23,6 @@ namespace ntp_snippets {
// Exposed for tests.
extern const int kArticlesRemoteId;
extern const int kChromeReaderDefaultExpiryTimeMins;
class SnippetProto;
......@@ -35,17 +34,9 @@ class RemoteSuggestion {
~RemoteSuggestion();
// Creates a RemoteSuggestion from a dictionary, as returned by Chrome Reader.
// Returns a null pointer if the dictionary doesn't correspond to a valid
// suggestion. The keys in the dictionary are expected to be the same as the
// property name, with exceptions documented in the property comment.
static std::unique_ptr<RemoteSuggestion> CreateFromChromeReaderDictionary(
const base::DictionaryValue& dict,
const base::Time& fetch_date);
// Creates a RemoteSuggestion from a dictionary, as returned by Chrome Content
// Suggestions. Returns a null pointer if the dictionary doesn't correspond to
// a valid suggestion. Maps field names to Chrome Reader field names.
// a valid suggestion.
static std::unique_ptr<RemoteSuggestion>
CreateFromContentSuggestionsDictionary(const base::DictionaryValue& dict,
int remote_category_id,
......@@ -87,13 +78,10 @@ class RemoteSuggestion {
const std::string& snippet() const { return snippet_; }
// Link to an image representative of the content. Do not fetch this image
// directly. If initialized by CreateFromChromeReaderDictionary() the relevant
// key is 'thumbnailUrl'
// directly.
const GURL& salient_image_url() const { return salient_image_url_; }
// When the page pointed by this suggestion was published. If initialized by
// CreateFromChromeReaderDictionary() the relevant key is
// 'creationTimestampSec'
// When the page pointed by this suggestion was published.
const base::Time& publish_date() const { return publish_date_; }
// After this expiration date this suggestion should no longer be presented to
......@@ -131,10 +119,6 @@ class RemoteSuggestion {
base::Time fetch_date() const { return fetch_date_; }
// Public for testing.
static base::Time TimeFromJsonString(const std::string& timestamp_str);
static std::string TimeToJsonString(const base::Time& time);
private:
RemoteSuggestion(const std::vector<std::string>& ids, int remote_category_id);
......
components/ntp_snippets/remote/remote_suggestion_builder.cc
View file @ 7acc981d
......@@ -148,7 +148,7 @@ std::unique_ptr<RemoteSuggestion> RemoteSuggestionBuilder::Build() const {
proto.set_score(score_.value_or(1));
proto.set_dismissed(is_dismissed_.value_or(false));
proto.set_remote_category_id(remote_category_id_.value_or(1));
auto* source = proto.add_sources();
auto* source = proto.mutable_source();
source->set_url(url_.value_or("http://url.com/"));
source->set_publisher_name(publisher_name_.value_or("Publisher"));
source->set_amp_url(amp_url_.value_or("http://amp_url.com/"));
......
components/ntp_snippets/remote/remote_suggestions_database_unittest.cc
View file @ 7acc981d
......@@ -44,7 +44,7 @@ std::unique_ptr<RemoteSuggestion> CreateTestSuggestion() {
SnippetProto proto;
proto.add_ids("http://localhost");
proto.set_remote_category_id(1); // Articles
auto* source = proto.add_sources();
auto* source = proto.mutable_source();
source->set_url("http://localhost");
source->set_publisher_name("Publisher");
source->set_amp_url("http://amp");
......
components/ntp_snippets/remote/remote_suggestions_fetcher_impl.h
View file @ 7acc981d
......@@ -64,22 +64,6 @@ class RemoteSuggestionsFetcherImpl : public RemoteSuggestionsFetcher {
}
private:
FRIEND_TEST_ALL_PREFIXES(ChromeReaderSnippetsFetcherTest,
BuildRequestAuthenticated);
FRIEND_TEST_ALL_PREFIXES(ChromeReaderSnippetsFetcherTest,
BuildRequestUnauthenticated);
FRIEND_TEST_ALL_PREFIXES(ChromeReaderSnippetsFetcherTest,
BuildRequestExcludedIds);
FRIEND_TEST_ALL_PREFIXES(ChromeReaderSnippetsFetcherTest,
BuildRequestNoUserClass);
FRIEND_TEST_ALL_PREFIXES(ChromeReaderSnippetsFetcherTest,
BuildRequestWithTwoLanguages);
FRIEND_TEST_ALL_PREFIXES(ChromeReaderSnippetsFetcherTest,
BuildRequestWithUILanguageOnly);
FRIEND_TEST_ALL_PREFIXES(ChromeReaderSnippetsFetcherTest,
BuildRequestWithOtherLanguageOnly);
friend class ChromeReaderSnippetsFetcherTest;
void FetchSnippetsNonAuthenticated(internal::JsonRequest::Builder builder,
SnippetsAvailableCallback callback);
void FetchSnippetsAuthenticated(internal::JsonRequest::Builder builder,
......
components/ntp_snippets/remote/remote_suggestions_provider_impl.cc
View file @ 7acc981d
......@@ -828,8 +828,6 @@ void RemoteSuggestionsProviderImpl::OnFetchFinished(
// from the server. crbug.com/653816
bool response_includes_article_category = false;
for (FetchedCategory& fetched_category : *fetched_categories) {
// TODO(tschumann): Remove this histogram once we only talk to the content
// suggestions cloud backend.
if (fetched_category.category == articles_category_) {
UMA_HISTOGRAM_SPARSE_SLOWLY(
"NewTabPage.Snippets.NumArticlesFetched",
......
components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc
View file @ 7acc981d
......@@ -151,7 +151,7 @@ std::unique_ptr<RemoteSuggestion> CreateTestRemoteSuggestion(
snippet_proto.set_publish_date(SerializeTime(GetDefaultCreationTime()));
snippet_proto.set_expiry_date(SerializeTime(GetDefaultExpirationTime()));
snippet_proto.set_remote_category_id(1);
auto* source = snippet_proto.add_sources();
auto* source = snippet_proto.mutable_source();
source->set_url(url);
source->set_publisher_name("Publisher");
source->set_amp_url(url + "amp");
......
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