Make ProfileIOData use URLRequestContextBuilder

net/url_request/url_request_context_builder.h

Index: net/url_request/url_request_context_builder.h
diff --git a/net/url_request/url_request_context_builder.h b/net/url_request/url_request_context_builder.h
index 9f695c39312854d96fbbca8924e511b90719191f..95e90dfaacbb3616eb12cdfe01314adee531f08b 100644
--- a/net/url_request/url_request_context_builder.h
+++ b/net/url_request/url_request_context_builder.h
@@ -23,6 +23,7 @@
 #include <utility>
 #include <vector>
 
+#include "base/callback.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
@@ -54,6 +55,7 @@ class CookieStore;
 class CTPolicyEnforcer;
 class CTVerifier;
 class HttpAuthHandlerFactory;
+class HttpUserAgentSettings;
 class HttpServerProperties;
 class NetworkQualityEstimator;
 class ProxyConfigService;
@@ -79,12 +81,18 @@ class URLRequestInterceptor;
 // Builder may be used to create only a single URLRequestContext.
 class NET_EXPORT URLRequestContextBuilder {
  public:
+  using CreateInterceptingJobFactory =
+      base::OnceCallback<std::unique_ptr<net::URLRequestJobFactory>(
+          std::unique_ptr<net::URLRequestJobFactory> inner_job_factory)>;
+
   struct NET_EXPORT HttpCacheParams {
     enum Type {
       // In-memory cache.
       IN_MEMORY,
       // Disk cache using "default" backend.
       DISK,
+      // Disk cache using "blockfile" backend (BackendImpl).
+      DISK_BLOCKFILE,
       // Disk cache using "simple" backend (SimpleBackendImpl).
       DISK_SIMPLE,
     };
@@ -166,12 +174,13 @@ class NET_EXPORT URLRequestContextBuilder {
   // Call these functions to specify hard-coded Accept-Language
   // or User-Agent header values for all requests that don't
   // have the headers already set.
-  void set_accept_language(const std::string& accept_language) {
-    accept_language_ = accept_language;
-  }
-  void set_user_agent(const std::string& user_agent) {
-    user_agent_ = user_agent;
-  }
+  void set_accept_language(const std::string& accept_language);
+  void set_user_agent(const std::string& user_agent);
+  // Makes the created URLRequestContext use a shared HttpUserAgentSettings
+  // object. Not compatible with set_accept_language() / set_user_agent().
+  // TODO(mmenke): Take ownership of the object instead.
+  void set_shared_http_user_agent_settings(
+      HttpUserAgentSettings* shared_http_user_agent_settings);

[Randy Smith (Not in Mondays), 2017/07/14 21:37:09]

I can't imagine it will surprise you that I'm somewhat offended by the existence
of the "set_*/set_shared_*" distinction in this class :-} (though if you are
surprised, let me know and I'll expand on my discomfort).  I get why you're
doing it, but I'd love to see it go away at some point.  I'm reaching for
pushing things in this category either into a truly shared global object for the
network stack, where we can audit privacy and security issues (aren't there
privacy issues with sharing a host resolver between profiles?) or just having
them separately owned. 

This isn't something reasonable to do in this CL, but I'd like to make sure we
don't lose track--could you file a bug and reference it from the code?  

Also, just because I'm OCD, could you put a "consumer must guarantee that this
object outlives the created URLRequestContext"?  (Applies to all "set_shared*"
methods.)   It seems obvious, but it's also vital, and in balance I think that
makes it worth saying.

(Separately I think the dependency on objects owned in the
SystemURLRequestContext implies a shutdown ordering requirement--is that
documented anywhere?  Not new in this CL, obviously; it just looks like a spike
that someone could impale themselves on at some point.)

[mmenke, 2017/07/14 22:45:37]

Much of this CL is adding technical debt that will hopefully be reasonable to
clean up after we switch configuration over to the NetworkContext.  I have no
intention of cleaning this stuff up while things are still configured by
ProfileIOData directly touching the URLRequestContextBuilder.  I want to
configure most everything through NetworkContext, which will lets us clean most
of these up, and then switch the App request contexts over to a network context,
resulting in duplicating a ton of objects, in the uncommon case where we have
App request contexts.

We could move AppRequestContexts over first, and then clean these up by
duplicating a ton of stuff using the current ProfileIOData code, but that seems
like a lot more work, since we'll want to switch over to having everything go
through NetworkContext, anyways (i.e., we'd need to set up UserAgentSettings
objects watching prefs on a per-URLRequestContex basis, basically creating a
LazyParams object for each AppRequestContext).

Anyhow, filed a bug, bug skeptical of the utility of that, given that the number
of open network bugs on the tracker is increasing monotonically, and eroman
seems to be the only person who occasionally takes on random code health bugs. 
:)
Done.
ProfilIOData talks about it a bit, but the whole thing is rather a mess.  Given
that I think we should not L-G-T-M anything that adds new objects to the
URLRequestContext, unless set up sanely through the builder, I'm not sure about
the utility of yet more documentation for stuff I'm actively trying to get rid
of, and that won't exist in the Brave New NetworkService World.

[Randy Smith (Not in Mondays), 2017/07/18 18:26:54]

That probably makes sense.  My sense is that we're moving away from App request
contexts and at some point should take the hit of deleting them, but that's a
Chrome level issue.
Fair enough.  Thanks for humoring me.  
Ok.

 
   // Control support for data:// requests. By default it's disabled.
   void set_data_enabled(bool enable) {
@@ -205,9 +214,12 @@ class NET_EXPORT URLRequestContextBuilder {
   void set_net_log(NetLog* net_log) { net_log_ = net_log; }
 
   // By default host_resolver is constructed with CreateDefaultResolver.
-  void set_host_resolver(std::unique_ptr<HostResolver> host_resolver) {
-    host_resolver_ = std::move(host_resolver);
-  }
+  void set_host_resolver(std::unique_ptr<HostResolver> host_resolver);
+  // Allows sharing the HostResolver with other URLRequestContexts. Should not
+  // be used if set_host_resolver() is used.
+  // TODO(mmenke): Figure out the cost/benefits of not supporting HostResolvers
+  // between URLRequestContexts.
+  void set_shared_host_resolver(HostResolver* host_resolver);
 
   // Uses BasicNetworkDelegate by default. Note that calling Build will unset
   // any custom delegate in builder, so this must be called each time before
@@ -216,20 +228,24 @@ class NET_EXPORT URLRequestContextBuilder {
     network_delegate_ = std::move(delegate);
   }
 
-  // Temporarily stores a ProxyDelegate. Ownership is transferred to
-  // UrlRequestContextStorage during Build.
-  void set_proxy_delegate(std::unique_ptr<ProxyDelegate> delegate) {
-    proxy_delegate_ = std::move(delegate);
-  }
+  // Sets the ProxyDelegate.
+  void set_proxy_delegate(std::unique_ptr<ProxyDelegate> proxy_delegate);
+  // Allows sharing the PreoxyDelegates with other URLRequestContexts. Should
+  // not be used if set_proxy_delegate() is used.
+  // TODO(mmenke): Remove this (And update consumers).
+  void set_shared_proxy_delegate(ProxyDelegate* shared_proxy_delegate);
 
   // Sets a specific HttpAuthHandlerFactory to be used by the URLRequestContext
   // rather than the default |HttpAuthHandlerRegistryFactory|. The builder
   // takes ownership of the factory and will eventually transfer it to the new
-  // URLRequestContext. Note that since Build will transfer ownership, the
-  // custom factory will be unset and this must be called before the next Build
-  // to set another custom one.
+  // URLRequestContext.
   void SetHttpAuthHandlerFactory(
       std::unique_ptr<HttpAuthHandlerFactory> factory);
+  // Makes the created URLRequestContext use a shared HttpAuthHandlerFactory
+  // object. Not compatible with SetHttpAuthHandlerFactory().
+  // TODO(mmenke): Evaluate if sharing is really needed.
+  void set_shared_http_auth_handler_factory(
+      HttpAuthHandlerFactory* shared_http_auth_handler_factory);
 
   // By default HttpCache is enabled with a default constructed HttpCacheParams.
   void EnableHttpCache(const HttpCacheParams& params);
@@ -246,6 +262,15 @@ class NET_EXPORT URLRequestContextBuilder {
     transport_security_persister_path_ = transport_security_persister_path;
   }
 
+  // Sets whether the TransportSecurityPersister only reads persisted
+  // information, or also writes it. By default, it both reads and writes.
+  // TODO(mmenke): Consider merging this with the above method.
+  void set_transport_security_persister_readonly(
+      bool transport_security_persister_readonly) {
+    transport_security_persister_readonly_ =
+        transport_security_persister_readonly;
+  }
+
   void SetSpdyAndQuicEnabled(bool spdy_enabled,
                              bool quic_enabled);
 
@@ -259,6 +284,11 @@ class NET_EXPORT URLRequestContextBuilder {
 
   void SetCertVerifier(std::unique_ptr<CertVerifier> cert_verifier);
 
+  // Makes the created URLRequestContext use a shared CertVerifier object.
+  // Should not be used it SetCertVerifier() is used.
+  // TODO(mmenke): Figure out if this can take ownership of the object instead.
+  void set_shared_cert_verifier(CertVerifier* shared_cert_verifier);
+
 #if BUILDFLAG(ENABLE_REPORTING)
   void set_reporting_policy(
       std::unique_ptr<net::ReportingPolicy> reporting_policy);
@@ -267,6 +297,13 @@ class NET_EXPORT URLRequestContextBuilder {
   void SetInterceptors(std::vector<std::unique_ptr<URLRequestInterceptor>>
                            url_request_interceptors);
 
+  // Sets a callback that is passed ownership of the URLRequestJobFactory, and
+  // can wrap it in another URLRequestJobFactory. URLRequestInterceptors can't
+  // handle intercepting unsupported protocols, while this case.
+  // TODO(mmenke): Remove this, once it's no longer needed.
+  void set_create_intercepting_job_factory(
+      CreateInterceptingJobFactory create_intercepting_job_factory);
+
   // Override the default in-memory cookie store and channel id service.
   // If both |cookie_store| and |channel_id_service| are NULL, CookieStore and
   // ChannelIDService will be disabled for this context.
@@ -322,6 +359,8 @@ class NET_EXPORT URLRequestContextBuilder {
 
   std::string accept_language_;
   std::string user_agent_;
+  HttpUserAgentSettings* shared_http_user_agent_settings_;
+
   // Include support for data:// requests.
   bool data_enabled_;
 #if !BUILDFLAG(DISABLE_FILE_SUPPORT)
@@ -341,8 +380,10 @@ class NET_EXPORT URLRequestContextBuilder {
   HttpCacheParams http_cache_params_;
   HttpNetworkSession::Params http_network_session_params_;
   base::FilePath transport_security_persister_path_;
+  bool transport_security_persister_readonly_;
   NetLog* net_log_;
   std::unique_ptr<HostResolver> host_resolver_;
+  net::HostResolver* shared_host_resolver_;
   std::unique_ptr<ChannelIDService> channel_id_service_;
   std::unique_ptr<ProxyConfigService> proxy_config_service_;
   bool pac_quick_check_enabled_;
@@ -351,15 +392,19 @@ class NET_EXPORT URLRequestContextBuilder {
   scoped_refptr<net::SSLConfigService> ssl_config_service_;
   std::unique_ptr<NetworkDelegate> network_delegate_;
   std::unique_ptr<ProxyDelegate> proxy_delegate_;
+  ProxyDelegate* shared_proxy_delegate_;
   std::unique_ptr<CookieStore> cookie_store_;
   std::unique_ptr<HttpAuthHandlerFactory> http_auth_handler_factory_;
+  HttpAuthHandlerFactory* shared_http_auth_handler_factory_;
   std::unique_ptr<CertVerifier> cert_verifier_;
+  CertVerifier* shared_cert_verifier_;
   std::unique_ptr<CTVerifier> ct_verifier_;
   std::unique_ptr<CTPolicyEnforcer> ct_policy_enforcer_;
 #if BUILDFLAG(ENABLE_REPORTING)
   std::unique_ptr<net::ReportingPolicy> reporting_policy_;
 #endif  // BUILDFLAG(ENABLE_REPORTING)
   std::vector<std::unique_ptr<URLRequestInterceptor>> url_request_interceptors_;
+  CreateInterceptingJobFactory create_intercepting_job_factory_;
   std::unique_ptr<HttpServerProperties> http_server_properties_;
   std::map<std::string, std::unique_ptr<URLRequestJobFactory::ProtocolHandler>>
       protocol_handlers_;