From 53229e3d6c544c68137497ed09a074250c1af0fc Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 26 Feb 2018 23:24:00 +0900 Subject: [PATCH] Fix network delegate race condition (#12044) * Fix race condition when getting network delegate * Remove the evil URLRequestContextGetter::network_delegate * Move the arguments instead of const referrencing Safer and more efficient. --- atom/browser/api/atom_api_session.cc | 13 +++++---- atom/browser/api/atom_api_web_request.cc | 29 +++++++++++++++---- atom/browser/net/atom_network_delegate.cc | 12 ++++---- atom/browser/net/atom_network_delegate.h | 8 ++--- .../browser/url_request_context_getter.h | 5 ---- 5 files changed, 41 insertions(+), 26 deletions(-) diff --git a/atom/browser/api/atom_api_session.cc b/atom/browser/api/atom_api_session.cc index c9fc0837f5df..86d4e4bb227b 100644 --- a/atom/browser/api/atom_api_session.cc +++ b/atom/browser/api/atom_api_session.cc @@ -441,14 +441,15 @@ void DownloadIdCallback(content::DownloadManager* download_manager, } void SetDevToolsNetworkEmulationClientIdInIO( - brightray::URLRequestContextGetter* context_getter, + brightray::URLRequestContextGetter* url_request_context_getter, const std::string& client_id) { - if (!context_getter) + if (!url_request_context_getter) return; - auto network_delegate = - static_cast(context_getter->network_delegate()); - if (network_delegate) - network_delegate->SetDevToolsNetworkEmulationClientId(client_id); + net::URLRequestContext* context = + url_request_context_getter->GetURLRequestContext(); + AtomNetworkDelegate* network_delegate = + static_cast(context->network_delegate()); + network_delegate->SetDevToolsNetworkEmulationClientId(client_id); } } // namespace diff --git a/atom/browser/api/atom_api_web_request.cc b/atom/browser/api/atom_api_web_request.cc index 3f53f4d3d720..ac84ac196b69 100644 --- a/atom/browser/api/atom_api_web_request.cc +++ b/atom/browser/api/atom_api_web_request.cc @@ -37,6 +37,26 @@ namespace atom { namespace api { +namespace { + +template +void CallNetworkDelegateMethod( + brightray::URLRequestContextGetter* url_request_context_getter, + Method method, + Event type, + URLPatterns patterns, + Listener listener) { + // Force creating network delegate. + net::URLRequestContext* context = + url_request_context_getter->GetURLRequestContext(); + // Then call the method. + AtomNetworkDelegate* network_delegate = + static_cast(context->network_delegate()); + (network_delegate->*method)(type, std::move(patterns), std::move(listener)); +} + +} // namespace + WebRequest::WebRequest(v8::Isolate* isolate, AtomBrowserContext* browser_context) : browser_context_(browser_context) { @@ -74,16 +94,15 @@ void WebRequest::SetListener(Method method, Event type, mate::Arguments* args) { return; } - auto url_request_context_getter = + brightray::URLRequestContextGetter* url_request_context_getter = browser_context_->url_request_context_getter(); if (!url_request_context_getter) return; BrowserThread::PostTask( BrowserThread::IO, FROM_HERE, - base::Bind(method, - base::Unretained(static_cast( - url_request_context_getter->network_delegate())), - type, patterns, listener)); + base::Bind(&CallNetworkDelegateMethod, + base::RetainedRef(url_request_context_getter), + method, type, std::move(patterns), std::move(listener))); } // static diff --git a/atom/browser/net/atom_network_delegate.cc b/atom/browser/net/atom_network_delegate.cc index a8d18c57890c..680459a14c86 100644 --- a/atom/browser/net/atom_network_delegate.cc +++ b/atom/browser/net/atom_network_delegate.cc @@ -227,22 +227,22 @@ AtomNetworkDelegate::~AtomNetworkDelegate() { void AtomNetworkDelegate::SetSimpleListenerInIO( SimpleEvent type, - const URLPatterns& patterns, - const SimpleListener& callback) { + URLPatterns patterns, + SimpleListener callback) { if (callback.is_null()) simple_listeners_.erase(type); else - simple_listeners_[type] = { patterns, callback }; + simple_listeners_[type] = { std::move(patterns), std::move(callback) }; } void AtomNetworkDelegate::SetResponseListenerInIO( ResponseEvent type, - const URLPatterns& patterns, - const ResponseListener& callback) { + URLPatterns patterns, + ResponseListener callback) { if (callback.is_null()) response_listeners_.erase(type); else - response_listeners_[type] = { patterns, callback }; + response_listeners_[type] = { std::move(patterns), std::move(callback) }; } void AtomNetworkDelegate::SetDevToolsNetworkEmulationClientId( diff --git a/atom/browser/net/atom_network_delegate.h b/atom/browser/net/atom_network_delegate.h index e00c26ff2cbc..ad37a926d473 100644 --- a/atom/browser/net/atom_network_delegate.h +++ b/atom/browser/net/atom_network_delegate.h @@ -62,11 +62,11 @@ class AtomNetworkDelegate : public brightray::NetworkDelegate { ~AtomNetworkDelegate() override; void SetSimpleListenerInIO(SimpleEvent type, - const URLPatterns& patterns, - const SimpleListener& callback); + URLPatterns patterns, + SimpleListener callback); void SetResponseListenerInIO(ResponseEvent type, - const URLPatterns& patterns, - const ResponseListener& callback); + URLPatterns patterns, + ResponseListener callback); void SetDevToolsNetworkEmulationClientId(const std::string& client_id); diff --git a/brightray/browser/url_request_context_getter.h b/brightray/browser/url_request_context_getter.h index b0a2549280e8..51b0ed532260 100644 --- a/brightray/browser/url_request_context_getter.h +++ b/brightray/browser/url_request_context_getter.h @@ -84,11 +84,6 @@ class URLRequestContextGetter : public net::URLRequestContextGetter { net::HostResolver* host_resolver(); net::URLRequestJobFactory* job_factory() const { return job_factory_; } - net::NetworkDelegate* network_delegate() const { - if (url_request_context_) - return url_request_context_->network_delegate(); - return nullptr; - } private: Delegate* delegate_;