[Media Feeds] Rename cache expiry field

Rename cache expiry field to be the last fetch
time that we did not hit the cache.

BUG=1053599

Change-Id: I4568e2494b84369430eef436d77bce8a43b260d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2137507Reviewed-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@{#757219}

chrome/browser/media/feeds/media_feeds_store.mojom

@@ -31,8 +31,8 @@ struct MediaFeed {
   // The number of times in a row the feed has failed to be fetched.
   int64 fetch_failed_count;
 
-  // The time the previously fetched feed should expire in the cache.
-  mojo_base.mojom.Time? cache_expiry_time;
+  // The time we previously fetched the feed that did not hit the cache.
+  mojo_base.mojom.Time? last_fetch_time_not_cache_hit;
 
   // The number of items in the last feed fetch.
   int64 last_fetch_item_count;

chrome/browser/media/history/media_history_feeds_table.cc

@@ -38,29 +38,29 @@ sql::InitStatus MediaHistoryFeedsTable::CreateTableIfNonExistent() {
   if (!CanAccessDatabase())
     return sql::INIT_FAILURE;
 
-  bool success =
-      DB()->Execute(base::StringPrintf("CREATE TABLE IF NOT EXISTS %s("
-                                       "id INTEGER PRIMARY KEY AUTOINCREMENT,"
-                                       "origin_id INTEGER NOT NULL UNIQUE,"
-                                       "url TEXT NOT NULL, "
-                                       "last_discovery_time_s INTEGER, "
-                                       "last_fetch_time_s INTEGER, "
-                                       "user_status INTEGER DEFAULT 0, "
-                                       "last_fetch_result INTEGER DEFAULT 0, "
-                                       "fetch_failed_count INTEGER, "
-                                       "cache_expiry_time_s INTEGER, "
-                                       "last_fetch_item_count INTEGER, "
-                                       "last_fetch_play_next_count INTEGER, "
-                                       "last_fetch_content_types INTEGER, "
-                                       "logo BLOB, "
-                                       "display_name TEXT, "
-                                       "CONSTRAINT fk_origin "
-                                       "FOREIGN KEY (origin_id) "
-                                       "REFERENCES origin(id) "
-                                       "ON DELETE CASCADE"
-                                       ")",
-                                       kTableName)
-                        .c_str());
+  bool success = DB()->Execute(
+      base::StringPrintf("CREATE TABLE IF NOT EXISTS %s("
+                         "id INTEGER PRIMARY KEY AUTOINCREMENT,"
+                         "origin_id INTEGER NOT NULL UNIQUE,"
+                         "url TEXT NOT NULL, "
+                         "last_discovery_time_s INTEGER, "
+                         "last_fetch_time_s INTEGER, "
+                         "user_status INTEGER DEFAULT 0, "
+                         "last_fetch_result INTEGER DEFAULT 0, "
+                         "fetch_failed_count INTEGER, "
+                         "last_fetch_time_not_cache_hit_s INTEGER, "
+                         "last_fetch_item_count INTEGER, "
+                         "last_fetch_play_next_count INTEGER, "
+                         "last_fetch_content_types INTEGER, "
+                         "logo BLOB, "
+                         "display_name TEXT, "
+                         "CONSTRAINT fk_origin "
+                         "FOREIGN KEY (origin_id) "
+                         "REFERENCES origin(id) "
+                         "ON DELETE CASCADE"
+                         ")",
+                         kTableName)
+          .c_str());
 
   if (success) {
     success = DB()->Execute(
@@ -142,7 +142,7 @@ MediaHistoryFeedsTable::GetRows() {
   sql::Statement statement(DB()->GetUniqueStatement(
       "SELECT id, url, last_discovery_time_s, last_fetch_time_s, "
       "user_status, last_fetch_result, fetch_failed_count, "
-      "cache_expiry_time_s, "
+      "last_fetch_time_not_cache_hit_s, "
       "last_fetch_item_count, last_fetch_play_next_count, "
       "last_fetch_content_types, "
       "logo, display_name FROM mediaFeed"));
@@ -195,8 +195,9 @@ MediaHistoryFeedsTable::GetRows() {
     feed->fetch_failed_count = statement.ColumnInt64(6);
 
     if (statement.GetColumnType(7) == sql::ColumnType::kInteger) {
-      feed->cache_expiry_time = base::Time::FromDeltaSinceWindowsEpoch(
-          base::TimeDelta::FromSeconds(statement.ColumnInt64(7)));
+      feed->last_fetch_time_not_cache_hit =
+          base::Time::FromDeltaSinceWindowsEpoch(
+              base::TimeDelta::FromSeconds(statement.ColumnInt64(7)));
     }
 
     feed->last_fetch_item_count = statement.ColumnInt64(8);
@@ -215,7 +216,7 @@ MediaHistoryFeedsTable::GetRows() {
 bool MediaHistoryFeedsTable::UpdateFeedFromFetch(
     const int64_t feed_id,
     const media_feeds::mojom::FetchResult result,
-    const base::Time& expiry_time,
+    const bool was_fetched_from_cache,
     const int item_count,
     const int item_play_next_count,
     const int item_content_types,
@@ -242,32 +243,48 @@ bool MediaHistoryFeedsTable::UpdateFeedFromFetch(
     }
   }
 
-  sql::Statement statement(DB()->GetCachedStatement(
-      SQL_FROM_HERE,
-      "UPDATE mediaFeed SET last_fetch_time_s = ?, last_fetch_result = ?, "
-      "fetch_failed_count = ?, cache_expiry_time_s = ?, last_fetch_item_count "
-      "= ?, "
-      "last_fetch_play_next_count = ?, last_fetch_content_types = ?, "
-      "logo = ?, display_name = ?  WHERE id = ?"));
+  sql::Statement statement;
+  if (was_fetched_from_cache) {
+    statement.Assign(DB()->GetCachedStatement(
+        SQL_FROM_HERE,
+        "UPDATE mediaFeed SET last_fetch_time_s = ?, last_fetch_result = ?, "
+        "fetch_failed_count = ?, last_fetch_item_count = ?, "
+        "last_fetch_play_next_count = ?, last_fetch_content_types = ?, "
+        "logo = ?, display_name = ?  WHERE id = ?"));
+  } else {
+    statement.Assign(DB()->GetCachedStatement(
+        SQL_FROM_HERE,
+        "UPDATE mediaFeed SET last_fetch_time_s = ?, last_fetch_result = ?, "
+        "fetch_failed_count = ?, last_fetch_item_count = ?, "
+        "last_fetch_play_next_count = ?, last_fetch_content_types = ?, "
+        "logo = ?, display_name = ?, last_fetch_time_not_cache_hit_s = ? WHERE "
+        "id = ?"));
+  }
 
   statement.BindInt64(0,
                       base::Time::Now().ToDeltaSinceWindowsEpoch().InSeconds());
   statement.BindInt64(1, static_cast<int>(result));
   statement.BindInt64(2, fetch_failed_count);
-  statement.BindInt64(3, expiry_time.ToDeltaSinceWindowsEpoch().InSeconds());
-  statement.BindInt64(4, item_count);
-  statement.BindInt64(5, item_play_next_count);
-  statement.BindInt64(6, item_content_types);
+  statement.BindInt64(3, item_count);
+  statement.BindInt64(4, item_play_next_count);
+  statement.BindInt64(5, item_content_types);
 
   if (!logos.empty()) {
-    BindProto(statement, 7,
+    BindProto(statement, 6,
               media_feeds::MediaImagesToProto(logos, kMaxLogoCount));
   } else {
-    statement.BindNull(7);
+    statement.BindNull(6);
   }
 
-  statement.BindString(8, display_name);
-  statement.BindInt64(9, feed_id);
+  statement.BindString(7, display_name);
+
+  if (was_fetched_from_cache) {
+    statement.BindInt64(8, feed_id);
+  } else {
+    statement.BindInt64(
+        8, base::Time::Now().ToDeltaSinceWindowsEpoch().InSeconds());
+    statement.BindInt64(9, feed_id);
+  }
 
   return statement.Run() && DB()->GetLastChangeCount() == 1;
 }

chrome/browser/media/history/media_history_feeds_table.h

@@ -54,7 +54,7 @@ class MediaHistoryFeedsTable : public MediaHistoryTableBase {
   // Updates the feed following a fetch.
   bool UpdateFeedFromFetch(const int64_t feed_id,
                            const media_feeds::mojom::FetchResult result,
-                           const base::Time& expiry_time,
+                           const bool was_fetched_from_cache,
                            const int item_count,
                            const int item_play_next_count,
                            const int item_content_types,

chrome/browser/media/history/media_history_keyed_service.cc

@@ -211,12 +211,13 @@ void MediaHistoryKeyedService::StoreMediaFeedFetchResult(
     const int64_t feed_id,
     std::vector<media_feeds::mojom::MediaFeedItemPtr> items,
     const media_feeds::mojom::FetchResult result,
-    const base::Time& expiry_time,
+    const bool was_fetched_from_cache,
     const std::vector<media_session::MediaImage>& logos,
     const std::string& display_name) {
   if (auto* store = store_->GetForWrite()) {
     store->StoreMediaFeedFetchResult(feed_id, std::move(items), result,
-                                     expiry_time, logos, display_name);
+                                     was_fetched_from_cache, logos,
+                                     display_name);
   }
 }
 

chrome/browser/media/history/media_history_keyed_service.h

@@ -94,12 +94,13 @@ class MediaHistoryKeyedService : public KeyedService,
 
   // Replaces the media items in |feed_id|. This will delete any old feed items
   // and store the new ones in |items|. This will also update the |result|,
-  // |expiry_time|, |logos| and |display_name| for the feed.
+  // |logos| and |display_name| for the feed. If the feed was fetched from the
+  // browser cache then |was_fetched_from_cache| should be true.
   void StoreMediaFeedFetchResult(
       const int64_t feed_id,
       std::vector<media_feeds::mojom::MediaFeedItemPtr> items,
       const media_feeds::mojom::FetchResult result,
-      const base::Time& expiry_time,
+      const bool was_fetched_from_cache,
       const std::vector<media_session::MediaImage>& logos,
       const std::string& display_name);
 

chrome/browser/media/history/media_history_keyed_service_unittest.cc

@@ -398,10 +398,12 @@ TEST_P(MediaHistoryKeyedServiceTest, CleanUpDatabaseWhenOriginIsDeleted) {
   // Store the feed data.
   service()->StoreMediaFeedFetchResult(
       1, GetExpectedItems(), media_feeds::mojom::FetchResult::kSuccess,
-      base::Time::Now(), std::vector<media_session::MediaImage>(), "Test");
+      /* was_fetched_from_cache= */ false,
+      std::vector<media_session::MediaImage>(), "Test");
   service()->StoreMediaFeedFetchResult(
       2, GetExpectedItems(), media_feeds::mojom::FetchResult::kSuccess,
-      base::Time::Now(), std::vector<media_session::MediaImage>(), "test");
+      /* was_fetched_from_cache= */ false,
+      std::vector<media_session::MediaImage>(), "test");
 
   // Wait until the feed data has finished saving.
   WaitForDB();
@@ -613,10 +615,12 @@ TEST_P(MediaHistoryKeyedServiceTest, CleanUpDatabaseWhenURLIsDeleted) {
   // Store the feed data.
   service()->StoreMediaFeedFetchResult(
       1, GetExpectedItems(), media_feeds::mojom::FetchResult::kSuccess,
-      base::Time::Now(), std::vector<media_session::MediaImage>(), "Test");
+      /* was_fetched_from_cache= */ false,
+      std::vector<media_session::MediaImage>(), "Test");
   service()->StoreMediaFeedFetchResult(
       2, GetExpectedItems(), media_feeds::mojom::FetchResult::kSuccess,
-      base::Time::Now(), std::vector<media_session::MediaImage>(), "test");
+      /* was_fetched_from_cache= */ false,
+      std::vector<media_session::MediaImage>(), "test");
 
   // Wait until the feed data has finished saving.
   WaitForDB();

chrome/browser/media/history/media_history_store.cc

@@ -100,7 +100,7 @@ class MediaHistoryStoreInternal
       const int64_t feed_id,
       std::vector<media_feeds::mojom::MediaFeedItemPtr> items,
       const media_feeds::mojom::FetchResult result,
-      const base::Time& expiry_time,
+      const bool was_fetched_from_cache,
       const std::vector<media_session::MediaImage>& logos,
       const std::string& display_name);
 
@@ -630,7 +630,7 @@ void MediaHistoryStoreInternal::StoreMediaFeedFetchResult(
     const int64_t feed_id,
     std::vector<media_feeds::mojom::MediaFeedItemPtr> items,
     const media_feeds::mojom::FetchResult result,
-    const base::Time& expiry_time,
+    const bool was_fetched_from_cache,
     const std::vector<media_session::MediaImage>& logos,
     const std::string& display_name) {
   DCHECK(db_task_runner_->RunsTasksInCurrentSequence());
@@ -674,8 +674,8 @@ void MediaHistoryStoreInternal::StoreMediaFeedFetchResult(
 
   // Update the metadata associated with this feed.
   if (!feeds_table_->UpdateFeedFromFetch(
-          feed_id, result, expiry_time, items.size(), item_play_next_count,
-          item_content_types, logos, display_name)) {
+          feed_id, result, was_fetched_from_cache, items.size(),
+          item_play_next_count, item_content_types, logos, display_name)) {
     DB()->RollbackTransaction();
     return;
   }
@@ -915,14 +915,14 @@ void MediaHistoryStore::StoreMediaFeedFetchResult(
     const int64_t feed_id,
     std::vector<media_feeds::mojom::MediaFeedItemPtr> items,
     const media_feeds::mojom::FetchResult result,
-    const base::Time& expiry_time,
+    const bool was_fetched_from_cache,
     const std::vector<media_session::MediaImage>& logos,
     const std::string& display_name) {
   db_->db_task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&MediaHistoryStoreInternal::StoreMediaFeedFetchResult, db_,
-                     feed_id, std::move(items), result, expiry_time, logos,
-                     display_name));
+                     feed_id, std::move(items), result, was_fetched_from_cache,
+                     logos, display_name));
 }
 
 void MediaHistoryStore::GetItemsForMediaFeedForDebug(

chrome/browser/media/history/media_history_store.h

@@ -148,7 +148,7 @@ class MediaHistoryStore {
       const int64_t feed_id,
       std::vector<media_feeds::mojom::MediaFeedItemPtr> items,
       const media_feeds::mojom::FetchResult result,
-      const base::Time& expiry_time,
+      const bool was_fetched_from_cache,
       const std::vector<media_session::MediaImage>& logos,
       const std::string& display_name);
 

chrome/browser/resources/media/media_feeds.html

@@ -110,8 +110,8 @@
         <th sort-key="fetchFailedCount">
           Fetch Failed Count
         </th>
-        <th sort-key="cacheExpiryTime">
-          Cache Expiry Time
+        <th sort-key="lastFetchTimeNotCacheHit">
+          Last Fetch Time (not cache hit)
         </th>
         <th sort-key="lastFetchItemCount">
           Last Fetch Item Count

chrome/browser/resources/media/media_feeds.js

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

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

@@ -111,8 +111,7 @@ MediaFeedsWebUIBrowserTest.prototype = {
     GEN('logos.push_back(logo2);');
     GEN('service->StoreMediaFeedFetchResult(');
     GEN('  1, std::move(items), media_feeds::mojom::FetchResult::kSuccess,');
-    GEN('  base::Time::FromDeltaSinceWindowsEpoch(');
-    GEN('  base::TimeDelta::FromMinutes(40)), logos, "Test Feed");');
+    GEN('  false, logos, "Test Feed");');
     GEN('base::RunLoop run_loop;');
     GEN('service->PostTaskToDBForTest(run_loop.QuitClosure());');
     GEN('run_loop.Run();');
@@ -137,7 +136,7 @@ TEST_F('MediaFeedsWebUIBrowserTest', 'All', function() {
         [
           'ID', 'Url', 'Display Name', 'Last Discovery Time', 'Last Fetch Time',
           'User Status', 'Last Fetch Result', 'Fetch Failed Count',
-          'Cache Expiry Time', 'Last Fetch Item Count',
+          'Last Fetch Time (not cache hit)', 'Last Fetch Item Count',
           'Last Fetch Play Next Count', 'Last Fetch Content Types', 'Logos',
           'Actions'
         ],