Commit 13ee72a5 authored by Sam Bowen's avatar Sam Bowen Committed by Commit Bot

[Media Feeds] Add method to fetch top feeds in the background

See the bug description. This is step (1) of the scaled back
requirements. We will post this task in an upcoming CL.

For mojo change, we need to add the reset token to be saved in
the DB so we can bypass an unnecessary DB call in this new code
path.

Bug: 1064751
Change-Id: I31ac1261a5f0e9fccb89787994eba65830cef092
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2243671
Commit-Queue: Sam Bowen <sgbowen@google.com>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarBecca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#780462}
parent c92a89d5
......@@ -5,12 +5,18 @@
#include "chrome/browser/media/feeds/media_feeds_service.h"
#include "base/bind.h"
#include "base/callback_forward.h"
#include "base/feature_list.h"
#include "base/metrics/histogram_functions.h"
#include "base/optional.h"
#include "base/threading/thread_checker.h"
#include "base/time/clock.h"
#include "base/time/default_clock.h"
#include "base/time/time_to_iso8601.h"
#include "chrome/browser/media/feeds/media_feeds_converter.h"
#include "chrome/browser/media/feeds/media_feeds_fetcher.h"
#include "chrome/browser/media/feeds/media_feeds_service_factory.h"
#include "chrome/browser/media/feeds/media_feeds_store.mojom-forward.h"
#include "chrome/browser/media/feeds/media_feeds_store.mojom-shared.h"
#include "chrome/browser/media/feeds/media_feeds_store.mojom.h"
#include "chrome/browser/media/history/media_history_keyed_service.h"
......@@ -46,6 +52,15 @@ GURL Normalize(const GURL& url) {
return url.ReplaceComponents(replacements);
}
media_history::MediaHistoryKeyedService::MediaFeedFetchDetails
FetchDetailsFromFeed(const mojom::MediaFeedPtr& media_feed) {
media_history::MediaHistoryKeyedService::MediaFeedFetchDetails details;
details.url = media_feed->url;
details.last_fetch_result = media_feed->last_fetch_result;
details.reset_token = media_feed->reset_token;
return details;
}
class CookieChangeListener : public network::mojom::CookieChangeListener {
public:
using CookieCallback =
......@@ -120,13 +135,21 @@ class CookieChangeListener : public network::mojom::CookieChangeListener {
const char MediaFeedsService::kSafeSearchResultHistogramName[] =
"Media.Feeds.SafeSearch.Result";
// The maximum number of feeds to fetch when getting the top feeds.
const int kMaxTopFeedsToFetch = 5;
// The minimum watchtime required on the feed's origin before a feed can be
// considered a top feed.
constexpr base::TimeDelta kTopFeedsMinWatchTime =
base::TimeDelta::FromMinutes(30);
MediaFeedsService::MediaFeedsService(Profile* profile)
:
cookie_change_listener_(std::make_unique<CookieChangeListener>(
: cookie_change_listener_(std::make_unique<CookieChangeListener>(
profile,
base::BindRepeating(&MediaFeedsService::OnResetOriginFromCookie,
base::Unretained(this)))),
profile_(profile) {
profile_(profile),
clock_(base::DefaultClock::GetInstance()) {
DCHECK(!profile->IsOffTheRecord());
pref_change_registrar_.Init(profile_->GetPrefs());
......@@ -182,7 +205,9 @@ void MediaFeedsService::SetSafeSearchCompletionCallbackForTest(
safe_search_completion_callback_ = std::move(callback);
}
void MediaFeedsService::FetchMediaFeed(int64_t feed_id,
void MediaFeedsService::FetchMediaFeed(const int64_t feed_id,
const bool bypass_cache,
media_feeds::mojom::MediaFeedPtr feed,
FetchMediaFeedCallback callback) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
......@@ -198,9 +223,19 @@ void MediaFeedsService::FetchMediaFeed(int64_t feed_id,
GetURLLoaderFactoryForFetcher()),
std::move(callback)));
if (feed) {
OnGotFetchDetails(feed_id, bypass_cache, FetchDetailsFromFeed(feed));
} else {
GetMediaHistoryService()->GetMediaFeedFetchDetails(
feed_id, base::BindOnce(&MediaFeedsService::OnGotFetchDetails,
weak_factory_.GetWeakPtr(), feed_id));
feed_id,
base::BindOnce(&MediaFeedsService::OnGotFetchDetails,
weak_factory_.GetWeakPtr(), feed_id, bypass_cache));
}
}
void MediaFeedsService::FetchMediaFeed(int64_t feed_id,
FetchMediaFeedCallback callback) {
FetchMediaFeed(feed_id, false, nullptr, std::move(callback));
}
media_history::MediaHistoryKeyedService*
......@@ -296,6 +331,45 @@ void MediaFeedsService::ResetMediaFeed(const url::Origin& origin,
GetMediaHistoryService()->ResetMediaFeed(origin, reason);
}
void MediaFeedsService::FetchTopMediaFeeds(base::OnceClosure callback) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
if (!IsBackgroundFetchingEnabled())
return;
GetMediaHistoryService()->GetMediaFeeds(
media_history::MediaHistoryKeyedService::GetMediaFeedsRequest::
CreateTopFeedsForFetch(kMaxTopFeedsToFetch, kTopFeedsMinWatchTime),
base::BindOnce(&MediaFeedsService::OnGotTopFeeds,
weak_factory_.GetWeakPtr(), std::move(callback)));
}
void MediaFeedsService::OnGotTopFeeds(
base::OnceClosure callback,
std::vector<media_feeds::mojom::MediaFeedPtr> feeds) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
int feeds_fetched = 0;
auto it = feeds.begin();
while (feeds_fetched < kMaxTopFeedsToFetch && it != feeds.end()) {
auto& feed = *it;
auto background_fetch_feed_settings = GetBackgroundFetchFeedSettings(feed);
if (background_fetch_feed_settings.should_fetch) {
auto feed_id = feed->id;
FetchMediaFeed(
feed_id, /*bypass_cache=*/background_fetch_feed_settings.bypass_cache,
std::move(feed), base::DoNothing());
feeds_fetched++;
}
++it;
}
std::move(callback).Run();
}
void MediaFeedsService::OnCheckURLDone(
const media_history::MediaHistoryKeyedService::SafeSearchID id,
const GURL& original_url,
......@@ -388,6 +462,7 @@ MediaFeedsService::InflightSafeSearchCheck::~InflightSafeSearchCheck() =
void MediaFeedsService::OnGotFetchDetails(
const int64_t feed_id,
bool bypass_cache,
base::Optional<
media_history::MediaHistoryKeyedService::MediaFeedFetchDetails>
details) {
......@@ -399,11 +474,12 @@ void MediaFeedsService::OnGotFetchDetails(
return;
}
bool should_bypass_cache =
bypass_cache ||
details->last_fetch_result != media_feeds::mojom::FetchResult::kSuccess;
fetches_.at(feed_id).fetcher->FetchFeed(
details->url,
// If the last fetch result was not successful then we should make sure
// we don't get the bad result from the cache.
details->last_fetch_result != media_feeds::mojom::FetchResult::kSuccess,
details->url, should_bypass_cache,
// Use of unretained is safe because the callback is owned
// by fetcher_, which will not outlive this.
base::BindOnce(&MediaFeedsService::OnFetchResponse,
......@@ -480,6 +556,48 @@ void MediaFeedsService::OnResetOriginFromCookie(
std::move(cookie_change_callback_).Run();
}
MediaFeedsService::BackgroundFetchFeedSettings
MediaFeedsService::GetBackgroundFetchFeedSettings(
const media_feeds::mojom::MediaFeedPtr& feed) {
BackgroundFetchFeedSettings settings;
settings.should_fetch = false;
settings.bypass_cache = false;
// Fetches should be spaced 15 minutes apart with exponential backoff
// based on how many sequential times the fetch has failed.
if (feed->last_fetch_time.has_value()) {
// TODO(crbug.com/1064751): Consider using net::BackoffEntry for this.
base::Time next_fetch_time =
feed->last_fetch_time.value() +
base::TimeDelta::FromMinutes(15 * pow(2, feed->fetch_failed_count));
settings.should_fetch = next_fetch_time < clock_->Now();
}
// If we haven't gotten a non-cached version of the feed in a while, we should
// fetch.
if (feed->last_fetch_time_not_cache_hit.has_value()) {
base::Time next_fetch_time = feed->last_fetch_time_not_cache_hit.value() +
base::TimeDelta::FromHours(24);
if (next_fetch_time < clock_->Now()) {
settings.should_fetch = true;
settings.bypass_cache = true;
}
}
// If the feed has never been fetched, we should fetch it.
if (!feed->last_fetch_time.has_value()) {
settings.should_fetch = true;
}
// If the feed has been reset, we should fetch and ignore the cache.
if (feed->reset_reason != mojom::ResetReason::kNone) {
settings.should_fetch = true;
settings.bypass_cache = true;
}
return settings;
}
scoped_refptr<::network::SharedURLLoaderFactory>
MediaFeedsService::GetURLLoaderFactoryForFetcher() {
if (test_url_loader_factory_for_fetcher_)
......
......@@ -8,6 +8,7 @@
#include <memory>
#include <set>
#include "base/callback_forward.h"
#include "base/memory/weak_ptr.h"
#include "base/threading/thread_checker.h"
#include "chrome/browser/media/feeds/media_feeds_converter.h"
......@@ -20,6 +21,10 @@
class Profile;
class GURL;
namespace base {
class Clock;
}
namespace safe_search_api {
enum class Classification;
class URLChecker;
......@@ -70,6 +75,12 @@ class MediaFeedsService : public KeyedService {
// Fetches a media feed with the given ID and then store it in the
// feeds table in media history. Runs the given callback after storing. The
// fetch will be skipped if another fetch is currently ongoing.
// If the feed is not supplied, it will be looked up in media history store in
// order to get details related to fetching.
void FetchMediaFeed(const int64_t feed_id,
const bool bypass_cache,
media_feeds::mojom::MediaFeedPtr feed,
FetchMediaFeedCallback callback);
void FetchMediaFeed(int64_t feed_id, FetchMediaFeedCallback callback);
// Stores a callback to be called once we have completed all inflight checks.
......@@ -84,11 +95,17 @@ class MediaFeedsService : public KeyedService {
void ResetMediaFeed(const url::Origin& origin,
media_feeds::mojom::ResetReason reason);
// Check the list of discovered feeds and fetch a collection of those with the
// highest watchtime. This should be called periodically in the background.
void FetchTopMediaFeeds(base::OnceClosure callback);
bool HasCookieObserverForTest() const;
private:
friend class MediaFeedsServiceTest;
void SetClockForTesting(base::Clock* clock) { clock_ = clock; }
bool AddInflightSafeSearchCheck(
const media_history::MediaHistoryKeyedService::SafeSearchID id,
const std::set<GURL>& urls);
......@@ -110,8 +127,12 @@ class MediaFeedsService : public KeyedService {
bool IsSafeSearchCheckingEnabled() const;
void OnGotTopFeeds(base::OnceClosure callback,
std::vector<media_feeds::mojom::MediaFeedPtr> feeds);
void OnGotFetchDetails(
const int64_t feed_id,
bool bypass_cache,
base::Optional<
media_history::MediaHistoryKeyedService::MediaFeedFetchDetails>
details);
......@@ -134,6 +155,18 @@ class MediaFeedsService : public KeyedService {
void OnDiscoveredFeed();
// Settings related to fetching a feed in the background.
struct BackgroundFetchFeedSettings {
// Whether this feed should be fetched now.
bool should_fetch;
// Whether to use cached data (false) or bypass and get fresh data (true).
bool bypass_cache;
};
// Returns whether the feed should be fetched in the background as a top feed.
BackgroundFetchFeedSettings GetBackgroundFetchFeedSettings(
const media_feeds::mojom::MediaFeedPtr& feed);
media_history::MediaHistoryKeyedService* GetMediaHistoryService();
scoped_refptr<::network::SharedURLLoaderFactory>
......@@ -185,6 +218,9 @@ class MediaFeedsService : public KeyedService {
std::unique_ptr<safe_search_api::URLChecker> safe_search_url_checker_;
Profile* const profile_;
// An internal clock for testing.
base::Clock* clock_;
THREAD_CHECKER(thread_checker_);
base::WeakPtrFactory<MediaFeedsService> weak_factory_{this};
......
......@@ -10,6 +10,7 @@ import "mojo/public/mojom/base/time.mojom";
import "url/mojom/origin.mojom";
import "ui/gfx/geometry/mojom/geometry.mojom";
import "url/mojom/url.mojom";
import "mojo/public/mojom/base/unguessable_token.mojom";
struct MediaFeed {
// The ID of the field in storage.
......@@ -67,6 +68,12 @@ struct MediaFeed {
// have been deleted and the feed status has been reset to default.
ResetReason reset_reason;
// Token used to invalidate ongoing fetches for the feed. If there is a token
// mismatch when feed fetch data returns (the reset token when the fetch began
// does not match the feed's current reset token), the data is stale and
// shouldn't be stored because the feed has been reset.
mojo_base.mojom.UnguessableToken? reset_token;
// Contains details about the user signed into the website.
UserIdentifier? user_identifier;
......
......@@ -197,7 +197,8 @@ std::vector<media_feeds::mojom::MediaFeedPtr> MediaHistoryFeedsTable::GetRows(
"mediaFeed.reset_reason, "
"mediaFeed.user_identifier, "
"mediaFeed.cookie_name_filter, "
"mediaFeed.safe_search_result");
"mediaFeed.safe_search_result, "
"mediaFeed.reset_token ");
sql::Statement statement;
......@@ -373,6 +374,12 @@ std::vector<media_feeds::mojom::MediaFeedPtr> MediaHistoryFeedsTable::GetRows(
if (statement.GetColumnType(17) == sql::ColumnType::kText)
feed->cookie_name_filter = statement.ColumnString(17);
if (statement.GetColumnType(19) == sql::ColumnType::kBlob) {
media_feeds::FeedResetToken token;
if (GetProto(statement, 19, token))
feed->reset_token = ProtoToUnguessableToken(token);
}
feeds.push_back(std::move(feed));
// If we are returning top feeds then we should apply a limit here.
......
......@@ -72,7 +72,8 @@ void MediaFeedsUI::GetItemsForMediaFeed(int64_t feed_id,
void MediaFeedsUI::FetchMediaFeed(int64_t feed_id,
FetchMediaFeedCallback callback) {
GetMediaFeedsService()->FetchMediaFeed(feed_id, std::move(callback));
GetMediaFeedsService()->FetchMediaFeed(feed_id, /*bypass_cache=*/false,
nullptr, std::move(callback));
}
void MediaFeedsUI::GetDebugInformation(GetDebugInformationCallback callback) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment