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..812a48fcea468b8932ca966c941484ea03004428 100644
--- a/base/task_scheduler/task.cc
+++ b/base/task_scheduler/task.cc
@@ -7,15 +7,30 @@
 namespace base {
 namespace internal {
 
+namespace {
+
+// Changes the shutdown behavior in |traits| to SKIP_ON_SHUTDOWN if |delay| is
+// non-zero and the existing shutdown behavior is BLOCK_SHUTDOWN. This prevents
+// delayed tasks from blocking shutdown before they are scheduled.
+TaskTraits AdjustShutdownBehavior(TaskTraits traits, TimeDelta delay) {
+  return (traits.shutdown_behavior() == TaskShutdownBehavior::BLOCK_SHUTDOWN &&
+          !delay.is_zero())
+             ? traits.WithShutdownBehavior(
+                   TaskShutdownBehavior::SKIP_ON_SHUTDOWN)
+             : traits;
+}
+
+}  // namespace
+
 Task::Task(const tracked_objects::Location& posted_from,
            const Closure& task,
            const TaskTraits& traits,
-           const TimeDelta& delay)
+           TimeDelta delay)

[robliao, 2016/09/30 18:57:20]

This should remain a const ref.

AdjustShutdownBehavior can also adjust to be a const ref as well it appears.

[fdoray, 2016/09/30 19:10:11]

No. https://cs.chromium.org/chromium/src/base/time/time.h?l=24

     : PendingTask(posted_from,
                   task,
                   delay.is_zero() ? TimeTicks() : TimeTicks::Now() + delay,
                   false),  // Not nestable.
-      traits(traits) {}
+      traits(AdjustShutdownBehavior(traits, delay)) {}

[gab, 2016/09/30 19:01:26]

So this potentially results in 3 copies (one to pass argument, one for return
value, on into |traits|), instead of a single one into |traits|.

Copy-eliding (since the return value is an rvalue) probably makes this only 2
copies and inlining could potentially bring it back to one but neither is
guaranteed.

Is it necessary to adjust the |traits| or should TaskTracker handle this on its
own (it knows both delay and shutdown behavior)?

[fdoray, 2016/09/30 19:10:11]

I'll make a new CL that changes TaskTracker instead of Task.

 
 Task::~Task() = default;