Use URLRequestContextBuilderMojo to create the SystemURLRequestContext.

net/url_request/url_request_context_builder_mojo.h

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef NET_URL_REQUEST_URL_REQUEST_CONTEXT_BUILDER_MOJO_H_
 #define NET_URL_REQUEST_URL_REQUEST_CONTEXT_BUILDER_MOJO_H_
 
 #include <memory>
 
 #include "base/macros.h"
@@ skipping 16 matching lines @@
 // URLRequestContextBuilder API, it will be used instead.
 class URLRequestContextBuilderMojo : public URLRequestContextBuilder {
  public:
   URLRequestContextBuilderMojo();
   ~URLRequestContextBuilderMojo() override;
 
   // Overrides default DhcpProxyScriptFetcherFactory. Ignored if no
   // MojoProxyResolverFactory is provided.
   void set_dhcp_fetcher_factory(
       std::unique_ptr<DhcpProxyScriptFetcherFactory> dhcp_fetcher_factory) {
-    dhcp_fetcher_factory = std::move(dhcp_fetcher_factory_);

[Randy Smith (Not in Mondays), 2017/06/13 16:42:15]

Snort.  That's cute.  Is there a (presumably new) test that would catch this?

[mmenke, 2017/06/13 17:02:22]

This is caught by integration tests...If you look at set 4, a rather large
number number of ChromeOS browser tests failed, as well as a couple IOThread
unittests.  They all fail on a DCHECK in CreateProxyServiceUsingMojoFactory
(https://cs.chromium.org/chromium/src/net/proxy/proxy_service_mojo.cc?type=cs&...).
 I'm not sure if we have any test that actually depends on a functional
DhcpFetcherFactory being set (Either a real one or a mock one).

That having been said, there are no URLRequestContextBuilderMojo unit tests that
catch this, and I'm not sure we really want unit tests that check every
individual setter in URLRequestContextBuilder[Mojo].

That having been said, I'd love it if we switched to using custom
URLRequestContextBulders in net unittests instead of
TestURLRequestContextBuilders, to better exercise this code.  And this fix was
the thing that inspired my comment on unit tests.

[Randy Smith (Not in Mondays), 2017/06/15 19:13:30]

I'm torn here, but I sorta think that if we're going to expose an API we should
test it.  But it's at "todo" level; I'm certainly not going to ask you to do it
for this CL.
How would you feel (honest question; I'm not clear this isn't excessive and I'm
not asking for it, just musing) about making sure there are unit tests for
everything that you exercise from io_thread.cc?

[mmenke, 2017/06/15 19:33:24]

I really don't want to have 5 sets of tests for everything (unit tests,
URLRequest-layer tests, URLRequestContextBuilder layer tests, NetworkService
tests, and chrome/content level integration tests) that just make sure
configuring each thing works.  Seven sets if you throw in both incognito and
IOThread configuration.
I'd really like to have tests at the NetworkService layer instead, which what
consumers should hopefully be using (And I don't want NetworkService and
URLRequestContextBuilder tests for everything they can set).  I'm all for tests,
but at some point, have to actually write some production code.  :)

Testing at this layer also requires a lot of set up - DHCP scripts, injecting
ProxyConfigs, injecting ProxyConfigServices, mocking out
DhcpScriptFetcherFactories (And DhcpScriptFetchers), etc.  It's not just do the
same thing a bunch of times with minor variations.

That's not to say I don't think we should have any tests - I did add one for the
basic mojo resolver path, just not for the DhcpProxyScriptFetcher logic.

+    dhcp_fetcher_factory_ = std::move(dhcp_fetcher_factory);
   }
 
   // Sets Mojo factory used to create ProxyResolvers. If not set, falls back to
   // URLRequestContext's default behavior. The passed in factory must outlive
   // the URLRequestContext the builder creates.
   void set_mojo_proxy_resolver_factory(
       MojoProxyResolverFactory* mojo_proxy_resolver_factory) {
     mojo_proxy_resolver_factory_ = mojo_proxy_resolver_factory;
   }
 
  private:
   std::unique_ptr<ProxyService> CreateProxyService(
       std::unique_ptr<ProxyConfigService> proxy_config_service,
       URLRequestContext* url_request_context,
       HostResolver* host_resolver,
       NetworkDelegate* network_delegate,
       NetLog* net_log) override;
 
   std::unique_ptr<DhcpProxyScriptFetcherFactory> dhcp_fetcher_factory_;
 
   MojoProxyResolverFactory* mojo_proxy_resolver_factory_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(URLRequestContextBuilderMojo);
 };
 
 }  // namespace net
 
 #endif  // NET_URL_REQUEST_URL_REQUEST_CONTEXT_BUILDER_MOJO_H_