TaskScheduler: Add delayed run time to the constructor of Task.

TaskScheduler: Add delayed run time to the constructor of Task.

Require a delayed run time (can be a null TimeTicks for no delay) when
constructing a Task. There is a single call site for the constructor of
Task (excluding unit tests).
unit tests).

BUG=553459
Committed: https://crrev.com/357c389142869acef70b01ab514b8d88622af321
Cr-Commit-Position: refs/heads/master@{#387944}

[fdoray, 2016-04-15 17:33:56]

fdoray@chromium.org changed reviewers:
+ gab@chromium.org, robliao@chromium.org

[fdoray, 2016-04-15 17:33:57]

Can you review this CL? Thanks.

[gab, 2016-04-18 16:12:14]

lgtm w/ comment

https://codereview.chromium.org/1890163003/diff/1/base/task_scheduler/task.h
File base/task_scheduler/task.h (right):

https://codereview.chromium.org/1890163003/diff/1/base/task_scheduler/task.h#...
base/task_scheduler/task.h:24: const TimeTicks& delayed_run_time);
Document parameters (in particular is |delayed_run_time| a delay or the time at
which the delay expires).

[gab, 2016-04-18 16:20:39]

https://codereview.chromium.org/1890163003/diff/1/base/task_scheduler/task.h
File base/task_scheduler/task.h (right):

https://codereview.chromium.org/1890163003/diff/1/base/task_scheduler/task.h#...
base/task_scheduler/task.h:21: Task(const tracked_objects::Location&
posted_from,
Also, I don't mind forcing the explicit parameter if you think that's better,
but given only one caller cares about providing this parameter I'd also be fine
with defining a delegated constructor (same as this one minus
|delayed_run_time|) which is delegated as:

Task(const tracked_objects::Location& posted_from, 
     const Closure& task,
     const TaskTraits& traits) 
    : Task(posted_from, task, traits, TimeTicks() {}

[fdoray, 2016-04-18 16:33:44]

https://codereview.chromium.org/1890163003/diff/1/base/task_scheduler/task.h
File base/task_scheduler/task.h (right):

https://codereview.chromium.org/1890163003/diff/1/base/task_scheduler/task.h#...
base/task_scheduler/task.h:21: Task(const tracked_objects::Location&
posted_from,
On 2016/04/18 16:20:39, gab wrote:
> Also, I don't mind forcing the explicit parameter if you think that's better,
> but given only one caller cares about providing this parameter I'd also be
fine
> with defining a delegated constructor (same as this one minus
> |delayed_run_time|) which is delegated as:
> 
> Task(const tracked_objects::Location& posted_from, 
>      const Closure& task,
>      const TaskTraits& traits) 
>     : Task(posted_from, task, traits, TimeTicks() {}

I prefer forcing |delayed_run_time| because it is always provided when a Task is
constructed in production (I don't want to add a constructor that will only be
used by tests).

https://codereview.chromium.org/1890163003/diff/1/base/task_scheduler/task.h#...
base/task_scheduler/task.h:24: const TimeTicks& delayed_run_time);
On 2016/04/18 16:12:14, gab wrote:
> Document parameters (in particular is |delayed_run_time| a delay or the time
at
> which the delay expires).

Done.

[fdoray, 2016-04-18 16:33:48]

The CQ bit was checked by fdoray@chromium.org

[fdoray, 2016-04-18 16:33:49]

The patchset sent to the CQ was uploaded after l-g-t-m from gab@chromium.org
Link to the patchset: https://codereview.chromium.org/1890163003/#ps40001
(title: "self review")

[commit-bot: I haz the power, 2016-04-18 16:34:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1890163003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1890163003/40001

[robliao, 2016-04-18 16:59:40]

lgtm

[gab, 2016-04-18 17:00:13]

Thanks, lgtm++

https://codereview.chromium.org/1890163003/diff/1/base/task_scheduler/task.h
File base/task_scheduler/task.h (right):

https://codereview.chromium.org/1890163003/diff/1/base/task_scheduler/task.h#...
base/task_scheduler/task.h:21: Task(const tracked_objects::Location&
posted_from,
On 2016/04/18 16:33:44, fdoray wrote:
> On 2016/04/18 16:20:39, gab wrote:
> > Also, I don't mind forcing the explicit parameter if you think that's
better,
> > but given only one caller cares about providing this parameter I'd also be
> fine
> > with defining a delegated constructor (same as this one minus
> > |delayed_run_time|) which is delegated as:
> > 
> > Task(const tracked_objects::Location& posted_from, 
> >      const Closure& task,
> >      const TaskTraits& traits) 
> >     : Task(posted_from, task, traits, TimeTicks() {}
> 
> I prefer forcing |delayed_run_time| because it is always provided when a Task
is
> constructed in production (I don't want to add a constructor that will only be
> used by tests).

Good point, hadn't realized all other uses were for tests.

[commit-bot: I haz the power, 2016-04-18 17:46:41]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-18 17:48:10]

Description was changed from

==========
TaskScheduler: Add delayed run time to the constructor of Task.

Require a delayed run time (can be a null TimeTicks for no delay) when
constructing a Task. There is a single call site for the constructor of
Task (excluding unit tests).
unit tests).

BUG=553459
==========

to

==========
TaskScheduler: Add delayed run time to the constructor of Task.

Require a delayed run time (can be a null TimeTicks for no delay) when
constructing a Task. There is a single call site for the constructor of
Task (excluding unit tests).
unit tests).

BUG=553459

Committed: https://crrev.com/357c389142869acef70b01ab514b8d88622af321
Cr-Commit-Position: refs/heads/master@{#387944}
==========

[commit-bot: I haz the power, 2016-04-18 17:48:11]

Patchset 3 (id:??) landed as
https://crrev.com/357c389142869acef70b01ab514b8d88622af321
Cr-Commit-Position: refs/heads/master@{#387944}