Support FileDescriptorWatcher in TaskScheduler.

base/task_scheduler/task_tracker.h

Index: base/task_scheduler/task_tracker.h
diff --git a/base/task_scheduler/task_tracker.h b/base/task_scheduler/task_tracker.h
index 8c19456e8772ed04407d5b078f0aad8753b45f37..5fdc50c6979779be0fa5cc27467e58b3b884ff18 100644
--- a/base/task_scheduler/task_tracker.h
+++ b/base/task_scheduler/task_tracker.h
@@ -17,20 +17,29 @@
 #include "base/task_scheduler/sequence.h"
 #include "base/task_scheduler/task.h"
 #include "base/task_scheduler/task_traits.h"
+#include "build/build_config.h"
 
 namespace base {
 
 class ConditionVariable;
+class MessageLoopForIO;
 class SequenceToken;
 
 namespace internal {
 
 // All tasks go through the scheduler's TaskTracker when they are posted and
-// when they are executed. The TaskTracker enforces shutdown semantics and takes
-// care of tracing and profiling. This class is thread-safe.
+// when they are executed. The TaskTracker sets up the environment to run a
+// task, enforces shutdown semantics and takes care of tracing and profiling.
+// This class is thread-safe.
 class BASE_EXPORT TaskTracker {
  public:
-  TaskTracker();
+  // |watch_file_descriptor_message_loop| is used to initialize a
+  // FileDescriptorWatcher in the scope in which a Task runs.
+  TaskTracker(
+#if defined(OS_POSIX) && !defined(OS_NACL_SFI)
+      MessageLoopForIO* watch_file_descriptor_message_loop

[gab, 2016/10/18 14:40:43]

It feels a bit intrusive to add this directly to TaskTracker. To me TaskTracker
is a no-op place used for instrumentation/metrics/tracking/etc, not logic.

What do you think of adding TaskTracker::TaskObserver instead which can be
registered to be called before/after a task runs (with its TaskTraits say)? This
isn't scary like your typical observer IMO because you still need an instance of
TaskTracker which means the task_scheduler code still controls who gets to be
able to register observers.

It also gets rid of all if-defs (which are also a sign to me that it doesn't
belong here) but the single callsite that will register.

And it probably makes tests cleaner as well by not coupling the need for a
|watch_file_descriptor_message_loop_| in production with the TaskTracker logic
(the need to adapt every test is another sign IMO that this is the wrong place).

[fdoray, 2016/10/18 15:30:43]

We already register TaskRunner handles from TaskTracker::RunTask. I think it's a
good thing to put all the work to set up the environment to run a task in
TaskTracker::RunTask because that method could easily be reused from a
SchedulerWorkerPool impl backed by the Windows thread pool.
The environment set up code can still be reused if it's moved to a TaskObserver
invoked by TaskTracker::RunTask. However, we loose the ability to instantiate
objects on the stack for the scope of the task.

i.e.
FileDescriptorWatcher file_descriptor_watcher(message_loop);
becomes
void BeforeRunTask() {
  // Heap allocation :(
  file_descriptor_watcher_ = MakeUnique<FileDescriptorWatcher>();
}
void AfterRunTask() {
  // Extra line that we could forget to add :(
  file_descriptor_watcher_.reset();
}

Instead of having a TaskObserver, WDYT of a TaskTracker::Delegate:

class TaskTracker::Delegate {
 public:
  // Must run the callback in |task|. May perform additional
  // work before and after running the callback.
  void RunTask(std::unique_ptr<Task> task) = 0;
};

Keeping track of the number of tasks blocking shutdown is handled by
TaskTracker. The delegate handles setting up the environment to run the task and
tracing [1].
I agree.

[1] Since tracing and running the task is done by TaskAnnotator::RunTask, it's
would be hard to keep tracing in TaskTracker.

+#endif
+      );
   ~TaskTracker();
 
   // Synchronously shuts down the scheduler. Once this is called, only tasks
@@ -131,6 +140,10 @@ class BASE_EXPORT TaskTracker {
   // Number of BLOCK_SHUTDOWN tasks posted during shutdown.
   HistogramBase::Sample num_block_shutdown_tasks_posted_during_shutdown_ = 0;
 
+#if defined(OS_POSIX) && !defined(OS_NACL_SFI)
+  MessageLoopForIO* const watch_file_descriptor_message_loop_;
+#endif
+
   DISALLOW_COPY_AND_ASSIGN(TaskTracker);
 };