From 57356036dbb9a4c7a845959ae342d5d6b3e36e1f Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Wed, 7 Nov 2018 19:54:05 +0530 Subject: [PATCH] fix: initialize system network context from IOThread --- atom/browser/browser_process_impl.cc | 5 +- atom/browser/io_thread.cc | 114 +++++------------- atom/browser/io_thread.h | 38 ++++-- .../net/system_network_context_manager.cc | 39 +++++- .../net/system_network_context_manager.h | 7 +- 5 files changed, 105 insertions(+), 98 deletions(-) diff --git a/atom/browser/browser_process_impl.cc b/atom/browser/browser_process_impl.cc index 76dc8dc5c700..dc72cb36d6ec 100644 --- a/atom/browser/browser_process_impl.cc +++ b/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(net_log_.get()); + io_thread_ = std::make_unique( + net_log_.get(), system_network_context_manager_.get()); } void BrowserProcessImpl::PostDestroyThreads() { @@ -153,7 +154,7 @@ net::URLRequestContextGetter* BrowserProcessImpl::system_request_context() { scoped_refptr BrowserProcessImpl::shared_url_loader_factory() { - return nullptr; + return system_network_context_manager()->GetSharedURLLoaderFactory(); } variations::VariationsService* BrowserProcessImpl::variations_service() { diff --git a/atom/browser/io_thread.cc b/atom/browser/io_thread.cc index 6334c2526ae6..72a6da88828f 100644 --- a/atom/browser/io_thread.cc +++ b/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 #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() { + std::unique_ptr builder = + std::make_unique(); + + auto cert_verifier = std::make_unique( + std::make_unique( + net::CertVerifyProc::CreateDefault())); + builder->SetCertVerifier(std::move(cert_verifier)); + // 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)); + network::NetworkService* network_service = content::GetNetworkServiceImpl(); + network_service->SetUpHttpAuth(std::move(http_auth_static_params_)); + network_service->ConfigureHttpAuthPrefs(std::move(http_auth_dynamic_params_)); - 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()); - -#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 + 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(); diff --git a/atom/browser/io_thread.h b/atom/browser/io_thread.h index e4dfdf43219c..5c6fac2f757b 100644 --- a/atom/browser/io_thread.h +++ b/atom/browser/io_thread.h @@ -7,14 +7,14 @@ #include +#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 url_request_context_; - scoped_refptr 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); }; diff --git a/atom/browser/net/system_network_context_manager.cc b/atom/browser/net/system_network_context_manager.cc index 1b6484b69fbc..b98695147284 100644 --- a/atom/browser/net/system_network_context_manager.cc +++ b/atom/browser/net/system_network_context_manager.cc @@ -8,6 +8,8 @@ #include #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::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. diff --git a/atom/browser/net/system_network_context_manager.h b/atom/browser/net/system_network_context_manager.h index 58260b70a605..ea09d2d68721 100644 --- a/atom/browser/net/system_network_context_manager.h +++ b/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