fix: initialize system network context from IOThread

atom/browser/browser_process_impl.cc

@@ -109,7 +109,8 @@ void BrowserProcessImpl::PreCreateThreads(
   net_log_->net_export_file_writer()->Initialize();
 
   // Manage global state of net and other IO thread related.
-  io_thread_ = std::make_unique<IOThread>(net_log_.get());
+  io_thread_ = std::make_unique<IOThread>(
+      net_log_.get(), system_network_context_manager_.get());
 }
 
 void BrowserProcessImpl::PostDestroyThreads() {
@@ -153,7 +154,7 @@ net::URLRequestContextGetter* BrowserProcessImpl::system_request_context() {
 
 scoped_refptr<network::SharedURLLoaderFactory>
 BrowserProcessImpl::shared_url_loader_factory() {
-  return nullptr;
+  return system_network_context_manager()->GetSharedURLLoaderFactory();
 }
 
 variations::VariationsService* BrowserProcessImpl::variations_service() {

atom/browser/io_thread.cc

@@ -3,57 +3,31 @@
 // found in the LICENSE file.
 
 #include "atom/browser/io_thread.h"
-#include "atom/common/options_switches.h"
+
+#include <utility>
 
 #include "components/net_log/chrome_net_log.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/network_service_instance.h"
+#include "net/cert/caching_cert_verifier.h"
+#include "net/cert/cert_verifier.h"
+#include "net/cert/cert_verify_proc.h"
+#include "net/cert/multi_threaded_cert_verifier.h"
 #include "net/proxy_resolution/proxy_resolution_service.h"
 #include "net/url_request/url_request_context.h"
-#include "net/url_request/url_request_context_builder.h"
-#include "net/url_request/url_request_context_getter.h"
 #include "services/network/network_service.h"
-
-#if defined(USE_NSS_CERTS)
-#include "net/cert_net/nss_ocsp.h"
-#endif
-
-#if defined(OS_LINUX) || defined(OS_MACOSX)
-#include "net/cert/cert_net_fetcher.h"
-#include "net/cert_net/cert_net_fetcher_impl.h"
-#endif
+#include "services/network/url_request_context_builder_mojo.h"
 
 using content::BrowserThread;
 
-namespace {
-
-network::mojom::HttpAuthStaticParamsPtr CreateHttpAuthStaticParams() {
-  network::mojom::HttpAuthStaticParamsPtr auth_static_params =
-      network::mojom::HttpAuthStaticParams::New();
-
-  auth_static_params->supported_schemes = {"basic", "digest", "ntlm",
-                                           "negotiate"};
-
-  return auth_static_params;
-}
-
-network::mojom::HttpAuthDynamicParamsPtr CreateHttpAuthDynamicParams(
-    const base::CommandLine& command_line) {
-  network::mojom::HttpAuthDynamicParamsPtr auth_dynamic_params =
-      network::mojom::HttpAuthDynamicParams::New();
-
-  auth_dynamic_params->server_whitelist =
-      command_line.GetSwitchValueASCII(atom::switches::kAuthServerWhitelist);
-  auth_dynamic_params->delegate_whitelist = command_line.GetSwitchValueASCII(
-      atom::switches::kAuthNegotiateDelegateWhitelist);
-
-  return auth_dynamic_params;
-}
-
-}  // namespace
-
-IOThread::IOThread(net_log::ChromeNetLog* net_log) : net_log_(net_log) {
+IOThread::IOThread(net_log::ChromeNetLog* net_log,
+                   SystemNetworkContextManager* system_network_context_manager)
+    : net_log_(net_log) {
   BrowserThread::SetIOThreadDelegate(this);
+
+  system_network_context_manager->SetUp(
+      &network_context_request_, &network_context_params_,
+      &http_auth_static_params_, &http_auth_dynamic_params_);
 }
 
 IOThread::~IOThread() {
@@ -61,53 +35,31 @@ IOThread::~IOThread() {
 }
 
 void IOThread::Init() {
-  // Create the network service, so that shared host resolver
-  // gets created which is required to set the auth preferences below.
-  auto& command_line = *base::CommandLine::ForCurrentProcess();
-  auto* network_service = content::GetNetworkServiceImpl();
-  network_service->SetUpHttpAuth(CreateHttpAuthStaticParams());
-  network_service->ConfigureHttpAuthPrefs(
-      CreateHttpAuthDynamicParams(command_line));
+  std::unique_ptr<network::URLRequestContextBuilderMojo> builder =
+      std::make_unique<network::URLRequestContextBuilderMojo>();
 
-  net::URLRequestContextBuilder builder;
-  // TODO(deepak1556): We need to respoect user proxy configurations,
-  // the following initialization has to happen before any request
-  // contexts are utilized by the io thread, so that proper cert validation
-  // take place, solutions:
-  // 1) Use the request context from default partition, but since
-  //    an app can completely run on a custom session without ever creating
-  //    the default session, we will have to force create the default session
-  //    in those scenarios.
-  // 2) Add a new api on app module that sets the proxy configuration
-  //    for the global requests, like the cert fetchers below and
-  //    geolocation requests.
-  // 3) There is also ongoing work in upstream which will eventually allow
-  //    localizing these global fetchers to their own URLRequestContexts.
-  builder.set_proxy_resolution_service(
-      net::ProxyResolutionService::CreateDirect());
-  url_request_context_ = builder.Build();
-  url_request_context_getter_ = new net::TrivialURLRequestContextGetter(
-      url_request_context_.get(), base::ThreadTaskRunnerHandle::Get());
+  auto cert_verifier = std::make_unique<net::CachingCertVerifier>(
+      std::make_unique<net::MultiThreadedCertVerifier>(
+          net::CertVerifyProc::CreateDefault()));
+  builder->SetCertVerifier(std::move(cert_verifier));
 
-#if defined(USE_NSS_CERTS)
-  net::SetURLRequestContextForNSSHttpIO(url_request_context_.get());
-#endif
-#if defined(OS_LINUX) || defined(OS_MACOSX)
-  net::SetGlobalCertNetFetcher(
-      net::CreateCertNetFetcher(url_request_context_.get()));
-#endif
+  // Create the network service, so that shared host resolver
+  // gets created which is required to set the auth preferences below.
+  network::NetworkService* network_service = content::GetNetworkServiceImpl();
+  network_service->SetUpHttpAuth(std::move(http_auth_static_params_));
+  network_service->ConfigureHttpAuthPrefs(std::move(http_auth_dynamic_params_));
+
+  system_network_context_ =
+      network_service
+          ->CreateNetworkContextWithBuilder(std::move(network_context_request_),
+                                            std::move(network_context_params_),
+                                            std::move(builder),
+                                            &system_request_context_)
+          .release();
 }
 
 void IOThread::CleanUp() {
-#if defined(USE_NSS_CERTS)
-  net::SetURLRequestContextForNSSHttpIO(nullptr);
-#endif
-#if defined(OS_LINUX) || defined(OS_MACOSX)
-  net::ShutdownGlobalCertNetFetcher();
-#endif
-  // Explicitly release before the IO thread gets destroyed.
-  url_request_context_.reset();
-  url_request_context_getter_ = nullptr;
+  system_request_context_->proxy_resolution_service()->OnShutdown();
 
   if (net_log_)
     net_log_->ShutDownBeforeTaskScheduler();

atom/browser/io_thread.h

@@ -7,14 +7,14 @@
 
 #include <memory>
 
+#include "atom/browser/net/system_network_context_manager.h"
 #include "base/macros.h"
-#include "base/memory/scoped_refptr.h"
 #include "content/public/browser/browser_thread_delegate.h"
+#include "services/network/public/mojom/network_service.mojom.h"
 
 namespace net {
 class URLRequestContext;
-class URLRequestContextGetter;
-}  // namespace net
+}
 
 namespace net_log {
 class ChromeNetLog;
@@ -22,13 +22,11 @@ class ChromeNetLog;
 
 class IOThread : public content::BrowserThreadDelegate {
  public:
-  explicit IOThread(net_log::ChromeNetLog* net_log);
+  explicit IOThread(
+      net_log::ChromeNetLog* net_log,
+      SystemNetworkContextManager* system_network_context_manager);
   ~IOThread() override;
 
-  net::URLRequestContextGetter* GetRequestContext() {
-    return url_request_context_getter_.get();
-  }
-
  protected:
   // BrowserThreadDelegate Implementation, runs on the IO thread.
   void Init() override;
@@ -38,8 +36,28 @@ class IOThread : public content::BrowserThreadDelegate {
   // The NetLog is owned by the browser process, to allow logging from other
   // threads during shutdown, but is used most frequently on the IOThread.
   net_log::ChromeNetLog* net_log_;
-  std::unique_ptr<net::URLRequestContext> url_request_context_;
-  scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
+
+  // When the network service is disabled, this holds on to a
+  // content::NetworkContext class that owns |system_request_context_|.
+  // TODO(deepak1556): primary network context has to be destroyed after
+  // other active contexts, but since the ownership of latter is not released
+  // before IO thread is destroyed, it results in a DCHECK failure.
+  // We leak the reference to primary context to workaround this issue,
+  // since there is only one instance for the entire lifetime of app, it is
+  // safe.
+  network::mojom::NetworkContext* system_network_context_;
+  net::URLRequestContext* system_request_context_;
+
+  // These are set on the UI thread, and then consumed during initialization on
+  // the IO thread.
+  network::mojom::NetworkContextRequest network_context_request_;
+  network::mojom::NetworkContextParamsPtr network_context_params_;
+
+  // Initial HTTP auth configuration used when setting up the NetworkService on
+  // the IO Thread. Future updates are sent using the NetworkService mojo
+  // interface, but initial state needs to be set non-racily.
+  network::mojom::HttpAuthStaticParamsPtr http_auth_static_params_;
+  network::mojom::HttpAuthDynamicParamsPtr http_auth_dynamic_params_;
 
   DISALLOW_COPY_AND_ASSIGN(IOThread);
 };

atom/browser/net/system_network_context_manager.cc

@@ -8,6 +8,8 @@
 #include <utility>
 
 #include "atom/browser/io_thread.h"
+#include "atom/common/options_switches.h"
+#include "base/command_line.h"
 #include "base/lazy_instance.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/net/chrome_mojo_proxy_resolver_factory.h"
@@ -26,6 +28,33 @@
 base::LazyInstance<SystemNetworkContextManager>::Leaky
     g_system_network_context_manager = LAZY_INSTANCE_INITIALIZER;
 
+namespace {
+
+network::mojom::HttpAuthStaticParamsPtr CreateHttpAuthStaticParams() {
+  network::mojom::HttpAuthStaticParamsPtr auth_static_params =
+      network::mojom::HttpAuthStaticParams::New();
+
+  auth_static_params->supported_schemes = {"basic", "digest", "ntlm",
+                                           "negotiate"};
+
+  return auth_static_params;
+}
+
+network::mojom::HttpAuthDynamicParamsPtr CreateHttpAuthDynamicParams() {
+  auto* command_line = base::CommandLine::ForCurrentProcess();
+  network::mojom::HttpAuthDynamicParamsPtr auth_dynamic_params =
+      network::mojom::HttpAuthDynamicParams::New();
+
+  auth_dynamic_params->server_whitelist =
+      command_line->GetSwitchValueASCII(atom::switches::kAuthServerWhitelist);
+  auth_dynamic_params->delegate_whitelist = command_line->GetSwitchValueASCII(
+      atom::switches::kAuthNegotiateDelegateWhitelist);
+
+  return auth_dynamic_params;
+}
+
+}  // namespace
+
 // SharedURLLoaderFactory backed by a SystemNetworkContextManager and its
 // network context. Transparently handles crashes.
 class SystemNetworkContextManager::URLLoaderFactoryForSystem
@@ -137,7 +166,9 @@ SystemNetworkContextManager::CreateDefaultNetworkContextParams() {
 
 void SystemNetworkContextManager::SetUp(
     network::mojom::NetworkContextRequest* network_context_request,
-    network::mojom::NetworkContextParamsPtr* network_context_params) {
+    network::mojom::NetworkContextParamsPtr* network_context_params,
+    network::mojom::HttpAuthStaticParamsPtr* http_auth_static_params,
+    network::mojom::HttpAuthDynamicParamsPtr* http_auth_dynamic_params) {
   if (!base::FeatureList::IsEnabled(network::features::kNetworkService)) {
     *network_context_request = mojo::MakeRequest(&io_thread_network_context_);
     *network_context_params = CreateNetworkContextParams();
@@ -146,6 +177,8 @@ void SystemNetworkContextManager::SetUp(
     // CreateNetworkContextParams() can only be called once.
     *network_context_params = CreateDefaultNetworkContextParams();
   }
+  *http_auth_static_params = CreateHttpAuthStaticParams();
+  *http_auth_dynamic_params = CreateHttpAuthDynamicParams();
 }
 
 SystemNetworkContextManager::SystemNetworkContextManager()
@@ -162,8 +195,8 @@ void SystemNetworkContextManager::OnNetworkServiceCreated(
   if (!base::FeatureList::IsEnabled(network::features::kNetworkService))
     return;
 
-  // network_service->SetUpHttpAuth(CreateHttpAuthStaticParams());
-  // network_service->ConfigureHttpAuthPrefs(CreateHttpAuthDynamicParams());
+  network_service->SetUpHttpAuth(CreateHttpAuthStaticParams());
+  network_service->ConfigureHttpAuthPrefs(CreateHttpAuthDynamicParams());
 
   // The system NetworkContext must be created first, since it sets
   // |primary_network_context| to true.

atom/browser/net/system_network_context_manager.h

@@ -50,8 +50,11 @@ class SystemNetworkContextManager {
   // help set up the IOThread's in-process URLRequestContext.
   //
   // Must be called before the system NetworkContext is first used.
-  void SetUp(network::mojom::NetworkContextRequest* network_context_request,
-             network::mojom::NetworkContextParamsPtr* network_context_params);
+  void SetUp(
+      network::mojom::NetworkContextRequest* network_context_request,
+      network::mojom::NetworkContextParamsPtr* network_context_params,
+      network::mojom::HttpAuthStaticParamsPtr* http_auth_static_params,
+      network::mojom::HttpAuthDynamicParamsPtr* http_auth_dynamic_params);
 
   // Returns the System NetworkContext. May only be called after SetUp(). Does
   // any initialization of the NetworkService that may be needed when first