[Media Feeds] Enhance GetMediaFeeds API

Enhance the GetMediaFeeds API to sort results
by audio+video watchtime percentile as well as
limiting the number of results and having a
minimum watchtime.

This will be used for getting feeds to fetch
and for display.

BUG=1053599

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

chrome/browser/media/feeds/media_feeds_store.mojom

@@ -48,6 +48,11 @@ struct MediaFeed {
 
   // A display name for the feed.
   string display_name;
+
+  // The audio+video watchtime percentile for this feed's origin. Calculated as
+  // 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 result of fetching the feed. This enum is committed to storage so do not

chrome/browser/media/history/media_history_feeds_table.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/media/history/media_history_feeds_table.h"
 
 #include "base/metrics/histogram_functions.h"
+#include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/updateable_sequenced_task_runner.h"
 #include "chrome/browser/media/feeds/media_feeds.pb.h"
@@ -134,23 +135,93 @@ bool MediaHistoryFeedsTable::DiscoverFeed(const GURL& url) {
   }
 }
 
-std::vector<media_feeds::mojom::MediaFeedPtr>
-MediaHistoryFeedsTable::GetRows() {
+std::vector<media_feeds::mojom::MediaFeedPtr> MediaHistoryFeedsTable::GetRows(
+    const MediaHistoryKeyedService::GetMediaFeedsRequest& request) {
   std::vector<media_feeds::mojom::MediaFeedPtr> feeds;
   if (!CanAccessDatabase())
     return feeds;
 
-  sql::Statement statement(DB()->GetUniqueStatement(
-      "SELECT id, url, last_discovery_time_s, last_fetch_time_s, "
-      "user_status, last_fetch_result, fetch_failed_count, "
-      "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"));
+  base::Optional<double> origin_count;
+  double rank = 0;
+
+  std::vector<std::string> sql;
+  sql.push_back(
+      "SELECT "
+      "mediaFeed.id, "
+      "mediaFeed.url, "
+      "mediaFeed.last_discovery_time_s, "
+      "mediaFeed.last_fetch_time_s, "
+      "mediaFeed.user_status, "
+      "mediaFeed.last_fetch_result, "
+      "mediaFeed.fetch_failed_count, "
+      "mediaFeed.last_fetch_time_not_cache_hit_s, "
+      "mediaFeed.last_fetch_item_count, "
+      "mediaFeed.last_fetch_play_next_count, "
+      "mediaFeed.last_fetch_content_types, "
+      "mediaFeed.logo, "
+      "mediaFeed.display_name ");
+
+  if (request.include_origin_watchtime_percentile_data) {
+    // If we need the percentile data we should select rows from the origin
+    // table and LEFT JOIN mediaFeed. This means there should be a row for each
+    // origin and if there is a media feed that will be included.
+    sql.push_back(
+        "FROM origin "
+        "LEFT JOIN mediaFeed "
+        "ON origin.id = mediaFeed.origin_id");
+  } else if (request.audio_video_watchtime_min) {
+    // If we need to filter by |audio_video_watchtime_min| then we should join
+    // the origin table to get the watchtime data.
+    sql.push_back(
+        "FROM mediaFeed "
+        "LEFT JOIN origin "
+        "ON origin.id = mediaFeed.origin_id");
+  } else {
+    sql.push_back("FROM mediaFeed");
+  }
+
+  // If we need a minimum watchtime then we should add it.
+  if (request.audio_video_watchtime_min) {
+    sql.push_back(base::StringPrintf(
+        "WHERE origin.aggregate_watchtime_audio_video_s > %f",
+        request.audio_video_watchtime_min->InSecondsF()));
+  }
+
+  // Sorts the feeds in descending watchtime order. We also need the total count
+  // of the origins so we can calculate a percentile.
+  if (request.include_origin_watchtime_percentile_data) {
+    sql::Statement statement(DB()->GetCachedStatement(
+        SQL_FROM_HERE, "SELECT COUNT(id) FROM origin"));
+
+    while (statement.Step()) {
+      origin_count = statement.ColumnDouble(0);
+      rank = *origin_count;
+    }
+
+    DCHECK(origin_count.has_value());
+
+    sql.push_back("ORDER BY origin.aggregate_watchtime_audio_video_s DESC");
+  }
+
+  // Add a SQL limit if we need one. If we are calculating the percentile then
+  // we should use a C++ limit since we might have rows that do not have an
+  // associated media feed.
+  if (request.limit.has_value() &&
+      !request.include_origin_watchtime_percentile_data) {
+    sql.push_back(base::StringPrintf("LIMIT %i", *request.limit));
+  }
+
+  sql::Statement statement(
+      DB()->GetUniqueStatement(base::JoinString(sql, " ").c_str()));
 
   while (statement.Step()) {
-    media_feeds::mojom::MediaFeedPtr feed(media_feeds::mojom::MediaFeed::New());
+    rank--;
+
+    // If there is no mediaFeed data then skip this.
+    if (statement.GetColumnType(0) == sql::ColumnType::kNull)
+      continue;
 
+    auto feed = media_feeds::mojom::MediaFeed::New();
     feed->user_status = static_cast<media_feeds::mojom::FeedUserStatus>(
         statement.ColumnInt64(4));
     feed->last_fetch_result =
@@ -207,7 +278,17 @@ MediaHistoryFeedsTable::GetRows() {
 
     feed->display_name = statement.ColumnString(12);
 
+    if (request.include_origin_watchtime_percentile_data && origin_count > 1) {
+      feed->origin_audio_video_watchtime_percentile =
+          (rank / (*origin_count - 1)) * 100;
+    }
+
     feeds.push_back(std::move(feed));
+
+    if (request.include_origin_watchtime_percentile_data &&
+        request.limit.has_value() && feeds.size() >= *request.limit) {
+      break;
+    }
   }
 
   DCHECK(statement.Succeeded());

chrome/browser/media/history/media_history_feeds_table.h

@@ -8,6 +8,7 @@
 #include <vector>
 
 #include "chrome/browser/media/feeds/media_feeds_store.mojom.h"
+#include "chrome/browser/media/history/media_history_keyed_service.h"
 #include "chrome/browser/media/history/media_history_table_base.h"
 #include "sql/init_status.h"
 #include "url/gurl.h"
@@ -62,7 +63,8 @@ class MediaHistoryFeedsTable : public MediaHistoryTableBase {
                            const std::string& display_name);
 
   // Returns the feed rows in the database.
-  std::vector<media_feeds::mojom::MediaFeedPtr> GetRows();
+  std::vector<media_feeds::mojom::MediaFeedPtr> GetRows(
+      const MediaHistoryKeyedService::GetMediaFeedsRequest& request);
 };
 
 }  // namespace media_history

chrome/browser/media/history/media_history_keyed_service.cc

@@ -317,13 +317,29 @@ void MediaHistoryKeyedService::PostTaskToDBForTest(base::OnceClosure callback) {
       FROM_HERE, base::DoNothing(), std::move(callback));
 }
 
-void MediaHistoryKeyedService::GetMediaFeedsForDebug(
+MediaHistoryKeyedService::GetMediaFeedsRequest::GetMediaFeedsRequest(
+    bool include_origin_watchtime_percentile_data,
+    base::Optional<unsigned> limit,
+    base::Optional<base::TimeDelta> audio_video_watchtime_min)
+    : include_origin_watchtime_percentile_data(
+          include_origin_watchtime_percentile_data),
+      limit(limit),
+      audio_video_watchtime_min(audio_video_watchtime_min) {}
+
+MediaHistoryKeyedService::GetMediaFeedsRequest::GetMediaFeedsRequest() =
+    default;
+
+MediaHistoryKeyedService::GetMediaFeedsRequest::GetMediaFeedsRequest(
+    const GetMediaFeedsRequest& t) = default;
+
+void MediaHistoryKeyedService::GetMediaFeeds(
+    const GetMediaFeedsRequest& request,
     base::OnceCallback<void(std::vector<media_feeds::mojom::MediaFeedPtr>)>
         callback) {
   base::PostTaskAndReplyWithResult(
       store_->GetForRead()->db_task_runner_.get(), FROM_HERE,
-      base::BindOnce(&MediaHistoryStore::GetMediaFeedsForDebug,
-                     store_->GetForRead()),
+      base::BindOnce(&MediaHistoryStore::GetMediaFeeds, store_->GetForRead(),
+                     request),
       std::move(callback));
 }
 

chrome/browser/media/history/media_history_keyed_service.h

@@ -134,9 +134,26 @@ class MediaHistoryKeyedService : public KeyedService,
   // for waiting for database operations in tests.
   void PostTaskToDBForTest(base::OnceClosure callback);
 
-  // Returns all the rows in the media feeds table.  This is only used for
-  // debugging because it loads all rows in the table.
-  void GetMediaFeedsForDebug(
+  // Returns Media Feeds. If |include_origin_watchtime_percentile_data| is true
+  // then we will return the feeds sorted by audio+video watchtime descending
+  // and we will also populate the |origin_audio_video_watchtime_percentile|
+  // field in |MediaFeedPtr|. If |limit| is specified then we will limit the
+  // number of results to this. If |audio_video_watchtime_min| is specified then
+  // this will require a minimum watchtime for feeds to be returned.
+  struct GetMediaFeedsRequest {
+    GetMediaFeedsRequest(
+        bool include_origin_watchtime_percentile_data,
+        base::Optional<unsigned> limit,
+        base::Optional<base::TimeDelta> audio_video_watchtime_min);
+    GetMediaFeedsRequest();
+    GetMediaFeedsRequest(const GetMediaFeedsRequest& t);
+
+    bool include_origin_watchtime_percentile_data = false;
+    base::Optional<unsigned> limit;
+    base::Optional<base::TimeDelta> audio_video_watchtime_min;
+  };
+  void GetMediaFeeds(
+      const GetMediaFeedsRequest& request,
       base::OnceCallback<void(std::vector<media_feeds::mojom::MediaFeedPtr>)>
           callback);
 

chrome/browser/media/history/media_history_origin_table.cc

@@ -45,6 +45,13 @@ sql::InitStatus MediaHistoryOriginTable::CreateTableIfNonExistent() {
                          kTableName)
           .c_str());
 
+  if (success) {
+    success = DB()->Execute(
+        "CREATE INDEX IF NOT EXISTS "
+        "origin_aggregate_watchtime_audio_video_s_index ON "
+        "origin (aggregate_watchtime_audio_video_s)");
+  }
+
   if (!success) {
     ResetDB();
     LOG(ERROR) << "Failed to create media history origin table.";

chrome/browser/media/history/media_history_store.cc

@@ -399,13 +399,13 @@ MediaHistoryStore::GetMediaHistoryPlaybackRowsForDebug() {
   return playback_table_->GetPlaybackRows();
 }
 
-std::vector<media_feeds::mojom::MediaFeedPtr>
-MediaHistoryStore::GetMediaFeedsForDebug() {
+std::vector<media_feeds::mojom::MediaFeedPtr> MediaHistoryStore::GetMediaFeeds(
+    const MediaHistoryKeyedService::GetMediaFeedsRequest& request) {
   DCHECK(db_task_runner_->RunsTasksInCurrentSequence());
   if (!CanAccessDatabase() || !feeds_table_)
     return std::vector<media_feeds::mojom::MediaFeedPtr>();
 
-  return feeds_table_->GetRows();
+  return feeds_table_->GetRows(request);
 }
 
 int MediaHistoryStore::GetTableRowCount(const std::string& table_name) {

chrome/browser/media/history/media_history_store.h

@@ -125,7 +125,8 @@ class MediaHistoryStore : public base::RefCountedThreadSafe<MediaHistoryStore> {
   std::vector<mojom::MediaHistoryPlaybackRowPtr>
   GetMediaHistoryPlaybackRowsForDebug();
 
-  std::vector<media_feeds::mojom::MediaFeedPtr> GetMediaFeedsForDebug();
+  std::vector<media_feeds::mojom::MediaFeedPtr> GetMediaFeeds(
+      const MediaHistoryKeyedService::GetMediaFeedsRequest& request);
 
   void SavePlaybackSession(
       const GURL& url,

chrome/browser/media/history/media_history_store_unittest.cc

@@ -196,15 +196,18 @@ class MediaHistoryStoreUnitTest
   }
 
   std::vector<media_feeds::mojom::MediaFeedPtr> GetMediaFeedsSync(
-      MediaHistoryKeyedService* service) {
+      MediaHistoryKeyedService* service,
+      const MediaHistoryKeyedService::GetMediaFeedsRequest& request =
+          MediaHistoryKeyedService::GetMediaFeedsRequest()) {
     base::RunLoop run_loop;
     std::vector<media_feeds::mojom::MediaFeedPtr> out;
 
-    service->GetMediaFeedsForDebug(base::BindLambdaForTesting(
-        [&](std::vector<media_feeds::mojom::MediaFeedPtr> rows) {
-          out = std::move(rows);
-          run_loop.Quit();
-        }));
+    service->GetMediaFeeds(
+        request, base::BindLambdaForTesting(
+                     [&](std::vector<media_feeds::mojom::MediaFeedPtr> rows) {
+                       out = std::move(rows);
+                       run_loop.Quit();
+                     }));
 
     run_loop.Run();
     return out;
@@ -1119,6 +1122,20 @@ TEST_P(MediaHistoryStoreFeedsTest, StoreMediaFeedFetchResult_MultipleFeeds) {
     // The OTR service should have the same data.
     EXPECT_EQ(items, GetItemsForMediaFeedSync(otr_service(), feed_id_b));
   }
+
+  {
+    // Check the feeds limit function works.
+    auto feeds = GetMediaFeedsSync(
+        service(), MediaHistoryKeyedService::GetMediaFeedsRequest(
+                       false, 1, base::nullopt));
+
+    if (IsReadOnly()) {
+      EXPECT_TRUE(feeds.empty());
+    } else {
+      ASSERT_EQ(1u, feeds.size());
+      EXPECT_EQ(feed_id_a, feeds[0]->id);
+    }
+  }
 }
 
 TEST_P(MediaHistoryStoreFeedsTest, StoreMediaFeedFetchResult_BadType) {
@@ -1589,4 +1606,230 @@ TEST_P(MediaHistoryStoreFeedsTest, SafeSearchCheck) {
   }
 }
 
+TEST_P(MediaHistoryStoreFeedsTest, GetMediaFeedsSortByWatchtimePercentile) {
+  // We add 111 origins with watchtime and feeds for all but one of these.
+  const unsigned kNumberOfOrigins = 111;
+  const unsigned kNumberOfFeeds = 110;
+
+  // The starting percentile always has one percentage value taken off. This
+  // is because we have one extra origin with the highest watchtime that does
+  // not have a feed.
+  const double kPercentageValue = 100.0 / kNumberOfOrigins;
+  const double kStartingPercentile = 100 - kPercentageValue;
+
+  // Generate a bunch of media feeds.
+  std::set<GURL> feeds;
+  for (unsigned i = 0; i < kNumberOfOrigins; i++) {
+    GURL url(base::StringPrintf("https://www.google%i.com/feed", i));
+    feeds.insert(url);
+
+    // Each origin will have a ascending amount of watchtime from 0 to
+    // |kNumberOfOrigins|.
+    auto watchtime = base::TimeDelta::FromMinutes(i);
+    content::MediaPlayerWatchTime watch_time(url, url.GetOrigin(), watchtime,
+                                             base::TimeDelta(), true, true);
+
+    service()->SavePlayback(watch_time);
+
+    if (i < kNumberOfFeeds)
+      service()->DiscoverMediaFeed(url);
+  }
+
+  WaitForDB();
+
+  {
+    // Check the feeds and origins were stored.
+    auto feeds = GetMediaFeedsSync(service());
+    auto origins = GetOriginRowsSync(service());
+
+    if (IsReadOnly()) {
+      EXPECT_TRUE(feeds.empty());
+      EXPECT_TRUE(origins.empty());
+    } else {
+      EXPECT_EQ(kNumberOfFeeds, feeds.size());
+      EXPECT_EQ(kNumberOfOrigins, origins.size());
+
+      int i = 0;
+      for (auto& origin : origins) {
+        auto watchtime = base::TimeDelta::FromMinutes(i);
+
+        EXPECT_EQ(watchtime, origin->cached_audio_video_watchtime);
+        EXPECT_EQ(watchtime, origin->actual_audio_video_watchtime);
+
+        i++;
+      }
+    }
+
+    // The OTR service should have the same data.
+    EXPECT_EQ(feeds, GetMediaFeedsSync(otr_service()));
+    EXPECT_EQ(origins, GetOriginRowsSync(otr_service()));
+  }
+
+  {
+    // Check the media feed sorting by sort_by_audio_video_watchtime_desc works.
+    auto feeds = GetMediaFeedsSync(
+        service(), MediaHistoryKeyedService::GetMediaFeedsRequest(
+                       true, base::nullopt, base::nullopt));
+
+    if (IsReadOnly()) {
+      EXPECT_TRUE(feeds.empty());
+    } else {
+      EXPECT_EQ(kNumberOfFeeds, feeds.size());
+
+      unsigned count = kNumberOfFeeds;
+      double percentile = kStartingPercentile;
+      double last_percentile = 101.0;
+      for (auto& feed : feeds) {
+        GURL url(
+            base::StringPrintf("https://www.google%i.com/feed", count - 1));
+
+        EXPECT_EQ(count, feed->id);
+        EXPECT_EQ(url, feed->url);
+        EXPECT_FALSE(feed->last_fetch_time.has_value());
+        EXPECT_EQ(media_feeds::mojom::FetchResult::kNone,
+                  feed->last_fetch_result);
+        EXPECT_EQ(0, feed->fetch_failed_count);
+        EXPECT_FALSE(feed->last_fetch_time_not_cache_hit.has_value());
+        EXPECT_EQ(0, feed->last_fetch_item_count);
+        EXPECT_EQ(0, feed->last_fetch_play_next_count);
+        EXPECT_EQ(0, feed->last_fetch_content_types);
+        EXPECT_TRUE(feed->logos.empty());
+        EXPECT_TRUE(feed->display_name.empty());
+        EXPECT_NEAR(percentile, feed->origin_audio_video_watchtime_percentile,
+                    1);
+        EXPECT_GT(last_percentile,
+                  feed->origin_audio_video_watchtime_percentile);
+
+        last_percentile = feed->origin_audio_video_watchtime_percentile;
+        percentile = percentile - kPercentageValue;
+        count--;
+      }
+    }
+  }
+
+  {
+    // Check the media feed sorting by sort_by_audio_video_watchtime_desc works
+    // with a limit applied.
+    auto feeds = GetMediaFeedsSync(
+        service(), MediaHistoryKeyedService::GetMediaFeedsRequest(
+                       true, 10, base::nullopt));
+
+    if (IsReadOnly()) {
+      EXPECT_TRUE(feeds.empty());
+    } else {
+      EXPECT_EQ(10u, feeds.size());
+
+      unsigned count = kNumberOfFeeds;
+      double percentile = kStartingPercentile;
+      double last_percentile = 101.0;
+      for (auto& feed : feeds) {
+        GURL url(
+            base::StringPrintf("https://www.google%i.com/feed", count - 1));
+
+        EXPECT_EQ(count, feed->id);
+        EXPECT_EQ(url, feed->url);
+        EXPECT_NEAR(percentile, feed->origin_audio_video_watchtime_percentile,
+                    1);
+        EXPECT_GT(last_percentile,
+                  feed->origin_audio_video_watchtime_percentile);
+
+        last_percentile = feed->origin_audio_video_watchtime_percentile;
+        percentile = percentile - kPercentageValue;
+        count--;
+      }
+    }
+  }
+
+  {
+    // Check the media feed sorting by sort_by_audio_video_watchtime_desc works
+    // with a minimum watchtime requirement and ranking.
+    auto feeds = GetMediaFeedsSync(
+        service(), MediaHistoryKeyedService::GetMediaFeedsRequest(
+                       true, base::nullopt, base::TimeDelta::FromMinutes(30)));
+
+    if (IsReadOnly()) {
+      EXPECT_TRUE(feeds.empty());
+    } else {
+      EXPECT_EQ(79u, feeds.size());
+
+      unsigned count = kNumberOfFeeds;
+      double percentile = kStartingPercentile;
+      double last_percentile = 101.0;
+      for (auto& feed : feeds) {
+        GURL url(
+            base::StringPrintf("https://www.google%i.com/feed", count - 1));
+
+        EXPECT_EQ(count, feed->id);
+        EXPECT_EQ(url, feed->url);
+        EXPECT_NEAR(percentile, feed->origin_audio_video_watchtime_percentile,
+                    1);
+        EXPECT_GT(last_percentile,
+                  feed->origin_audio_video_watchtime_percentile);
+
+        last_percentile = feed->origin_audio_video_watchtime_percentile;
+        percentile = percentile - kPercentageValue;
+        count--;
+      }
+    }
+  }
+
+  {
+    // Check the media feed sorting by sort_by_audio_video_watchtime_desc works
+    // with a minimum watchtime requirement, ranking and limit.
+    auto feeds = GetMediaFeedsSync(
+        service(), MediaHistoryKeyedService::GetMediaFeedsRequest(
+                       true, 10, base::TimeDelta::FromMinutes(30)));
+
+    if (IsReadOnly()) {
+      EXPECT_TRUE(feeds.empty());
+    } else {
+      EXPECT_EQ(10u, feeds.size());
+
+      unsigned count = kNumberOfFeeds;
+      double percentile = kStartingPercentile;
+      double last_percentile = 101.0;
+      for (auto& feed : feeds) {
+        GURL url(
+            base::StringPrintf("https://www.google%i.com/feed", count - 1));
+
+        EXPECT_EQ(count, feed->id);
+        EXPECT_EQ(url, feed->url);
+        EXPECT_NEAR(percentile, feed->origin_audio_video_watchtime_percentile,
+                    1);
+        EXPECT_GT(last_percentile,
+                  feed->origin_audio_video_watchtime_percentile);
+
+        last_percentile = feed->origin_audio_video_watchtime_percentile;
+        percentile = percentile - kPercentageValue;
+        count--;
+      }
+    }
+  }
+
+  {
+    // Check the media feed minimum watchtime ranking works without the
+    // percentile data.
+    auto feeds = GetMediaFeedsSync(
+        service(), MediaHistoryKeyedService::GetMediaFeedsRequest(
+                       false, base::nullopt, base::TimeDelta::FromMinutes(30)));
+
+    if (IsReadOnly()) {
+      EXPECT_TRUE(feeds.empty());
+    } else {
+      EXPECT_EQ(79u, feeds.size());
+
+      unsigned count = 32;
+      for (auto& feed : feeds) {
+        GURL url(
+            base::StringPrintf("https://www.google%i.com/feed", count - 1));
+
+        EXPECT_EQ(count, feed->id);
+        EXPECT_EQ(url, feed->url);
+
+        count++;
+      }
+    }
+  }
+}
+
 }  // namespace media_history

chrome/browser/ui/webui/media/media_feeds_ui.cc

@@ -53,7 +53,9 @@ void MediaFeedsUI::BindInterface(
 }
 
 void MediaFeedsUI::GetMediaFeeds(GetMediaFeedsCallback callback) {
-  GetMediaHistoryService()->GetMediaFeedsForDebug(std::move(callback));
+  GetMediaHistoryService()->GetMediaFeeds(
+      media_history::MediaHistoryKeyedService::GetMediaFeedsRequest(),
+      std::move(callback));
 }
 
 void MediaFeedsUI::GetItemsForMediaFeed(int64_t feed_id,