[Media Feeds] Store feed item images

Store a maximum of 5 images for a feed item.

BUG=1057765

Change-Id: I49b97831d1d4adff0d5c75acb7f218e183948258
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2115885
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753263}

chrome/browser/BUILD.gn

@@ -690,6 +690,8 @@ jumbo_static_library("browser") {
     "media/feeds/media_feeds_service.h",
     "media/feeds/media_feeds_service_factory.cc",
     "media/feeds/media_feeds_service_factory.h",
+    "media/feeds/media_feeds_utils.cc",
+    "media/feeds/media_feeds_utils.h",
     "media/history/media_history_contents_observer.cc",
     "media/history/media_history_contents_observer.h",
     "media/history/media_history_feed_items_table.cc",

chrome/browser/media/feeds/media_feeds.proto

@@ -118,15 +118,16 @@ message TVEpisode {
 
 // A reference to an image along with the width and height in px. Only stores
 // the URL and not the image data.
-// https://wicg.github.io/media-feeds-api/index.html#media-image
+// https://wicg.github.io/media-feeds/index.html#media-image
 message Image {
   string url = 1;
   int64 width = 2;
   int64 height = 3;
 }
 
-// Stores up to 5 logo images of different sizes.
-// https://wicg.github.io/media-feeds-api/index.html#media-feed-document
-message Logos {
-  repeated Image logo = 1;
+// Stores up to 5 images of different sizes.
+// https://wicg.github.io/media-feeds/index.html#media-feed-document
+// https://wicg.github.io/media-feeds/index.html#media-feed-item
+message ImageSet {
+  repeated Image image = 1;
 }

chrome/browser/media/feeds/media_feeds_store.mojom

@@ -203,7 +203,7 @@ struct LiveDetails {
 
 // The Media Feed Item is an individual item stored in a Media Feed. It
 // represents a single recommendation such as a video or TV show.
-// https://wicg.github.io/media-feeds-api/index.html#media-feed-item
+// https://wicg.github.io/media-feeds/index.html#media-feed-item
 struct MediaFeedItem {
   // The type of this feed item such as a video or TV show.
   MediaFeedItemType type;
@@ -255,6 +255,9 @@ struct MediaFeedItem {
 
   // Whether the user has previously clicked on this feed item.
   bool clicked;
+
+  // The images for the feed item.
+  array<media_session.mojom.MediaImage> images;
 };
 
 // MediaFeedStore allows the Media Feeds WebUI to access data from the Media

chrome/browser/media/feeds/media_feeds_utils.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/media/feeds/media_feeds_utils.h"
+
+#include "chrome/browser/media/feeds/media_feeds.pb.h"
+#include "services/media_session/public/cpp/media_image.h"
+
+namespace media_feeds {
+
+void MediaImageToProto(Image* proto, const media_session::MediaImage& image) {
+  proto->set_url(image.src.spec());
+
+  if (image.sizes.empty())
+    return;
+
+  DCHECK_EQ(1u, image.sizes.size());
+
+  proto->set_width(image.sizes[0].width());
+  proto->set_height(image.sizes[0].height());
+}
+
+ImageSet MediaImagesToProto(
+    const std::vector<media_session::MediaImage>& images,
+    int max_number) {
+  ImageSet image_set;
+
+  for (auto& image : images) {
+    MediaImageToProto(image_set.add_image(), image);
+
+    if (image_set.image().size() >= max_number)
+      break;
+  }
+
+  return image_set;
+}
+
+media_session::MediaImage ProtoToMediaImage(const Image& proto) {
+  media_session::MediaImage image;
+  image.src = GURL(proto.url());
+
+  if (proto.width() > 0 && proto.height() > 0)
+    image.sizes.push_back(gfx::Size(proto.width(), proto.height()));
+
+  return image;
+}
+
+std::vector<media_session::MediaImage> ProtoToMediaImages(
+    const ImageSet& image_set,
+    unsigned max_number) {
+  std::vector<media_session::MediaImage> images;
+
+  for (auto& proto : image_set.image()) {
+    images.push_back(ProtoToMediaImage(proto));
+
+    if (images.size() >= max_number)
+      break;
+  }
+
+  return images;
+}
+
+}  // namespace media_feeds

chrome/browser/media/feeds/media_feeds_utils.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_MEDIA_FEEDS_MEDIA_FEEDS_UTILS_H_
+#define CHROME_BROWSER_MEDIA_FEEDS_MEDIA_FEEDS_UTILS_H_
+
+#include <vector>
+
+namespace media_session {
+struct MediaImage;
+}  // namespace media_session
+
+namespace media_feeds {
+
+class Image;
+class ImageSet;
+
+void MediaImageToProto(Image* proto, const media_session::MediaImage& image);
+
+ImageSet MediaImagesToProto(
+    const std::vector<media_session::MediaImage>& images,
+    int max_number);
+
+media_session::MediaImage ProtoToMediaImage(const Image& proto);
+
+std::vector<media_session::MediaImage> ProtoToMediaImages(
+    const ImageSet& image_set,
+    unsigned max_number);
+
+}  // namespace media_feeds
+
+#endif  // CHROME_BROWSER_MEDIA_FEEDS_MEDIA_FEEDS_UTILS_H_

chrome/browser/media/history/media_history_feed_items_table.cc

@@ -7,6 +7,7 @@
 #include "base/metrics/histogram_functions.h"
 #include "base/updateable_sequenced_task_runner.h"
 #include "chrome/browser/media/feeds/media_feeds.pb.h"
+#include "chrome/browser/media/feeds/media_feeds_utils.h"
 #include "chrome/browser/media/history/media_history_store.h"
 #include "sql/statement.h"
 #include "url/gurl.h"
@@ -15,6 +16,9 @@ namespace media_history {
 
 namespace {
 
+// The maximum number of images to allow.
+const int kMaxImageCount = 5;
+
 media_feeds::InteractionCounter_Type Convert(
     const media_feeds::mojom::InteractionCounterType& type) {
   switch (type) {
@@ -145,6 +149,7 @@ sql::InitStatus MediaHistoryFeedItemsTable::CreateTableIfNonExistent() {
       "identifiers BLOB, "
       "shown_count INTEGER,"
       "clicked INTEGER, "
+      "images BLOB, "
       "CONSTRAINT fk_feed "
       "FOREIGN KEY (feed_id) "
       "REFERENCES mediaFeed(id) "
@@ -180,8 +185,8 @@ bool MediaHistoryFeedItemsTable::SaveItem(
       "action_status, genre, duration_s, is_live, live_start_time_s, "
       "live_end_time_s, shown_count, clicked, author, action, "
       "interaction_counters, content_rating, identifiers, tv_episode, "
-      "play_next_candidate) VALUES "
-      "(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"));
+      "play_next_candidate, images) VALUES "
+      "(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"));
 
   statement.BindInt64(0, feed_id);
   statement.BindInt64(1, static_cast<int>(item->type));
@@ -317,6 +322,13 @@ bool MediaHistoryFeedItemsTable::SaveItem(
     statement.BindNull(19);
   }
 
+  if (!item->images.empty()) {
+    BindProto(statement, 20,
+              media_feeds::MediaImagesToProto(item->images, kMaxImageCount));
+  } else {
+    statement.BindNull(20);
+  }
+
   return statement.Run();
 }
 
@@ -343,7 +355,7 @@ MediaHistoryFeedItemsTable::GetItemsForFeed(const int64_t feed_id) {
       "action_status, genre, duration_s, is_live, live_start_time_s, "
       "live_end_time_s, shown_count, clicked, author, action, "
       "interaction_counters, content_rating, identifiers, tv_episode, "
-      "play_next_candidate FROM "
+      "play_next_candidate, images FROM "
       "mediaFeedItem WHERE feed_id = ?"));
 
   statement.BindInt64(0, feed_id);
@@ -490,6 +502,19 @@ MediaHistoryFeedItemsTable::GetItemsForFeed(const int64_t feed_id) {
         item->play_next_candidate->identifiers.push_back(Convert(identifier));
     }
 
+    if (statement.GetColumnType(19) == sql::ColumnType::kBlob) {
+      media_feeds::ImageSet image_set;
+      if (!GetProto(statement, 19, image_set)) {
+        base::UmaHistogramEnumeration(
+            MediaHistoryFeedItemsTable::kFeedItemReadResultHistogramName,
+            FeedItemReadResult::kBadImages);
+
+        continue;
+      }
+
+      item->images = media_feeds::ProtoToMediaImages(image_set, kMaxImageCount);
+    }
+
     base::UmaHistogramEnumeration(kFeedItemReadResultHistogramName,
                                   FeedItemReadResult::kSuccess);
 

chrome/browser/media/history/media_history_feed_items_table.h

@@ -38,7 +38,8 @@ class MediaHistoryFeedItemsTable : public MediaHistoryTableBase {
     kBadIdentifiers = 7,
     kBadTVEpisode = 8,
     kBadPlayNextCandidate = 9,
-    kMaxValue = kBadPlayNextCandidate,
+    kBadImages = 10,
+    kMaxValue = kBadImages,
   };
 
   MediaHistoryFeedItemsTable(const MediaHistoryFeedItemsTable&) = delete;

chrome/browser/media/history/media_history_feeds_table.cc

@@ -8,6 +8,7 @@
 #include "base/strings/stringprintf.h"
 #include "base/updateable_sequenced_task_runner.h"
 #include "chrome/browser/media/feeds/media_feeds.pb.h"
+#include "chrome/browser/media/feeds/media_feeds_utils.h"
 #include "chrome/browser/media/history/media_history_store.h"
 #include "services/media_session/public/cpp/media_image.h"
 #include "sql/statement.h"
@@ -20,19 +21,6 @@ namespace {
 // The maximum number of logos to allow.
 const int kMaxLogoCount = 5;
 
-void FillImage(media_feeds::Image* proto,
-               const media_session::MediaImage& image) {
-  proto->set_url(image.src.spec());
-
-  if (image.sizes.empty())
-    return;
-
-  DCHECK_EQ(1u, image.sizes.size());
-
-  proto->set_width(image.sizes[0].width());
-  proto->set_height(image.sizes[0].height());
-}
-
 }  // namespace
 
 const char MediaHistoryFeedsTable::kTableName[] = "mediaFeed";
@@ -180,27 +168,15 @@ MediaHistoryFeedsTable::GetRows() {
     }
 
     if (statement.GetColumnType(11) == sql::ColumnType::kBlob) {
-      media_feeds::Logos logos;
-      if (!GetProto(statement, 11, logos)) {
+      media_feeds::ImageSet image_set;
+      if (!GetProto(statement, 11, image_set)) {
         base::UmaHistogramEnumeration(kFeedReadResultHistogramName,
                                       FeedReadResult::kBadLogo);
 
         continue;
       }
 
-      for (auto& logo : logos.logo()) {
-        media_session::MediaImage image;
-        image.src = GURL(logo.url());
-
-        if (logo.width() > 0 && logo.height() > 0)
-          image.sizes.push_back(gfx::Size(logo.width(), logo.height()));
-
-        if (image.src.is_valid())
-          feed->logos.push_back(image);
-
-        if (feed->logos.size() >= kMaxLogoCount)
-          break;
-      }
+      feed->logos = media_feeds::ProtoToMediaImages(image_set, kMaxLogoCount);
     }
 
     base::UmaHistogramEnumeration(kFeedReadResultHistogramName,
@@ -284,22 +260,15 @@ bool MediaHistoryFeedsTable::UpdateFeedFromFetch(
   statement.BindInt64(6, item_content_types);
 
   if (!logos.empty()) {
-    media_feeds::Logos proto_logos;
-
-    for (auto& logo : logos) {
-      FillImage(proto_logos.add_logo(), logo);
-
-      if (proto_logos.logo().size() >= kMaxLogoCount)
-        break;
-    }
-
-    BindProto(statement, 7, proto_logos);
+    BindProto(statement, 7,
+              media_feeds::MediaImagesToProto(logos, kMaxLogoCount));
   } else {
     statement.BindNull(7);
   }
 
   statement.BindString(8, display_name);
   statement.BindInt64(9, feed_id);
+
   return statement.Run() && DB()->GetLastChangeCount() == 1;
 }
 

chrome/browser/media/history/media_history_store_unittest.cc

@@ -654,6 +654,20 @@ class MediaHistoryStoreFeedsTest : public MediaHistoryStoreUnitTest {
       item->play_next_candidate->action->url = GURL("https://www.example.com");
       item->play_next_candidate->identifiers.push_back(CreateIdentifier(
           media_feeds::mojom::Identifier::Type::kTMSId, "TEST4"));
+
+      {
+        media_session::MediaImage image;
+        image.src = GURL("https://www.example.org/image1.png");
+        item->images.push_back(image);
+      }
+
+      {
+        media_session::MediaImage image;
+        image.src = GURL("https://www.example.org/image2.png");
+        image.sizes.push_back(gfx::Size(10, 10));
+        item->images.push_back(image);
+      }
+
       items.push_back(std::move(item));
     }
 
@@ -1284,4 +1298,76 @@ TEST_P(MediaHistoryStoreFeedsTest, StoreMediaFeedFetchResult_CheckLogoMax) {
   }
 }
 
+TEST_P(MediaHistoryStoreFeedsTest, StoreMediaFeedFetchResult_CheckImageMax) {
+  service()->DiscoverMediaFeed(GURL("https://www.google.com/feed"));
+  WaitForDB();
+
+  // If we are read only we should use -1 as a placeholder feed id because the
+  // feed will not have been stored. This is so we can run the rest of the test
+  // to ensure a no-op.
+  const int feed_id = IsReadOnly() ? -1 : GetMediaFeedsSync(service())[0]->id;
+
+  auto item = media_feeds::mojom::MediaFeedItem::New();
+  item->name = base::ASCIIToUTF16("The Movie");
+  item->type = media_feeds::mojom::MediaFeedItemType::kMovie;
+
+  {
+    media_session::MediaImage image;
+    image.src = GURL("https://www.example.org/image1.png");
+    item->images.push_back(image);
+  }
+
+  {
+    media_session::MediaImage image;
+    image.src = GURL("https://www.example.org/image2.png");
+    item->images.push_back(image);
+  }
+
+  {
+    media_session::MediaImage image;
+    image.src = GURL("https://www.example.org/image3.png");
+    item->images.push_back(image);
+  }
+
+  {
+    media_session::MediaImage image;
+    image.src = GURL("https://www.example.org/image4.png");
+    item->images.push_back(image);
+  }
+
+  {
+    media_session::MediaImage image;
+    image.src = GURL("https://www.example.org/image5.png");
+    item->images.push_back(image);
+  }
+
+  {
+    media_session::MediaImage image;
+    image.src = GURL("https://www.example.org/image6.png");
+    item->images.push_back(image);
+  }
+
+  std::vector<media_feeds::mojom::MediaFeedItemPtr> items;
+  items.push_back(std::move(item));
+
+  service()->StoreMediaFeedFetchResult(
+      feed_id, std::move(items), media_feeds::mojom::FetchResult::kSuccess,
+      base::Time::Now(), GetExpectedLogos(), kExpectedDisplayName);
+  WaitForDB();
+
+  {
+    // The item should have at most 5 images.
+    auto items = GetItemsForMediaFeedSync(service(), feed_id);
+
+    if (IsReadOnly()) {
+      EXPECT_TRUE(items.empty());
+    } else {
+      EXPECT_EQ(5u, items[0]->images.size());
+    }
+
+    // The OTR service should have the same data.
+    EXPECT_EQ(items, GetItemsForMediaFeedSync(otr_service(), feed_id));
+  }
+}
+
 }  // namespace media_history

tools/metrics/histograms/enums.xml

@@ -42716,6 +42716,7 @@ Called by update_use_counter_css.py.-->
   <int value="7" label="Bad identifiers"/>
   <int value="8" label="Bad tv episode"/>
   <int value="9" label="Bad play next candidate"/>
+  <int value="10" label="Bad images"/>
 </enum>
 
 <enum name="MediaFeedReadResult">