Add Support to Get a COM STA Task Runner from the Task Scheduler API

Add Support to Get a COM STA Task Runner from the Task Scheduler API

This also adds support for COM STA Task Runner for TaskSchedulerImpl as
well as ScopedTaskScheduler.

BUG=662122
Review-Url: https://codereview.chromium.org/2768873007
Cr-Commit-Position: refs/heads/master@{#460430}
Committed: https://chromium.googlesource.com/chromium/src/+/75dd50b325a709221dd971e61eb3e6f53de1a8aa

[robliao, 2017-03-24 21:36:49]

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

[commit-bot: I haz the power, 2017-03-24 21:37:28]

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-24 22:53:10]

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

[commit-bot: I haz the power, 2017-03-24 22:53:10]

Dry run: This issue passed the CQ dry run.

[robliao, 2017-03-27 18:53:24]

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

[fdoray, 2017-03-27 20:11:50]

https://codereview.chromium.org/2768873007/diff/1/base/task_scheduler/post_ta...
File base/task_scheduler/post_task.h (right):

https://codereview.chromium.org/2768873007/diff/1/base/task_scheduler/post_ta...
base/task_scheduler/post_task.h:179: // SingleThreadTaskRunner. Different
SingleThreadTaskRunners may be associated
// The Single-Threaded Apartment assigned to the returned SingleThreadTaskRunner
may be shared with other SingleThreadTaskRunners.

https://codereview.chromium.org/2768873007/diff/1/base/test/scoped_task_sched...
File base/test/scoped_task_scheduler.cc (right):

https://codereview.chromium.org/2768873007/diff/1/base/test/scoped_task_sched...
base/test/scoped_task_scheduler.cc:178: PostTask(MakeUnique<internal::Task>(
If new tasks are posted during shutdown to a COM TaskRunner, they will run after
COM uninitialization. Do we need to explicitly reset |scoped_com_inializer_|? We
could let the member be destroyed automatically.

[robliao, 2017-03-27 20:28:05]

https://codereview.chromium.org/2768873007/diff/1/base/task_scheduler/post_ta...
File base/task_scheduler/post_task.h (right):

https://codereview.chromium.org/2768873007/diff/1/base/task_scheduler/post_ta...
base/task_scheduler/post_task.h:179: // SingleThreadTaskRunner. Different
SingleThreadTaskRunners may be associated
On 2017/03/27 20:11:50, fdoray wrote:
> // The Single-Threaded Apartment assigned to the returned
SingleThreadTaskRunner
> may be shared with other SingleThreadTaskRunners.

Changed the note to...
There is not necessarily a one-to-one correspondence between
SingleThreadTaskRunners and Single-Threaded Apartments. The implementation is
free to share apartments or create new apartments as necessary. In either case,
care should be taken to make sure COM pointers are not smuggled across
apartments.

https://codereview.chromium.org/2768873007/diff/1/base/test/scoped_task_sched...
File base/test/scoped_task_scheduler.cc (right):

https://codereview.chromium.org/2768873007/diff/1/base/test/scoped_task_sched...
base/test/scoped_task_scheduler.cc:178: PostTask(MakeUnique<internal::Task>(
On 2017/03/27 20:11:50, fdoray wrote:
> If new tasks are posted during shutdown to a COM TaskRunner, they will run
after
> COM uninitialization. Do we need to explicitly reset |scoped_com_inializer_|?
We
> could let the member be destroyed automatically.

For correctness, this task does need to be posted to the message loop (someone
could have sent us a message loop running on a different thread, right?). Maybe
a message loop destruction observer is the way to go here?

[robliao, 2017-03-27 21:31:28]

Patchset #3 (id:40001) has been deleted

[robliao, 2017-03-27 21:32:32]

https://codereview.chromium.org/2768873007/diff/1/base/test/scoped_task_sched...
File base/test/scoped_task_scheduler.cc (right):

https://codereview.chromium.org/2768873007/diff/1/base/test/scoped_task_sched...
base/test/scoped_task_scheduler.cc:178: PostTask(MakeUnique<internal::Task>(
On 2017/03/27 20:28:05, robliao wrote:
> On 2017/03/27 20:11:50, fdoray wrote:
> > If new tasks are posted during shutdown to a COM TaskRunner, they will run
> after
> > COM uninitialization. Do we need to explicitly reset
|scoped_com_inializer_|?
> We
> > could let the member be destroyed automatically.
> 
> For correctness, this task does need to be posted to the message loop (someone
> could have sent us a message loop running on a different thread, right?).
Maybe
> a message loop destruction observer is the way to go here?

Simpler: Run another task after shutdown. That's what I went with here.

[fdoray, 2017-03-28 14:14:46]

https://codereview.chromium.org/2768873007/diff/1/base/test/scoped_task_sched...
File base/test/scoped_task_scheduler.cc (right):

https://codereview.chromium.org/2768873007/diff/1/base/test/scoped_task_sched...
base/test/scoped_task_scheduler.cc:178: PostTask(MakeUnique<internal::Task>(
On 2017/03/27 21:32:32, robliao wrote:
> On 2017/03/27 20:28:05, robliao wrote:
> > On 2017/03/27 20:11:50, fdoray wrote:
> > > If new tasks are posted during shutdown to a COM TaskRunner, they will run
> > after
> > > COM uninitialization. Do we need to explicitly reset
> |scoped_com_inializer_|?
> > We
> > > could let the member be destroyed automatically.
> > 
> > For correctness, this task does need to be posted to the message loop
(someone
> > could have sent us a message loop running on a different thread, right?).
> Maybe
> > a message loop destruction observer is the way to go here?
> 
> Simpler: Run another task after shutdown. That's what I went with here.

ScopedTaskScheduler is not supposed to be used with a MessageLoop that belongs
to another thread
https://cs.chromium.org/chromium/src/base/test/scoped_task_scheduler.h?sq=pac...
The description could probably be clearer about that.

https://codereview.chromium.org/2768873007/diff/60001/base/test/scoped_task_s...
File base/test/scoped_task_scheduler.cc (right):

https://codereview.chromium.org/2768873007/diff/60001/base/test/scoped_task_s...
base/test/scoped_task_scheduler.cc:188: RunLoop().RunUntilIdle();
RunLoop().RunUntilIdle() runs tasks on the current thread. Therefore, if the
MessageLoop runs on another thread, it is not guaranteed that the
scoped_com_initializer->reset() task has run when this returns (DCHECK fails and
use-after-free). Anyways, ScopedTaskScheduler is not intended to be used with a
MessageLoop from another thread (use ScopedAsyncTaskScheduler for async
execution).

[robliao, 2017-03-28 17:15:02]

https://codereview.chromium.org/2768873007/diff/60001/base/test/scoped_task_s...
File base/test/scoped_task_scheduler.cc (right):

https://codereview.chromium.org/2768873007/diff/60001/base/test/scoped_task_s...
base/test/scoped_task_scheduler.cc:188: RunLoop().RunUntilIdle();
On 2017/03/28 14:14:46, fdoray wrote:
> RunLoop().RunUntilIdle() runs tasks on the current thread. Therefore, if the
> MessageLoop runs on another thread, it is not guaranteed that the
> scoped_com_initializer->reset() task has run when this returns (DCHECK fails
and
> use-after-free). Anyways, ScopedTaskScheduler is not intended to be used with
a
> MessageLoop from another thread (use ScopedAsyncTaskScheduler for async
> execution).

Yeah, Shutdown() seemed pretty strange to me when I saw
RunLoop().RunUntilIdle(). We can simplify this path. Done.

[robliao, 2017-03-28 17:15:06]

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

[commit-bot: I haz the power, 2017-03-28 17:15:40]

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-28 20:20:04]

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

[commit-bot: I haz the power, 2017-03-28 20:20:05]

Dry run: This issue passed the CQ dry run.

[fdoray, 2017-03-29 12:14:05]

lgtm

[gab, 2017-03-29 17:01:31]

lgtm

[robliao, 2017-03-29 17:01:48]

The CQ bit was checked by robliao@chromium.org

[commit-bot: I haz the power, 2017-03-29 17:02:52]

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-29 17:11:29]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1490806908078990,
"parent_rev": "1d6e7115c66667ae3f401de68bf7cfbc7e694f79", "commit_rev":
"75dd50b325a709221dd971e61eb3e6f53de1a8aa"}

[commit-bot: I haz the power, 2017-03-29 17:12:17]

Description was changed from

==========
Add Support to Get a COM STA Task Runner from the Task Scheduler API

This also adds support for COM STA Task Runner for TaskSchedulerImpl as
well as ScopedTaskScheduler.

BUG=662122
==========

to

==========
Add Support to Get a COM STA Task Runner from the Task Scheduler API

This also adds support for COM STA Task Runner for TaskSchedulerImpl as
well as ScopedTaskScheduler.

BUG=662122

Review-Url: https://codereview.chromium.org/2768873007
Cr-Commit-Position: refs/heads/master@{#460430}
Committed:
https://chromium.googlesource.com/chromium/src/+/75dd50b325a709221dd971e61eb3...
==========

[commit-bot: I haz the power, 2017-03-29 17:12:18]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/75dd50b325a709221dd971e61eb3...