Add non-automatic selection fetching

Adds fetching that is not automatic selection
and instead uses which feeds a user has picked.

BUG=1053599

Change-Id: Ib1c16d0efa409354c68845472484bfd33bf5145c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2362997
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800256}

chrome/browser/media/feeds/media_feeds_service.cc

@@ -22,6 +22,7 @@
 #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_keyed_service_factory.h"
+#include "chrome/browser/media/kaleidoscope/kaleidoscope_prefs.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "components/pref_registry/pref_registry_syncable.h"
@@ -354,11 +355,23 @@ void MediaFeedsService::FetchTopMediaFeeds(base::OnceClosure callback) {
   if (!IsBackgroundFetchingEnabled())
     return;
 
-  GetMediaHistoryService()->GetMediaFeeds(
-      media_history::MediaHistoryKeyedService::GetMediaFeedsRequest::
-          CreateTopFeedsForFetch(kMaxTopFeedsToFetch, kTopFeedsMinWatchTime),
-      base::BindOnce(&MediaFeedsService::OnGotTopFeeds,
-                     weak_factory_.GetWeakPtr(), std::move(callback)));
+  // If the user has opted into auto selection of media feeds then we should get
+  // the top media feeds based on heuristics. Otherwise, we should fallback to
+  // feeds the user has opted into.
+  if (profile_->GetPrefs()->GetBoolean(
+          kaleidoscope::prefs::kKaleidoscopeAutoSelectMediaFeeds)) {
+    GetMediaHistoryService()->GetMediaFeeds(
+        media_history::MediaHistoryKeyedService::GetMediaFeedsRequest::
+            CreateTopFeedsForFetch(kMaxTopFeedsToFetch, kTopFeedsMinWatchTime),
+        base::BindOnce(&MediaFeedsService::OnGotTopFeeds,
+                       weak_factory_.GetWeakPtr(), std::move(callback)));
+  } else {
+    GetMediaHistoryService()->GetMediaFeeds(
+        media_history::MediaHistoryKeyedService::GetMediaFeedsRequest::
+            CreateSelectedFeedsForFetch(),
+        base::BindOnce(&MediaFeedsService::OnGotTopFeeds,
+                       weak_factory_.GetWeakPtr(), std::move(callback)));
+  }
 }
 
 void MediaFeedsService::OnGotTopFeeds(

chrome/browser/media/feeds/media_feeds_service_unittest.cc

@@ -17,6 +17,7 @@
 #include "chrome/browser/media/feeds/media_feeds_store.mojom-shared.h"
 #include "chrome/browser/media/history/media_history_keyed_service.h"
 #include "chrome/browser/media/history/media_history_test_utils.h"
+#include "chrome/browser/media/kaleidoscope/kaleidoscope_prefs.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "chrome/test/base/testing_profile.h"
@@ -345,6 +346,11 @@ class MediaFeedsServiceTest : public ChromeRenderViewHostTestHarness {
     return rv;
   }
 
+  void SetAutomaticSelectionEnabled() {
+    profile()->GetPrefs()->SetBoolean(
+        kaleidoscope::prefs::kKaleidoscopeAutoSelectMediaFeeds, true);
+  }
+
   safe_search_api::StubURLChecker* safe_search_checker() {
     return stub_url_checker_.get();
   }
@@ -2086,6 +2092,7 @@ TEST_P(MediaFeedsSpecTest, RunOpenSourceTest) {
 // FetchTopMediaFeeds should fetch a feed with enough watchtime on that origin
 // even if it hasn't been fetched before.
 TEST_F(MediaFeedsServiceTest, FetchTopMediaFeeds_SuccessNewFetch) {
+  SetAutomaticSelectionEnabled();
   base::HistogramTester histogram_tester;
 
   const GURL feed_url("https://www.google.com/feed");
@@ -2126,6 +2133,7 @@ TEST_F(MediaFeedsServiceTest, FetchTopMediaFeeds_SuccessNewFetch) {
 
 // Fetch top feeds should periodically fetch the feed from cache if available.
 TEST_F(MediaFeedsServiceTest, FetchTopMediaFeeds_SuccessFromCache) {
+  SetAutomaticSelectionEnabled();
   base::HistogramTester histogram_tester;
 
   const GURL feed_url("https://www.google.com/feed");
@@ -2173,6 +2181,7 @@ TEST_F(MediaFeedsServiceTest, FetchTopMediaFeeds_SuccessFromCache) {
 // FetchTopMediaFeeds should back off if the feed fails to fetch. But after 24
 // hours, it should fetch regardless of failures, bypassing the cache.
 TEST_F(MediaFeedsServiceTest, FetchTopMediaFeeds_BacksOffFailedFetches) {
+  SetAutomaticSelectionEnabled();
   base::HistogramTester histogram_tester;
   const int times_to_fail = 10;
 
@@ -2230,6 +2239,7 @@ TEST_F(MediaFeedsServiceTest, FetchTopMediaFeeds_BacksOffFailedFetches) {
 // After 24 hours, FetchTopMediaFeeds should fetch the feed and bypass the
 // cache.
 TEST_F(MediaFeedsServiceTest, FetchTopMediaFeeds_SuccessBypassCache) {
+  SetAutomaticSelectionEnabled();
   base::HistogramTester histogram_tester;
 
   const GURL feed_url("https://www.google.com/feed");
@@ -2275,6 +2285,7 @@ TEST_F(MediaFeedsServiceTest, FetchTopMediaFeeds_SuccessBypassCache) {
 
 // After a feed reset, FetchTopMediaFeeds should fetch anyway.
 TEST_F(MediaFeedsServiceTest, FetchTopMediaFeeds_SuccessResetFeed) {
+  SetAutomaticSelectionEnabled();
   base::HistogramTester histogram_tester;
 
   const GURL feed_url("https://www.google.com/feed");
@@ -2320,6 +2331,7 @@ TEST_F(MediaFeedsServiceTest, FetchTopMediaFeeds_SuccessResetFeed) {
 // After enabling the pref, top feeds should fetch immediately and then again
 // after 15 minutes.
 TEST_F(MediaFeedsServiceTest, FetchTopMediaFeeds_SuccessRepeatsPeriodically) {
+  SetAutomaticSelectionEnabled();
   base::HistogramTester histogram_tester;
 
   const GURL feed_url("https://www.google.com/feed");
@@ -2360,4 +2372,42 @@ TEST_F(MediaFeedsServiceTest, FetchTopMediaFeeds_SuccessRepeatsPeriodically) {
       MediaFeedsFetcher::kFetchSizeKbHistogramName, 15, 2);
 }
 
+// FetchTopMediaFeeds should fetch a feed with enough watchtime on that origin
+// even if it hasn't been fetched before.
+TEST_F(MediaFeedsServiceTest, FetchTopMediaFeeds_DisableAutoSelection) {
+  base::HistogramTester histogram_tester;
+
+  const GURL feed_url_a("https://www.google.com/feed");
+  const GURL feed_url_b("https://www.google.co.uk/feed");
+
+  // Store a couple of Media Feeds.
+  GetMediaFeedsService()->DiscoverMediaFeed(feed_url_a);
+  GetMediaFeedsService()->DiscoverMediaFeed(feed_url_b);
+  WaitForDB();
+
+  // The first feed we should opt into.
+  GetMediaHistoryService()->UpdateFeedUserStatus(
+      1, media_feeds::mojom::FeedUserStatus::kEnabled);
+  WaitForDB();
+
+  SetBackgroundFetchingEnabled(true);
+  task_environment()->RunUntilIdle();
+
+  // The first feed should be fetched and the second one should be ignored since
+  // the user has not enabled it.
+  ASSERT_TRUE(RespondToPendingFeedFetch(feed_url_a));
+  ASSERT_FALSE(RespondToPendingFeedFetch(feed_url_b));
+
+  auto feeds = GetMediaFeedsSync();
+  ASSERT_EQ(2u, feeds.size());
+  EXPECT_TRUE(feeds[0]->last_fetch_time_not_cache_hit);
+  EXPECT_EQ(media_feeds::mojom::FetchResult::kSuccess,
+            feeds[0]->last_fetch_result);
+  EXPECT_EQ(media_feeds::mojom::FetchResult::kNone,
+            feeds[1]->last_fetch_result);
+
+  histogram_tester.ExpectUniqueSample(
+      MediaFeedsFetcher::kFetchSizeKbHistogramName, 15, 1);
+}
+
 }  // namespace media_feeds

chrome/browser/media/feeds/media_feeds_store.mojom

@@ -145,6 +145,9 @@ enum FeedUserStatus {
 
   // The user has opted out of seeing the feed.
   kDisabled = 1,
+
+  // The user has opted into seeing the feed.
+  kEnabled = 2,
 };
 
 // The type of the feed item. This enum is committed to storage so do not

chrome/browser/media/history/media_history_feeds_table.cc

@@ -120,6 +120,12 @@ sql::InitStatus MediaHistoryFeedsTable::CreateTableIfNonExistent() {
         "mediaFeed (last_fetch_content_types, safe_search_result)");
   }
 
+  if (success) {
+    success = DB()->Execute(
+        "CREATE INDEX IF NOT EXISTS mediaFeed_user_status_index ON "
+        "mediaFeed (user_status)");
+  }
+
   if (!success) {
     ResetDB();
     LOG(ERROR) << "Failed to create media history feeds table.";
@@ -290,6 +296,15 @@ std::vector<media_feeds::mojom::MediaFeedPtr> MediaHistoryFeedsTable::GetRows(
       statement.BindInt64(bind_index++,
                           static_cast<int>(*request.filter_by_type));
     }
+  } else if (request.type == MediaHistoryKeyedService::GetMediaFeedsRequest::
+                                 Type::kSelectedFeedsForFetch) {
+    sql.push_back("FROM mediaFeed WHERE user_status = ?");
+
+    statement.Assign(DB()->GetCachedStatement(
+        SQL_FROM_HERE, base::JoinString(sql, " ").c_str()));
+
+    statement.BindInt64(
+        0, static_cast<int>(media_feeds::mojom::FeedUserStatus::kEnabled));
   } else {
     sql.push_back("FROM mediaFeed");
 
@@ -773,4 +788,14 @@ bool MediaHistoryFeedsTable::StoreSafeSearchResult(
   return statement.Run();
 }
 
+bool MediaHistoryFeedsTable::UpdateFeedUserStatus(
+    const int64_t feed_id,
+    media_feeds::mojom::FeedUserStatus status) {
+  sql::Statement statement(DB()->GetCachedStatement(
+      SQL_FROM_HERE, "UPDATE mediaFeed SET user_status = ? WHERE id = ?"));
+  statement.BindInt64(0, static_cast<int>(status));
+  statement.BindInt64(1, feed_id);
+  return statement.Run();
+}
+
 }  // namespace media_history

chrome/browser/media/history/media_history_feeds_table.h

@@ -109,6 +109,10 @@ class MediaHistoryFeedsTable : public MediaHistoryTableBase {
   // Stores the safe search result for |feed_id| and returns true if successful.
   bool StoreSafeSearchResult(int64_t feed_id,
                              media_feeds::mojom::SafeSearchResult result);
+
+  // Updates the user status and returns true if successful.
+  bool UpdateFeedUserStatus(const int64_t feed_id,
+                            media_feeds::mojom::FeedUserStatus status);
 };
 
 }  // namespace media_history

chrome/browser/media/history/media_history_keyed_service.cc

@@ -418,6 +418,13 @@ MediaHistoryKeyedService::GetMediaFeedsRequest::CreateTopFeedsForDisplay(
   return request;
 }
 
+MediaHistoryKeyedService::GetMediaFeedsRequest
+MediaHistoryKeyedService::GetMediaFeedsRequest::CreateSelectedFeedsForFetch() {
+  GetMediaFeedsRequest request;
+  request.type = Type::kSelectedFeedsForFetch;
+  return request;
+}
+
 MediaHistoryKeyedService::GetMediaFeedsRequest::GetMediaFeedsRequest() =
     default;
 
@@ -516,6 +523,16 @@ void MediaHistoryKeyedService::DeleteMediaFeed(const int64_t feed_id,
   }
 }
 
+void MediaHistoryKeyedService::UpdateFeedUserStatus(
+    const int64_t feed_id,
+    media_feeds::mojom::FeedUserStatus status) {
+  if (auto* store = store_->GetForWrite()) {
+    store->db_task_runner_->PostTask(
+        FROM_HERE, base::BindOnce(&MediaHistoryStore::UpdateFeedUserStatus,
+                                  store, feed_id, status));
+  }
+}
+
 MediaHistoryKeyedService::MediaFeedFetchDetails::MediaFeedFetchDetails() =
     default;
 

chrome/browser/media/history/media_history_keyed_service.h

@@ -247,7 +247,10 @@ class MediaHistoryKeyedService : public KeyedService,
       // Returns the top feeds to be displayed. These will be sorted by the
       // by audio+video watchtime descending and we will also populate the
       // |origin_audio_video_watchtime_percentile| field in |MediaFeedPtr|.
-      kTopFeedsForDisplay
+      kTopFeedsForDisplay,
+
+      // Returns the feeeds that have been selected by the user to be fetched.
+      kSelectedFeedsForFetch,
     };
 
     static GetMediaFeedsRequest CreateTopFeedsForFetch(
@@ -260,6 +263,8 @@ class MediaHistoryKeyedService : public KeyedService,
         bool fetched_items_min_should_be_safe,
         base::Optional<media_feeds::mojom::MediaFeedItemType> filter_by_type);
 
+    static GetMediaFeedsRequest CreateSelectedFeedsForFetch();
+
     GetMediaFeedsRequest();
     GetMediaFeedsRequest(const GetMediaFeedsRequest& t);
 
@@ -332,6 +337,10 @@ class MediaHistoryKeyedService : public KeyedService,
   void GetMediaFeedFetchDetails(const int64_t feed_id,
                                 GetMediaFeedFetchDetailsCallback callback);
 
+  // Updates the FeedUserStatus for a feed.
+  void UpdateFeedUserStatus(const int64_t feed_id,
+                            media_feeds::mojom::FeedUserStatus status);
+
  protected:
   friend class media_feeds::MediaFeedsService;
 

chrome/browser/media/history/media_history_store.cc

@@ -1196,4 +1196,26 @@ MediaHistoryStore::GetMediaFeedFetchDetails(const int64_t feed_id) {
   return feeds_table_->GetFetchDetails(feed_id);
 }
 
+void MediaHistoryStore::UpdateFeedUserStatus(
+    const int64_t feed_id,
+    media_feeds::mojom::FeedUserStatus status) {
+  if (!CanAccessDatabase())
+    return;
+
+  if (!feeds_table_)
+    return;
+
+  if (!DB()->BeginTransaction()) {
+    DLOG(ERROR) << "Failed to begin the transaction.";
+    return;
+  }
+
+  if (!feeds_table_->UpdateFeedUserStatus(feed_id, status)) {
+    DB()->RollbackTransaction();
+    return;
+  }
+
+  DB()->CommitTransaction();
+}
+
 }  // namespace media_history

chrome/browser/media/history/media_history_store.h

@@ -199,6 +199,9 @@ class MediaHistoryStore : public base::RefCountedThreadSafe<MediaHistoryStore> {
   base::Optional<MediaHistoryKeyedService::MediaFeedFetchDetails>
   GetMediaFeedFetchDetails(const int64_t feed_id);
 
+  void UpdateFeedUserStatus(const int64_t feed_id,
+                            media_feeds::mojom::FeedUserStatus status);
+
  private:
   friend class base::RefCountedThreadSafe<MediaHistoryStore>;
 

chrome/browser/media/history/media_history_store_unittest.cc

@@ -2957,6 +2957,40 @@ TEST_P(MediaHistoryStoreFeedsTest, GetItemsForFeed) {
   }
 }
 
+TEST_P(MediaHistoryStoreFeedsTest, GetSelectedFeedsForFetch) {
+  const GURL feed_url_a("https://www.google.com/feed");
+  const GURL feed_url_b("https://www.google.co.uk/feed");
+  const GURL feed_url_c("https://www.google.co.tv/feed");
+
+  DiscoverMediaFeed(feed_url_a);
+  DiscoverMediaFeed(feed_url_b);
+  DiscoverMediaFeed(feed_url_c);
+  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_a = IsReadOnly() ? -1 : GetMediaFeedsSync(service())[0]->id;
+  const int feed_id_b = IsReadOnly() ? -1 : GetMediaFeedsSync(service())[1]->id;
+
+  service()->UpdateFeedUserStatus(
+      feed_id_a, media_feeds::mojom::FeedUserStatus::kDisabled);
+  service()->UpdateFeedUserStatus(feed_id_b,
+                                  media_feeds::mojom::FeedUserStatus::kEnabled);
+  WaitForDB();
+
+  auto feeds = GetMediaFeedsSync(
+      service(), MediaHistoryKeyedService::GetMediaFeedsRequest::
+                     CreateSelectedFeedsForFetch());
+
+  if (IsReadOnly()) {
+    EXPECT_TRUE(feeds.empty());
+  } else {
+    ASSERT_EQ(1u, feeds.size());
+    EXPECT_EQ(feed_id_b, feeds[0]->id);
+  }
+}
+
 #endif  // !defined(OS_ANDROID)
 
 }  // namespace media_history

chrome/browser/media/kaleidoscope/kaleidoscope_data_provider_impl.cc

@@ -190,8 +190,15 @@ void KaleidoscopeDataProviderImpl::SetMediaFeedsConsent(
   prefs->SetBoolean(kaleidoscope::prefs::kKaleidoscopeAutoSelectMediaFeeds,
                     accepted_auto_select_media_feeds);
 
-  // TODO(b/154517281): Update the MediaFeeds service with the allowlisted
-  // feeds from |enabled_feed_ids| and |disabled_feed_ids|.
+  for (auto& feed_id : disabled_feed_ids) {
+    GetMediaHistoryService()->UpdateFeedUserStatus(
+        feed_id, media_feeds::mojom::FeedUserStatus::kDisabled);
+  }
+
+  for (auto& feed_id : enabled_feed_ids) {
+    GetMediaHistoryService()->UpdateFeedUserStatus(
+        feed_id, media_feeds::mojom::FeedUserStatus::kEnabled);
+  }
 }
 
 void KaleidoscopeDataProviderImpl::GetHighWatchTimeOrigins(

chrome/browser/resources/media/media_feeds.js

@@ -105,6 +105,8 @@ class MediaFeedsTableDelegate {
         td.textContent = 'Auto';
       } else if (data == mediaFeeds.mojom.FeedUserStatus.kDisabled) {
         td.textContent = 'Disabled';
+      } else if (data == mediaFeeds.mojom.FeedUserStatus.kEnabled) {
+        td.textContent = 'Enabled';
       }
     } else if (key === 'lastFetchResult') {
       // Format a FetchResult.