Commit fcb79c90 authored by Xing Liu's avatar Xing Liu Committed by Commit Bot

Download service: Use TaskScheduler in scheduler.

Previously we use a dummy interface, this CL makes the scheduler to use
the new API to schedule OS level background tasks.

Bug: 728472
Change-Id: I1e298c8ba82631cd8c390e40c76365df55cc2492
Reviewed-on: https://chromium-review.googlesource.com/536274
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: default avatarShakti Sahu <shaktisahu@chromium.org>
Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479839}
parent 96368334
......@@ -20,7 +20,7 @@ namespace android {
class DownloadTaskScheduler : public TaskScheduler {
public:
DownloadTaskScheduler();
~DownloadTaskScheduler();
~DownloadTaskScheduler() override;
// TaskScheduler implementation.
void ScheduleTask(DownloadTaskType task_type,
......
......@@ -22,7 +22,7 @@ class BrowserContext;
class DownloadTaskSchedulerImpl : public download::TaskScheduler {
public:
explicit DownloadTaskSchedulerImpl(content::BrowserContext* context);
~DownloadTaskSchedulerImpl();
~DownloadTaskSchedulerImpl() override;
// TaskScheduler implementation.
void ScheduleTask(download::DownloadTaskType task_type,
......
BatteryListenerTest.*
DeviceStatusListenerTest.*
DownloadDriverImplTest.*
DownloadSchedulerImplTest.*
DownloadServiceClientSetTest.*
DownloadServiceControllerImplTest.*
DownloadServiceImplTest.*
......
......@@ -43,7 +43,8 @@ DownloadService* CreateDownloadService(
std::move(entry_db));
auto model = base::MakeUnique<ModelImpl>(std::move(store));
auto device_status_listener = base::MakeUnique<DeviceStatusListener>();
auto scheduler = base::MakeUnique<SchedulerImpl>(nullptr, client_set.get());
auto scheduler = base::MakeUnique<SchedulerImpl>(
task_scheduler.get(), config.get(), client_set.get());
auto controller = base::MakeUnique<ControllerImpl>(
config.get(), std::move(client_set), std::move(driver), std::move(model),
......
......@@ -31,6 +31,12 @@ const uint32_t kDefaultMaxRetryCount = 5;
// 12 hours by default.
const uint32_t kDefaultFileKeepAliveTimeMinutes = 12 * 60;
// Default value for the start window time for OS to schedule background task.
const uint32_t kDefaultWindowStartTimeSeconds = 300; /* 5 minutes. */
// Default value for the end window time for OS to schedule background task.
const uint32_t kDefaultWindowEndTimeSeconds = 3600 * 8; /* 8 hours. */
// Helper routine to get Finch experiment parameter. If no Finch seed was found,
// use the |default_value|. The |name| should match an experiment
// parameter in Finch server configuration.
......@@ -57,6 +63,10 @@ std::unique_ptr<Configuration> Configuration::CreateFromFinch() {
config->file_keep_alive_time =
base::TimeDelta::FromMinutes(base::saturated_cast<int>(GetFinchConfigUInt(
kFileKeepAliveTimeMinutesConfig, kDefaultFileKeepAliveTimeMinutes)));
config->window_start_time_seconds = GetFinchConfigUInt(
kWindowStartTimeConfig, kDefaultWindowStartTimeSeconds);
config->window_end_time_seconds =
GetFinchConfigUInt(kWindowEndTimeConfig, kDefaultWindowEndTimeSeconds);
return config;
}
......@@ -66,6 +76,8 @@ Configuration::Configuration()
max_scheduled_downloads(kDefaultMaxScheduledDownloads),
max_retry_count(kDefaultMaxRetryCount),
file_keep_alive_time(base::TimeDelta::FromMinutes(
base::saturated_cast<int>(kDefaultFileKeepAliveTimeMinutes))) {}
base::saturated_cast<int>(kDefaultFileKeepAliveTimeMinutes))),
window_start_time_seconds(kDefaultWindowStartTimeSeconds),
window_end_time_seconds(kDefaultWindowEndTimeSeconds) {}
} // namespace download
......@@ -27,6 +27,12 @@ constexpr char kMaxRetryCountConfig[] = "max_retry_count";
// Configuration name for file keep alive time.
constexpr char kFileKeepAliveTimeMinutesConfig[] = "file_keep_alive_time";
// Configuration name for window start time.
constexpr char kWindowStartTimeConfig[] = "window_start_time_seconds";
// Configuration name for window end time.
constexpr char kWindowEndTimeConfig[] = "window_end_time_seconds";
// Download service configuration.
//
// Loaded based on experiment parameters from the server. Use default values if
......@@ -56,6 +62,14 @@ struct Configuration {
// deleting them if the client hasn't handle the files.
base::TimeDelta file_keep_alive_time;
// The start window time in seconds for OS to schedule background task.
// The OS will trigger the background task in this window.
uint32_t window_start_time_seconds;
// The end window time in seconds for OS to schedule background task.
// The OS will trigger the background task in this window.
uint32_t window_end_time_seconds;
private:
DISALLOW_COPY_AND_ASSIGN(Configuration);
};
......
......@@ -35,7 +35,7 @@ namespace {
class MockTaskScheduler : public TaskScheduler {
public:
MockTaskScheduler() = default;
~MockTaskScheduler() = default;
~MockTaskScheduler() override = default;
// TaskScheduler implementation.
MOCK_METHOD5(ScheduleTask, void(DownloadTaskType, bool, bool, long, long));
......
......@@ -30,15 +30,6 @@ class Scheduler {
virtual ~Scheduler() {}
};
// Interface to schedule platform dependent background tasks that can run after
// browser being closed.
class PlatformTaskScheduler {
public:
virtual void ScheduleDownloadTask(const Criteria& criteria) = 0;
virtual void CancelDownloadTask() = 0;
virtual ~PlatformTaskScheduler() {}
};
} // namespace download
#endif // COMPONENTS_DOWNLOAD_CORE_SCHEDULER_H_
......@@ -5,9 +5,11 @@
#include "components/download/internal/scheduler/scheduler_impl.h"
#include "components/download/internal/client_set.h"
#include "components/download/internal/config.h"
#include "components/download/internal/entry_utils.h"
#include "components/download/internal/scheduler/device_status.h"
#include "components/download/public/download_params.h"
#include "components/download/public/task_scheduler.h"
namespace download {
......@@ -25,35 +27,41 @@ std::vector<T> ToList(const std::set<T>& set) {
} // namespace
SchedulerImpl::SchedulerImpl(PlatformTaskScheduler* platform_scheduler,
SchedulerImpl::SchedulerImpl(TaskScheduler* task_scheduler,
Configuration* config,
const ClientSet* clients)
: SchedulerImpl(platform_scheduler,
: SchedulerImpl(task_scheduler,
config,
ToList<DownloadClient>(clients->GetRegisteredClients())) {}
SchedulerImpl::SchedulerImpl(PlatformTaskScheduler* platform_scheduler,
SchedulerImpl::SchedulerImpl(TaskScheduler* task_scheduler,
Configuration* config,
const std::vector<DownloadClient>& clients)
: platform_scheduler_(platform_scheduler),
: task_scheduler_(task_scheduler),
config_(config),
download_clients_(clients),
current_client_index_(0) {
DCHECK(!clients.empty());
DCHECK(!download_clients_.empty());
DCHECK(task_scheduler_);
}
SchedulerImpl::~SchedulerImpl() = default;
void SchedulerImpl::Reschedule(const Model::EntryList& entries) {
if (entries.empty()) {
if (platform_scheduler_)
platform_scheduler_->CancelDownloadTask();
task_scheduler_->CancelTask(DownloadTaskType::DOWNLOAD_TASK);
return;
}
// TODO(xingliu): Figure out if we need to pass the time window to platform
// scheduler, and support NetworkRequirements::OPTIMISTIC.
if (platform_scheduler_) {
platform_scheduler_->CancelDownloadTask();
platform_scheduler_->ScheduleDownloadTask(
util::GetSchedulingCriteria(entries));
}
// TODO(xingliu): Support NetworkRequirements::OPTIMISTIC.
task_scheduler_->CancelTask(DownloadTaskType::DOWNLOAD_TASK);
Criteria criteria = util::GetSchedulingCriteria(entries);
task_scheduler_->ScheduleTask(
DownloadTaskType::DOWNLOAD_TASK, criteria.requires_unmetered_network,
criteria.requires_battery_charging,
base::saturated_cast<long>(config_->window_start_time_seconds),
base::saturated_cast<long>(config_->window_end_time_seconds));
}
Entry* SchedulerImpl::Next(const Model::EntryList& entries,
......
......@@ -16,6 +16,8 @@
namespace download {
class ClientSet;
class TaskScheduler;
struct Configuration;
// Scheduler implementation that
// 1. Creates platform background task based on the states of download entries.
......@@ -25,9 +27,11 @@ class ClientSet;
// Provides load balancing between download clients using the service.
class SchedulerImpl : public Scheduler {
public:
SchedulerImpl(PlatformTaskScheduler* platform_scheduler,
SchedulerImpl(TaskScheduler* task_scheduler,
Configuration* config,
const ClientSet* clients);
SchedulerImpl(PlatformTaskScheduler* platform_scheduler,
SchedulerImpl(TaskScheduler* task_scheduler,
Configuration* config,
const std::vector<DownloadClient>& clients);
~SchedulerImpl() override;
......@@ -46,7 +50,10 @@ class SchedulerImpl : public Scheduler {
const DeviceStatus& device_status);
// Used to create platform dependent background tasks.
PlatformTaskScheduler* platform_scheduler_;
TaskScheduler* task_scheduler_;
// Download service configuration.
Configuration* config_;
// List of all download client id, used in round robin load balancing.
// Downloads will be delivered to clients with incremental order based on
......
......@@ -8,20 +8,26 @@
#include "base/memory/ptr_util.h"
#include "base/strings/string_number_conversions.h"
#include "components/download/internal/config.h"
#include "components/download/internal/entry.h"
#include "components/download/internal/scheduler/device_status.h"
#include "components/download/public/task_scheduler.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
using testing::_;
using testing::InSequence;
namespace download {
namespace {
class MockPlatformTaskScheduler : public PlatformTaskScheduler {
class MockTaskScheduler : public TaskScheduler {
public:
MOCK_METHOD1(ScheduleDownloadTask, void(const Criteria&));
MOCK_METHOD0(CancelDownloadTask, void());
MockTaskScheduler() = default;
~MockTaskScheduler() override = default;
MOCK_METHOD5(ScheduleTask, void(DownloadTaskType, bool, bool, long, long));
MOCK_METHOD1(CancelTask, void(DownloadTaskType));
};
class DownloadSchedulerImplTest : public testing::Test {
......@@ -33,7 +39,7 @@ class DownloadSchedulerImplTest : public testing::Test {
void BuildScheduler(const std::vector<DownloadClient> clients) {
scheduler_ =
base::MakeUnique<SchedulerImpl>(&platform_task_scheduler_, clients);
base::MakeUnique<SchedulerImpl>(&task_scheduler_, &config_, clients);
}
void DestroyScheduler() { scheduler_.reset(); }
......@@ -82,7 +88,8 @@ class DownloadSchedulerImplTest : public testing::Test {
protected:
std::unique_ptr<SchedulerImpl> scheduler_;
MockPlatformTaskScheduler platform_task_scheduler_;
MockTaskScheduler task_scheduler_;
Configuration config_;
// Entries owned by the test fixture.
std::vector<Entry> entries_;
......@@ -384,27 +391,36 @@ TEST_F(DownloadSchedulerImplTest, Reschedule) {
SchedulingParams::NetworkRequirements::UNMETERED;
Criteria criteria;
EXPECT_CALL(platform_task_scheduler_, CancelDownloadTask())
EXPECT_CALL(task_scheduler_, CancelTask(DownloadTaskType::DOWNLOAD_TASK))
.RetiresOnSaturation();
EXPECT_CALL(platform_task_scheduler_, ScheduleDownloadTask(criteria))
EXPECT_CALL(task_scheduler_,
ScheduleTask(DownloadTaskType::DOWNLOAD_TASK,
criteria.requires_unmetered_network,
criteria.requires_battery_charging, _, _))
.RetiresOnSaturation();
scheduler_->Reschedule(entries());
entries_[0].scheduling_params.battery_requirements =
SchedulingParams::BatteryRequirements::BATTERY_INSENSITIVE;
criteria.requires_battery_charging = false;
EXPECT_CALL(platform_task_scheduler_, CancelDownloadTask())
EXPECT_CALL(task_scheduler_, CancelTask(DownloadTaskType::DOWNLOAD_TASK))
.RetiresOnSaturation();
EXPECT_CALL(platform_task_scheduler_, ScheduleDownloadTask(criteria))
EXPECT_CALL(task_scheduler_,
ScheduleTask(DownloadTaskType::DOWNLOAD_TASK,
criteria.requires_unmetered_network,
criteria.requires_battery_charging, _, _))
.RetiresOnSaturation();
scheduler_->Reschedule(entries());
entries_[0].scheduling_params.network_requirements =
SchedulingParams::NetworkRequirements::NONE;
criteria.requires_unmetered_network = false;
EXPECT_CALL(platform_task_scheduler_, CancelDownloadTask())
EXPECT_CALL(task_scheduler_, CancelTask(DownloadTaskType::DOWNLOAD_TASK))
.RetiresOnSaturation();
EXPECT_CALL(platform_task_scheduler_, ScheduleDownloadTask(criteria))
EXPECT_CALL(task_scheduler_,
ScheduleTask(DownloadTaskType::DOWNLOAD_TASK,
criteria.requires_unmetered_network,
criteria.requires_battery_charging, _, _))
.RetiresOnSaturation();
scheduler_->Reschedule(entries());
}
......
......@@ -27,6 +27,8 @@ class TaskScheduler {
// Cancels a pre-scheduled task of type |task_type|.
virtual void CancelTask(DownloadTaskType task_type) = 0;
virtual ~TaskScheduler() {}
};
} // namespace download
......
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