Commit 2665ec85 authored by Carlos Knippschild's avatar Carlos Knippschild Committed by Commit Bot

Prefetching download task takes limitless mode into account.

This change takes into account the "limitless prefetching" feature flag
to choose to ignore the daily download quota and increase the concurrent
downloads limit when it is enabled.

Bug: 793109
Change-Id: I6a3aea110976bcd1c2afac1dbfa86f06e0327b25
Reviewed-on: https://chromium-review.googlesource.com/817705
Commit-Queue: Carlos Knippschild <carlosk@chromium.org>
Reviewed-by: default avatarFilip Gorski <fgorski@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523608}
parent d816df28
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/time/default_clock.h" #include "base/time/default_clock.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "components/offline_pages/core/offline_page_feature.h"
#include "components/offline_pages/core/offline_store_utils.h" #include "components/offline_pages/core/offline_store_utils.h"
#include "components/offline_pages/core/prefetch/prefetch_downloader.h" #include "components/offline_pages/core/prefetch/prefetch_downloader.h"
#include "components/offline_pages/core/prefetch/prefetch_types.h" #include "components/offline_pages/core/prefetch/prefetch_types.h"
...@@ -86,11 +87,16 @@ std::unique_ptr<ItemsToDownload> SelectAndMarkItemsForDownloadSync( ...@@ -86,11 +87,16 @@ std::unique_ptr<ItemsToDownload> SelectAndMarkItemsForDownloadSync(
if (!transaction.Begin()) if (!transaction.Begin())
return nullptr; return nullptr;
const int max_concurrent_downloads =
IsLimitlessPrefetchingEnabled()
? DownloadArchivesTask::kMaxConcurrentDownloadsForLimitless
: DownloadArchivesTask::kMaxConcurrentDownloads;
// Get current count of concurrent downloads and bail early if we are already // Get current count of concurrent downloads and bail early if we are already
// downloading more than we can. // downloading more than we can.
std::unique_ptr<int> concurrent_downloads(CountDownloadsInProgress(db)); std::unique_ptr<int> concurrent_downloads(CountDownloadsInProgress(db));
if (!concurrent_downloads || if (!concurrent_downloads ||
*concurrent_downloads >= DownloadArchivesTask::kMaxConcurrentDownloads) { *concurrent_downloads >= max_concurrent_downloads) {
return nullptr; return nullptr;
} }
...@@ -102,7 +108,7 @@ std::unique_ptr<ItemsToDownload> SelectAndMarkItemsForDownloadSync( ...@@ -102,7 +108,7 @@ std::unique_ptr<ItemsToDownload> SelectAndMarkItemsForDownloadSync(
base::DefaultClock clock; base::DefaultClock clock;
PrefetchDownloaderQuota downloader_quota(db, &clock); PrefetchDownloaderQuota downloader_quota(db, &clock);
int64_t available_quota = downloader_quota.GetAvailableQuotaBytes(); int64_t available_quota = downloader_quota.GetAvailableQuotaBytes();
if (available_quota <= 0) if (available_quota <= 0 && !IsLimitlessPrefetchingEnabled())
return nullptr; return nullptr;
ItemsToDownload ready_items = FindItemsReadyForDownload(db); ItemsToDownload ready_items = FindItemsReadyForDownload(db);
...@@ -115,12 +121,16 @@ std::unique_ptr<ItemsToDownload> SelectAndMarkItemsForDownloadSync( ...@@ -115,12 +121,16 @@ std::unique_ptr<ItemsToDownload> SelectAndMarkItemsForDownloadSync(
auto items_to_download = base::MakeUnique<ItemsToDownload>(); auto items_to_download = base::MakeUnique<ItemsToDownload>();
for (auto& ready_item : ready_items) { for (auto& ready_item : ready_items) {
// Concurrent downloads check. // Concurrent downloads check.
if (*concurrent_downloads >= DownloadArchivesTask::kMaxConcurrentDownloads) if (*concurrent_downloads >= max_concurrent_downloads)
break; break;
// Quota check. Skips all items that violate quota. // Quota check. Skips all items that violate quota if limitless prefetching
if (ready_item.archive_body_length > available_quota) // is disabled. If it is enabled then always proceed but still decrement the
// quota allowing it to become negative.
if (ready_item.archive_body_length > available_quota &&
!IsLimitlessPrefetchingEnabled()) {
continue; continue;
}
available_quota -= ready_item.archive_body_length; available_quota -= ready_item.archive_body_length;
// Explicitly not reusing the GUID from the last archive download attempt // Explicitly not reusing the GUID from the last archive download attempt
...@@ -133,7 +143,8 @@ std::unique_ptr<ItemsToDownload> SelectAndMarkItemsForDownloadSync( ...@@ -133,7 +143,8 @@ std::unique_ptr<ItemsToDownload> SelectAndMarkItemsForDownloadSync(
++(*concurrent_downloads); ++(*concurrent_downloads);
} }
// Write new remaining quota with date here. // Write new remaining quota, limited to a minimum of 0.
available_quota = available_quota < 0 ? 0 : available_quota;
if (!downloader_quota.SetAvailableQuotaBytes(available_quota)) if (!downloader_quota.SetAvailableQuotaBytes(available_quota))
return nullptr; return nullptr;
...@@ -148,6 +159,13 @@ std::unique_ptr<ItemsToDownload> SelectAndMarkItemsForDownloadSync( ...@@ -148,6 +159,13 @@ std::unique_ptr<ItemsToDownload> SelectAndMarkItemsForDownloadSync(
// static // static
const int DownloadArchivesTask::kMaxConcurrentDownloads = 2; const int DownloadArchivesTask::kMaxConcurrentDownloads = 2;
// This value matches the maximum number of concurrent downloads allowed by the
// downloads service.
// TODO(https://crbug.com/793158): obtain this value directly from the downloads
// service once it is exposed.
// static
const int DownloadArchivesTask::kMaxConcurrentDownloadsForLimitless = 4;
DownloadArchivesTask::DownloadArchivesTask( DownloadArchivesTask::DownloadArchivesTask(
PrefetchStore* prefetch_store, PrefetchStore* prefetch_store,
PrefetchDownloader* prefetch_downloader) PrefetchDownloader* prefetch_downloader)
......
...@@ -24,6 +24,9 @@ class DownloadArchivesTask : public Task { ...@@ -24,6 +24,9 @@ class DownloadArchivesTask : public Task {
// Maximum number of parallel downloads. // Maximum number of parallel downloads.
static const int kMaxConcurrentDownloads; static const int kMaxConcurrentDownloads;
// Maximum number of parallel downloads when limitless prefetching is enabled.
static const int kMaxConcurrentDownloadsForLimitless;
// Represents item to be downloaded as a result of running the task. // Represents item to be downloaded as a result of running the task.
struct DownloadItem { struct DownloadItem {
int64_t offline_id; int64_t offline_id;
......
...@@ -8,7 +8,10 @@ ...@@ -8,7 +8,10 @@
#include <set> #include <set>
#include "base/guid.h" #include "base/guid.h"
#include "base/numerics/safe_conversions.h"
#include "base/test/histogram_tester.h" #include "base/test/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
#include "components/offline_pages/core/offline_page_feature.h"
#include "components/offline_pages/core/prefetch/store/prefetch_downloader_quota.h" #include "components/offline_pages/core/prefetch/store/prefetch_downloader_quota.h"
#include "components/offline_pages/core/prefetch/store/prefetch_store_test_util.h" #include "components/offline_pages/core/prefetch/store/prefetch_store_test_util.h"
#include "components/offline_pages/core/prefetch/store/prefetch_store_utils.h" #include "components/offline_pages/core/prefetch/store/prefetch_store_utils.h"
...@@ -303,16 +306,84 @@ TEST_F(DownloadArchivesTaskTest, TooManyArchivesToDownload) { ...@@ -303,16 +306,84 @@ TEST_F(DownloadArchivesTaskTest, TooManyArchivesToDownload) {
const PrefetchItem* download_item_before = const PrefetchItem* download_item_before =
FindPrefetchItemByOfflineId(items_before_run, item_ids[i]); FindPrefetchItemByOfflineId(items_before_run, item_ids[i]);
const PrefetchItem* download_item_after = const PrefetchItem* download_item_after =
FindPrefetchItemByOfflineId(items_after_run, item_ids[i]);
ASSERT_TRUE(download_item_before);
ASSERT_TRUE(download_item_after);
EXPECT_EQ(*download_item_before, *download_item_after);
EXPECT_EQ(PrefetchItemState::RECEIVED_BUNDLE, download_item_after->state);
}
histogram_tester.ExpectUniqueSample(
"OfflinePages.Prefetching.DownloadExpectedFileSize",
kSmallArchiveSize / 1024, DownloadArchivesTask::kMaxConcurrentDownloads);
}
TEST_F(DownloadArchivesTaskTest,
ManyLargeArchivesToDownloadWithLimitlessEnabled) {
// Enable limitless prefetching.
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeatures(
{kPrefetchingOfflinePagesFeature,
kOfflinePagesLimitlessPrefetchingFeature},
{});
// Check the concurrent downloads limit is greater for limitless.
ASSERT_GT(DownloadArchivesTask::kMaxConcurrentDownloadsForLimitless,
DownloadArchivesTask::kMaxConcurrentDownloads);
// Create more archives than we allow to download in parallel with limitless
// and put them in the fresher |item_ids| in front.
const size_t max_concurrent_downloads = base::checked_cast<size_t>(
DownloadArchivesTask::kMaxConcurrentDownloadsForLimitless);
const size_t total_items = max_concurrent_downloads + 2;
std::vector<int64_t> item_ids;
for (size_t i = 0; i < total_items; ++i)
item_ids.insert(item_ids.begin(), InsertItemToDownload(kLargeArchiveSize));
std::set<PrefetchItem> items_before_run;
EXPECT_EQ(total_items, store_util()->GetAllItems(&items_before_run));
DownloadArchivesTask task(store(), prefetch_downloader());
base::HistogramTester histogram_tester;
ExpectTaskCompletes(&task);
task.Run();
RunUntilIdle();
std::set<PrefetchItem> items_after_run;
EXPECT_EQ(total_items, store_util()->GetAllItems(&items_after_run));
std::map<std::string, std::string> requested_downloads =
prefetch_downloader()->requested_downloads();
EXPECT_EQ(max_concurrent_downloads, requested_downloads.size());
// The freshest |kMaxConcurrentDownloadsForLimitless| items should be started
// as with limitless enabled there's no download size restrictions.
for (size_t i = 0; i < max_concurrent_downloads; ++i) {
const PrefetchItem* download_item =
FindPrefetchItemByOfflineId(items_after_run, item_ids[i]);
ASSERT_TRUE(download_item);
EXPECT_EQ(PrefetchItemState::DOWNLOADING, download_item->state);
auto it = requested_downloads.find(download_item->guid);
ASSERT_TRUE(it != requested_downloads.end());
EXPECT_EQ(it->second, download_item->archive_body_name);
}
// Remaining items shouldn't have been started.
for (size_t i = max_concurrent_downloads; i < total_items; ++i) {
const PrefetchItem* download_item_before =
FindPrefetchItemByOfflineId(items_before_run, item_ids[i]); FindPrefetchItemByOfflineId(items_before_run, item_ids[i]);
const PrefetchItem* download_item_after =
FindPrefetchItemByOfflineId(items_after_run, item_ids[i]);
ASSERT_TRUE(download_item_before); ASSERT_TRUE(download_item_before);
ASSERT_TRUE(download_item_after); ASSERT_TRUE(download_item_after);
EXPECT_EQ(*download_item_before, *download_item_before); EXPECT_EQ(*download_item_before, *download_item_after);
EXPECT_EQ(PrefetchItemState::RECEIVED_BUNDLE, download_item_after->state); EXPECT_EQ(PrefetchItemState::RECEIVED_BUNDLE, download_item_after->state);
} }
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
"OfflinePages.Prefetching.DownloadExpectedFileSize", "OfflinePages.Prefetching.DownloadExpectedFileSize",
kSmallArchiveSize / 1024, 2); kLargeArchiveSize / 1024, max_concurrent_downloads);
} }
TEST_F(DownloadArchivesTaskTest, SingleArchiveSecondAttempt) { TEST_F(DownloadArchivesTaskTest, SingleArchiveSecondAttempt) {
......
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