Commit 1b5841fa authored by Thomas Guilbert's avatar Thomas Guilbert Committed by Chromium LUCI CQ

Convert SyncTaskManager::Task to OnceCallback

This CL converts SyncTaskManager::Task and SyncTaskManager::Continuation
to OnceCallbacks. Since some of the closures are stored in a priority
queue (which doesn't support move-only types), they are wrapped with
base::AdaptCallbackForRepeating.

There is also an opportunistic deletion of some dead code, and renaming
a test callback to something more useful.

Bug: 1152272
Change-Id: I39fd928029e215b118f0c09cbaf84780862197cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2576443
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Auto-Submit: Thomas Guilbert <tguilbert@chromium.org>
Reviewed-by: default avatarVictor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835082}
parent 1b6aee66
......@@ -41,8 +41,8 @@ void ConflictResolver::RunPreflight(std::unique_ptr<SyncTaskToken> token) {
task_blocker->exclusive = true;
SyncTaskManager::UpdateTaskBlocker(
std::move(token), std::move(task_blocker),
base::Bind(&ConflictResolver::RunExclusive,
weak_ptr_factory_.GetWeakPtr()));
base::BindOnce(&ConflictResolver::RunExclusive,
weak_ptr_factory_.GetWeakPtr()));
}
void ConflictResolver::RunExclusive(std::unique_ptr<SyncTaskToken> token) {
......
......@@ -65,19 +65,6 @@ const typename Container::mapped_type& LookUpMap(
return found->second;
}
template <typename R, typename S, typename T>
R ComposeFunction(const base::Callback<T()>& g,
const base::Callback<R(S)>& f) {
return f.Run(g.Run());
}
template <typename R, typename S, typename T>
base::Callback<R()> CreateComposedFunction(
const base::Callback<T()>& g,
const base::Callback<R(S)>& f) {
return base::Bind(&ComposeFunction<R, S, T>, g, f);
}
} // namespace drive_backend
} // namespace sync_file_system
......
......@@ -43,9 +43,9 @@ void ListChangesTask::RunPreflight(std::unique_ptr<SyncTaskToken> token) {
}
SyncTaskManager::UpdateTaskBlocker(
std::move(token), std::unique_ptr<TaskBlocker>(new TaskBlocker),
base::Bind(&ListChangesTask::StartListing,
weak_ptr_factory_.GetWeakPtr()));
std::move(token), std::make_unique<TaskBlocker>(),
base::BindOnce(&ListChangesTask::StartListing,
weak_ptr_factory_.GetWeakPtr()));
}
void ListChangesTask::StartListing(std::unique_ptr<SyncTaskToken> token) {
......@@ -107,7 +107,7 @@ void ListChangesTask::DidListChanges(
return;
}
std::unique_ptr<TaskBlocker> task_blocker(new TaskBlocker);
auto task_blocker = std::make_unique<TaskBlocker>();
task_blocker->exclusive = true;
SyncTaskManager::UpdateTaskBlocker(
std::move(token), std::move(task_blocker),
......
......@@ -276,8 +276,8 @@ void LocalToRemoteSyncer::MoveToBackground(
// After the invocation of ContinueAsBackgroundTask
SyncTaskManager::UpdateTaskBlocker(
std::move(token), std::move(blocker),
base::Bind(&LocalToRemoteSyncer::ContinueAsBackgroundTask,
weak_ptr_factory_.GetWeakPtr(), continuation));
base::BindOnce(&LocalToRemoteSyncer::ContinueAsBackgroundTask,
weak_ptr_factory_.GetWeakPtr(), continuation));
}
void LocalToRemoteSyncer::ContinueAsBackgroundTask(
......
......@@ -312,7 +312,7 @@ void RemoteToLocalSyncer::ResolveRemoteChange(
}
void RemoteToLocalSyncer::MoveToBackground(std::unique_ptr<SyncTaskToken> token,
const Continuation& continuation) {
Continuation continuation) {
DCHECK(dirty_tracker_);
std::unique_ptr<TaskBlocker> blocker(new TaskBlocker);
......@@ -324,12 +324,12 @@ void RemoteToLocalSyncer::MoveToBackground(std::unique_ptr<SyncTaskToken> token,
SyncTaskManager::UpdateTaskBlocker(
std::move(token), std::move(blocker),
base::Bind(&RemoteToLocalSyncer::ContinueAsBackgroundTask,
weak_ptr_factory_.GetWeakPtr(), continuation));
base::BindOnce(&RemoteToLocalSyncer::ContinueAsBackgroundTask,
weak_ptr_factory_.GetWeakPtr(), std::move(continuation)));
}
void RemoteToLocalSyncer::ContinueAsBackgroundTask(
const Continuation& continuation,
Continuation continuation,
std::unique_ptr<SyncTaskToken> token) {
DCHECK(dirty_tracker_);
......@@ -391,7 +391,7 @@ void RemoteToLocalSyncer::ContinueAsBackgroundTask(
return;
}
}
continuation.Run(std::move(token));
std::move(continuation).Run(std::move(token));
}
void RemoteToLocalSyncer::HandleMissingRemoteMetadata(
......
......@@ -106,8 +106,8 @@ class RemoteToLocalSyncer : public SyncTask {
void ResolveRemoteChange(std::unique_ptr<SyncTaskToken> token);
void MoveToBackground(std::unique_ptr<SyncTaskToken> token,
const Continuation& continuation);
void ContinueAsBackgroundTask(const Continuation& continuation,
Continuation continuation);
void ContinueAsBackgroundTask(Continuation continuation,
std::unique_ptr<SyncTaskToken> token);
// Handles missing remote metadata case.
......
......@@ -8,6 +8,7 @@
#include <utility>
#include "base/bind.h"
#include "base/callback_helpers.h"
#include "base/location.h"
#include "base/logging.h"
#include "base/macros.h"
......@@ -25,11 +26,12 @@ namespace {
class SyncTaskAdapter : public ExclusiveTask {
public:
explicit SyncTaskAdapter(const SyncTaskManager::Task& task) : task_(task) {}
~SyncTaskAdapter() override {}
explicit SyncTaskAdapter(SyncTaskManager::Task task)
: task_(std::move(task)) {}
~SyncTaskAdapter() override = default;
void RunExclusive(const SyncStatusCallback& callback) override {
task_.Run(callback);
std::move(task_).Run(callback);
}
private:
......@@ -40,11 +42,14 @@ class SyncTaskAdapter : public ExclusiveTask {
} // namespace
SyncTaskManager::PendingTask::PendingTask() {}
SyncTaskManager::PendingTask::PendingTask() = default;
SyncTaskManager::PendingTask::PendingTask(
const base::Closure& task, Priority pri, int seq)
: task(task), priority(pri), seq(seq) {}
SyncTaskManager::PendingTask::PendingTask(base::OnceClosure task,
Priority pri,
int seq)
: wrapped_once_closure(base::AdaptCallbackForRepeating(std::move(task))),
priority(pri),
seq(seq) {}
SyncTaskManager::PendingTask::PendingTask(const PendingTask& other) = default;
......@@ -85,14 +90,14 @@ void SyncTaskManager::Initialize(SyncStatusCode status) {
}
void SyncTaskManager::ScheduleTask(const base::Location& from_here,
const Task& task,
Task task,
Priority priority,
const SyncStatusCallback& callback) {
DCHECK(sequence_checker_.CalledOnValidSequence());
ScheduleSyncTask(from_here,
std::unique_ptr<SyncTask>(new SyncTaskAdapter(task)),
priority, callback);
std::make_unique<SyncTaskAdapter>(std::move(task)), priority,
callback);
}
void SyncTaskManager::ScheduleSyncTask(const base::Location& from_here,
......@@ -103,24 +108,22 @@ void SyncTaskManager::ScheduleSyncTask(const base::Location& from_here,
std::unique_ptr<SyncTaskToken> token(GetToken(from_here, callback));
if (!token) {
PushPendingTask(
base::Bind(&SyncTaskManager::ScheduleSyncTask,
weak_ptr_factory_.GetWeakPtr(), from_here,
base::Passed(&task), priority, callback),
priority);
PushPendingTask(base::BindOnce(&SyncTaskManager::ScheduleSyncTask,
weak_ptr_factory_.GetWeakPtr(), from_here,
std::move(task), priority, callback),
priority);
return;
}
RunTask(std::move(token), std::move(task));
}
bool SyncTaskManager::ScheduleTaskIfIdle(const base::Location& from_here,
const Task& task,
Task task,
const SyncStatusCallback& callback) {
DCHECK(sequence_checker_.CalledOnValidSequence());
return ScheduleSyncTaskIfIdle(
from_here, std::unique_ptr<SyncTask>(new SyncTaskAdapter(task)),
callback);
from_here, std::make_unique<SyncTaskAdapter>(std::move(task)), callback);
}
bool SyncTaskManager::ScheduleSyncTaskIfIdle(
......@@ -158,13 +161,13 @@ void SyncTaskManager::NotifyTaskDone(std::unique_ptr<SyncTaskToken> token,
void SyncTaskManager::UpdateTaskBlocker(
std::unique_ptr<SyncTaskToken> current_task_token,
std::unique_ptr<TaskBlocker> task_blocker,
const Continuation& continuation) {
Continuation continuation) {
DCHECK(current_task_token);
SyncTaskManager* manager = current_task_token->manager();
if (current_task_token->token_id() == SyncTaskToken::kTestingTaskTokenID) {
DCHECK(!manager);
continuation.Run(std::move(current_task_token));
std::move(continuation).Run(std::move(current_task_token));
return;
}
......@@ -182,7 +185,7 @@ void SyncTaskManager::UpdateTaskBlocker(
manager->UpdateTaskBlockerBody(
std::move(foreground_task_token), std::move(background_task_token),
std::move(task_log), std::move(task_blocker), continuation);
std::move(task_log), std::move(task_blocker), std::move(continuation));
}
bool SyncTaskManager::IsRunningTask(int64_t token_id) const {
......@@ -261,7 +264,7 @@ void SyncTaskManager::UpdateTaskBlockerBody(
std::unique_ptr<SyncTaskToken> background_task_token,
std::unique_ptr<TaskLogger::TaskLog> task_log,
std::unique_ptr<TaskBlocker> task_blocker,
const Continuation& continuation) {
Continuation continuation) {
DCHECK(sequence_checker_.CalledOnValidSequence());
// Run the task directly if the parallelization is disabled.
......@@ -269,7 +272,7 @@ void SyncTaskManager::UpdateTaskBlockerBody(
DCHECK(foreground_task_token);
DCHECK(!background_task_token);
foreground_task_token->SetTaskLog(std::move(task_log));
continuation.Run(std::move(foreground_task_token));
std::move(continuation).Run(std::move(foreground_task_token));
return;
}
......@@ -288,13 +291,11 @@ void SyncTaskManager::UpdateTaskBlockerBody(
SyncStatusCallback());
if (!foreground_task_token) {
PushPendingTask(
base::Bind(&SyncTaskManager::UpdateTaskBlockerBody,
weak_ptr_factory_.GetWeakPtr(),
base::Passed(&foreground_task_token),
base::Passed(&background_task_token),
base::Passed(&task_log),
base::Passed(&task_blocker),
continuation),
base::BindOnce(&SyncTaskManager::UpdateTaskBlockerBody,
weak_ptr_factory_.GetWeakPtr(),
std::move(foreground_task_token),
std::move(background_task_token), std::move(task_log),
std::move(task_blocker), std::move(continuation)),
PRIORITY_HIGH);
MaybeStartNextForegroundTask(nullptr);
return;
......@@ -313,14 +314,10 @@ void SyncTaskManager::UpdateTaskBlockerBody(
DCHECK(pending_backgrounding_task_.is_null());
// Wait for NotifyTaskDone to release a |task_blocker|.
pending_backgrounding_task_ =
base::Bind(&SyncTaskManager::UpdateTaskBlockerBody,
weak_ptr_factory_.GetWeakPtr(),
base::Passed(&foreground_task_token),
base::Passed(&background_task_token),
base::Passed(&task_log),
base::Passed(&task_blocker),
continuation);
pending_backgrounding_task_ = base::BindOnce(
&SyncTaskManager::UpdateTaskBlockerBody, weak_ptr_factory_.GetWeakPtr(),
std::move(foreground_task_token), std::move(background_task_token),
std::move(task_log), std::move(task_blocker), std::move(continuation));
return;
}
......@@ -342,7 +339,7 @@ void SyncTaskManager::UpdateTaskBlockerBody(
token_ = std::move(foreground_task_token);
MaybeStartNextForegroundTask(nullptr);
background_task_token->SetTaskLog(std::move(task_log));
continuation.Run(std::move(background_task_token));
std::move(continuation).Run(std::move(background_task_token));
}
std::unique_ptr<SyncTaskToken> SyncTaskManager::GetToken(
......@@ -356,11 +353,12 @@ std::unique_ptr<SyncTaskToken> SyncTaskManager::GetToken(
return std::move(token_);
}
void SyncTaskManager::PushPendingTask(
const base::Closure& closure, Priority priority) {
void SyncTaskManager::PushPendingTask(base::OnceClosure closure,
Priority priority) {
DCHECK(sequence_checker_.CalledOnValidSequence());
pending_tasks_.push(PendingTask(closure, priority, pending_task_seq_++));
pending_tasks_.push(
PendingTask(std::move(closure), priority, pending_task_seq_++));
}
void SyncTaskManager::RunTask(std::unique_ptr<SyncTaskToken> token,
......@@ -382,9 +380,7 @@ void SyncTaskManager::MaybeStartNextForegroundTask(
}
if (!pending_backgrounding_task_.is_null()) {
base::Closure closure = pending_backgrounding_task_;
pending_backgrounding_task_.Reset();
closure.Run();
std::move(pending_backgrounding_task_).Run();
return;
}
......@@ -392,9 +388,9 @@ void SyncTaskManager::MaybeStartNextForegroundTask(
return;
if (!pending_tasks_.empty()) {
base::Closure closure = pending_tasks_.top().task;
base::RepeatingClosure closure = pending_tasks_.top().wrapped_once_closure;
pending_tasks_.pop();
closure.Run();
std::move(closure).Run();
return;
}
......
......@@ -43,9 +43,9 @@ struct TaskBlocker;
// doesn't block the new background task, and queues it up if it can't run.
class SyncTaskManager {
public:
typedef base::Callback<void(const SyncStatusCallback& callback)> Task;
typedef base::Callback<void(std::unique_ptr<SyncTaskToken> token)>
Continuation;
using Task = base::OnceCallback<void(const SyncStatusCallback& callback)>;
using Continuation =
base::OnceCallback<void(std::unique_ptr<SyncTaskToken> token)>;
enum Priority {
PRIORITY_LOW,
......@@ -83,7 +83,7 @@ class SyncTaskManager {
// Schedules a task at the given priority.
void ScheduleTask(const base::Location& from_here,
const Task& task,
Task task,
Priority priority,
const SyncStatusCallback& callback);
void ScheduleSyncTask(const base::Location& from_here,
......@@ -91,10 +91,10 @@ class SyncTaskManager {
Priority priority,
const SyncStatusCallback& callback);
// Runs the posted task only when we're idle. Returns true if tha task is
// Runs the posted task only when we're idle. Returns true if that task is
// scheduled.
bool ScheduleTaskIfIdle(const base::Location& from_here,
const Task& task,
Task task,
const SyncStatusCallback& callback);
bool ScheduleSyncTaskIfIdle(const base::Location& from_here,
std::unique_ptr<SyncTask> task,
......@@ -119,7 +119,7 @@ class SyncTaskManager {
static void UpdateTaskBlocker(
std::unique_ptr<SyncTaskToken> current_task_token,
std::unique_ptr<TaskBlocker> task_blocker,
const Continuation& continuation);
Continuation continuation);
bool IsRunningTask(int64_t task_token_id) const;
......@@ -127,12 +127,15 @@ class SyncTaskManager {
private:
struct PendingTask {
base::Closure task;
// TODO: Change |wrapped_once_closure| to base::OnceTask if
// std::priority_queue supports move-only type. In the meantime, we can
// wrap a base::OnceClosure via AdaptCallbackForRepeating.
base::RepeatingClosure wrapped_once_closure;
Priority priority;
int64_t seq;
PendingTask();
PendingTask(const base::Closure& task, Priority pri, int seq);
PendingTask(base::OnceClosure task, Priority pri, int seq);
PendingTask(const PendingTask& other);
~PendingTask();
};
......@@ -152,7 +155,7 @@ class SyncTaskManager {
std::unique_ptr<SyncTaskToken> background_task_token,
std::unique_ptr<TaskLogger::TaskLog> task_log,
std::unique_ptr<TaskBlocker> task_blocker,
const Continuation& continuation);
Continuation continuation);
// This should be called when an async task needs to get a task token.
std::unique_ptr<SyncTaskToken> GetToken(const base::Location& from_here,
......@@ -163,7 +166,7 @@ class SyncTaskManager {
const SyncStatusCallback& callback,
std::unique_ptr<TaskBlocker> task_blocker);
void PushPendingTask(const base::Closure& closure, Priority priority);
void PushPendingTask(base::OnceClosure closure, Priority priority);
void RunTask(std::unique_ptr<SyncTaskToken> token,
std::unique_ptr<SyncTask> task);
......@@ -185,7 +188,7 @@ class SyncTaskManager {
size_t maximum_background_task_;
// Holds pending continuation to move task to background.
base::Closure pending_backgrounding_task_;
base::OnceClosure pending_backgrounding_task_;
std::priority_queue<PendingTask, std::vector<PendingTask>,
PendingTaskComparator> pending_tasks_;
......
......@@ -32,8 +32,8 @@ namespace drive_backend {
namespace {
void DumbTask(SyncStatusCode status,
const SyncStatusCallback& callback) {
void PostCallbackTask(SyncStatusCode status,
const SyncStatusCallback& callback) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(callback, status));
}
......@@ -78,17 +78,16 @@ class TaskManagerClient
const SyncStatusCallback& callback) {
task_manager_->ScheduleTask(
FROM_HERE,
base::Bind(&TaskManagerClient::DoTask, AsWeakPtr(),
status_to_return, false /* idle */),
SyncTaskManager::PRIORITY_MED,
callback);
base::BindOnce(&TaskManagerClient::DoTask, AsWeakPtr(),
status_to_return, false /* idle */),
SyncTaskManager::PRIORITY_MED, callback);
}
void ScheduleTaskIfIdle(SyncStatusCode status_to_return) {
task_manager_->ScheduleTaskIfIdle(
FROM_HERE,
base::Bind(&TaskManagerClient::DoTask, AsWeakPtr(),
status_to_return, true /* idle */),
base::BindOnce(&TaskManagerClient::DoTask, AsWeakPtr(),
status_to_return, true /* idle */),
SyncStatusCallback());
}
......@@ -183,8 +182,8 @@ class BackgroundTask : public SyncTask {
SyncTaskManager::UpdateTaskBlocker(
std::move(token), std::move(task_blocker),
base::Bind(&BackgroundTask::RunAsBackgroundTask,
weak_ptr_factory_.GetWeakPtr()));
base::BindOnce(&BackgroundTask::RunAsBackgroundTask,
weak_ptr_factory_.GetWeakPtr()));
}
private:
......@@ -254,8 +253,8 @@ class BlockerUpdateTestHelper : public SyncTask {
SyncTaskManager::UpdateTaskBlocker(
std::move(token), std::move(task_blocker),
base::Bind(&BlockerUpdateTestHelper::UpdateBlockerSoon,
weak_ptr_factory_.GetWeakPtr(), updating_to));
base::BindOnce(&BlockerUpdateTestHelper::UpdateBlockerSoon,
weak_ptr_factory_.GetWeakPtr(), updating_to));
}
void UpdateBlockerSoon(const std::string& updated_to,
......@@ -293,8 +292,7 @@ TEST(SyncTaskManagerTest, ScheduleTask) {
SyncStatusCode callback_status = SYNC_STATUS_OK;
client.ScheduleTask(kStatus1, base::Bind(&IncrementAndAssign, 0,
&callback_count,
&callback_status));
&callback_count, &callback_status));
base::RunLoop().RunUntilIdle();
EXPECT_EQ(kStatus1, callback_status);
......@@ -412,36 +410,31 @@ TEST(SyncTaskManagerTest, ScheduleTaskAtPriority) {
// This will run first even if its priority is low, since there're no
// pending tasks.
task_manager.ScheduleTask(
FROM_HERE,
base::Bind(&DumbTask, kStatus1),
FROM_HERE, base::BindOnce(&PostCallbackTask, kStatus1),
SyncTaskManager::PRIORITY_LOW,
base::Bind(&IncrementAndAssign, 0, &callback_count, &callback_status1));
// This runs last (expected counter == 4).
task_manager.ScheduleTask(
FROM_HERE,
base::Bind(&DumbTask, kStatus2),
FROM_HERE, base::BindOnce(&PostCallbackTask, kStatus2),
SyncTaskManager::PRIORITY_LOW,
base::Bind(&IncrementAndAssign, 4, &callback_count, &callback_status2));
// This runs second (expected counter == 1).
task_manager.ScheduleTask(
FROM_HERE,
base::Bind(&DumbTask, kStatus3),
FROM_HERE, base::BindOnce(&PostCallbackTask, kStatus3),
SyncTaskManager::PRIORITY_HIGH,
base::Bind(&IncrementAndAssign, 1, &callback_count, &callback_status3));
// This runs fourth (expected counter == 3).
task_manager.ScheduleTask(
FROM_HERE,
base::Bind(&DumbTask, kStatus4),
FROM_HERE, base::BindOnce(&PostCallbackTask, kStatus4),
SyncTaskManager::PRIORITY_MED,
base::Bind(&IncrementAndAssign, 3, &callback_count, &callback_status4));
// This runs third (expected counter == 2).
task_manager.ScheduleTask(
FROM_HERE,
base::Bind(&DumbTask, kStatus5),
FROM_HERE, base::BindOnce(&PostCallbackTask, kStatus5),
SyncTaskManager::PRIORITY_HIGH,
base::Bind(&IncrementAndAssign, 2, &callback_count, &callback_status5));
......
......@@ -421,9 +421,7 @@ void SyncFileSystemService::CheckIfIdle() {
if (idle_callback_.is_null())
return;
base::Closure callback = idle_callback_;
idle_callback_.Reset();
callback.Run();
std::move(idle_callback_).Run();
}
SyncServiceState SyncFileSystemService::GetSyncServiceState() {
......@@ -435,10 +433,9 @@ SyncFileSystemService* SyncFileSystemService::GetSyncService() {
return this;
}
void SyncFileSystemService::CallOnIdleForTesting(
const base::Closure& callback) {
void SyncFileSystemService::CallOnIdleForTesting(base::OnceClosure callback) {
DCHECK(idle_callback_.is_null());
idle_callback_ = callback;
idle_callback_ = std::move(callback);
CheckIfIdle();
}
......
......@@ -85,7 +85,7 @@ class SyncFileSystemService
TaskLogger* task_logger() { return &task_logger_; }
void CallOnIdleForTesting(const base::Closure& callback);
void CallOnIdleForTesting(base::OnceClosure callback);
private:
friend class SyncFileSystemServiceFactory;
......@@ -181,7 +181,7 @@ class SyncFileSystemService
base::ObserverList<SyncEventObserver>::Unchecked observers_;
bool promoting_demoted_changes_;
base::Closure idle_callback_;
base::OnceClosure idle_callback_;
DISALLOW_COPY_AND_ASSIGN(SyncFileSystemService);
};
......
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