Use TaskRunner passed in to URLRequestContextBuilder only for the cache

Use TaskRunner passed in to URLRequestContextBuilder only for the cache

It used to be used for file URLs and TransportSecurityStatePersister as
well, but this CL moves those over to using the task scheduler. Can't
just always use that for the cache as well due to the cache needing a
SingleThreadTaskRunner, and getting one of those from the task scheduler
is a bit more resource intensive.

BUG=734199
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2977693003
Cr-Commit-Position: refs/heads/master@{#485919}
Committed: https://chromium.googlesource.com/chromium/src/+/5f72b7fbfd9626c67decee2bfff2f6c26d6d85af

[mmenke, 2017-07-11 16:58:11]

Description was changed from

==========
Use TaskRunner passed in to URLRequestContextBuilder only for the cache

It used to be used for file URLs and TransportSecurityStatePersister as
well, but this CL moves those over to using the task scheduler. Can't
just always use that for the cache as well due to the cache needing a
SingleThreadTaskRunner, and getting one of those from the task scheduler
is a bit more resource intensive.

BUG=734199
==========

to

==========
Use TaskRunner passed in to URLRequestContextBuilder only for the cache

It used to be used for file URLs and TransportSecurityStatePersister as
well, but this CL moves those over to using the task scheduler. Can't
just always use that for the cache as well due to the cache needing a
SingleThreadTaskRunner, and getting one of those from the task scheduler
is a bit more resource intensive.

BUG=734199
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

[mmenke, 2017-07-11 16:59:41]

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

[commit-bot: I haz the power, 2017-07-11 16:59:51]

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-07-11 17:02:45]

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

[commit-bot: I haz the power, 2017-07-11 17:02:45]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[mmenke, 2017-07-11 17:12:58]

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

[commit-bot: I haz the power, 2017-07-11 17:13:13]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mmenke, 2017-07-11 18:01:49]

The CQ bit was unchecked by mmenke@chromium.org

[mmenke, 2017-07-11 18:19:02]

mmenke@chromium.org changed reviewers:
+ mef@chromium.org, rdsmith@chromium.org

[mmenke, 2017-07-11 18:19:03]

[rdsmith]:  Please review net/.  Eventually, we'll probably want to have net
components grab TaskRunners themselves from the scheduler, instead of passing
them in.

[mef]:  Please review Cronet (Want your signoff on the behavior change before
sending this out more widely).

[Randy Smith (Not in Mondays), 2017-07-11 18:27:20]

The new behavior in /net LGTM.  How concerned are you about/how useful would it
be for me to investigate for what consumers this would be a change in behavior
and if there are any gotchas in the change rather than the new behavior?

[mmenke, 2017-07-11 18:31:31]

On 2017/07/11 18:27:20, Randy Smith (Not in Mondays) wrote:
> The new behavior in /net LGTM.  How concerned are you about/how useful would
it
> be for me to investigate for what consumers this would be a change in behavior
> and if there are any gotchas in the change rather than the new behavior?

Since I renamed the method to set the task runner, all consumers that will see a
change in behavior in this CL are actually being modified in this CL.  I think
we should just defer to the owners of those classes on whether this will create
problems.

Other than Cronet, I had to modified Android Webview's main context (Where I
think this is an improvement, or at least it makes us match Chrome behavior),
headless's main context (ditto), and remoting (Which I'm less sure of).

[Randy Smith (Not in Mondays), 2017-07-11 18:35:18]

On 2017/07/11 18:31:31, mmenke wrote:
> On 2017/07/11 18:27:20, Randy Smith (Not in Mondays) wrote:
> > The new behavior in /net LGTM.  How concerned are you about/how useful would
> it
> > be for me to investigate for what consumers this would be a change in
behavior
> > and if there are any gotchas in the change rather than the new behavior?
> 
> Since I renamed the method to set the task runner, all consumers that will see
a
> change in behavior in this CL are actually being modified in this CL.  I think
> we should just defer to the owners of those classes on whether this will
create
> problems.

Good point.  Top level LGTM.

> 
> Other than Cronet, I had to modified Android Webview's main context (Where I
> think this is an improvement, or at least it makes us match Chrome behavior),
> headless's main context (ditto), and remoting (Which I'm less sure of).

[mmenke, 2017-07-11 19:49:44]

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

[commit-bot: I haz the power, 2017-07-11 19:50:03]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mmenke, 2017-07-11 20:57:42]

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

[commit-bot: I haz the power, 2017-07-11 20:58:14]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mmenke, 2017-07-11 21:06:40]

mmenke@chromium.org changed reviewers:
+ eseckler@chromium.org, sergeyu@chromium.org, sgurun@chromium.org

[mmenke, 2017-07-11 21:06:41]

[+sgurun]: Please review
android_webview/browser/net/aw_url_request_context_getter.cc
[+eseckler] Please review
headless/lib/browser/headless_url_request_context_getter.cc
[+sergeyu]:  Please review remoting/base/url_request_context_getter.cc

The passed in task runner is now only used for the disk cache, so consumers that
don't use a disk cache don't need to pass in a task runner. 
URLRequestContextBuilder now uses TaskScheduler for all the other threads it
needs, so this CL implicitly requires that using a TaskSchedulers does not cause
problems for your use cases.  It only uses TaskSchedulers for file URLs and
TransportSecurityPerister, anyways, so not a huge change.

[Do not use (sergeyu), 2017-07-11 21:16:31]

sergeyu@google.com changed reviewers:
+ sergeyu@google.com

[Do not use (sergeyu), 2017-07-11 21:16:31]

remoting lgtm

[Sergey Ulanov, 2017-07-11 21:20:27]

Description was changed from

==========
Use TaskRunner passed in to URLRequestContextBuilder only for the cache

It used to be used for file URLs and TransportSecurityStatePersister as
well, but this CL moves those over to using the task scheduler. Can't
just always use that for the cache as well due to the cache needing a
SingleThreadTaskRunner, and getting one of those from the task scheduler
is a bit more resource intensive.

BUG=734199
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
Use TaskRunner passed in to URLRequestContextBuilder only for the cache

It used to be used for file URLs and TransportSecurityStatePersister as
well, but this CL moves those over to using the task scheduler. Can't
just always use that for the cache as well due to the cache needing a
SingleThreadTaskRunner, and getting one of those from the task scheduler
is a bit more resource intensive.

BUG=734199
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

[Sergey Ulanov, 2017-07-11 21:20:27]

sergeyu@chromium.org changed reviewers:
- sergeyu@google.com

[Sergey Ulanov, 2017-07-11 21:20:39]

lgtm from the right account

[mef, 2017-07-11 21:23:39]

cronet lgtm

[sgurun-gerrit only, 2017-07-11 21:40:37]

aw lgtm

[commit-bot: I haz the power, 2017-07-11 22:59:20]

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

[commit-bot: I haz the power, 2017-07-11 22:59:21]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has
not started yet; builder either lacks capacity or does not exist (misspelled?))
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet;
builder either lacks capacity or does not exist (misspelled?))
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does
not exist (misspelled?))
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does
not exist (misspelled?))
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does
not exist (misspelled?))
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, build
has not started yet; builder either lacks capacity or does not exist
(misspelled?))

[Eric Seckler, 2017-07-12 08:00:04]

headless lgtm. Thanks!

[mmenke, 2017-07-12 09:41:40]

The CQ bit was checked by mmenke@chromium.org

[mmenke, 2017-07-12 09:41:41]

The patchset sent to the CQ was uploaded after l-g-t-m from rdsmith@chromium.org
Link to the patchset: https://codereview.chromium.org/2977693003/#ps60001
(title: "Actually, don't need to set the thread everywhere")

[commit-bot: I haz the power, 2017-07-12 09:41:55]

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-07-12 10:54:14]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1499852500862920,
"parent_rev": "18f863a17129ac080b63b6efd370ffc4f85e89b6", "commit_rev":
"5f72b7fbfd9626c67decee2bfff2f6c26d6d85af"}

[commit-bot: I haz the power, 2017-07-12 10:54:33]

Description was changed from

==========
Use TaskRunner passed in to URLRequestContextBuilder only for the cache

It used to be used for file URLs and TransportSecurityStatePersister as
well, but this CL moves those over to using the task scheduler. Can't
just always use that for the cache as well due to the cache needing a
SingleThreadTaskRunner, and getting one of those from the task scheduler
is a bit more resource intensive.

BUG=734199
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
Use TaskRunner passed in to URLRequestContextBuilder only for the cache

It used to be used for file URLs and TransportSecurityStatePersister as
well, but this CL moves those over to using the task scheduler. Can't
just always use that for the cache as well due to the cache needing a
SingleThreadTaskRunner, and getting one of those from the task scheduler
is a bit more resource intensive.

BUG=734199
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2977693003
Cr-Commit-Position: refs/heads/master@{#485919}
Committed:
https://chromium.googlesource.com/chromium/src/+/5f72b7fbfd9626c67decee2bfff2...
==========

[commit-bot: I haz the power, 2017-07-12 10:54:34]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/5f72b7fbfd9626c67decee2bfff2...