TaskScheduler: Add TaskRunnerHandle support to TaskScheduler tasks

base/task_scheduler/task.h

Index: base/task_scheduler/task.h
diff --git a/base/task_scheduler/task.h b/base/task_scheduler/task.h
index dcdda00d45d186dc97a792b865b8ace726d0fcab..184ca2e3521a1b457b55b6b268814d196d9b53c7 100644
--- a/base/task_scheduler/task.h
+++ b/base/task_scheduler/task.h
@@ -8,7 +8,11 @@
 #include "base/base_export.h"
 #include "base/callback_forward.h"
 #include "base/location.h"
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
 #include "base/pending_task.h"
+#include "base/sequenced_task_runner.h"
+#include "base/single_thread_task_runner.h"
 #include "base/task_scheduler/task_traits.h"
 #include "base/time/time.h"
 
@@ -36,6 +40,17 @@ struct BASE_EXPORT Task : public PendingTask {
   // time after the task's delay has expired. If the task hasn't been inserted
   // in a sequence yet, this defaults to a null TimeTicks.
   TimeTicks sequenced_time;
+
+  // A reference to the SequencedTaskRunner or SingleThreadedTaskRunner that
+  // posted this task, if any. Used to set ThreadTaskRunnerHandle and/or
+  // SequencedTaskRunnerHandle while the task is running.
+  scoped_refptr<SequencedTaskRunner> sequenced_task_runner_ref;
+  scoped_refptr<SingleThreadTaskRunner> single_thread_task_runner_ref;

[fdoray, 2016/04/22 14:43:12]

Should we comment about the ownership cycle that this creates?
Sequence -> Task -> TaskRunner -> Sequence -> ...

We must be careful when we drop a reference to a non-empty Sequence.

[gab, 2016/04/25 18:31:52]

Done, this cycle is okay (and in fact required) as I tried to explain in the
added comment.
Why? This would only happen if a TaskRunner goes away while its Sequence is
non-empty and this is precisely why this cycle is required. I don't see what we
have to be careful about?

[fdoray, 2016/04/25 18:53:27]

We could easily make a change that creates a memory leak (e.g. add an API to
cancel a Sequence, which releases a ref without emptying the Sequence first). It
would be nice to DCHECK_GT(number of refs, number of Tasks in Sequence) whenever
a ref is released, but RefCountedThreadSafe doesn't give us access to the number
of refs.

[gab, 2016/04/26 11:54:57]

Ah I see, shall we add a DCHECK(IsEmpty()) in Sequence's destructor?

[gab, 2016/04/26 21:20:20]

ping to keep this question on our mind, WDYT?

[fdoray, 2016/04/27 18:11:20]

It's a good idea, but it doesn't solve the problem. I think the best we can do
is to warn people about the cycle + maybe add a comment that says that a
non-empty Sequence must always be owned by a worker thread or a PQ (it's not ok
if the only scoped_refptr to a non-empty Sequence is in a TaskRunner).

[gab, 2016/04/27 20:21:40]

Ah right, that's not such a great idea actually since it merely documents that
Tasks will hold on to the Sequence but wouldn't fire in the case we're worried
about.
Added documentation to sequence.h (it's sad to mention implementations details
in that header, but I don't see where else to put that comment that would make
sure people reading code relevant to this would see it).

Let me know what you think :-)

+
+ private:
+  // Disallow copies to make sure no unecessary ref-bumps are incurred. Making
+  // it move-only would be an option, but isn't necessary for now.
+  DISALLOW_COPY_AND_ASSIGN(Task);
 };
 
 }  // namespace internal