Expose different task runner in RenderFrame for convenience

Expose different task runner in RenderFrame for convenience

BUG=676805
Review-Url: https://codereview.chromium.org/2643633003
Cr-Commit-Position: refs/heads/master@{#445040}
Committed: https://chromium.googlesource.com/chromium/src/+/06fe10c00a7cdeabd54105bc61f8e27fcdb8486e

[altimin, 2017-01-18 17:28:48]

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

[nasko, 2017-01-18 17:30:39]

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

[nasko, 2017-01-18 17:30:39]

Please include in the patch the code that will use those APIs.
Thanks!

[altimin, 2017-01-18 18:20:00]

On 2017/01/18 17:30:39, nasko wrote:
> Please include in the patch the code that will use those APIs.
> Thanks!

Changing actual usage is somewhat risky (task queue-related changes can always
go wrong and may need to be reverted), so I prefer to land API first and land
~dozen usage cases later to avoid conflicts when reverting.
I uploaded one of the CLs (merged with current one, only
content/renderer/pepper/renderer_ppapi_host_impl.cc is relevant):
https://codereview.chromium.org/2644583003.

[nasko, 2017-01-18 23:50:49]

On 2017/01/18 18:20:00, altimin wrote:
> On 2017/01/18 17:30:39, nasko wrote:
> > Please include in the patch the code that will use those APIs.
> > Thanks!
> 
> Changing actual usage is somewhat risky (task queue-related changes can always
> go wrong and may need to be reverted), so I prefer to land API first and land
> ~dozen usage cases later to avoid conflicts when reverting.

I would suggest then landing one usage of the API (the least likely to break)
and let it pass a few days of Canary, then the rest can be easily landed
afterwards.

> I uploaded one of the CLs (merged with current one, only
> content/renderer/pepper/renderer_ppapi_host_impl.cc is relevant):
> https://codereview.chromium.org/2644583003.

This CL also does not use the API from outside of content/, so it doesn't need
to be a public API.

[altimin, 2017-01-19 13:27:01]

On 2017/01/18 23:50:49, nasko wrote:
> On 2017/01/18 18:20:00, altimin wrote:
> > On 2017/01/18 17:30:39, nasko wrote:
> > > Please include in the patch the code that will use those APIs.
> > > Thanks!
> > 
> > Changing actual usage is somewhat risky (task queue-related changes can
always
> > go wrong and may need to be reverted), so I prefer to land API first and
land
> > ~dozen usage cases later to avoid conflicts when reverting.
> 
> I would suggest then landing one usage of the API (the least likely to break)
> and let it pass a few days of Canary, then the rest can be easily landed
> afterwards.
> 
> > I uploaded one of the CLs (merged with current one, only
> > content/renderer/pepper/renderer_ppapi_host_impl.cc is relevant):
> > https://codereview.chromium.org/2644583003.
> 
> This CL also does not use the API from outside of content/, so it doesn't need
> to be a public API.

Valid point, but I have more examples if you want. This one is from
extensions/renderer: https://codereview.chromium.org/2640903005/

[jochen (gone - plz use gerrit), 2017-01-19 17:31:09]

https://codereview.chromium.org/2643633003/diff/1/content/public/renderer/ren...
File content/public/renderer/render_frame.h (right):

https://codereview.chromium.org/2643633003/diff/1/content/public/renderer/ren...
content/public/renderer/render_frame.h:251: virtual
base::SingleThreadTaskRunner* GetTimerTaskRunner() = 0;
please add comments that explain the differences, and when to use which (or
point at documentation about that)

[altimin, 2017-01-19 17:49:51]

PTAL

https://codereview.chromium.org/2643633003/diff/1/content/public/renderer/ren...
File content/public/renderer/render_frame.h (right):

https://codereview.chromium.org/2643633003/diff/1/content/public/renderer/ren...
content/public/renderer/render_frame.h:251: virtual
base::SingleThreadTaskRunner* GetTimerTaskRunner() = 0;
On 2017/01/19 17:31:09, jochen (at summit until 19th) wrote:
> please add comments that explain the differences, and when to use which (or
> point at documentation about that)

Done.

[altimin, 2017-01-19 17:49:51]

PTAL

https://codereview.chromium.org/2643633003/diff/1/content/public/renderer/ren...
File content/public/renderer/render_frame.h (right):

https://codereview.chromium.org/2643633003/diff/1/content/public/renderer/ren...
content/public/renderer/render_frame.h:251: virtual
base::SingleThreadTaskRunner* GetTimerTaskRunner() = 0;
On 2017/01/19 17:31:09, jochen (at summit until 19th) wrote:
> please add comments that explain the differences, and when to use which (or
> point at documentation about that)

Done.

[jochen (gone - plz use gerrit), 2017-01-20 07:38:25]

lgtm

[altimin, 2017-01-20 10:44:45]

The CQ bit was checked by altimin@chromium.org

[commit-bot: I haz the power, 2017-01-20 10:44:54]

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

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484909085952730,
"parent_rev": "f116503fb64582fe3c85a1589a1898570976d3cb", "commit_rev":
"06fe10c00a7cdeabd54105bc61f8e27fcdb8486e"}

[commit-bot: I haz the power, 2017-01-20 11:28:45]

Description was changed from

==========
Expose different task runner in RenderFrame for convenience

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

to

==========
Expose different task runner in RenderFrame for convenience

BUG=676805

Review-Url: https://codereview.chromium.org/2643633003
Cr-Commit-Position: refs/heads/master@{#445040}
Committed:
https://chromium.googlesource.com/chromium/src/+/06fe10c00a7cdeabd54105bc61f8...
==========

[commit-bot: I haz the power, 2017-01-20 11:28:46]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/06fe10c00a7cdeabd54105bc61f8...