TaskScheduler: Adjust shutdown behavior of delayed tasks.

base/task_scheduler/task.cc

Index: base/task_scheduler/task.cc
diff --git a/base/task_scheduler/task.cc b/base/task_scheduler/task.cc
index 8a589a202186dc2b3404c234e104a856ca57d8dd..2161769693b40ed695c970a40084e0fc391a0e02 100644
--- a/base/task_scheduler/task.cc
+++ b/base/task_scheduler/task.cc
@@ -9,13 +9,20 @@ namespace internal {
 
 Task::Task(const tracked_objects::Location& posted_from,
            const Closure& task,
-           const TaskTraits& traits,
+           const TaskTraits& traits_in,
            const TimeDelta& delay)
     : PendingTask(posted_from,
                   task,
                   delay.is_zero() ? TimeTicks() : TimeTicks::Now() + delay,
                   false),  // Not nestable.
-      traits(traits) {}
+      traits_in(traits) {
+  // Prevent a delayed BLOCK_SHUTDOWN task from blocking shutdown before being
+  // scheduled by changing its shutdown behavior to SKIP_ON_SHUTDOWN.
+  if (!delay.is_zero() &&
+      traits.shutdown_behavior() == TaskShutdownBehavior::BLOCK_SHUTDOWN) {
+    traits_in.WithShutdownBehavior(TaskShutdownBehavior::SKIP_ON_SHUTDOWN);

[robliao, 2016/10/03 17:16:35]

Should this be traits instead of traits_in?

Actually, I'm surprised this works given the const TaskTraits& traits_in.

Also add a TODO to DCHECK this once all the callers are cleaned up.

[fdoray, 2016/10/03 17:27:48]

I uploaded the wrong patch. This should be |traits| instead of |traits_in|.

Is the TODO still required since this code doesn't compile? It would also not
have passed unit tests if the const for |traits_in| had been removed to make the
code compile.

[robliao, 2016/10/03 17:30:12]

I'm meant a TODO that we're rewriting the shutdown behavior as skip on shutdown.
Long term, we don't want callers to fall into this category, right?

[fdoray, 2016/10/03 17:36:33]

From our chat discussion, I thought we wanted to allow callers to post delayed
tasks to a BLOCK_SHUTDOWN TaskRunner and automatically adjust shutdown behavior
to SKIP_ON_SHTUDOWN (i.e. we don't want to change call sites).

That's not the case?

[robliao, 2016/10/03 17:50:25]

Discussed offline: We'll go ahead and keep this long term and reassess later.

+  }
+}
 
 Task::~Task() = default;