Use a shared HostResolverManager in Chromecast

Each individual URLRequestContext gets its own HostResolver instance.

Also, because that HostResolver becomes a dependency of the
HttpNetworkSession, constructed a new session for the media context
rather than reusing the session from the main context and built the
whole context from scratch rather than relying on the dangerous
URLRequestContext::CopyFrom(). Saved the sessions to a unique_ptr
rather than leaking it as was done previously.

Bug: 934402
Change-Id: I1c61f58406c2df66786da72146c4eb2631f6450c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1529518Reviewed-by: Luke Halliwell <halliwell@chromium.org>
Reviewed-by: Asanka Herath <asanka@chromium.org>
Commit-Queue: Eric Orth <ericorth@chromium.org>
Cr-Commit-Position: refs/heads/master@{#648703}

chromecast/browser/url_request_context_factory.cc

@@ -32,6 +32,7 @@
 #include "net/cert_net/nss_ocsp.h"
 #include "net/cookies/cookie_store.h"
 #include "net/dns/host_resolver.h"
+#include "net/dns/host_resolver_manager.h"
 #include "net/http/http_auth_handler_factory.h"
 #include "net/http/http_network_layer.h"
 #include "net/http/http_server_properties_impl.h"
@@ -233,9 +234,8 @@ void URLRequestContextFactory::InitializeSystemContextDependencies() {
   if (system_dependencies_initialized_)
     return;
 
-  // TODO(crbug.com/934402): Use a shared HostResolverManager instead of a
-  // global HostResolver.
-  host_resolver_ = net::HostResolver::CreateStandaloneResolver(nullptr);
+  host_resolver_manager_ = std::make_unique<net::HostResolverManager>(
+      net::HostResolver::Options(), nullptr);
   cert_verifier_ = net::CertVerifier::CreateDefault();
   ssl_config_service_.reset(new net::SSLConfigServiceDefaults);
   transport_security_state_.reset(new net::TransportSecurityState());
@@ -252,6 +252,10 @@ void URLRequestContextFactory::InitializeSystemContextDependencies() {
   proxy_resolution_service_ =
       net::ProxyResolutionService::CreateUsingSystemProxyResolver(
           std::move(proxy_config_service_), nullptr);
+
+  system_host_resolver_ =
+      net::HostResolver::CreateResolver(host_resolver_manager_.get());
+
   system_dependencies_initialized_ = true;
 }
 
@@ -300,15 +304,18 @@ void URLRequestContextFactory::InitializeMainContextDependencies(
 
   main_job_factory_ = std::move(top_job_factory);
 
+  main_host_resolver_ =
+      net::HostResolver::CreateResolver(host_resolver_manager_.get());
+
   main_dependencies_initialized_ = true;
 }
 
-void URLRequestContextFactory::InitializeMediaContextDependencies(
-    net::HttpTransactionFactory* transaction_factory) {
+void URLRequestContextFactory::InitializeMediaContextDependencies() {
   if (media_dependencies_initialized_)
     return;
 
-  media_transaction_factory_.reset(transaction_factory);
+  media_host_resolver_ =
+      net::HostResolver::CreateResolver(host_resolver_manager_.get());
   media_dependencies_initialized_ = true;
 }
 
@@ -338,6 +345,18 @@ void URLRequestContextFactory::PopulateNetworkSessionParams(
             << session_params->disable_idle_sockets_close_on_memory_pressure;
 }
 
+std::unique_ptr<net::HttpNetworkSession>
+URLRequestContextFactory::CreateNetworkSession(
+    const net::URLRequestContext* context) {
+  net::HttpNetworkSession::Params session_params;
+  net::HttpNetworkSession::Context session_context;
+  PopulateNetworkSessionParams(IgnoreCertificateErrors(), &session_params);
+  net::URLRequestContextBuilder::SetHttpNetworkSessionComponents(
+      context, &session_context);
+  return std::make_unique<net::HttpNetworkSession>(session_params,
+                                                   session_context);
+}
+
 net::URLRequestContext* URLRequestContextFactory::CreateSystemRequestContext() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   InitializeSystemContextDependencies();
@@ -347,15 +366,12 @@ net::URLRequestContext* URLRequestContextFactory::CreateSystemRequestContext() {
 
   net::URLRequestContext* system_context = new net::URLRequestContext();
   ConfigureURLRequestContext(system_context, system_job_factory_,
-                             system_cookie_store_, system_network_delegate_);
+                             system_cookie_store_, system_network_delegate_,
+                             system_host_resolver_);
 
-  net::HttpNetworkSession::Params session_params;
-  net::HttpNetworkSession::Context session_context;
-  PopulateNetworkSessionParams(IgnoreCertificateErrors(), &session_params);
-  net::URLRequestContextBuilder::SetHttpNetworkSessionComponents(
-      system_context, &session_context);
-  system_transaction_factory_.reset(new net::HttpNetworkLayer(
-      new net::HttpNetworkSession(session_params, session_context)));
+  system_network_session_ = CreateNetworkSession(system_context);
+  system_transaction_factory_ =
+      std::make_unique<net::HttpNetworkLayer>(system_network_session_.get());
   system_context->set_http_transaction_factory(
       system_transaction_factory_.get());
 
@@ -366,18 +382,20 @@ net::URLRequestContext* URLRequestContextFactory::CreateMediaRequestContext() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   DCHECK(main_getter_.get())
       << "Getting MediaRequestContext before MainRequestContext";
-  net::URLRequestContext* main_context = main_getter_->GetURLRequestContext();
 
-  // Set non caching backend.
-  net::HttpNetworkSession* main_session =
-      main_transaction_factory_->GetSession();
-  InitializeMediaContextDependencies(
-      new net::HttpNetworkLayer(main_session));
+  InitializeMediaContextDependencies();
 
+  // Reuse main context dependencies except HostResolver and
+  // HttpTransactionFactory.
   net::URLRequestContext* media_context = new net::URLRequestContext();
-  media_context->CopyFrom(main_context);
-  media_context->set_http_transaction_factory(
-      media_transaction_factory_.get());
+  ConfigureURLRequestContext(media_context, main_job_factory_,
+                             main_cookie_store_, app_network_delegate_,
+                             media_host_resolver_);
+
+  media_network_session_ = CreateNetworkSession(media_context);
+  media_transaction_factory_ =
+      std::make_unique<net::HttpNetworkLayer>(media_network_session_.get());
+  media_context->set_http_transaction_factory(media_transaction_factory_.get());
 
   return media_context;
 }
@@ -396,15 +414,12 @@ net::URLRequestContext* URLRequestContextFactory::CreateMainRequestContext(
 
   net::URLRequestContext* main_context = new net::URLRequestContext();
   ConfigureURLRequestContext(main_context, main_job_factory_,
-                             main_cookie_store_, app_network_delegate_);
+                             main_cookie_store_, app_network_delegate_,
+                             main_host_resolver_);
 
-  net::HttpNetworkSession::Params session_params;
-  net::HttpNetworkSession::Context session_context;
-  PopulateNetworkSessionParams(IgnoreCertificateErrors(), &session_params);
-  net::URLRequestContextBuilder::SetHttpNetworkSessionComponents(
-      main_context, &session_context);
-  main_transaction_factory_.reset(new net::HttpNetworkLayer(
-      new net::HttpNetworkSession(session_params, session_context)));
+  main_network_session_ = CreateNetworkSession(main_context);
+  main_transaction_factory_ =
+      std::make_unique<net::HttpNetworkLayer>(main_network_session_.get());
   main_context->set_http_transaction_factory(main_transaction_factory_.get());
 
   return main_context;
@@ -414,9 +429,9 @@ void URLRequestContextFactory::ConfigureURLRequestContext(
     net::URLRequestContext* context,
     const std::unique_ptr<net::URLRequestJobFactory>& job_factory,
     const std::unique_ptr<net::CookieStore>& cookie_store,
-    const std::unique_ptr<CastNetworkDelegate>& network_delegate) {
+    const std::unique_ptr<CastNetworkDelegate>& network_delegate,
+    const std::unique_ptr<net::HostResolver>& host_resolver) {
   // common settings
-  context->set_host_resolver(host_resolver_.get());
   context->set_channel_id_service(channel_id_service_.get());
   context->set_cert_verifier(cert_verifier_.get());
   context->set_cert_transparency_verifier(cert_transparency_verifier_.get());
@@ -433,6 +448,9 @@ void URLRequestContextFactory::ConfigureURLRequestContext(
   context->set_job_factory(job_factory.get());
   context->set_cookie_store(cookie_store.get());
   context->set_network_delegate(network_delegate.get());
+  context->set_host_resolver(host_resolver.get());
+
+  host_resolver->SetRequestContext(context);
 }
 
 void URLRequestContextFactory::InitializeNetworkDelegates() {

chromecast/browser/url_request_context_factory.h

@@ -20,6 +20,8 @@ class FilePath;
 namespace net {
 class ChannelIDService;
 class CookieStore;
+class HostResolver;
+class HostResolverManager;
 class HttpTransactionFactory;
 class HttpUserAgentSettings;
 class NetLog;
@@ -60,10 +62,6 @@ class URLRequestContextFactory {
     return app_network_delegate_.get();
   }
 
-  net::HostResolver* host_resolver() const {
-    return host_resolver_.get();
-  }
-
   // Initialize the CastNetworkDelegate objects. This needs to be done
   // after the CastService is created, but before any URL requests are made.
   void InitializeNetworkDelegates();
@@ -78,11 +76,13 @@ class URLRequestContextFactory {
   void InitializeMainContextDependencies(
       content::ProtocolHandlerMap* protocol_handlers,
       content::URLRequestInterceptorScopedVector request_interceptors);
-  void InitializeMediaContextDependencies(net::HttpTransactionFactory* factory);
+  void InitializeMediaContextDependencies();
 
   void PopulateNetworkSessionParams(
       bool ignore_certificate_errors,
       net::HttpNetworkSession::Params* session_params);
+  std::unique_ptr<net::HttpNetworkSession> CreateNetworkSession(
+      const net::URLRequestContext* context);
 
   // These are called by the RequestContextGetters to create each
   // RequestContext.
@@ -99,7 +99,8 @@ class URLRequestContextFactory {
       net::URLRequestContext* context,
       const std::unique_ptr<net::URLRequestJobFactory>& job_factory,
       const std::unique_ptr<net::CookieStore>& cookie_store,
-      const std::unique_ptr<CastNetworkDelegate>& network_delegate);
+      const std::unique_ptr<CastNetworkDelegate>& network_delegate,
+      const std::unique_ptr<net::HostResolver>& host_resolver);
 
   scoped_refptr<net::URLRequestContextGetter> system_getter_;
   scoped_refptr<net::URLRequestContextGetter> media_getter_;
@@ -113,7 +114,7 @@ class URLRequestContextFactory {
   // The URLRequestContextStorage class manages dependent resources for a single
   // instance of URLRequestContext only.
   bool system_dependencies_initialized_;
-  std::unique_ptr<net::HostResolver> host_resolver_;
+  std::unique_ptr<net::HostResolverManager> host_resolver_manager_;
   std::unique_ptr<net::ChannelIDService> channel_id_service_;
   std::unique_ptr<net::CertVerifier> cert_verifier_;
   std::unique_ptr<net::SSLConfigService> ssl_config_service_;
@@ -125,17 +126,23 @@ class URLRequestContextFactory {
   std::unique_ptr<net::HttpAuthHandlerFactory> http_auth_handler_factory_;
   std::unique_ptr<net::HttpServerProperties> http_server_properties_;
   std::unique_ptr<net::HttpUserAgentSettings> http_user_agent_settings_;
+  std::unique_ptr<net::HttpNetworkSession> system_network_session_;
   std::unique_ptr<net::HttpTransactionFactory> system_transaction_factory_;
   std::unique_ptr<net::CookieStore> system_cookie_store_;
   std::unique_ptr<net::URLRequestJobFactory> system_job_factory_;
+  std::unique_ptr<net::HostResolver> system_host_resolver_;
 
   bool main_dependencies_initialized_;
+  std::unique_ptr<net::HttpNetworkSession> main_network_session_;
   std::unique_ptr<net::HttpTransactionFactory> main_transaction_factory_;
   std::unique_ptr<net::CookieStore> main_cookie_store_;
   std::unique_ptr<net::URLRequestJobFactory> main_job_factory_;
+  std::unique_ptr<net::HostResolver> main_host_resolver_;
 
   bool media_dependencies_initialized_;
+  std::unique_ptr<net::HttpNetworkSession> media_network_session_;
   std::unique_ptr<net::HttpTransactionFactory> media_transaction_factory_;
+  std::unique_ptr<net::HostResolver> media_host_resolver_;
 
   std::unique_ptr<PrefProxyConfigTracker> pref_proxy_config_tracker_impl_;
 

net/dns/BUILD.gn

@@ -213,6 +213,10 @@ source_set("host_resolver_manager") {
   # Whitelist-only access so we can keep track of all usage external to the
   # network stack and network service.
   friend = [
+    # chromecast/browser/url_request_context_factory.cc
+    # URLRequestContext creation for chromecast.
+    "//chromecast/browser",
+
     # Network stack/service.
     "//net/*",
     "//services/network/*",

net/url_request/url_request_context.h

@@ -35,12 +35,6 @@ class ProcessMemoryDump;
 }
 }
 
-namespace chromecast {
-namespace shell {
-class URLRequestContextFactory;
-}
-}  // namespace chromecast
-
 namespace safe_browsing {
 class SafeBrowsingURLRequestContextGetter;
 }  // namespace safe_browsing
@@ -311,7 +305,6 @@ class NET_EXPORT URLRequestContext
   // Whitelist legacy usage of now-deprecated CopyFrom().
   friend class ::ChromeBrowserStateImplIOData;
   friend class ::ProfileImplIOData;
-  friend class chromecast::shell::URLRequestContextFactory;
   friend class safe_browsing::SafeBrowsingURLRequestContextGetter;
 
   // Copies the state from |other| into this context.