[Media Feeds] Add last display time field

Add a last display time field that is
used for picking feeds that have not been
shown recently.

BUG=1053599

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

chrome/browser/media/feeds/media_feeds_store.mojom

@@ -53,6 +53,9 @@ struct MediaFeed {
   // the number of other origins with less watchtime divided by the total
   // number of other origins. The value is between 0.0 and 100.0.
   double origin_audio_video_watchtime_percentile;
+
+  // The last time this feed was displayed in the UI.
+  mojo_base.mojom.Time? last_display_time;
 };
 
 // The result of fetching the feed. This enum is committed to storage so do not

chrome/browser/media/history/media_history_feeds_table.cc

@@ -56,6 +56,7 @@ sql::InitStatus MediaHistoryFeedsTable::CreateTableIfNonExistent() {
                          "last_fetch_content_types INTEGER, "
                          "logo BLOB, "
                          "display_name TEXT, "
+                         "last_display_time_s INTEGER, "
                          "CONSTRAINT fk_origin "
                          "FOREIGN KEY (origin_id) "
                          "REFERENCES origin(id) "
@@ -159,7 +160,8 @@ std::vector<media_feeds::mojom::MediaFeedPtr> MediaHistoryFeedsTable::GetRows(
       "mediaFeed.last_fetch_play_next_count, "
       "mediaFeed.last_fetch_content_types, "
       "mediaFeed.logo, "
-      "mediaFeed.display_name ");
+      "mediaFeed.display_name, "
+      "mediaFeed.last_display_time_s");
 
   if (request.include_origin_watchtime_percentile_data) {
     // If we need the percentile data we should select rows from the origin
@@ -278,6 +280,11 @@ std::vector<media_feeds::mojom::MediaFeedPtr> MediaHistoryFeedsTable::GetRows(
 
     feed->display_name = statement.ColumnString(12);
 
+    if (statement.GetColumnType(13) == sql::ColumnType::kInteger) {
+      feed->last_display_time = base::Time::FromDeltaSinceWindowsEpoch(
+          base::TimeDelta::FromSeconds(statement.ColumnInt64(13)));
+    }
+
     if (request.include_origin_watchtime_percentile_data && origin_count > 1) {
       feed->origin_audio_video_watchtime_percentile =
           (rank / (*origin_count - 1)) * 100;
@@ -371,4 +378,20 @@ bool MediaHistoryFeedsTable::UpdateFeedFromFetch(
   return statement.Run() && DB()->GetLastChangeCount() == 1;
 }
 
+bool MediaHistoryFeedsTable::UpdateDisplayTime(const int64_t feed_id) {
+  DCHECK_LT(0, DB()->transaction_nesting());
+  if (!CanAccessDatabase())
+    return false;
+
+  sql::Statement statement(DB()->GetCachedStatement(
+      SQL_FROM_HERE,
+      "UPDATE mediaFeed SET last_display_time_s = ? WHERE id = ?"));
+
+  statement.BindInt64(0,
+                      base::Time::Now().ToDeltaSinceWindowsEpoch().InSeconds());
+  statement.BindInt64(1, feed_id);
+
+  return statement.Run() && DB()->GetLastChangeCount() == 1;
+}
+
 }  // namespace media_history

chrome/browser/media/history/media_history_feeds_table.h

@@ -65,6 +65,9 @@ class MediaHistoryFeedsTable : public MediaHistoryTableBase {
   // Returns the feed rows in the database.
   std::vector<media_feeds::mojom::MediaFeedPtr> GetRows(
       const MediaHistoryKeyedService::GetMediaFeedsRequest& request);
+
+  // Updates the display time to now and returns a boolean if it was saved.
+  bool UpdateDisplayTime(const int64_t feed_id);
 };
 
 }  // namespace media_history

chrome/browser/media/history/media_history_keyed_service.cc

@@ -346,4 +346,14 @@ void MediaHistoryKeyedService::GetMediaFeeds(
       std::move(callback));
 }
 
+void MediaHistoryKeyedService::UpdateMediaFeedDisplayTime(
+    const int64_t feed_id) {
+  if (auto* store = store_->GetForWrite()) {
+    store->db_task_runner_->PostTask(
+        FROM_HERE,
+        base::BindOnce(&MediaHistoryStore::UpdateMediaFeedDisplayTime, store,
+                       feed_id));
+  }
+}
+
 }  // namespace media_history

chrome/browser/media/history/media_history_keyed_service.h

@@ -157,6 +157,9 @@ class MediaHistoryKeyedService : public KeyedService,
       base::OnceCallback<void(std::vector<media_feeds::mojom::MediaFeedPtr>)>
           callback);
 
+  // Updates the display time for the Media Feed with |feed_id| to now.
+  void UpdateMediaFeedDisplayTime(const int64_t feed_id);
+
  private:
   class StoreHolder;
 

chrome/browser/media/history/media_history_store.cc

@@ -733,6 +733,27 @@ bool MediaHistoryStore::CanAccessDatabase() const {
   return initialization_successful_ && db_ && db_->is_open();
 }
 
+void MediaHistoryStore::UpdateMediaFeedDisplayTime(const int64_t feed_id) {
+  DCHECK(db_task_runner_->RunsTasksInCurrentSequence());
+  if (!initialization_successful_)
+    return;
+
+  if (!feeds_table_)
+    return;
+
+  if (!DB()->BeginTransaction()) {
+    LOG(ERROR) << "Failed to begin the transaction.";
+    return;
+  }
+
+  if (!feeds_table_->UpdateDisplayTime(feed_id)) {
+    DB()->RollbackTransaction();
+    return;
+  }
+
+  DB()->CommitTransaction();
+}
+
 void MediaHistoryStore::Close() {
   DCHECK(db_task_runner_->RunsTasksInCurrentSequence());
 

chrome/browser/media/history/media_history_store.h

@@ -163,6 +163,8 @@ class MediaHistoryStore : public base::RefCountedThreadSafe<MediaHistoryStore> {
   void StoreMediaFeedItemSafeSearchResults(
       std::map<int64_t, media_feeds::mojom::SafeSearchResult> results);
 
+  void UpdateMediaFeedDisplayTime(const int64_t feed_id);
+
   void Close();
 
  private:

chrome/browser/media/history/media_history_store_unittest.cc

@@ -904,6 +904,7 @@ TEST_P(MediaHistoryStoreFeedsTest, StoreMediaFeedFetchResult) {
       EXPECT_EQ(kExpectedFetchContentTypes, feeds[0]->last_fetch_content_types);
       EXPECT_EQ(GetExpectedLogos(), feeds[0]->logos);
       EXPECT_EQ(kExpectedDisplayName, feeds[0]->display_name);
+      EXPECT_FALSE(feeds[0]->last_display_time.has_value());
 
       EXPECT_EQ(GetExpectedItems(), items);
     }
@@ -944,6 +945,7 @@ TEST_P(MediaHistoryStoreFeedsTest, StoreMediaFeedFetchResult) {
                 feeds[0]->last_fetch_content_types);
       EXPECT_TRUE(feeds[0]->logos.empty());
       EXPECT_EQ(kExpectedDisplayName, feeds[0]->display_name);
+      EXPECT_FALSE(feeds[0]->last_display_time.has_value());
 
       EXPECT_EQ(GetAltExpectedItems(), items);
 
@@ -984,6 +986,7 @@ TEST_P(MediaHistoryStoreFeedsTest, StoreMediaFeedFetchResult) {
                 feeds[0]->last_fetch_content_types);
       EXPECT_TRUE(feeds[0]->logos.empty());
       EXPECT_EQ(kExpectedDisplayName, feeds[0]->display_name);
+      EXPECT_FALSE(feeds[0]->last_display_time.has_value());
 
       EXPECT_EQ(GetAltExpectedItems(), items);
 
@@ -995,6 +998,24 @@ TEST_P(MediaHistoryStoreFeedsTest, StoreMediaFeedFetchResult) {
     EXPECT_EQ(feeds, GetMediaFeedsSync(otr_service()));
     EXPECT_EQ(items, GetItemsForMediaFeedSync(otr_service(), feed_id));
   }
+
+  service()->UpdateMediaFeedDisplayTime(feed_id);
+  WaitForDB();
+
+  {
+    // The media feed should have a display time.
+    auto feeds = GetMediaFeedsSync(service());
+
+    if (IsReadOnly()) {
+      EXPECT_TRUE(feeds.empty());
+    } else {
+      EXPECT_EQ(feed_id, feeds[0]->id);
+      EXPECT_TRUE(feeds[0]->last_display_time.has_value());
+    }
+
+    // The OTR service should have the same data.
+    EXPECT_EQ(feeds, GetMediaFeedsSync(otr_service()));
+  }
 }
 
 TEST_P(MediaHistoryStoreFeedsTest, StoreMediaFeedFetchResult_WithEmpty) {

chrome/browser/resources/media/media_feeds.html

@@ -122,6 +122,9 @@
         <th sort-key="lastFetchContentTypes">
           Last Fetch Content Types
         </th>
+        <th sort-key="lastDisplayTime">
+          Last Display Time
+        </th>
         <th data-key="logos">
           Logos
         <th data-key="actions">

chrome/browser/resources/media/media_feeds.js

@@ -68,7 +68,8 @@ class MediaFeedsTableDelegate {
       td.textContent = data.url;
     } else if (
         key === 'lastDiscoveryTime' || key === 'lastFetchTime' ||
-        key === 'lastFetchTimeNotCacheHit' || key === 'datePublished') {
+        key === 'lastFetchTimeNotCacheHit' || key === 'datePublished' ||
+        key === 'lastDisplayTime') {
       // Format a mojo time.
       td.textContent =
           convertMojoTimeToJS(/** @type {mojoBase.mojom.Time} */ (data))
@@ -284,7 +285,8 @@ class MediaFeedsTableDelegate {
       return val1 > val2 ? 1 : -1;
     } else if (
         sortKey === 'lastDiscoveryTime' || sortKey === 'lastFetchTime' ||
-        sortKey === 'lastFetchTimeNotCacheHit') {
+        sortKey === 'lastFetchTimeNotCacheHit' ||
+        sortKey === 'lastDisplayTime') {
       return val1.internalValue > val2.internalValue ? 1 : -1;
     }
 

chrome/test/data/webui/media/media_feeds_webui_browsertest.js

@@ -112,6 +112,7 @@ MediaFeedsWebUIBrowserTest.prototype = {
     GEN('service->StoreMediaFeedFetchResult(');
     GEN('  1, std::move(items), media_feeds::mojom::FetchResult::kSuccess,');
     GEN('  false, logos, "Test Feed", base::DoNothing());');
+    GEN('service->UpdateMediaFeedDisplayTime(1);');
     GEN('base::RunLoop run_loop;');
     GEN('service->PostTaskToDBForTest(run_loop.QuitClosure());');
     GEN('run_loop.Run();');
@@ -137,8 +138,8 @@ TEST_F('MediaFeedsWebUIBrowserTest', 'All', function() {
           'ID', 'Url', 'Display Name', 'Last Discovery Time', 'Last Fetch Time',
           'User Status', 'Last Fetch Result', 'Fetch Failed Count',
           'Last Fetch Time (not cache hit)', 'Last Fetch Item Count',
-          'Last Fetch Play Next Count', 'Last Fetch Content Types', 'Logos',
-          'Actions'
+          'Last Fetch Play Next Count', 'Last Fetch Content Types',
+          'Last Display Time', 'Logos', 'Actions'
         ],
         feedsHeaders.map(x => x.textContent.trim()));
 
@@ -155,15 +156,16 @@ TEST_F('MediaFeedsWebUIBrowserTest', 'All', function() {
     assertEquals('1', feedsContents.childNodes[9].textContent.trim());
     assertEquals('1', feedsContents.childNodes[10].textContent.trim());
     assertEquals('Movie', feedsContents.childNodes[11].textContent.trim());
+    assertNotEquals('', feedsContents.childNodes[12].textContent.trim());
     assertEquals(
         'https://www.example.org/logo1.pnghttps://www.example.org/logo2.png',
-        feedsContents.childNodes[12].textContent.trim());
+        feedsContents.childNodes[13].textContent.trim());
     assertEquals(
         'Show ContentsFetch Feed',
-        feedsContents.childNodes[13].textContent.trim());
+        feedsContents.childNodes[14].textContent.trim());
 
     // Click on the show contents button.
-    feedsContents.childNodes[13].firstChild.click();
+    feedsContents.childNodes[14].firstChild.click();
 
     return whenFeedTableIsPopulatedForTest().then(() => {
       assertEquals(