Introduce TaskRunnerHandle.

Introduce TaskRunnerHandle.

Deprecates ThreadTaskRunnerHandle and SequencedTaskRunnerHandle.

Design doc: https://docs.google.com/document/d/1A_LRKyTOCzhRPOY4Q3RsePuw4UCsvxuFYx6D18BaYk0/edit

CLs will follow to migrate SequencedTaskRunnerHandle callsites (with
goal to have no callsite depending on a SequencedTaskRunner to be
returned from unsequenced task):
 - https://codereview.chromium.org/2076153002/
 - https://codereview.chromium.org/2076163002/

ThreadTaskRunnerHandle will not be auto migrated as callers of
TaskRunnerHandle's getters should have made an explicit decision
(i.e. TaskRunnerHandle::GetSingleThreaded() should indicate thread-affinity
not merely a migration from ThreadTaskRunnerHandle::Get() which is
often over-specific in today's usage).

BUG=618043

[gab, 2016-06-07 19:09:10]

Description was changed from

==========
Introduce TaskRunnerHandle.

Deprecates ThreadTaskRunnerHandle and SequencedTaskRunnerHandle.

Design doc:
https://docs.google.com/document/d/1A_LRKyTOCzhRPOY4Q3RsePuw4UCsvxuFYx6D18BaY...

CLs will follow to migrate SequencedTaskRunnerHandle callsites (with
goal to have no callsite depending on a SequencedTaskRunner to be
returned from unsequenced task).
ThreadTaskRunnerHandle will not be auto migrated as callers of
TaskRunnerHandle's getters should have made an explicit decision
(i.e. TaskRunnerHandle::GetSingleThreaded() indicates thread-affinity
not merely a migration from ThreadTaskRunnerHandle::Get() which is
often over-specific in today's usage).

BUG=618043
==========

to

==========
Introduce TaskRunnerHandle.

Deprecates ThreadTaskRunnerHandle and SequencedTaskRunnerHandle.

Design doc:
https://docs.google.com/document/d/1A_LRKyTOCzhRPOY4Q3RsePuw4UCsvxuFYx6D18BaY...

CLs will follow to migrate SequencedTaskRunnerHandle callsites (with
goal to have no callsite depending on a SequencedTaskRunner to be
returned from unsequenced task).

ThreadTaskRunnerHandle will not be auto migrated as callers of
TaskRunnerHandle's getters should have made an explicit decision
(i.e. TaskRunnerHandle::GetSingleThreaded() should indicate thread-affinity
not merely a migration from ThreadTaskRunnerHandle::Get() which is
often over-specific in today's usage).

BUG=618043
==========

[gab, 2016-06-07 19:13:18]

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

[gab, 2016-06-07 19:13:18]

Rob/Francois WDYT? Still need to write tests (and follow-up Cls), planning to
send to Dana when she's back.

[fdoray, 2016-06-08 18:10:44]

interface lg

https://codereview.chromium.org/2042383004/diff/20001/base/threading/task_run...
File base/threading/task_runner_handle.cc (right):

https://codereview.chromium.org/2042383004/diff/20001/base/threading/task_run...
base/threading/task_runner_handle.cc:35: if
(single_thread_task_scope_tls.Pointer()->Get()) {
if (HasSingleThreadTaskScope())
  return GetSingleThreaded()
...

https://codereview.chromium.org/2042383004/diff/20001/base/threading/task_run...
File base/threading/task_runner_handle.h (right):

https://codereview.chromium.org/2042383004/diff/20001/base/threading/task_run...
base/threading/task_runner_handle.h:41: static bool HasTaskScope();
HasTaskContext? or just HasContext? since the comment above Get() says that it
returns a TaskRunner which will run tasks in the "same context"

HasSequencedTaskContext? or HasSequencedContext?
HasSingleThreadTaskContext? or HasSingleThreadContext?

https://codereview.chromium.org/2042383004/diff/20001/base/threading/task_run...
base/threading/task_runner_handle.h:48: static bool HasSequencedTaskScope();
HasSequencedContext?

https://codereview.chromium.org/2042383004/diff/20001/base/threading/task_run...
base/threading/task_runner_handle.h:54: // Each task in Chromium should run in
one and only one Scope. The
Remove Chromium?

[gab, 2016-06-08 20:29:20]

Discussion below, thanks.

https://codereview.chromium.org/2042383004/diff/20001/base/threading/task_run...
File base/threading/task_runner_handle.cc (right):

https://codereview.chromium.org/2042383004/diff/20001/base/threading/task_run...
base/threading/task_runner_handle.cc:35: if
(single_thread_task_scope_tls.Pointer()->Get()) {
On 2016/06/08 18:10:44, fdoray wrote:
> if (HasSingleThreadTaskScope())
>   return GetSingleThreaded()
> ...

That feels inconsistent (because it wouldn't be optimal to use "if
(HasSequencedTaskScope())" in the next if).

https://codereview.chromium.org/2042383004/diff/20001/base/threading/task_run...
File base/threading/task_runner_handle.h (right):

https://codereview.chromium.org/2042383004/diff/20001/base/threading/task_run...
base/threading/task_runner_handle.h:41: static bool HasTaskScope();
On 2016/06/08 18:10:44, fdoray wrote:
> HasTaskContext? or just HasContext? since the comment above Get() says that it
> returns a TaskRunner which will run tasks in the "same context"
> 
> HasSequencedTaskContext? or HasSequencedContext?
> HasSingleThreadTaskContext? or HasSingleThreadContext?

I agree that sounds a bit better but it clashes with *TaskScope, perhaps the
scopes should be named *TaskContext?

https://codereview.chromium.org/2042383004/diff/20001/base/threading/task_run...
base/threading/task_runner_handle.h:48: static bool HasSequencedTaskScope();
On 2016/06/08 18:10:44, fdoray wrote:
> HasSequencedContext?

Acknowledged.

https://codereview.chromium.org/2042383004/diff/20001/base/threading/task_run...
base/threading/task_runner_handle.h:54: // Each task in Chromium should run in
one and only one Scope. The
On 2016/06/08 18:10:44, fdoray wrote:
> Remove Chromium?

Done.

[robliao, 2016-06-09 15:40:18]

https://codereview.chromium.org/2042383004/diff/60001/base/threading/sequence...
File base/threading/sequenced_task_runner_handle.h (right):

https://codereview.chromium.org/2042383004/diff/60001/base/threading/sequence...
base/threading/sequenced_task_runner_handle.h:19: // Deprecated, use
TaskRunnerHandle::GetSequenced() instead.
Might be helpful to put a deprecated banner above the class as well since most
everything here is deprecated.

https://codereview.chromium.org/2042383004/diff/60001/base/threading/task_run...
File base/threading/task_runner_handle.cc (right):

https://codereview.chromium.org/2042383004/diff/60001/base/threading/task_run...
base/threading/task_runner_handle.cc:23:
LazyInstance<ThreadLocalPointer<TaskRunnerHandle::TaskScope>>::Leaky
Can this be indirected into a struct to save on TLS usage?

https://codereview.chromium.org/2042383004/diff/60001/base/threading/task_run...
File base/threading/task_runner_handle.h (right):

https://codereview.chromium.org/2042383004/diff/60001/base/threading/task_run...
base/threading/task_runner_handle.h:21: // current task. That context could, for
example, be: a MessageLoop, a
Nit: Remove "for example." The hypothetical is clear from "could"

https://codereview.chromium.org/2042383004/diff/60001/base/threading/task_run...
base/threading/task_runner_handle.h:28: // sequencing is not required.
TODO(gab): Migrate all callsites using
Linebreak this TODO so it's easier to see.

https://codereview.chromium.org/2042383004/diff/60001/base/threading/task_run...
base/threading/task_runner_handle.h:95: // single- threaded context
(SingleThreadedTaskRunner) in which tasks on the
Nit: single- threaded -> single-threaded

https://codereview.chromium.org/2042383004/diff/60001/base/threading/thread_t...
File base/threading/thread_task_runner_handle.h (right):

https://codereview.chromium.org/2042383004/diff/60001/base/threading/thread_t...
base/threading/thread_task_runner_handle.h:19: // Deprecated, use
TaskRunnerHandle::Get.*() instead.
Similar deprecated class banner here.

[gab, 2016-06-14 15:36:52]

Patchset #5 (id:80001) has been deleted

[gab, 2016-06-14 15:38:59]

Here's a redesigned version of the impl that uses a union to avoid using 3 TLS
slots. Using a union forces the use of a type enum as well which actually comes
out very handy to make the Has*TaskScope() methods short (similarly the union
makes the getters short :-)).

Debating getting rid of the union and instead holding only a TaskRunner, forcing
a cast based on type on request.

https://codereview.chromium.org/2042383004/diff/60001/base/threading/sequence...
File base/threading/sequenced_task_runner_handle.h (right):

https://codereview.chromium.org/2042383004/diff/60001/base/threading/sequence...
base/threading/sequenced_task_runner_handle.h:19: // Deprecated, use
TaskRunnerHandle::GetSequenced() instead.
On 2016/06/09 15:40:18, robliao wrote:
> Might be helpful to put a deprecated banner above the class as well since most
> everything here is deprecated.

Done.

https://codereview.chromium.org/2042383004/diff/60001/base/threading/task_run...
File base/threading/task_runner_handle.cc (right):

https://codereview.chromium.org/2042383004/diff/60001/base/threading/task_run...
base/threading/task_runner_handle.cc:23:
LazyInstance<ThreadLocalPointer<TaskRunnerHandle::TaskScope>>::Leaky
On 2016/06/09 15:40:18, robliao wrote:
> Can this be indirected into a struct to save on TLS usage?

Re-did the impl using a union + enum for type (the type makes for really small
code for the Has*TaskScope() methods and the union makes the getters easy :-)).

Also thinking I could get rid of the union and do cast based on |type|, WDYT?

https://codereview.chromium.org/2042383004/diff/60001/base/threading/task_run...
File base/threading/task_runner_handle.h (right):

https://codereview.chromium.org/2042383004/diff/60001/base/threading/task_run...
base/threading/task_runner_handle.h:21: // current task. That context could, for
example, be: a MessageLoop, a
On 2016/06/09 15:40:18, robliao wrote:
> Nit: Remove "for example." The hypothetical is clear from "could"

Done.

https://codereview.chromium.org/2042383004/diff/60001/base/threading/task_run...
base/threading/task_runner_handle.h:28: // sequencing is not required.
TODO(gab): Migrate all callsites using
On 2016/06/09 15:40:18, robliao wrote:
> Linebreak this TODO so it's easier to see.

Done.

https://codereview.chromium.org/2042383004/diff/60001/base/threading/task_run...
base/threading/task_runner_handle.h:95: // single- threaded context
(SingleThreadedTaskRunner) in which tasks on the
On 2016/06/09 15:40:18, robliao wrote:
> Nit: single- threaded -> single-threaded

Done.

https://codereview.chromium.org/2042383004/diff/60001/base/threading/thread_t...
File base/threading/thread_task_runner_handle.h (right):

https://codereview.chromium.org/2042383004/diff/60001/base/threading/thread_t...
base/threading/thread_task_runner_handle.h:19: // Deprecated, use
TaskRunnerHandle::Get.*() instead.
On 2016/06/09 15:40:18, robliao wrote:
> Similar deprecated class banner here.

Done.

[gab, 2016-06-14 16:00:37]

Patchset #6 (id:120001) has been deleted

[fdoray, 2016-06-17 14:29:02]

https://codereview.chromium.org/2042383004/diff/180001/base/BUILD.gn
File base/BUILD.gn (right):

https://codereview.chromium.org/2042383004/diff/180001/base/BUILD.gn#newcode825
base/BUILD.gn:825: "threading/task_runner_handle.cc",
Why is this in base/threading/? Could it be in base/ like task_runner.h? Or
could we move this, task_runner.h and (sequenced|single_thread)_task_runner.h in
base/task/?

IMO, base/threading/ should be only for:
- Managing threads and their properties (platform_thread.h, simple_thread.h,
sequenced_worker_pool.h, thread_id_name_manager.h, ...)
- Providing access to thread-local storage (thread_local.h, ...)
- Verifying that code runs on the appropriate thread (non_thread_safe.h,
thread_restrictions.h, ...)

https://codereview.chromium.org/2042383004/diff/180001/base/threading/sequenc...
File base/threading/sequenced_task_runner_handle.h (right):

https://codereview.chromium.org/2042383004/diff/180001/base/threading/sequenc...
base/threading/sequenced_task_runner_handle.h:17: class BASE_EXPORT
SequencedTaskRunnerHandle {
We should be able to migrate call sites away from this shortly after this CL
lands? Add TODO(gab): Delete this file.

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
File base/threading/task_runner_handle.cc (right):

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
base/threading/task_runner_handle.cc:146: task_runner.get()->Release();
I'm not strongly against the union, but with a cast, no deep knowledge of C++
would be required to understand how TaskScopeState works.

http://www.stroustrup.com/C++11FAQ.html#unions

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
File base/threading/task_runner_handle.h (right):

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
base/threading/task_runner_handle.h:28: // sequencing is not required.
s/This must only be called.../This DCHECKs if !HasSequencedTaskScope()./

We don't expect users to call HasSequencedTaskScope() if they know that they are
on a sequence. They shouldn't do this:

DCHECK(TaskRunnerHandle::HasSequencedTaskScope());
TaskRunnerHandle::GetSequenced()->...

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
base/threading/task_runner_handle.h:36: // single-threaded sequence. This must
only be called if
s/This must only be called.../This DCHECKS if !HasSingleThreadedTaskScope()/

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
base/threading/task_runner_handle.h:39: static
scoped_refptr<SingleThreadTaskRunner> GetSingleThreaded();
Why do we have GetSingleThread*ed*() and HasSingleThread*TaskScope()?

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
base/threading/task_runner_handle.h:48: // to the TaskScheduler.
TODO(gab): Remove this comment after SequencedWorkerPool has been migrated to
the TaskScheduler.

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
base/threading/task_runner_handle.h:65: TaskScope(scoped_refptr<TaskRunner>
task_runner);
The destructor is not trivial. ~TaskScope() here and TaskScope::~TaskScope() =
default; in the .cc file?

https://www.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-When-y...

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
base/threading/task_runner_handle.h:88: // single- threaded context
(SingleThreadedTaskRunner) in which tasks on the
single- threaded
remove extra space

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
base/threading/task_runner_handle.h:88: // single- threaded context
(SingleThreadedTaskRunner) in which tasks on the
SingleThread**TaskRunner :)

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
File base/threading/task_runner_handle_unittest.cc (right):

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
base/threading/task_runner_handle_unittest.cc:119: }
Maybe instantiate a SequencedWorkerPool?

void TestTask() {
  EXPECT_TRUE(TaskRunnerHandle::HasTaskScope());
  EXPECT_TRUE(TaskRunnerHandle::HasSequencedTaskScope());
  EXPECT_FALSE(TaskRunnerHandle::HasSingleThreadTaskScope());

  EXPECT_TRUE(!!TaskRunnerHandle::Get());
  EXPECT_TRUE(!!TaskRunnerHandle::GetSequenced());
}

scoped_refptr<SequencedWorkerPool> sequenced_worker_pool(new
SequencedWorkerPool(1, "name"));
sequenced_worker_pool->PostTask(FROM_HERE, Bind(&TestTask));
sequenced_worker_pool->FlushForTesting();

[gab, 2016-06-17 19:22:38]

Description was changed from

==========
Introduce TaskRunnerHandle.

Deprecates ThreadTaskRunnerHandle and SequencedTaskRunnerHandle.

Design doc:
https://docs.google.com/document/d/1A_LRKyTOCzhRPOY4Q3RsePuw4UCsvxuFYx6D18BaY...

CLs will follow to migrate SequencedTaskRunnerHandle callsites (with
goal to have no callsite depending on a SequencedTaskRunner to be
returned from unsequenced task).

ThreadTaskRunnerHandle will not be auto migrated as callers of
TaskRunnerHandle's getters should have made an explicit decision
(i.e. TaskRunnerHandle::GetSingleThreaded() should indicate thread-affinity
not merely a migration from ThreadTaskRunnerHandle::Get() which is
often over-specific in today's usage).

BUG=618043
==========

to

==========
Introduce TaskRunnerHandle.

Deprecates ThreadTaskRunnerHandle and SequencedTaskRunnerHandle.

Design doc:
https://docs.google.com/document/d/1A_LRKyTOCzhRPOY4Q3RsePuw4UCsvxuFYx6D18BaY...

CLs will follow to migrate SequencedTaskRunnerHandle callsites (with
goal to have no callsite depending on a SequencedTaskRunner to be
returned from unsequenced task):
 - https://codereview.chromium.org/2076153002/
 - https://codereview.chromium.org/2076163002/

ThreadTaskRunnerHandle will not be auto migrated as callers of
TaskRunnerHandle's getters should have made an explicit decision
(i.e. TaskRunnerHandle::GetSingleThreaded() should indicate thread-affinity
not merely a migration from ThreadTaskRunnerHandle::Get() which is
often over-specific in today's usage).

BUG=618043
==========

[gab, 2016-06-20 19:55:36]

@fdoray: done (sorry for merge noise in this PS for GYP/GN -- other files should
be clear)

https://codereview.chromium.org/2042383004/diff/180001/base/BUILD.gn
File base/BUILD.gn (right):

https://codereview.chromium.org/2042383004/diff/180001/base/BUILD.gn#newcode825
base/BUILD.gn:825: "threading/task_runner_handle.cc",
On 2016/06/17 14:29:01, fdoray wrote:
> Why is this in base/threading/? Could it be in base/ like task_runner.h? Or
> could we move this, task_runner.h and (sequenced|single_thread)_task_runner.h
in
> base/task/?
> 
> IMO, base/threading/ should be only for:
> - Managing threads and their properties (platform_thread.h, simple_thread.h,
> sequenced_worker_pool.h, thread_id_name_manager.h, ...)
> - Providing access to thread-local storage (thread_local.h, ...)
> - Verifying that code runs on the appropriate thread (non_thread_safe.h,
> thread_restrictions.h, ...)

I agree that this is weird, I had the same thought multiple times (I hate that
task_runners are in base/ root and also find it weird for the
task_runner_handles to be in base/threading/.

If we introduce something like base/task, maybe we could put post_task.h in it
as well? Keeping base/task_scheduler for the implementation? Do you want to kick
off a discussion on this in a bug with base/ OWNERS?

https://codereview.chromium.org/2042383004/diff/180001/base/threading/sequenc...
File base/threading/sequenced_task_runner_handle.h (right):

https://codereview.chromium.org/2042383004/diff/180001/base/threading/sequenc...
base/threading/sequenced_task_runner_handle.h:17: class BASE_EXPORT
SequencedTaskRunnerHandle {
On 2016/06/17 14:29:02, fdoray wrote:
> We should be able to migrate call sites away from this shortly after this CL
> lands? Add TODO(gab): Delete this file.

Done.

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
File base/threading/task_runner_handle.cc (right):

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
base/threading/task_runner_handle.cc:146: task_runner.get()->Release();
On 2016/06/17 14:29:02, fdoray wrote:
> I'm not strongly against the union, but with a cast, no deep knowledge of C++
> would be required to understand how TaskScopeState works.
> 
> http://www.stroustrup.com/C++11FAQ.html#unions

Right, done (based on what I read in this I think the current impl freed the
TaskRunner but not the scoped_refptr itself -- which was probably okay if it
only used stack memory but skipping its destructor is not very kosher).

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
File base/threading/task_runner_handle.h (right):

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
base/threading/task_runner_handle.h:28: // sequencing is not required.
On 2016/06/17 14:29:02, fdoray wrote:
> s/This must only be called.../This DCHECKs if !HasSequencedTaskScope()./
> 
> We don't expect users to call HasSequencedTaskScope() if they know that they
are
> on a sequence. They shouldn't do this:
> 
> DCHECK(TaskRunnerHandle::HasSequencedTaskScope());
> TaskRunnerHandle::GetSequenced()->...

Done.

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
base/threading/task_runner_handle.h:36: // single-threaded sequence. This must
only be called if
On 2016/06/17 14:29:02, fdoray wrote:
> s/This must only be called.../This DCHECKS if !HasSingleThreadedTaskScope()/

Done.

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
base/threading/task_runner_handle.h:39: static
scoped_refptr<SingleThreadTaskRunner> GetSingleThreaded();
On 2016/06/17 14:29:02, fdoray wrote:
> Why do we have GetSingleThread*ed*() and HasSingleThread*TaskScope()?

Because we have SingleThread*TaskRunner already but GetSingleThread() sounds
silly IMO. Because of the legacy SingleThreadTaskRunner naming I decided to use
SingleThreadTaskScope as its related name and things derived from it except for
GetSingleThread()... Happy to field other suggestions.

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
base/threading/task_runner_handle.h:48: // to the TaskScheduler.
On 2016/06/17 14:29:02, fdoray wrote:
> TODO(gab): Remove this comment after SequencedWorkerPool has been migrated to
> the TaskScheduler.

Reworded to include TODO

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
base/threading/task_runner_handle.h:65: TaskScope(scoped_refptr<TaskRunner>
task_runner);
On 2016/06/17 14:29:02, fdoray wrote:
> The destructor is not trivial. ~TaskScope() here and TaskScope::~TaskScope() =
> default; in the .cc file?
> 
>
https://www.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-When-y...

It isn't but it only deletes TaskScopeState which itself has an explicit
out-of-line destructor. So IMO it's okay to have TaskScope's destructor
inline...

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
base/threading/task_runner_handle.h:88: // single- threaded context
(SingleThreadedTaskRunner) in which tasks on the
On 2016/06/17 14:29:02, fdoray wrote:
> single- threaded
> remove extra space

Done.

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
base/threading/task_runner_handle.h:88: // single- threaded context
(SingleThreadedTaskRunner) in which tasks on the
On 2016/06/17 14:29:02, fdoray wrote:
> SingleThread**TaskRunner :)

Done (I hate that name!)

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
File base/threading/task_runner_handle_unittest.cc (right):

https://codereview.chromium.org/2042383004/diff/180001/base/threading/task_ru...
base/threading/task_runner_handle_unittest.cc:119: }
On 2016/06/17 14:29:02, fdoray wrote:
> Maybe instantiate a SequencedWorkerPool?
> 
> void TestTask() {
>   EXPECT_TRUE(TaskRunnerHandle::HasTaskScope());
>   EXPECT_TRUE(TaskRunnerHandle::HasSequencedTaskScope());
>   EXPECT_FALSE(TaskRunnerHandle::HasSingleThreadTaskScope());
> 
>   EXPECT_TRUE(!!TaskRunnerHandle::Get());
>   EXPECT_TRUE(!!TaskRunnerHandle::GetSequenced());
> }
> 
> scoped_refptr<SequencedWorkerPool> sequenced_worker_pool(new
> SequencedWorkerPool(1, "name"));
> sequenced_worker_pool->PostTask(FROM_HERE, Bind(&TestTask));
> sequenced_worker_pool->FlushForTesting();
> 
> 

This is already tested by SequencedTaskRunnerHandle's unittests (which now use
this code by redirection). Those will be migrated here when it's removed.

[gab, 2016-06-21 19:39:54]

Dana, WDYT of this (and associated design doc which you've already seen)?

I'm thinking of getting second thoughts from eng-review on this before going
live too once you are happy with it.

Thanks!
Gab

[gab, 2016-06-21 19:41:45]

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

[gab, 2016-06-21 19:41:46]

Dana, WDYT of this (and associated design doc which you've already seen)?

I'm thinking of getting second thoughts from eng-review on this before going
live too once you are happy with it.

Thanks!
Gab

[danakj, 2016-06-22 22:33:02]

On 2016/06/21 19:41:46, gab (OOO back Monday) wrote:
> Dana, WDYT of this (and associated design doc which you've already seen)?
> 
> I'm thinking of getting second thoughts from eng-review on this before going
> live too once you are happy with it.
> 
> Thanks!
> Gab

I am worried I do not have the context around uses of SequencedTaskRunnerHandle
to understand all the implications, and rsleevi is intensely concerned about
this change on the doc. So I think we should resolve the plan some more. And
perhaps bring in some more people who are more familiar with uses of
SequencedTaskRunnerHandle to the discussion if you think that would be helpful.

[gab, 2016-07-13 21:19:25]

On 2016/06/22 22:33:02, danakj wrote:
> On 2016/06/21 19:41:46, gab (OOO back Monday) wrote:
> > Dana, WDYT of this (and associated design doc which you've already seen)?
> > 
> > I'm thinking of getting second thoughts from eng-review on this before going
> > live too once you are happy with it.
> > 
> > Thanks!
> > Gab
> 
> I am worried I do not have the context around uses of
SequencedTaskRunnerHandle
> to understand all the implications, and rsleevi is intensely concerned about
> this change on the doc. So I think we should resolve the plan some more. And
> perhaps bring in some more people who are more familiar with uses of
> SequencedTaskRunnerHandle to the discussion if you think that would be
helpful.

Closing this per conclusion of thread @
https://groups.google.com/a/chromium.org/d/topic/chromium-dev/cd9hldEhIEc/dis...
(see last 2 replies as tl;dr;)

[danakj, 2016-07-13 22:11:31]

On Wed, Jul 13, 2016 at 2:19 PM, <gab@chromium.org> wrote:

> On 2016/06/22 22:33:02, danakj wrote:
> > On 2016/06/21 19:41:46, gab (OOO back Monday) wrote:
> > > Dana, WDYT of this (and associated design doc which you've already
> seen)?
> > >
> > > I'm thinking of getting second thoughts from eng-review on this before
> going
> > > live too once you are happy with it.
> > >
> > > Thanks!
> > > Gab
> >
> > I am worried I do not have the context around uses of
> SequencedTaskRunnerHandle
> > to understand all the implications, and rsleevi is intensely concerned
> about
> > this change on the doc. So I think we should resolve the plan some more.
> And
> > perhaps bring in some more people who are more familiar with uses of
> > SequencedTaskRunnerHandle to the discussion if you think that would be
> helpful.
>
> Closing this per conclusion of thread @
>
>
https://groups.google.com/a/chromium.org/d/topic/chromium-dev/cd9hldEhIEc/dis...
> (see last 2 replies as tl;dr;)
>

Thanks, I'm happy with the outcome of that thread. :)

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.