Make SchedulerSingleThreadTaskRunner::RunsTasksOnCurrentThread sequence-affine.

Make SchedulerSingleThreadTaskRunner::RunsTasksOnCurrentThread sequence-affine.

Added test highlights the requirements (fails with previous impl).

BUG=662191
Committed: https://crrev.com/cdac39241efc49fadf95a13df932f1f1e1eab93d
Cr-Commit-Position: refs/heads/master@{#430364}

[gab, 2016-11-03 22:08:12]

Description was changed from

==========
Make SchedulerSingleThreadTaskRunner::RunsTasksOnCurrentThread sequence-affine.

BUG=662191
==========

to

==========
Make SchedulerSingleThreadTaskRunner::RunsTasksOnCurrentThread sequence-affine.

Added test highlights the requirements (fails with previous impl).

BUG=662191
==========

[gab, 2016-11-03 22:08:16]

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

[commit-bot: I haz the power, 2016-11-03 22:09:01]

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

[gab, 2016-11-03 22:11:08]

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

[gab, 2016-11-03 22:11:08]

Francois PTAL, thanks!

[gab, 2016-11-03 22:11:15]

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

[commit-bot: I haz the power, 2016-11-03 22:11:36]

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

[robliao, 2016-11-03 22:59:56]

robliao@chromium.org changed reviewers:
+ robliao@chromium.org

[robliao, 2016-11-03 22:59:57]

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
File base/task_scheduler/scheduler_worker_pool_impl.cc (right):

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
base/task_scheduler/scheduler_worker_pool_impl.cc:202: // to the same worker
doesn't return true by happenstance.
Nit: s/happenstance/coincidence/ ?

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
File base/task_scheduler/scheduler_worker_pool_impl_unittest.cc (right):

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:370: // tasks to be
assigned to the same worker.
If we stop round robining the workers for single threaded tasks, this test would
no longer work, right?

Could use a TLS to assert that this is the first task to run on a particular
worker to detect overlapping tasks and have the tasks saturate the pool.

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:372: constexpr size_t
kNumFloodingTaskRunners = 50;
Nit: Move this constexpr to the group near the top of the file.

[gab, 2016-11-03 23:17:44]

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
File base/task_scheduler/scheduler_worker_pool_impl_unittest.cc (right):

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:370: // tasks to be
assigned to the same worker.
On 2016/11/03 22:59:56, robliao wrote:
> If we stop round robining the workers for single threaded tasks, this test
would
> no longer work, right?
> 
> Could use a TLS to assert that this is the first task to run on a particular
> worker to detect overlapping tasks and have the tasks saturate the pool.

It would still work. Note that this test is parametrized and the flood is tested
in all 3 ExecutionModes. So long as one of these tasks as *a chance* to run on
the thread assigned to |single_thread_task_runner| then this test would at least
become flaky. But with 50 tasks I think it's pretty much guaranteed to fail (it
currently reliably fails in all 3 modes without the matching impl change in this
CL).

Using TLS to ensure that at least one of the task has touched this thread would
on the other hand make the test susceptible to false positive flakes (i.e. every
now and then it's possible that no tasks use the assigned thread).

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:372: constexpr size_t
kNumFloodingTaskRunners = 50;
On 2016/11/03 22:59:56, robliao wrote:
> Nit: Move this constexpr to the group near the top of the file.

No, it's specific to this test so it's preferred to keep it local (same is true
of say a string used in only one test body or a single method in a regular .cc
file).

[robliao, 2016-11-03 23:29:32]

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
File base/task_scheduler/scheduler_worker_pool_impl_unittest.cc (right):

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:370: // tasks to be
assigned to the same worker.
On 2016/11/03 23:17:44, gab wrote:
> On 2016/11/03 22:59:56, robliao wrote:
> > If we stop round robining the workers for single threaded tasks, this test
> would
> > no longer work, right?
> > 
> > Could use a TLS to assert that this is the first task to run on a particular
> > worker to detect overlapping tasks and have the tasks saturate the pool.
> 
> It would still work. Note that this test is parametrized and the flood is
tested
> in all 3 ExecutionModes. So long as one of these tasks as *a chance* to run on
> the thread assigned to |single_thread_task_runner| then this test would at
least
> become flaky. But with 50 tasks I think it's pretty much guaranteed to fail
(it
> currently reliably fails in all 3 modes without the matching impl change in
this
> CL).
> 
> Using TLS to ensure that at least one of the task has touched this thread
would
> on the other hand make the test susceptible to false positive flakes (i.e.
every
> now and then it's possible that no tasks use the assigned thread).

Ah, in that case would it be sufficient to simply saturate the pool instead with
kNumWorkersInWorkerPool and have each task wait?

We then signal all of the tasks after saturation.

[gab, 2016-11-03 23:39:50]

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
File base/task_scheduler/scheduler_worker_pool_impl_unittest.cc (right):

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:370: // tasks to be
assigned to the same worker.
On 2016/11/03 23:29:32, robliao wrote:
> On 2016/11/03 23:17:44, gab wrote:
> > On 2016/11/03 22:59:56, robliao wrote:
> > > If we stop round robining the workers for single threaded tasks, this test
> > would
> > > no longer work, right?
> > > 
> > > Could use a TLS to assert that this is the first task to run on a
particular
> > > worker to detect overlapping tasks and have the tasks saturate the pool.
> > 
> > It would still work. Note that this test is parametrized and the flood is
> tested
> > in all 3 ExecutionModes. So long as one of these tasks as *a chance* to run
on
> > the thread assigned to |single_thread_task_runner| then this test would at
> least
> > become flaky. But with 50 tasks I think it's pretty much guaranteed to fail
> (it
> > currently reliably fails in all 3 modes without the matching impl change in
> this
> > CL).
> > 
> > Using TLS to ensure that at least one of the task has touched this thread
> would
> > on the other hand make the test susceptible to false positive flakes (i.e.
> every
> > now and then it's possible that no tasks use the assigned thread).
> 
> Ah, in that case would it be sufficient to simply saturate the pool instead
with
> kNumWorkersInWorkerPool and have each task wait?
> 
> We then signal all of the tasks after saturation.

This could deadlock in the SINGLE_THREADED case (e.g. if we don't assign workers
in round-robin then it's possible that we don't fill the threads and wait
forever for them to be).

[commit-bot: I haz the power, 2016-11-04 00:29:40]

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

[commit-bot: I haz the power, 2016-11-04 00:29:41]

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_...)

[robliao, 2016-11-04 00:56:47]

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
File base/task_scheduler/scheduler_worker_pool_impl_unittest.cc (right):

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:370: // tasks to be
assigned to the same worker.
On 2016/11/03 23:39:50, gab wrote:
> On 2016/11/03 23:29:32, robliao wrote:
> > On 2016/11/03 23:17:44, gab wrote:
> > > On 2016/11/03 22:59:56, robliao wrote:
> > > > If we stop round robining the workers for single threaded tasks, this
test
> > > would
> > > > no longer work, right?
> > > > 
> > > > Could use a TLS to assert that this is the first task to run on a
> particular
> > > > worker to detect overlapping tasks and have the tasks saturate the pool.
> > > 
> > > It would still work. Note that this test is parametrized and the flood is
> > tested
> > > in all 3 ExecutionModes. So long as one of these tasks as *a chance* to
run
> on
> > > the thread assigned to |single_thread_task_runner| then this test would at
> > least
> > > become flaky. But with 50 tasks I think it's pretty much guaranteed to
fail
> > (it
> > > currently reliably fails in all 3 modes without the matching impl change
in
> > this
> > > CL).
> > > 
> > > Using TLS to ensure that at least one of the task has touched this thread
> > would
> > > on the other hand make the test susceptible to false positive flakes (i.e.
> > every
> > > now and then it's possible that no tasks use the assigned thread).
> > 
> > Ah, in that case would it be sufficient to simply saturate the pool instead
> with
> > kNumWorkersInWorkerPool and have each task wait?
> > 
> > We then signal all of the tasks after saturation.
> 
> This could deadlock in the SINGLE_THREADED case (e.g. if we don't assign
workers
> in round-robin then it's possible that we don't fill the threads and wait
> forever for them to be).

I think I would prefer a deadlocked test if/when we change the policy rather
than a flaky test.

I don't think it's possible for this test to be robust to any scheduling policy
we throw at it. However, it would be good to know that the coverage is similar
in the face of a changing policy. If the test deadlocks, then at least we know
this test should be updated.

[gab, 2016-11-04 14:47:59]

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
File base/task_scheduler/scheduler_worker_pool_impl_unittest.cc (right):

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:370: // tasks to be
assigned to the same worker.
On 2016/11/04 00:56:47, robliao wrote:
> On 2016/11/03 23:39:50, gab wrote:
> > On 2016/11/03 23:29:32, robliao wrote:
> > > On 2016/11/03 23:17:44, gab wrote:
> > > > On 2016/11/03 22:59:56, robliao wrote:
> > > > > If we stop round robining the workers for single threaded tasks, this
> test
> > > > would
> > > > > no longer work, right?
> > > > > 
> > > > > Could use a TLS to assert that this is the first task to run on a
> > particular
> > > > > worker to detect overlapping tasks and have the tasks saturate the
pool.
> > > > 
> > > > It would still work. Note that this test is parametrized and the flood
is
> > > tested
> > > > in all 3 ExecutionModes. So long as one of these tasks as *a chance* to
> run
> > on
> > > > the thread assigned to |single_thread_task_runner| then this test would
at
> > > least
> > > > become flaky. But with 50 tasks I think it's pretty much guaranteed to
> fail
> > > (it
> > > > currently reliably fails in all 3 modes without the matching impl change
> in
> > > this
> > > > CL).
> > > > 
> > > > Using TLS to ensure that at least one of the task has touched this
thread
> > > would
> > > > on the other hand make the test susceptible to false positive flakes
(i.e.
> > > every
> > > > now and then it's possible that no tasks use the assigned thread).
> > > 
> > > Ah, in that case would it be sufficient to simply saturate the pool
instead
> > with
> > > kNumWorkersInWorkerPool and have each task wait?
> > > 
> > > We then signal all of the tasks after saturation.
> > 
> > This could deadlock in the SINGLE_THREADED case (e.g. if we don't assign
> workers
> > in round-robin then it's possible that we don't fill the threads and wait
> > forever for them to be).
> 
> I think I would prefer a deadlocked test if/when we change the policy rather
> than a flaky test.
> 
> I don't think it's possible for this test to be robust to any scheduling
policy
> we throw at it. However, it would be good to know that the coverage is similar
> in the face of a changing policy. If the test deadlocks, then at least we know
> this test should be updated.

I disagree, the worker assignment policy is orthogonal to this test and as such
it's incorrect to intentionally have this test depend on the existing policy
IMO.

With 50 tasks I seriously doubt that this test would incorrectly pass as a false
negative flake very often if it became broken.

[fdoray, 2016-11-04 15:20:43]

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
File base/task_scheduler/scheduler_worker_pool_impl_unittest.cc (right):

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:367: // Verify that
the RunsTasksOnCurrentThread() method of a SINGLE_THREADED
s/SINGLE_THREADED TaskRunner/SingleThreadTaskRunner/

I can remove remaining ExecutionModes from base/task_scheduler/ comments in a
separate CL.

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:370: // tasks to be
assigned to the same worker.
On 2016/11/04 14:47:59, gab wrote:
> On 2016/11/04 00:56:47, robliao wrote:
> > On 2016/11/03 23:39:50, gab wrote:
> > > On 2016/11/03 23:29:32, robliao wrote:
> > > > On 2016/11/03 23:17:44, gab wrote:
> > > > > On 2016/11/03 22:59:56, robliao wrote:
> > > > > > If we stop round robining the workers for single threaded tasks,
this
> > test
> > > > > would
> > > > > > no longer work, right?
> > > > > > 
> > > > > > Could use a TLS to assert that this is the first task to run on a
> > > particular
> > > > > > worker to detect overlapping tasks and have the tasks saturate the
> pool.
> > > > > 
> > > > > It would still work. Note that this test is parametrized and the flood
> is
> > > > tested
> > > > > in all 3 ExecutionModes. So long as one of these tasks as *a chance*
to
> > run
> > > on
> > > > > the thread assigned to |single_thread_task_runner| then this test
would
> at
> > > > least
> > > > > become flaky. But with 50 tasks I think it's pretty much guaranteed to
> > fail
> > > > (it
> > > > > currently reliably fails in all 3 modes without the matching impl
change
> > in
> > > > this
> > > > > CL).
> > > > > 
> > > > > Using TLS to ensure that at least one of the task has touched this
> thread
> > > > would
> > > > > on the other hand make the test susceptible to false positive flakes
> (i.e.
> > > > every
> > > > > now and then it's possible that no tasks use the assigned thread).
> > > > 
> > > > Ah, in that case would it be sufficient to simply saturate the pool
> instead
> > > with
> > > > kNumWorkersInWorkerPool and have each task wait?
> > > > 
> > > > We then signal all of the tasks after saturation.
> > > 
> > > This could deadlock in the SINGLE_THREADED case (e.g. if we don't assign
> > workers
> > > in round-robin then it's possible that we don't fill the threads and wait
> > > forever for them to be).
> > 
> > I think I would prefer a deadlocked test if/when we change the policy rather
> > than a flaky test.
> > 
> > I don't think it's possible for this test to be robust to any scheduling
> policy
> > we throw at it. However, it would be good to know that the coverage is
similar
> > in the face of a changing policy. If the test deadlocks, then at least we
know
> > this test should be updated.
> 
> I disagree, the worker assignment policy is orthogonal to this test and as
such
> it's incorrect to intentionally have this test depend on the existing policy
> IMO.
> 
> With 50 tasks I seriously doubt that this test would incorrectly pass as a
false
> negative flake very often if it became broken.

After reading the "Resilience" section from
https://testing.googleblog.com/2014/05/testing-on-toilet-effective-testing.html
, I don't think this test should have to change if the thread assignment policy
changes.

A good compromise to be absolutely sure that this test fails when
RunsTasksOnCurrentThread() returns true for tasks posted to different
SingleThreadTaskRunners that happen to run on the same thread would be to use a
SchedulerWorkerPoolImpl with |max_threads| = 1.

[robliao, 2016-11-04 16:59:36]

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
File base/task_scheduler/scheduler_worker_pool_impl_unittest.cc (right):

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:370: // tasks to be
assigned to the same worker.
On 2016/11/04 15:20:43, fdoray wrote:
> On 2016/11/04 14:47:59, gab wrote:
> > On 2016/11/04 00:56:47, robliao wrote:
> > > On 2016/11/03 23:39:50, gab wrote:
> > > > On 2016/11/03 23:29:32, robliao wrote:
> > > > > On 2016/11/03 23:17:44, gab wrote:
> > > > > > On 2016/11/03 22:59:56, robliao wrote:
> > > > > > > If we stop round robining the workers for single threaded tasks,
> this
> > > test
> > > > > > would
> > > > > > > no longer work, right?
> > > > > > > 
> > > > > > > Could use a TLS to assert that this is the first task to run on a
> > > > particular
> > > > > > > worker to detect overlapping tasks and have the tasks saturate the
> > pool.
> > > > > > 
> > > > > > It would still work. Note that this test is parametrized and the
flood
> > is
> > > > > tested
> > > > > > in all 3 ExecutionModes. So long as one of these tasks as *a chance*
> to
> > > run
> > > > on
> > > > > > the thread assigned to |single_thread_task_runner| then this test
> would
> > at
> > > > > least
> > > > > > become flaky. But with 50 tasks I think it's pretty much guaranteed
to
> > > fail
> > > > > (it
> > > > > > currently reliably fails in all 3 modes without the matching impl
> change
> > > in
> > > > > this
> > > > > > CL).
> > > > > > 
> > > > > > Using TLS to ensure that at least one of the task has touched this
> > thread
> > > > > would
> > > > > > on the other hand make the test susceptible to false positive flakes
> > (i.e.
> > > > > every
> > > > > > now and then it's possible that no tasks use the assigned thread).
> > > > > 
> > > > > Ah, in that case would it be sufficient to simply saturate the pool
> > instead
> > > > with
> > > > > kNumWorkersInWorkerPool and have each task wait?
> > > > > 
> > > > > We then signal all of the tasks after saturation.
> > > > 
> > > > This could deadlock in the SINGLE_THREADED case (e.g. if we don't assign
> > > workers
> > > > in round-robin then it's possible that we don't fill the threads and
wait
> > > > forever for them to be).
> > > 
> > > I think I would prefer a deadlocked test if/when we change the policy
rather
> > > than a flaky test.
> > > 
> > > I don't think it's possible for this test to be robust to any scheduling
> > policy
> > > we throw at it. However, it would be good to know that the coverage is
> similar
> > > in the face of a changing policy. If the test deadlocks, then at least we
> know
> > > this test should be updated.
> > 
> > I disagree, the worker assignment policy is orthogonal to this test and as
> such
> > it's incorrect to intentionally have this test depend on the existing policy
> > IMO.
> > 
> > With 50 tasks I seriously doubt that this test would incorrectly pass as a
> false
> > negative flake very often if it became broken.
> 
> After reading the "Resilience" section from
>
https://testing.googleblog.com/2014/05/testing-on-toilet-effective-testing.html
> , I don't think this test should have to change if the thread assignment
policy
> changes.
> 
> A good compromise to be absolutely sure that this test fails when
> RunsTasksOnCurrentThread() returns true for tasks posted to different
> SingleThreadTaskRunners that happen to run on the same thread would be to use
a
> SchedulerWorkerPoolImpl with |max_threads| = 1.

I think I would be okay with this. That's a good compromise!

[gab, 2016-11-04 17:55:00]

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

[commit-bot: I haz the power, 2016-11-04 17:55:19]

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

[gab, 2016-11-04 17:56:13]

Done, PTAnL

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
File base/task_scheduler/scheduler_worker_pool_impl_unittest.cc (right):

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:367: // Verify that
the RunsTasksOnCurrentThread() method of a SINGLE_THREADED
On 2016/11/04 15:20:43, fdoray wrote:
> s/SINGLE_THREADED TaskRunner/SingleThreadTaskRunner/
> 
> I can remove remaining ExecutionModes from base/task_scheduler/ comments in a
> separate CL.

Okay but I used SchedulerSingleThreadTaskRunner as otherwise it's too generic.

https://codereview.chromium.org/2480613002/diff/20001/base/task_scheduler/sch...
base/task_scheduler/scheduler_worker_pool_impl_unittest.cc:370: // tasks to be
assigned to the same worker.
On 2016/11/04 16:59:36, robliao wrote:
> On 2016/11/04 15:20:43, fdoray wrote:
> > On 2016/11/04 14:47:59, gab wrote:
> > > On 2016/11/04 00:56:47, robliao wrote:
> > > > On 2016/11/03 23:39:50, gab wrote:
> > > > > On 2016/11/03 23:29:32, robliao wrote:
> > > > > > On 2016/11/03 23:17:44, gab wrote:
> > > > > > > On 2016/11/03 22:59:56, robliao wrote:
> > > > > > > > If we stop round robining the workers for single threaded tasks,
> > this
> > > > test
> > > > > > > would
> > > > > > > > no longer work, right?
> > > > > > > > 
> > > > > > > > Could use a TLS to assert that this is the first task to run on
a
> > > > > particular
> > > > > > > > worker to detect overlapping tasks and have the tasks saturate
the
> > > pool.
> > > > > > > 
> > > > > > > It would still work. Note that this test is parametrized and the
> flood
> > > is
> > > > > > tested
> > > > > > > in all 3 ExecutionModes. So long as one of these tasks as *a
chance*
> > to
> > > > run
> > > > > on
> > > > > > > the thread assigned to |single_thread_task_runner| then this test
> > would
> > > at
> > > > > > least
> > > > > > > become flaky. But with 50 tasks I think it's pretty much
guaranteed
> to
> > > > fail
> > > > > > (it
> > > > > > > currently reliably fails in all 3 modes without the matching impl
> > change
> > > > in
> > > > > > this
> > > > > > > CL).
> > > > > > > 
> > > > > > > Using TLS to ensure that at least one of the task has touched this
> > > thread
> > > > > > would
> > > > > > > on the other hand make the test susceptible to false positive
flakes
> > > (i.e.
> > > > > > every
> > > > > > > now and then it's possible that no tasks use the assigned thread).
> > > > > > 
> > > > > > Ah, in that case would it be sufficient to simply saturate the pool
> > > instead
> > > > > with
> > > > > > kNumWorkersInWorkerPool and have each task wait?
> > > > > > 
> > > > > > We then signal all of the tasks after saturation.
> > > > > 
> > > > > This could deadlock in the SINGLE_THREADED case (e.g. if we don't
assign
> > > > workers
> > > > > in round-robin then it's possible that we don't fill the threads and
> wait
> > > > > forever for them to be).
> > > > 
> > > > I think I would prefer a deadlocked test if/when we change the policy
> rather
> > > > than a flaky test.
> > > > 
> > > > I don't think it's possible for this test to be robust to any scheduling
> > > policy
> > > > we throw at it. However, it would be good to know that the coverage is
> > similar
> > > > in the face of a changing policy. If the test deadlocks, then at least
we
> > know
> > > > this test should be updated.
> > > 
> > > I disagree, the worker assignment policy is orthogonal to this test and as
> > such
> > > it's incorrect to intentionally have this test depend on the existing
policy
> > > IMO.
> > > 
> > > With 50 tasks I seriously doubt that this test would incorrectly pass as a
> > false
> > > negative flake very often if it became broken.
> > 
> > After reading the "Resilience" section from
> >
>
https://testing.googleblog.com/2014/05/testing-on-toilet-effective-testing.html
> > , I don't think this test should have to change if the thread assignment
> policy
> > changes.
> > 
> > A good compromise to be absolutely sure that this test fails when
> > RunsTasksOnCurrentThread() returns true for tasks posted to different
> > SingleThreadTaskRunners that happen to run on the same thread would be to
use
> a
> > SchedulerWorkerPoolImpl with |max_threads| = 1.
> 
> I think I would be okay with this. That's a good compromise!

I like this too, done.

[commit-bot: I haz the power, 2016-11-04 20:10:43]

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

[commit-bot: I haz the power, 2016-11-04 20:10:44]

Dry run: This issue passed the CQ dry run.

[robliao, 2016-11-07 18:38:43]

lgtm

[fdoray, 2016-11-07 18:53:09]

lgtm

[gab, 2016-11-07 19:01:55]

The CQ bit was checked by gab@chromium.org

[commit-bot: I haz the power, 2016-11-07 19:02:50]

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, 2016-11-07 20:41:17]

Description was changed from

==========
Make SchedulerSingleThreadTaskRunner::RunsTasksOnCurrentThread sequence-affine.

Added test highlights the requirements (fails with previous impl).

BUG=662191
==========

to

==========
Make SchedulerSingleThreadTaskRunner::RunsTasksOnCurrentThread sequence-affine.

Added test highlights the requirements (fails with previous impl).

BUG=662191
==========

[commit-bot: I haz the power, 2016-11-07 20:41:18]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-07 21:18:52]

Description was changed from

==========
Make SchedulerSingleThreadTaskRunner::RunsTasksOnCurrentThread sequence-affine.

Added test highlights the requirements (fails with previous impl).

BUG=662191
==========

to

==========
Make SchedulerSingleThreadTaskRunner::RunsTasksOnCurrentThread sequence-affine.

Added test highlights the requirements (fails with previous impl).

BUG=662191

Committed: https://crrev.com/cdac39241efc49fadf95a13df932f1f1e1eab93d
Cr-Commit-Position: refs/heads/master@{#430364}
==========

[commit-bot: I haz the power, 2016-11-07 21:18:54]

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