scheduler: Maintain a constant enqueue order for every task

scheduler: Maintain a constant enqueue order for every task

The scheduler uses a monotonically increasing counter called enqueue
order for sorting tasks. This value was previously assigned at post
time for immediate tasks and at delay expiration time for delayed tasks.

Because expired delayed tasks get processed in a consistent order
(i.e., sorted by their delay) within a single task queue, but in 
a random order between task queues, delayed tasks for the exact 
same point in time posted two distinct task queues will run in a
random order w.r.t. each other. This in turn causes a problem in
virtual time scenarios where delayed tasks should behave
deterministically.

This patch fixes the problem by eliminating the two-phase assignment
of enqueue orders to delayed tasks. This was originally added to avoid
a flood of expired delayed tasks starving immediate work, but because
the selector now makes sure delayed work doesn't starve immediate work
and vice versa, this mechanism can be removed.

A side effect of this removal is that there is no longer a guarantee
that expired delayed tasks won't run before immediate tasks that were
posted later. Note that the base::TaskRunner doesn't offer this 
guarantee either so it shouldn't be relied on.

BUG=696001, 701223

[Sami, 2017-03-31 18:05:51]

The CQ bit was checked by skyostil@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-31 18:06:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-31 18:33:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-31 18:33:53]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[Sami, 2017-03-31 18:47:09]

The CQ bit was checked by skyostil@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-31 18:47:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-31 19:11:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-31 19:11:54]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Sami, 2017-04-03 15:58:37]

The CQ bit was checked by skyostil@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-03 15:59:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-03 16:19:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-03 16:19:57]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Sami, 2017-04-03 19:08:01]

The CQ bit was checked by skyostil@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-03 19:08:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-03 19:12:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-03 19:12:10]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Sami, 2017-04-04 10:34:37]

The CQ bit was checked by skyostil@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-04 10:34:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-04 10:56:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-04 10:56:17]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Sami, 2017-04-04 11:03:40]

The CQ bit was checked by skyostil@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-04 11:03:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-04 11:19:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-04 11:19:33]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Sami, 2017-04-04 11:29:45]

The CQ bit was checked by skyostil@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-04 11:29:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-04 12:04:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-04 12:04:35]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Sami, 2017-04-04 12:12:51]

The CQ bit was checked by skyostil@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-04 12:13:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-04 12:23:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-04 12:23:50]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[Sami, 2017-04-05 16:22:48]

The CQ bit was checked by skyostil@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-05 16:23:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-05 17:41:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-05 17:41:11]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Sami, 2017-04-07 20:07:47]

The CQ bit was checked by skyostil@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-07 20:08:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-07 20:56:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-07 20:56:40]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Sami, 2017-04-08 12:17:19]

The CQ bit was checked by skyostil@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-08 12:17:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Sami, 2017-04-08 12:25:14]

Description was changed from

==========
scheduler: Maintain a constant enqueue order for every task

(WIP)
==========

to

==========
scheduler: Maintain a constant enqueue order for every task

The scheduler uses a monotonically increasing counter called enqueue
order for sorting tasks. This value was previously assigned at post
time for immediate tasks and at delay expiration time for delayed tasks.

Because expired delayed tasks get processed in a consistent order (i.e., sorted
by their delay) within a single task queue, but in 
a random order between task queues, delayed tasks for the exact 
same point in time posted two distinct task queues will run in a
random order w.r.t. each other. This in turn causes a problem in
virtual time scenarios where delayed tasks should behave
deterministically.

This patch fixes the problem by eliminating the two-phase assignment
of enqueue orders to delayed tasks. This was originally added to avoid
a flood of expired delayed tasks starving immediate work, but because
the selector now makes sure delayed work doesn't starve immediate work
and vice versa, this mechanism can be removed.

A side effect of this removal is that there is no longer a guarantee
that expired delayed tasks won't run before immediate tasks that were
posted later. Note that the base::TaskRunner doesn't offer this 
guarantee either so it shouldn't be relied on.

BUG=696001,701223
==========

[Sami, 2017-04-08 12:25:33]

Description was changed from

==========
scheduler: Maintain a constant enqueue order for every task

The scheduler uses a monotonically increasing counter called enqueue
order for sorting tasks. This value was previously assigned at post
time for immediate tasks and at delay expiration time for delayed tasks.

Because expired delayed tasks get processed in a consistent order (i.e., sorted
by their delay) within a single task queue, but in 
a random order between task queues, delayed tasks for the exact 
same point in time posted two distinct task queues will run in a
random order w.r.t. each other. This in turn causes a problem in
virtual time scenarios where delayed tasks should behave
deterministically.

This patch fixes the problem by eliminating the two-phase assignment
of enqueue orders to delayed tasks. This was originally added to avoid
a flood of expired delayed tasks starving immediate work, but because
the selector now makes sure delayed work doesn't starve immediate work
and vice versa, this mechanism can be removed.

A side effect of this removal is that there is no longer a guarantee
that expired delayed tasks won't run before immediate tasks that were
posted later. Note that the base::TaskRunner doesn't offer this 
guarantee either so it shouldn't be relied on.

BUG=696001,701223
==========

to

==========
scheduler: Maintain a constant enqueue order for every task

The scheduler uses a monotonically increasing counter called enqueue
order for sorting tasks. This value was previously assigned at post
time for immediate tasks and at delay expiration time for delayed tasks.

Because expired delayed tasks get processed in a consistent order
(i.e., sorted by their delay) within a single task queue, but in 
a random order between task queues, delayed tasks for the exact 
same point in time posted two distinct task queues will run in a
random order w.r.t. each other. This in turn causes a problem in
virtual time scenarios where delayed tasks should behave
deterministically.

This patch fixes the problem by eliminating the two-phase assignment
of enqueue orders to delayed tasks. This was originally added to avoid
a flood of expired delayed tasks starving immediate work, but because
the selector now makes sure delayed work doesn't starve immediate work
and vice versa, this mechanism can be removed.

A side effect of this removal is that there is no longer a guarantee
that expired delayed tasks won't run before immediate tasks that were
posted later. Note that the base::TaskRunner doesn't offer this 
guarantee either so it shouldn't be relied on.

BUG=696001,701223
==========

[commit-bot: I haz the power, 2017-04-08 13:09:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-08 13:09:17]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Sami, 2017-04-11 10:06:21]

Description was changed from

==========
scheduler: Maintain a constant enqueue order for every task

The scheduler uses a monotonically increasing counter called enqueue
order for sorting tasks. This value was previously assigned at post
time for immediate tasks and at delay expiration time for delayed tasks.

Because expired delayed tasks get processed in a consistent order
(i.e., sorted by their delay) within a single task queue, but in 
a random order between task queues, delayed tasks for the exact 
same point in time posted two distinct task queues will run in a
random order w.r.t. each other. This in turn causes a problem in
virtual time scenarios where delayed tasks should behave
deterministically.

This patch fixes the problem by eliminating the two-phase assignment
of enqueue orders to delayed tasks. This was originally added to avoid
a flood of expired delayed tasks starving immediate work, but because
the selector now makes sure delayed work doesn't starve immediate work
and vice versa, this mechanism can be removed.

A side effect of this removal is that there is no longer a guarantee
that expired delayed tasks won't run before immediate tasks that were
posted later. Note that the base::TaskRunner doesn't offer this 
guarantee either so it shouldn't be relied on.

BUG=696001,701223
==========

to

==========
scheduler: Maintain a constant enqueue order for every task

The scheduler uses a monotonically increasing counter called enqueue
order for sorting tasks. This value was previously assigned at post
time for immediate tasks and at delay expiration time for delayed tasks.

Because expired delayed tasks get processed in a consistent order
(i.e., sorted by their delay) within a single task queue, but in 
a random order between task queues, delayed tasks for the exact 
same point in time posted two distinct task queues will run in a
random order w.r.t. each other. This in turn causes a problem in
virtual time scenarios where delayed tasks should behave
deterministically.

This patch fixes the problem by eliminating the two-phase assignment
of enqueue orders to delayed tasks. This was originally added to avoid
a flood of expired delayed tasks starving immediate work, but because
the selector now makes sure delayed work doesn't starve immediate work
and vice versa, this mechanism can be removed.

A side effect of this removal is that there is no longer a guarantee
that expired delayed tasks won't run before immediate tasks that were
posted later. Note that the base::TaskRunner doesn't offer this 
guarantee either so it shouldn't be relied on.

BUG=696001,701223
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Sami, 2017-04-11 10:06:27]

The CQ bit was checked by skyostil@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-11 10:06:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-11 11:09:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-11 11:09:07]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Sami, 2017-04-12 09:54:16]

The CQ bit was checked by skyostil@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-12 09:54:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-12 10:06:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-12 10:06:48]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Sami, 2017-04-12 15:34:38]

Description was changed from

==========
scheduler: Maintain a constant enqueue order for every task

The scheduler uses a monotonically increasing counter called enqueue
order for sorting tasks. This value was previously assigned at post
time for immediate tasks and at delay expiration time for delayed tasks.

Because expired delayed tasks get processed in a consistent order
(i.e., sorted by their delay) within a single task queue, but in 
a random order between task queues, delayed tasks for the exact 
same point in time posted two distinct task queues will run in a
random order w.r.t. each other. This in turn causes a problem in
virtual time scenarios where delayed tasks should behave
deterministically.

This patch fixes the problem by eliminating the two-phase assignment
of enqueue orders to delayed tasks. This was originally added to avoid
a flood of expired delayed tasks starving immediate work, but because
the selector now makes sure delayed work doesn't starve immediate work
and vice versa, this mechanism can be removed.

A side effect of this removal is that there is no longer a guarantee
that expired delayed tasks won't run before immediate tasks that were
posted later. Note that the base::TaskRunner doesn't offer this 
guarantee either so it shouldn't be relied on.

BUG=696001,701223
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
scheduler: Maintain a constant enqueue order for every task

The scheduler uses a monotonically increasing counter called enqueue
order for sorting tasks. This value was previously assigned at post
time for immediate tasks and at delay expiration time for delayed tasks.

Because expired delayed tasks get processed in a consistent order
(i.e., sorted by their delay) within a single task queue, but in 
a random order between task queues, delayed tasks for the exact 
same point in time posted two distinct task queues will run in a
random order w.r.t. each other. This in turn causes a problem in
virtual time scenarios where delayed tasks should behave
deterministically.

This patch fixes the problem by eliminating the two-phase assignment
of enqueue orders to delayed tasks. This was originally added to avoid
a flood of expired delayed tasks starving immediate work, but because
the selector now makes sure delayed work doesn't starve immediate work
and vice versa, this mechanism can be removed.

A side effect of this removal is that there is no longer a guarantee
that expired delayed tasks won't run before immediate tasks that were
posted later. Note that the base::TaskRunner doesn't offer this 
guarantee either so it shouldn't be relied on.

BUG=696001,701223
==========

[Sami, 2017-04-12 15:34:38]

skyostil@chromium.org changed reviewers:
+ alexclarke@chromium.org

[Sami, 2017-04-12 15:49:58]

The CQ bit was checked by skyostil@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-12 15:50:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Sami, 2017-04-12 15:50:56]

I'm still chasing a couple of layout test failures but I think this is ready for
review. Alex, PTAL.

[commit-bot: I haz the power, 2017-04-12 16:56:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-12 16:56:44]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[alex clarke (OOO till 29th), 2017-04-13 07:46:01]

https://codereview.chromium.org/2786083005/diff/280001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scheduler/base/enqueue_order.h (left):

https://codereview.chromium.org/2786083005/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/enqueue_order.h:28: // A 64bit
integer used to provide ordering of tasks. NOTE The scheduler assumes
We should keep the comment about not overflowing.

https://codereview.chromium.org/2786083005/diff/280001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scheduler/base/enqueue_order.h (right):

https://codereview.chromium.org/2786083005/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/enqueue_order.h:22: uint64_t
sequence_num;
I wonder if we should have

using SequenceNum = uint64_t;

https://codereview.chromium.org/2786083005/diff/280001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc
(left):

https://codereview.chromium.org/2786083005/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc:305: int
thread_hop_task_sequence_number =
Oh wow that's a nasty bug fixed here.

https://codereview.chromium.org/2786083005/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc:451:
main_thread_only().delayed_work_queue->Push(std::move(task));
Something for a future patch, but I wonder if we actually need the
delayed_work_queue anymore.

https://codereview.chromium.org/2786083005/diff/280001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scheduler/base/work_queue.cc (right):

https://codereview.chromium.org/2786083005/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/work_queue.cc:164:
EnqueueOrder enqueue_order = {base::TimeTicks(), 0};
Can we have a default constructor?

https://codereview.chromium.org/2786083005/diff/280001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scheduler/base/work_queue_unittest.cc
(right):

https://codereview.chromium.org/2786083005/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/work_queue_unittest.cc:40:
TaskQueueImpl::Task FakeTaskWithEnqueueOrder(int enqueue_order) {
s/int/uint65_t

[Sami, 2017-04-13 14:58:47]

The CQ bit was checked by skyostil@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-13 14:59:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-13 15:54:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-13 15:54:49]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Sami, 2017-04-18 10:47:39]

The CQ bit was checked by skyostil@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-18 10:47:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Sami, 2017-04-18 10:47:49]

Thanks, all addressed!

https://codereview.chromium.org/2786083005/diff/280001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scheduler/base/enqueue_order.h (left):

https://codereview.chromium.org/2786083005/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/enqueue_order.h:28: // A 64bit
integer used to provide ordering of tasks. NOTE The scheduler assumes
On 2017/04/13 07:46:00, alex clarke wrote:
> We should keep the comment about not overflowing.

Yep, it's now further up the file where the enqueue order is defined.

https://codereview.chromium.org/2786083005/diff/280001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scheduler/base/enqueue_order.h (right):

https://codereview.chromium.org/2786083005/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/enqueue_order.h:22: uint64_t
sequence_num;
On 2017/04/13 07:46:00, alex clarke wrote:
> I wonder if we should have
> 
> using SequenceNum = uint64_t;

Good idea, done.

https://codereview.chromium.org/2786083005/diff/280001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc
(left):

https://codereview.chromium.org/2786083005/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc:305: int
thread_hop_task_sequence_number =
On 2017/04/13 07:46:00, alex clarke wrote:
> Oh wow that's a nasty bug fixed here.

You mean the overflow? Yeah :)

https://codereview.chromium.org/2786083005/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc:451:
main_thread_only().delayed_work_queue->Push(std::move(task));
On 2017/04/13 07:46:00, alex clarke wrote:
> Something for a future patch, but I wonder if we actually need the
> delayed_work_queue anymore.

Interesting point. We'd still need some way to round-robin immediate and delayed
work. Let's revisit this.

https://codereview.chromium.org/2786083005/diff/280001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scheduler/base/work_queue.cc (right):

https://codereview.chromium.org/2786083005/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/work_queue.cc:164:
EnqueueOrder enqueue_order = {base::TimeTicks(), 0};
On 2017/04/13 07:46:00, alex clarke wrote:
> Can we have a default constructor?

Done.

https://codereview.chromium.org/2786083005/diff/280001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scheduler/base/work_queue_unittest.cc
(right):

https://codereview.chromium.org/2786083005/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/work_queue_unittest.cc:40:
TaskQueueImpl::Task FakeTaskWithEnqueueOrder(int enqueue_order) {
On 2017/04/13 07:46:00, alex clarke wrote:
> s/int/uint65_t

Done.

[Sami, 2017-04-18 10:54:56]

The CQ bit was checked by skyostil@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-18 10:55:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[alex clarke (OOO till 29th), 2017-04-18 10:58:36]

lgtm

[commit-bot: I haz the power, 2017-04-18 11:51:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-18 11:51:31]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Sami, 2017-04-26 13:11:22]

For posterity: we decided against offering this ordering guarantee across task
queue since it would basically tie our hands when wanting to schedule distinct
task queues more freely. The specific virtual time issue is addressed by
https://codereview.chromium.org/2841463003/.