Pass Callback to TaskRunner by value and consume it on invocation (1)

base/threading/sequenced_worker_pool.cc

Index: base/threading/sequenced_worker_pool.cc
diff --git a/base/threading/sequenced_worker_pool.cc b/base/threading/sequenced_worker_pool.cc
index badd2936ee2a70b964b5d047c85a5c8ce0861925..9de93a33eba7fb635c11994c7a9d3022b26bbb48 100644
--- a/base/threading/sequenced_worker_pool.cc
+++ b/base/threading/sequenced_worker_pool.cc
@@ -16,6 +16,7 @@
 
 #include "base/atomic_sequence_num.h"
 #include "base/callback.h"
+#include "base/callback_helpers.h"
 #include "base/compiler_specific.h"
 #include "base/critical_closure.h"
 #include "base/debug/dump_without_crashing.h"
@@ -140,7 +141,7 @@ class SequencedWorkerPoolTaskRunner : public TaskRunner {
 
   // TaskRunner implementation
   bool PostDelayedTask(const tracked_objects::Location& from_here,
-                       const Closure& task,
+                       Closure task,
                        TimeDelta delay) override;
   bool RunsTasksOnCurrentThread() const override;
 
@@ -164,13 +165,13 @@ SequencedWorkerPoolTaskRunner::~SequencedWorkerPoolTaskRunner() {
 
 bool SequencedWorkerPoolTaskRunner::PostDelayedTask(
     const tracked_objects::Location& from_here,
-    const Closure& task,
+    Closure task,
     TimeDelta delay) {
   if (delay.is_zero()) {
-    return pool_->PostWorkerTaskWithShutdownBehavior(
-        from_here, task, shutdown_behavior_);
+    return pool_->PostWorkerTaskWithShutdownBehavior(from_here, std::move(task),
+                                                     shutdown_behavior_);
   }
-  return pool_->PostDelayedWorkerTask(from_here, task, delay);
+  return pool_->PostDelayedWorkerTask(from_here, std::move(task), delay);
 }
 
 bool SequencedWorkerPoolTaskRunner::RunsTasksOnCurrentThread() const {
@@ -194,13 +195,13 @@ class SequencedWorkerPool::PoolSequencedTaskRunner
 
   // TaskRunner implementation
   bool PostDelayedTask(const tracked_objects::Location& from_here,
-                       const Closure& task,
+                       Closure task,
                        TimeDelta delay) override;
   bool RunsTasksOnCurrentThread() const override;
 
   // SequencedTaskRunner implementation
   bool PostNonNestableDelayedTask(const tracked_objects::Location& from_here,
-                                  const Closure& task,
+                                  Closure task,
                                   TimeDelta delay) override;
 
  private:
@@ -227,15 +228,16 @@ SequencedWorkerPool::PoolSequencedTaskRunner::
 SequencedWorkerPool::PoolSequencedTaskRunner::
     ~PoolSequencedTaskRunner() = default;
 
-bool SequencedWorkerPool::PoolSequencedTaskRunner::
-    PostDelayedTask(const tracked_objects::Location& from_here,
-                    const Closure& task,
-                    TimeDelta delay) {
+bool SequencedWorkerPool::PoolSequencedTaskRunner::PostDelayedTask(
+    const tracked_objects::Location& from_here,
+    Closure task,
+    TimeDelta delay) {
   if (delay.is_zero()) {
     return pool_->PostSequencedWorkerTaskWithShutdownBehavior(
-        token_, from_here, task, shutdown_behavior_);
+        token_, from_here, std::move(task), shutdown_behavior_);
   }
-  return pool_->PostDelayedSequencedWorkerTask(token_, from_here, task, delay);
+  return pool_->PostDelayedSequencedWorkerTask(token_, from_here,
+                                               std::move(task), delay);
 }
 
 bool SequencedWorkerPool::PoolSequencedTaskRunner::
@@ -243,13 +245,13 @@ bool SequencedWorkerPool::PoolSequencedTaskRunner::
   return pool_->IsRunningSequenceOnCurrentThread(token_);
 }
 
-bool SequencedWorkerPool::PoolSequencedTaskRunner::
-    PostNonNestableDelayedTask(const tracked_objects::Location& from_here,
-                               const Closure& task,
-                               TimeDelta delay) {
+bool SequencedWorkerPool::PoolSequencedTaskRunner::PostNonNestableDelayedTask(
+    const tracked_objects::Location& from_here,
+    Closure task,
+    TimeDelta delay) {
   // There's no way to run nested tasks, so simply forward to
   // PostDelayedTask.
-  return PostDelayedTask(from_here, task, delay);
+  return PostDelayedTask(from_here, std::move(task), delay);
 }
 
 // Worker ---------------------------------------------------------------------
@@ -348,7 +350,7 @@ class SequencedWorkerPool::Inner {
                 SequenceToken sequence_token,
                 WorkerShutdown shutdown_behavior,
                 const tracked_objects::Location& from_here,
-                const Closure& task,
+                Closure task,
                 TimeDelta delay);
 
   bool RunsTasksOnCurrentThread() const;
@@ -692,7 +694,7 @@ bool SequencedWorkerPool::Inner::PostTask(
     SequenceToken sequence_token,
     WorkerShutdown shutdown_behavior,
     const tracked_objects::Location& from_here,
-    const Closure& task,
+    Closure task,
     TimeDelta delay) {
   // TODO(fdoray): Uncomment this DCHECK. It is initially commented to avoid a
   // revert of the CL that adds debug::DumpWithoutCrashing() if it fails on the
@@ -706,9 +708,9 @@ bool SequencedWorkerPool::Inner::PostTask(
   sequenced.sequence_token_id = sequence_token.id_;
   sequenced.shutdown_behavior = shutdown_behavior;
   sequenced.posted_from = from_here;
-  sequenced.task =
-      shutdown_behavior == BLOCK_SHUTDOWN ?
-      base::MakeCriticalClosure(task) : task;
+  sequenced.task = shutdown_behavior == BLOCK_SHUTDOWN
+                       ? base::MakeCriticalClosure(std::move(task))
+                       : std::move(task);
   sequenced.time_to_run = TimeTicks::Now() + delay;
 
   int create_thread_id = 0;
@@ -1018,7 +1020,7 @@ void SequencedWorkerPool::Inner::ThreadLoop(Worker* this_worker) {
 
           tracked_objects::TaskStopwatch stopwatch;
           stopwatch.Start();
-          task.task.Run();
+          base::ResetAndReturn(&task.task).Run();

[gab, 2017/03/15 19:13:42]

What's the difference between this and std::move(task.task).Run()? The
documentation on base::ResetAndReturn doesn't make it obvious why this should be
preferred, should it? Should the documentation be updated?

[tzik, 2017/03/21 05:43:20]

std::move(cb).Run() for Callback and OnceCallback behaves differently until
https://codereview.chromium.org/2753953003/ lands.
It doesn't clear |cb| if it's Callback, and does clear it if it's OnceCallback.

[tzik, 2017/03/22 08:52:03]

And it landed. Updated the CL to replace ResetAndReturn with std::move.

           stopwatch.Stop();
 
           tracked_objects::ThreadData::TallyRunOnNamedThreadIfTracking(
@@ -1029,7 +1031,7 @@ void SequencedWorkerPool::Inner::ThreadLoop(Worker* this_worker) {
           // Also, do it before calling reset_running_task_info() so
           // that sequence-checking from within the task's destructor
           // still works.
-          task.task = Closure();
+          DCHECK(!task.task);
 
           this_worker->reset_running_task_info();
         }
@@ -1532,71 +1534,71 @@ SequencedWorkerPool::GetTaskRunnerWithShutdownBehavior(
 
 bool SequencedWorkerPool::PostWorkerTask(
     const tracked_objects::Location& from_here,
-    const Closure& task) {
-  return inner_->PostTask(NULL, SequenceToken(), BLOCK_SHUTDOWN,
-                          from_here, task, TimeDelta());
+    Closure task) {
+  return inner_->PostTask(NULL, SequenceToken(), BLOCK_SHUTDOWN, from_here,
+                          std::move(task), TimeDelta());
 }
 
 bool SequencedWorkerPool::PostDelayedWorkerTask(
     const tracked_objects::Location& from_here,
-    const Closure& task,
+    Closure task,
     TimeDelta delay) {
   WorkerShutdown shutdown_behavior =
       delay.is_zero() ? BLOCK_SHUTDOWN : SKIP_ON_SHUTDOWN;
-  return inner_->PostTask(NULL, SequenceToken(), shutdown_behavior,
-                          from_here, task, delay);
+  return inner_->PostTask(NULL, SequenceToken(), shutdown_behavior, from_here,
+                          std::move(task), delay);
 }
 
 bool SequencedWorkerPool::PostWorkerTaskWithShutdownBehavior(
     const tracked_objects::Location& from_here,
-    const Closure& task,
+    Closure task,
     WorkerShutdown shutdown_behavior) {
-  return inner_->PostTask(NULL, SequenceToken(), shutdown_behavior,
-                          from_here, task, TimeDelta());
+  return inner_->PostTask(NULL, SequenceToken(), shutdown_behavior, from_here,
+                          std::move(task), TimeDelta());
 }
 
 bool SequencedWorkerPool::PostSequencedWorkerTask(
     SequenceToken sequence_token,
     const tracked_objects::Location& from_here,
-    const Closure& task) {
-  return inner_->PostTask(NULL, sequence_token, BLOCK_SHUTDOWN,
-                          from_here, task, TimeDelta());
+    Closure task) {
+  return inner_->PostTask(NULL, sequence_token, BLOCK_SHUTDOWN, from_here,
+                          std::move(task), TimeDelta());
 }
 
 bool SequencedWorkerPool::PostDelayedSequencedWorkerTask(
     SequenceToken sequence_token,
     const tracked_objects::Location& from_here,
-    const Closure& task,
+    Closure task,
     TimeDelta delay) {
   WorkerShutdown shutdown_behavior =
       delay.is_zero() ? BLOCK_SHUTDOWN : SKIP_ON_SHUTDOWN;
-  return inner_->PostTask(NULL, sequence_token, shutdown_behavior,
-                          from_here, task, delay);
+  return inner_->PostTask(NULL, sequence_token, shutdown_behavior, from_here,
+                          std::move(task), delay);
 }
 
 bool SequencedWorkerPool::PostNamedSequencedWorkerTask(
     const std::string& token_name,
     const tracked_objects::Location& from_here,
-    const Closure& task) {
+    Closure task) {
   DCHECK(!token_name.empty());
   return inner_->PostTask(&token_name, SequenceToken(), BLOCK_SHUTDOWN,
-                          from_here, task, TimeDelta());
+                          from_here, std::move(task), TimeDelta());
 }
 
 bool SequencedWorkerPool::PostSequencedWorkerTaskWithShutdownBehavior(
     SequenceToken sequence_token,
     const tracked_objects::Location& from_here,
-    const Closure& task,
+    Closure task,
     WorkerShutdown shutdown_behavior) {
-  return inner_->PostTask(NULL, sequence_token, shutdown_behavior,
-                          from_here, task, TimeDelta());
+  return inner_->PostTask(NULL, sequence_token, shutdown_behavior, from_here,
+                          std::move(task), TimeDelta());
 }
 
 bool SequencedWorkerPool::PostDelayedTask(
     const tracked_objects::Location& from_here,
-    const Closure& task,
+    Closure task,
     TimeDelta delay) {
-  return PostDelayedWorkerTask(from_here, task, delay);
+  return PostDelayedWorkerTask(from_here, std::move(task), delay);
 }
 
 bool SequencedWorkerPool::RunsTasksOnCurrentThread() const {