Switch to using URLRequestContextBuilder to create some URLRequestContexts.

Switch to using URLRequestContextBuilder to create some contexts.

Also add missing functionality to URLRequestContextBuilder to fit
their needs. This is intended to simplify code to initialize the
network stack, and make it easier to refactor network stack
initialization.

BUG=388836
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=281638

[mmenke, 2014-06-25 18:56:47]

This is pretty analogous to the CL where we switched Cronet to using
URLRequestContextBuilder.

This CL isn't too complicated despite the number of files it modifies, but if
you prefer, I can split this up into three or four CLs, for easier review.

Some of the builder changes fall into things we'd probably want to do for
CroNet, anyways - setting a ProxyService [flywheel is going to use its own] and
adding server bound cert support.

https://codereview.chromium.org/354693004/diff/140001/net/url_request/url_req...
File net/url_request/url_request_context_builder.cc (right):

https://codereview.chromium.org/354693004/diff/140001/net/url_request/url_req...
net/url_request/url_request_context_builder.cc:303:
context->GetFileThread()->message_loop_proxy()));
This is a behavior change, but it sounds like we should be doing this everywhere
anyways.

[darin (slow to review), 2014-06-25 18:58:29]

src/mojo/ changes LGTM ... actually they look fantastically awesome to me ;-)

[mmenke, 2014-06-30 20:39:51]

Misha:  Ping!

Like I said, there's no rush on this, but I'd like to get it (Or send it for
wider review) in next week.  Sending it to you because of builder's use in
Cronet.  Suspect we'll be spending time with the class in the future.

[mef, 2014-06-30 21:42:59]

sorry for the delay, I totally forgot. :-(

https://codereview.chromium.org/354693004/diff/140001/chrome/service/net/serv...
File chrome/service/net/service_url_request_context_getter.cc (left):

https://codereview.chromium.org/354693004/diff/140001/chrome/service/net/serv...
chrome/service/net/service_url_request_context_getter.cc:139:
storage_.set_cookie_store(new net::CookieMonster(NULL, NULL));
Do we need to set it up now as well?

https://codereview.chromium.org/354693004/diff/140001/mojo/services/network/n...
File mojo/services/network/network_context.cc (left):

https://codereview.chromium.org/354693004/diff/140001/mojo/services/network/n...
mojo/services/network/network_context.cc:65: new net::ServerBoundCertService(
do we need this?

https://codereview.chromium.org/354693004/diff/140001/mojo/services/network/n...
File mojo/services/network/network_context.cc (right):

https://codereview.chromium.org/354693004/diff/140001/mojo/services/network/n...
mojo/services/network/network_context.cc:25: cache_params.path =
base_path.Append(FILE_PATH_LITERAL("Cache"));
shouldn't this be |cache_path|?

https://codereview.chromium.org/354693004/diff/140001/mojo/services/network/n...
mojo/services/network/network_context.cc:26: cache_params.type =
net::URLRequestContextBuilder::HttpCacheParams::IN_MEMORY;
Why change from DISK_CACHE to IN_MEMORY?

https://codereview.chromium.org/354693004/diff/140001/net/url_request/url_req...
File net/url_request/url_request_context_builder.cc (right):

https://codereview.chromium.org/354693004/diff/140001/net/url_request/url_req...
net/url_request/url_request_context_builder.cc:146: cache_thread_.reset(new
base::Thread("Network cache Thread"));
nit: a bit strange that second word is not capitalized, but third one is.

[mmenke, 2014-06-30 21:55:25]

Thanks for the feedback!

https://codereview.chromium.org/354693004/diff/140001/chrome/service/net/serv...
File chrome/service/net/service_url_request_context_getter.cc (left):

https://codereview.chromium.org/354693004/diff/140001/chrome/service/net/serv...
chrome/service/net/service_url_request_context_getter.cc:139:
storage_.set_cookie_store(new net::CookieMonster(NULL, NULL));
On 2014/06/30 21:42:58, mef wrote:
> Do we need to set it up now as well?

We already do, don't we?

https://codereview.chromium.org/354693004/diff/140001/mojo/services/network/n...
File mojo/services/network/network_context.cc (left):

https://codereview.chromium.org/354693004/diff/140001/mojo/services/network/n...
mojo/services/network/network_context.cc:65: new net::ServerBoundCertService(
On 2014/06/30 21:42:58, mef wrote:
> do we need this?

We do, and I added this to the builder (See line 300 in the builder's cc file). 
This will affect all contexts, but I after talking to David, it sounds like this
is desired (And we'll want to add a way to allow it to be stored on disk, at
some point, presumably).

https://codereview.chromium.org/354693004/diff/140001/mojo/services/network/n...
File mojo/services/network/network_context.cc (right):

https://codereview.chromium.org/354693004/diff/140001/mojo/services/network/n...
mojo/services/network/network_context.cc:25: cache_params.path =
base_path.Append(FILE_PATH_LITERAL("Cache"));
On 2014/06/30 21:42:58, mef wrote:
> shouldn't this be |cache_path|?

Actually, I meant to delete the cache_path line (Which I've now done).

https://codereview.chromium.org/354693004/diff/140001/mojo/services/network/n...
mojo/services/network/network_context.cc:26: cache_params.type =
net::URLRequestContextBuilder::HttpCacheParams::IN_MEMORY;
On 2014/06/30 21:42:59, mef wrote:
> Why change from DISK_CACHE to IN_MEMORY?

Eee...That would have been bad!  Thanks for catching that.  Copy-paste error.

https://codereview.chromium.org/354693004/diff/140001/net/url_request/url_req...
File net/url_request/url_request_context_builder.cc (right):

https://codereview.chromium.org/354693004/diff/140001/net/url_request/url_req...
net/url_request/url_request_context_builder.cc:146: cache_thread_.reset(new
base::Thread("Network cache Thread"));
On 2014/06/30 21:42:59, mef wrote:
> nit: a bit strange that second word is not capitalized, but third one is.

Done.

[mef, 2014-07-01 12:08:09]

lgtm

https://codereview.chromium.org/354693004/diff/160001/chrome/service/net/serv...
File chrome/service/net/service_url_request_context_getter.cc (right):

https://codereview.chromium.org/354693004/diff/160001/chrome/service/net/serv...
chrome/service/net/service_url_request_context_getter.cc:79: // Returns the
default user agent.
nit/q: Is it actually customized elsewhere?

[mmenke, 2014-07-01 14:32:57]

https://codereview.chromium.org/354693004/diff/160001/chrome/service/net/serv...
File chrome/service/net/service_url_request_context_getter.cc (right):

https://codereview.chromium.org/354693004/diff/160001/chrome/service/net/serv...
chrome/service/net/service_url_request_context_getter.cc:79: // Returns the
default user agent.
On 2014/07/01 12:08:08, mef wrote:
> nit/q: Is it actually customized elsewhere?

Looks like cloud print uses the same getter, but a different user agent: 
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/service/clo...

[mmenke, 2014-07-01 14:40:24]

[+sergeyu]:  Please review remoting/.
[+ajwong]:  Please review chrome/service/.

Note that on Linux/Android, the builder doesn't create the system proxy service
by default, due to weird threading requirements, which is why the embedders
still have to create it themselves.

[Sergey Ulanov, 2014-07-03 07:01:28]

remoting - lgtm

[mmenke, 2014-07-07 21:14:20]

[+ajwong (For real, this time)]:  Please review chrome/service/.

[awong, 2014-07-07 21:20:04]

chrome/service LGTM

[mmenke, 2014-07-07 21:22:29]

The CQ bit was checked by mmenke@chromium.org

[commit-bot: I haz the power, 2014-07-07 21:23:20]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mmenke@chromium.org/354693004/160001

[commit-bot: I haz the power, 2014-07-07 21:51:22]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/9...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/20...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/25575)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/30026)

[commit-bot: I haz the power, 2014-07-07 21:54:02]

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

[commit-bot: I haz the power, 2014-07-07 21:54:03]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/25592)

[mmenke, 2014-07-07 23:35:33]

The CQ bit was checked by mmenke@chromium.org

[commit-bot: I haz the power, 2014-07-07 23:36:13]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mmenke@chromium.org/354693004/200001

[mmenke, 2014-07-07 23:39:12]

Failures were just due to a merge conflict with a moved header file.

[commit-bot: I haz the power, 2014-07-08 02:33:57]

Change committed as 281638