TaskScheduler: Add TaskRunnerHandle support to TaskScheduler tasks

base/task_scheduler/task.h

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef BASE_TASK_SCHEDULER_TASK_H_
 #define BASE_TASK_SCHEDULER_TASK_H_
 
 #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"
 
 namespace base {
 namespace internal {
 
 // A task is a unit of work inside the task scheduler. Support for tracing and
 // profiling inherited from PendingTask.
 struct BASE_EXPORT Task : public PendingTask {
   // |posted_from| is the site the task was posted from. |task| is the closure
   // to run. |traits| is metadata about the task. |delayed_run_time| is the time
   // at which the task should be run (null TimeTicks if the task can run
   // immediately).
   Task(const tracked_objects::Location& posted_from,
        const Closure& task,
        const TaskTraits& traits,
        const TimeTicks& delayed_run_time);
   ~Task();
 
   // The TaskTraits of this task.
   const TaskTraits traits;
 
   // The time at which the task was inserted in its sequence. For an undelayed
   // task, this happens at post time. For a delayed task, this happens some
   // 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
 }  // namespace base
 
 #endif  // BASE_TASK_SCHEDULER_TASK_H_