Use loading task queue in RendererPpapiHost

Use loading task quue in RendererPpapiHost

As a part of effort to post tasks to correct scheduler task queues instead
of default one, use frame-specific loading task runner in RendererPpapiHost.

BUG=676805

[altimin, 2017-01-18 18:16:26]

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

[altimin, 2017-01-18 18:16:50]

Description was changed from

==========
Use loading task quue in RendererPpapiHost

As a part of effort to post tasks to correct scheduler task queues instead
of default one, use frame-specific loading task runner in RendererPpapiHost.

BUG=676805
==========

to

==========
Use loading task quue in RendererPpapiHost

As a part of effort to post tasks to correct scheduler task queues instead
of default one, use frame-specific loading task runner in RendererPpapiHost.

BUG=676805
==========

[commit-bot: I haz the power, 2017-01-18 18:17: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-01-18 19:05:24]

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

[commit-bot: I haz the power, 2017-01-18 19:05:24]

Dry run: This issue passed the CQ dry run.

[altimin, 2017-01-20 19:58:18]

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

[commit-bot: I haz the power, 2017-01-20 19:58:45]

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

[altimin, 2017-01-20 19:58:57]

altimin@chromium.org changed reviewers:
+ raymes@chromium.org

[altimin, 2017-01-20 19:58:58]

PTAL

[commit-bot: I haz the power, 2017-01-20 20:57:13]

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

[commit-bot: I haz the power, 2017-01-20 20:57:14]

Dry run: This issue passed the CQ dry run.

[raymes, 2017-01-22 23:32:47]

Hey - unfortunately I don't know enough about task runners and there's not
enough context for me in the bug (design doc, etc.) to understand what you're
trying to do and whether this is right.

It may be good to have someone who is familiar with the refactoring you are
doing to review this change and then I can rubberstamp it. It may also be good
to add more detail to the bug in a design doc.

Thanks!

[altimin, 2017-01-23 11:30:46]

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

[altimin, 2017-01-23 11:30:46]

+alexclarke@ as an expert in scheduling

PTAL

[alex clarke (OOO till 29th), 2017-01-23 11:37:05]

https://codereview.chromium.org/2644583003/diff/20001/content/renderer/pepper...
File content/renderer/pepper/renderer_ppapi_host_impl.cc (right):

https://codereview.chromium.org/2644583003/diff/20001/content/renderer/pepper...
content/renderer/pepper/renderer_ppapi_host_impl.cc:265:
render_frame->GetLoadingTaskRunner()->PostTask(
So in theory I think that's a reasonable change. The tricky bit (and where I
don't know enough about PPAPI to be sure) is there might be some ordering
dependencies.

E.g. perhaps some of these need to change as well:

https://cs.chromium.org/search/?q=PostTask+f:renderer+f:pepper&sq=package:chr...

You might just have to try this and see.  It's a simple change so a revert if
needed is easy.

[alex clarke (OOO till 29th), 2017-01-23 12:24:14]

lgtm

[raymes, 2017-01-23 22:47:06]

> So in theory I think that's a reasonable change. The tricky bit (and where I
> don't know enough about PPAPI to be sure) is there might be some ordering
> dependencies.

Pepper does have complex ordering requirements due to synchronous messages and
handling of input events.  I don't know how those play into this CL though.
piman@ would know but he's on leave. Do you think it's still worth landing this
as a test? I would be a little worried about subtle bugs.

[altimin, 2017-01-23 22:53:25]

On 2017/01/23 22:47:06, raymes wrote:
> > So in theory I think that's a reasonable change. The tricky bit (and where I
> > don't know enough about PPAPI to be sure) is there might be some ordering
> > dependencies.
> 
> Pepper does have complex ordering requirements due to synchronous messages and
> handling of input events.  I don't know how those play into this CL though.
> piman@ would know but he's on leave. Do you think it's still worth landing
this
> as a test? I would be a little worried about subtle bugs.

I think I'll wait for piman@ and will try to refactor all usages of
ThreadTaskRunnerHandle::Get()
in content/renderer/pepper.

[raymes, 2017-01-23 22:54:34]

Thanks, sgtm