Commit 836db710 authored by Markus Heintz's avatar Markus Heintz Committed by Commit Bot

[Content Suggestions] Add a is_video_suggestion_ field.

This cl adds a is_video_suggestions field to the ContentSuggestions class and the Java equivalent SnippetArticle.
The field is set based on the content_type of a RemoteSnippet. RemoteSnippet is now supporting the content_type field of suggestions sent by the server.

Bug: 735072
Change-Id: I02401c2856bb2ff3c7bb95cc4cb91e67dc464e0d
Reviewed-on: https://chromium-review.googlesource.com/543142
Commit-Queue: Markus Heintz <markusheintz@chromium.org>
Reviewed-by: default avatarvitaliii <vitaliii@chromium.org>
Reviewed-by: default avatarMichael van Ouwerkerk <mvanouwerkerk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481869}
parent 4eb6812c
...@@ -45,6 +45,9 @@ public class SnippetArticle implements OfflinableSuggestion { ...@@ -45,6 +45,9 @@ public class SnippetArticle implements OfflinableSuggestion {
*/ */
public final long mFetchTimestampMilliseconds; public final long mFetchTimestampMilliseconds;
/** The flag that indicates whether this is a video suggestion. */
public boolean mIsVideoSuggestion;
/** The rank of this article within its section. */ /** The rank of this article within its section. */
private int mPerSectionRank = -1; private int mPerSectionRank = -1;
...@@ -79,8 +82,8 @@ public class SnippetArticle implements OfflinableSuggestion { ...@@ -79,8 +82,8 @@ public class SnippetArticle implements OfflinableSuggestion {
* Creates a SnippetArticleListItem object that will hold the data. * Creates a SnippetArticleListItem object that will hold the data.
*/ */
public SnippetArticle(int category, String idWithinCategory, String title, String publisher, public SnippetArticle(int category, String idWithinCategory, String title, String publisher,
String previewText, String url, long publishTimestamp, float score, String previewText, String url, long publishTimestamp, float score, long fetchTimestamp,
long fetchTimestamp) { boolean isVideoSuggestion) {
mCategory = category; mCategory = category;
mIdWithinCategory = idWithinCategory; mIdWithinCategory = idWithinCategory;
mTitle = title; mTitle = title;
...@@ -90,6 +93,7 @@ public class SnippetArticle implements OfflinableSuggestion { ...@@ -90,6 +93,7 @@ public class SnippetArticle implements OfflinableSuggestion {
mPublishTimestampMilliseconds = publishTimestamp; mPublishTimestampMilliseconds = publishTimestamp;
mScore = score; mScore = score;
mFetchTimestampMilliseconds = fetchTimestamp; mFetchTimestampMilliseconds = fetchTimestamp;
mIsVideoSuggestion = isVideoSuggestion;
} }
@Override @Override
......
...@@ -193,10 +193,10 @@ public class SnippetsBridge implements SuggestionsSource, DestructionObserver { ...@@ -193,10 +193,10 @@ public class SnippetsBridge implements SuggestionsSource, DestructionObserver {
@CalledByNative @CalledByNative
private static SnippetArticle addSuggestion(List<SnippetArticle> suggestions, int category, private static SnippetArticle addSuggestion(List<SnippetArticle> suggestions, int category,
String id, String title, String publisher, String previewText, String url, String id, String title, String publisher, String previewText, String url,
long timestamp, float score, long fetchTime) { long timestamp, float score, long fetchTime, boolean isVideoSuggestion) {
int position = suggestions.size(); int position = suggestions.size();
suggestions.add(new SnippetArticle( suggestions.add(new SnippetArticle(category, id, title, publisher, previewText, url,
category, id, title, publisher, previewText, url, timestamp, score, fetchTime)); timestamp, score, fetchTime, isVideoSuggestion));
return suggestions.get(position); return suggestions.get(position);
} }
......
...@@ -427,7 +427,7 @@ public class NewTabPageRecyclerViewTest { ...@@ -427,7 +427,7 @@ public class NewTabPageRecyclerViewTest {
String url = mTestServer.getURL(TEST_PAGE) + "#" + i; String url = mTestServer.getURL(TEST_PAGE) + "#" + i;
suggestions.add(new SnippetArticle(TEST_CATEGORY, "id" + i, "title" + i, suggestions.add(new SnippetArticle(TEST_CATEGORY, "id" + i, "title" + i,
"publisher" + i, "previewText" + i, url, FAKE_PUBLISH_TIMESTAMP + i, "publisher" + i, "previewText" + i, url, FAKE_PUBLISH_TIMESTAMP + i,
FAKE_SNIPPET_SCORE, FAKE_FETCH_TIMESTAMP)); FAKE_SNIPPET_SCORE, FAKE_FETCH_TIMESTAMP, false));
} }
return suggestions; return suggestions;
} }
......
...@@ -208,7 +208,7 @@ public class ArticleSnippetsTest { ...@@ -208,7 +208,7 @@ public class ArticleSnippetsTest {
SnippetArticle download = new SnippetArticle(KnownCategories.DOWNLOADS, "id1", SnippetArticle download = new SnippetArticle(KnownCategories.DOWNLOADS, "id1",
"test_image.jpg", "example.com", null, "http://example.com", timestamp, 10f, "test_image.jpg", "example.com", null, "http://example.com", timestamp, 10f,
timestamp); timestamp, false);
download.setAssetDownloadData("asdf", filePath, "image/jpeg"); download.setAssetDownloadData("asdf", filePath, "image/jpeg");
SuggestionsCategoryInfo categoryInfo = SuggestionsCategoryInfo categoryInfo =
new SuggestionsCategoryInfo(KnownCategories.DOWNLOADS, "Downloads", new SuggestionsCategoryInfo(KnownCategories.DOWNLOADS, "Downloads",
...@@ -248,7 +248,8 @@ public class ArticleSnippetsTest { ...@@ -248,7 +248,8 @@ public class ArticleSnippetsTest {
"Publisher", "Preview Text", "www.google.com", "Publisher", "Preview Text", "www.google.com",
1466614774, // Publish timestamp 1466614774, // Publish timestamp
10f, // Score 10f, // Score
1466634774); // Fetch timestamp 1466634774, // Fetch timestamp
false); // IsVideoSuggestion
Bitmap bitmap = BitmapFactory.decodeResource(mActivityTestRule.getActivity().getResources(), Bitmap bitmap = BitmapFactory.decodeResource(mActivityTestRule.getActivity().getResources(),
R.drawable.signin_promo_illustration); R.drawable.signin_promo_illustration);
int thumbnailSize = mActivityTestRule.getActivity().getResources().getDimensionPixelSize( int thumbnailSize = mActivityTestRule.getActivity().getResources().getDimensionPixelSize(
...@@ -263,20 +264,23 @@ public class ArticleSnippetsTest { ...@@ -263,20 +264,23 @@ public class ArticleSnippetsTest {
new String(new char[80]).replace("\0", "Preview Text "), "www.google.com", new String(new char[80]).replace("\0", "Preview Text "), "www.google.com",
1466614074, // Publish timestamp 1466614074, // Publish timestamp
20f, // Score 20f, // Score
1466634774); // Fetch timestamp 1466634774, // Fetch timestamp
false); // IsVideoSuggestion
SnippetArticle minimalSnippet = new SnippetArticle(minimalCategory, "id3", SnippetArticle minimalSnippet = new SnippetArticle(minimalCategory, "id3",
new String(new char[20]).replace("\0", "Bookmark "), "Publisher", new String(new char[20]).replace("\0", "Bookmark "), "Publisher",
"This should not be displayed", "www.google.com", "This should not be displayed", "www.google.com",
1466614774, // Publish timestamp 1466614774, // Publish timestamp
10f, // Score 10f, // Score
1466634774); // Fetch timestamp 1466634774, // Fetch timestamp
false); // IsVideoSuggestion
SnippetArticle minimalSnippet2 = new SnippetArticle(minimalCategory, "id4", "Bookmark", SnippetArticle minimalSnippet2 = new SnippetArticle(minimalCategory, "id4", "Bookmark",
"Publisher", "This should not be displayed", "www.google.com", "Publisher", "This should not be displayed", "www.google.com",
1466614774, // Publish timestamp 1466614774, // Publish timestamp
10f, // Score 10f, // Score
1466634774); // Fetch timestamp 1466634774, // Fetch timestamp
false); // IsVideoSuggestion
mSnippetsSource.setInfoForCategory(fullCategory, mSnippetsSource.setInfoForCategory(fullCategory,
new SuggestionsCategoryInfo(fullCategory, "Section Title", new SuggestionsCategoryInfo(fullCategory, "Section Title",
......
...@@ -346,7 +346,7 @@ public class NewTabPageAdapterTest { ...@@ -346,7 +346,7 @@ public class NewTabPageAdapterTest {
// Add another suggestion. // Add another suggestion.
suggestions.add(new SnippetArticle(TEST_CATEGORY, "https://site.com/url3", "title3", "pub3", suggestions.add(new SnippetArticle(TEST_CATEGORY, "https://site.com/url3", "title3", "pub3",
"txt3", "https://site.com/url3", 0, 0, 0)); "txt3", "https://site.com/url3", 0, 0, 0, false));
// When the provider is removed, we should not be able to load suggestions. The UI should // When the provider is removed, we should not be able to load suggestions. The UI should
// stay the same though. // stay the same though.
......
...@@ -68,7 +68,8 @@ ScopedJavaLocalRef<jobject> ToJavaSuggestionList( ...@@ -68,7 +68,8 @@ ScopedJavaLocalRef<jobject> ToJavaSuggestionList(
ConvertUTF16ToJavaString(env, suggestion.snippet_text()), ConvertUTF16ToJavaString(env, suggestion.snippet_text()),
ConvertUTF8ToJavaString(env, suggestion.url().spec()), ConvertUTF8ToJavaString(env, suggestion.url().spec()),
suggestion.publish_date().ToJavaTime(), suggestion.score(), suggestion.publish_date().ToJavaTime(), suggestion.score(),
suggestion.fetch_date().ToJavaTime()); suggestion.fetch_date().ToJavaTime(),
suggestion.is_video_suggestion());
if (suggestion.id().category().IsKnownCategory( if (suggestion.id().category().IsKnownCategory(
KnownCategories::DOWNLOADS) && KnownCategories::DOWNLOADS) &&
suggestion.download_suggestion_extra() != nullptr) { suggestion.download_suggestion_extra() != nullptr) {
......
...@@ -27,7 +27,7 @@ public final class ContentSuggestionsTestUtils { ...@@ -27,7 +27,7 @@ public final class ContentSuggestionsTestUtils {
for (int index = 0; index < count; index++) { for (int index = 0; index < count; index++) {
suggestions.add(new SnippetArticle(category, "https://site.com/url" + prefix + index, suggestions.add(new SnippetArticle(category, "https://site.com/url" + prefix + index,
prefix + "title" + index, "pub" + index, "txt" + index, prefix + "title" + index, "pub" + index, "txt" + index,
"https://site.com/url" + index, 0, 0, 0)); "https://site.com/url" + index, 0, 0, 0, false));
} }
return suggestions; return suggestions;
} }
......
...@@ -25,12 +25,15 @@ bool ContentSuggestion::ID::operator!=(const ID& rhs) const { ...@@ -25,12 +25,15 @@ bool ContentSuggestion::ID::operator!=(const ID& rhs) const {
} }
ContentSuggestion::ContentSuggestion(const ID& id, const GURL& url) ContentSuggestion::ContentSuggestion(const ID& id, const GURL& url)
: id_(id), url_(url), score_(0) {} : id_(id), url_(url), score_(0), is_video_suggestion_(false) {}
ContentSuggestion::ContentSuggestion(Category category, ContentSuggestion::ContentSuggestion(Category category,
const std::string& id_within_category, const std::string& id_within_category,
const GURL& url) const GURL& url)
: id_(category, id_within_category), url_(url), score_(0) {} : id_(category, id_within_category),
url_(url),
score_(0),
is_video_suggestion_(false) {}
ContentSuggestion::ContentSuggestion(ContentSuggestion&&) = default; ContentSuggestion::ContentSuggestion(ContentSuggestion&&) = default;
......
...@@ -141,6 +141,11 @@ class ContentSuggestion { ...@@ -141,6 +141,11 @@ class ContentSuggestion {
publisher_name_ = publisher_name; publisher_name_ = publisher_name;
} }
bool is_video_suggestion() const { return is_video_suggestion_; }
void set_is_video_suggestion(bool is_video_suggestion) {
is_video_suggestion_ = is_video_suggestion;
}
// TODO(pke): Remove the score from the ContentSuggestion class. The UI only // TODO(pke): Remove the score from the ContentSuggestion class. The UI only
// uses it to track user clicks (histogram data). Instead, the providers // uses it to track user clicks (histogram data). Instead, the providers
// should be informed about clicks and do appropriate logging themselves. // should be informed about clicks and do appropriate logging themselves.
...@@ -209,6 +214,8 @@ class ContentSuggestion { ...@@ -209,6 +214,8 @@ class ContentSuggestion {
// RemoteSuggestion. // RemoteSuggestion.
base::Time fetch_date_; base::Time fetch_date_;
bool is_video_suggestion_;
DISALLOW_COPY_AND_ASSIGN(ContentSuggestion); DISALLOW_COPY_AND_ASSIGN(ContentSuggestion);
}; };
......
...@@ -27,6 +27,12 @@ message SnippetProto { ...@@ -27,6 +27,12 @@ message SnippetProto {
optional int32 remote_category_id = 10; optional int32 remote_category_id = 10;
// The time when the snippet was fetched from the server. // The time when the snippet was fetched from the server.
optional int64 fetch_date = 11; optional int64 fetch_date = 11;
enum ContentType {
UNKNOWN = 0;
VIDEO = 1;
}
optional ContentType content_type = 12 [default = UNKNOWN];
} }
message SnippetImageProto { message SnippetImageProto {
......
...@@ -87,7 +87,8 @@ RemoteSuggestion::RemoteSuggestion(const std::vector<std::string>& ids, ...@@ -87,7 +87,8 @@ RemoteSuggestion::RemoteSuggestion(const std::vector<std::string>& ids,
score_(0), score_(0),
is_dismissed_(false), is_dismissed_(false),
remote_category_id_(remote_category_id), remote_category_id_(remote_category_id),
should_notify_(false) {} should_notify_(false),
content_type_(ContentType::UNKNOWN) {}
RemoteSuggestion::~RemoteSuggestion() = default; RemoteSuggestion::~RemoteSuggestion() = default;
...@@ -267,6 +268,21 @@ RemoteSuggestion::CreateFromContentSuggestionsDictionary( ...@@ -267,6 +268,21 @@ RemoteSuggestion::CreateFromContentSuggestionsDictionary(
} }
} }
// In the JSON dictionary contentType is an optional field. The field
// content_type_ of the class |RemoteSuggestion| is by default initialized to
// ContentType::UNKNOWN.
std::string content_type;
if (dict.GetString("contentType", &content_type)) {
if (content_type == "VIDEO") {
snippet->content_type_ = ContentType::VIDEO;
} else {
// The supported values are: VIDEO, UNKNOWN. Therefore if the field is
// present the value has to be "UNKNOWN" here.
DCHECK_EQ(content_type, "UNKNOWN");
snippet->content_type_ = ContentType::UNKNOWN;
}
}
return snippet; return snippet;
} }
...@@ -326,6 +342,10 @@ std::unique_ptr<RemoteSuggestion> RemoteSuggestion::CreateFromProto( ...@@ -326,6 +342,10 @@ std::unique_ptr<RemoteSuggestion> RemoteSuggestion::CreateFromProto(
snippet->fetch_date_ = base::Time::FromInternalValue(proto.fetch_date()); snippet->fetch_date_ = base::Time::FromInternalValue(proto.fetch_date());
} }
if (proto.content_type() == SnippetProto_ContentType_VIDEO) {
snippet->content_type_ = ContentType::VIDEO;
}
return snippet; return snippet;
} }
...@@ -365,6 +385,10 @@ SnippetProto RemoteSuggestion::ToProto() const { ...@@ -365,6 +385,10 @@ SnippetProto RemoteSuggestion::ToProto() const {
if (!fetch_date_.is_null()) { if (!fetch_date_.is_null()) {
result.set_fetch_date(fetch_date_.ToInternalValue()); result.set_fetch_date(fetch_date_.ToInternalValue());
} }
if (content_type_ == ContentType::VIDEO) {
result.set_content_type(SnippetProto_ContentType_VIDEO);
}
return result; return result;
} }
...@@ -393,6 +417,9 @@ ContentSuggestion RemoteSuggestion::ToContentSuggestion( ...@@ -393,6 +417,9 @@ ContentSuggestion RemoteSuggestion::ToContentSuggestion(
base::MakeUnique<NotificationExtra>(extra)); base::MakeUnique<NotificationExtra>(extra));
} }
suggestion.set_fetch_date(fetch_date_); suggestion.set_fetch_date(fetch_date_);
if (content_type_ == ContentType::VIDEO) {
suggestion.set_is_video_suggestion(true);
}
return suggestion; return suggestion;
} }
......
...@@ -31,6 +31,8 @@ class RemoteSuggestion { ...@@ -31,6 +31,8 @@ class RemoteSuggestion {
public: public:
using PtrVector = std::vector<std::unique_ptr<RemoteSuggestion>>; using PtrVector = std::vector<std::unique_ptr<RemoteSuggestion>>;
enum ContentType { UNKNOWN, VIDEO };
~RemoteSuggestion(); ~RemoteSuggestion();
// Creates a RemoteSuggestion from a dictionary, as returned by Chrome Reader. // Creates a RemoteSuggestion from a dictionary, as returned by Chrome Reader.
...@@ -107,6 +109,8 @@ class RemoteSuggestion { ...@@ -107,6 +109,8 @@ class RemoteSuggestion {
bool should_notify() const { return should_notify_; } bool should_notify() const { return should_notify_; }
base::Time notification_deadline() const { return notification_deadline_; } base::Time notification_deadline() const { return notification_deadline_; }
ContentType content_type() const { return content_type_; }
bool is_dismissed() const { return is_dismissed_; } bool is_dismissed() const { return is_dismissed_; }
void set_dismissed(bool dismissed) { is_dismissed_ = dismissed; } void set_dismissed(bool dismissed) { is_dismissed_ = dismissed; }
...@@ -148,6 +152,8 @@ class RemoteSuggestion { ...@@ -148,6 +152,8 @@ class RemoteSuggestion {
bool should_notify_; bool should_notify_;
base::Time notification_deadline_; base::Time notification_deadline_;
ContentType content_type_;
// The time when the remote suggestion was fetched from the server. // The time when the remote suggestion was fetched from the server.
base::Time fetch_date_; base::Time fetch_date_;
......
...@@ -400,6 +400,7 @@ TEST(RemoteSuggestionTest, CreateFromProtoToProtoRoundtrip) { ...@@ -400,6 +400,7 @@ TEST(RemoteSuggestionTest, CreateFromProtoToProtoRoundtrip) {
proto.set_dismissed(false); proto.set_dismissed(false);
proto.set_remote_category_id(1); proto.set_remote_category_id(1);
proto.set_fetch_date(1476364691); proto.set_fetch_date(1476364691);
proto.set_content_type(SnippetProto_ContentType_VIDEO);
auto* source = proto.add_sources(); auto* source = proto.add_sources();
source->set_url("http://cool-suggestions.com/"); source->set_url("http://cool-suggestions.com/");
source->set_publisher_name("Great Suggestions Inc."); source->set_publisher_name("Great Suggestions Inc.");
...@@ -565,5 +566,40 @@ TEST(RemoteSuggestionTest, ToContentSuggestionWithNotificationInfo) { ...@@ -565,5 +566,40 @@ TEST(RemoteSuggestionTest, ToContentSuggestionWithNotificationInfo) {
Eq(1467291697000)); Eq(1467291697000));
} }
TEST(RemoteSuggestionTest, ToContentSuggestionWithContentTypeVideo) {
auto json = ContentSuggestionSnippet();
json->SetString("contentType", "VIDEO");
auto snippet = RemoteSuggestion::CreateFromContentSuggestionsDictionary(
*json, 0, base::Time());
ASSERT_THAT(snippet, NotNull());
ContentSuggestion content_suggestion = snippet->ToContentSuggestion(
Category::FromKnownCategory(KnownCategories::ARTICLES));
EXPECT_THAT(content_suggestion.is_video_suggestion(), Eq(true));
}
TEST(RemoteSuggestionTest, ToContentSuggestionWithContentTypeUnknown) {
auto json = ContentSuggestionSnippet();
json->SetString("contentType", "UNKNOWN");
auto snippet = RemoteSuggestion::CreateFromContentSuggestionsDictionary(
*json, 0, base::Time());
ASSERT_THAT(snippet, NotNull());
ContentSuggestion content_suggestion = snippet->ToContentSuggestion(
Category::FromKnownCategory(KnownCategories::ARTICLES));
EXPECT_THAT(content_suggestion.is_video_suggestion(), Eq(false));
}
TEST(RemoteSuggestionTest, ToContentSuggestionWithMissingContentType) {
auto json = ContentSuggestionSnippet();
auto snippet = RemoteSuggestion::CreateFromContentSuggestionsDictionary(
*json, 0, base::Time());
ASSERT_THAT(snippet, NotNull());
ContentSuggestion content_suggestion = snippet->ToContentSuggestion(
Category::FromKnownCategory(KnownCategories::ARTICLES));
EXPECT_THAT(content_suggestion.is_video_suggestion(), Eq(false));
}
} // namespace } // namespace
} // namespace ntp_snippets } // namespace ntp_snippets
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