From ebb0e46380033a4336baad3e99f3646845ecefec Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Fri, 15 Dec 2017 23:02:33 +0530 Subject: [PATCH] REVIEW: create AtomNetworkDelegate on the IO thread --- atom/browser/api/atom_api_session.cc | 27 ++++++++++++++++--- atom/browser/api/atom_api_web_request.cc | 14 +++++++--- atom/browser/atom_browser_context.cc | 6 ++--- atom/browser/atom_browser_context.h | 5 +--- atom/browser/net/atom_network_delegate.cc | 11 ++------ atom/browser/net/atom_network_delegate.h | 2 -- brightray/browser/browser_context.cc | 4 +-- brightray/browser/browser_context.h | 2 +- .../browser/url_request_context_getter.cc | 5 ++-- .../browser/url_request_context_getter.h | 11 ++++++-- 10 files changed, 53 insertions(+), 34 deletions(-) diff --git a/atom/browser/api/atom_api_session.cc b/atom/browser/api/atom_api_session.cc index 39da83789102..9fb029cecbb5 100644 --- a/atom/browser/api/atom_api_session.cc +++ b/atom/browser/api/atom_api_session.cc @@ -439,6 +439,17 @@ void DownloadIdCallback(content::DownloadManager* download_manager, std::vector()); } +void SetDevToolsNetworkEmulationClientIdInIO( + brightray::URLRequestContextGetter* context_getter, + const std::string& client_id) { + if (!context_getter) + return; + auto network_delegate = + static_cast(context_getter->network_delegate()); + if (network_delegate) + network_delegate->SetDevToolsNetworkEmulationClientId(client_id); +} + } // namespace Session::Session(v8::Isolate* isolate, AtomBrowserContext* browser_context) @@ -548,16 +559,24 @@ void Session::EnableNetworkEmulation(const mate::Dictionary& options) { browser_context_->network_controller_handle()->SetNetworkState( devtools_network_emulation_client_id_, std::move(conditions)); - browser_context_->network_delegate()->SetDevToolsNetworkEmulationClientId( - devtools_network_emulation_client_id_); + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind( + &SetDevToolsNetworkEmulationClientIdInIO, + base::RetainedRef(browser_context_->url_request_context_getter()), + devtools_network_emulation_client_id_)); } void Session::DisableNetworkEmulation() { std::unique_ptr conditions; browser_context_->network_controller_handle()->SetNetworkState( devtools_network_emulation_client_id_, std::move(conditions)); - browser_context_->network_delegate()->SetDevToolsNetworkEmulationClientId( - std::string()); + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind( + &SetDevToolsNetworkEmulationClientIdInIO, + base::RetainedRef(browser_context_->url_request_context_getter()), + std::string())); } void Session::SetCertVerifyProc(v8::Local val, diff --git a/atom/browser/api/atom_api_web_request.cc b/atom/browser/api/atom_api_web_request.cc index d8526e03ad8f..3f53f4d3d720 100644 --- a/atom/browser/api/atom_api_web_request.cc +++ b/atom/browser/api/atom_api_web_request.cc @@ -74,10 +74,16 @@ void WebRequest::SetListener(Method method, Event type, mate::Arguments* args) { return; } - auto delegate = browser_context_->network_delegate(); - BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, - base::Bind(method, base::Unretained(delegate), type, - patterns, listener)); + auto 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)); } // static diff --git a/atom/browser/atom_browser_context.cc b/atom/browser/atom_browser_context.cc index 63fb8289529d..07b351b551b6 100644 --- a/atom/browser/atom_browser_context.cc +++ b/atom/browser/atom_browser_context.cc @@ -71,7 +71,6 @@ AtomBrowserContext::AtomBrowserContext(const std::string& partition, bool in_memory, const base::DictionaryValue& options) : brightray::BrowserContext(partition, in_memory), - network_delegate_(new AtomNetworkDelegate), cookie_delegate_(new AtomCookieDelegate) { // Construct user agent string. Browser* browser = Browser::Get(); @@ -104,8 +103,9 @@ void AtomBrowserContext::SetUserAgent(const std::string& user_agent) { user_agent_ = user_agent; } -net::NetworkDelegate* AtomBrowserContext::CreateNetworkDelegate() { - return network_delegate_; +std::unique_ptr +AtomBrowserContext::CreateNetworkDelegate() { + return base::MakeUnique(); } net::CookieMonsterDelegate* AtomBrowserContext::CreateCookieDelegate() { diff --git a/atom/browser/atom_browser_context.h b/atom/browser/atom_browser_context.h index 7e73fa0395be..359d3d6b71a5 100644 --- a/atom/browser/atom_browser_context.h +++ b/atom/browser/atom_browser_context.h @@ -32,7 +32,7 @@ class AtomBrowserContext : public brightray::BrowserContext { void SetUserAgent(const std::string& user_agent); // brightray::URLRequestContextGetter::Delegate: - net::NetworkDelegate* CreateNetworkDelegate() override; + std::unique_ptr CreateNetworkDelegate() override; net::CookieMonsterDelegate* CreateCookieDelegate() override; std::string GetUserAgent() override; std::unique_ptr CreateURLRequestJobFactory( @@ -52,7 +52,6 @@ class AtomBrowserContext : public brightray::BrowserContext { void RegisterPrefs(PrefRegistrySimple* pref_registry) override; AtomBlobReader* GetBlobReader(); - AtomNetworkDelegate* network_delegate() const { return network_delegate_; } AtomCookieDelegate* cookie_delegate() const { return cookie_delegate_.get(); } @@ -70,8 +69,6 @@ class AtomBrowserContext : public brightray::BrowserContext { std::string user_agent_; bool use_cache_; - // Managed by brightray::BrowserContext. - AtomNetworkDelegate* network_delegate_; scoped_refptr cookie_delegate_; DISALLOW_COPY_AND_ASSIGN(AtomBrowserContext); diff --git a/atom/browser/net/atom_network_delegate.cc b/atom/browser/net/atom_network_delegate.cc index 1306ae59a683..9b0f3a98dc53 100644 --- a/atom/browser/net/atom_network_delegate.cc +++ b/atom/browser/net/atom_network_delegate.cc @@ -247,7 +247,6 @@ void AtomNetworkDelegate::SetResponseListenerInIO( void AtomNetworkDelegate::SetDevToolsNetworkEmulationClientId( const std::string& client_id) { - base::AutoLock auto_lock(lock_); client_id_ = client_id; } @@ -266,16 +265,10 @@ int AtomNetworkDelegate::OnBeforeStartTransaction( net::URLRequest* request, const net::CompletionCallback& callback, net::HttpRequestHeaders* headers) { - std::string client_id; - { - base::AutoLock auto_lock(lock_); - client_id = client_id_; - } - - if (!client_id.empty()) + if (!client_id_.empty()) headers->SetHeader( DevToolsNetworkTransaction::kDevToolsEmulateNetworkConditionsClientId, - client_id); + client_id_); if (!base::ContainsKey(response_listeners_, kOnBeforeSendHeaders)) return brightray::NetworkDelegate::OnBeforeStartTransaction( request, callback, headers); diff --git a/atom/browser/net/atom_network_delegate.h b/atom/browser/net/atom_network_delegate.h index 7a50d6076f2f..e00c26ff2cbc 100644 --- a/atom/browser/net/atom_network_delegate.h +++ b/atom/browser/net/atom_network_delegate.h @@ -118,8 +118,6 @@ class AtomNetworkDelegate : public brightray::NetworkDelegate { std::map response_listeners_; std::map callbacks_; - base::Lock lock_; - // Client id for devtools network emulation. std::string client_id_; diff --git a/brightray/browser/browser_context.cc b/brightray/browser/browser_context.cc index e7d45b7b9083..d7a4c672926c 100644 --- a/brightray/browser/browser_context.cc +++ b/brightray/browser/browser_context.cc @@ -145,8 +145,8 @@ net::URLRequestContextGetter* BrowserContext::CreateRequestContext( return url_request_getter_.get(); } -net::NetworkDelegate* BrowserContext::CreateNetworkDelegate() { - return new NetworkDelegate; +std::unique_ptr BrowserContext::CreateNetworkDelegate() { + return base::MakeUnique(); } std::string BrowserContext::GetMediaDeviceIDSalt() { diff --git a/brightray/browser/browser_context.h b/brightray/browser/browser_context.h index b78a8f365817..6d0348db2538 100644 --- a/brightray/browser/browser_context.h +++ b/brightray/browser/browser_context.h @@ -90,7 +90,7 @@ class BrowserContext : public base::RefCounted, virtual void RegisterPrefs(PrefRegistrySimple* pref_registry) {} // URLRequestContextGetter::Delegate: - net::NetworkDelegate* CreateNetworkDelegate() override; + std::unique_ptr CreateNetworkDelegate() override; base::FilePath GetPath() const override; diff --git a/brightray/browser/url_request_context_getter.cc b/brightray/browser/url_request_context_getter.cc index 3003fbc7d231..2c607f88b8a3 100644 --- a/brightray/browser/url_request_context_getter.cc +++ b/brightray/browser/url_request_context_getter.cc @@ -185,12 +185,11 @@ net::URLRequestContext* URLRequestContextGetter::GetURLRequestContext() { url_request_context_->set_net_log(net_log_); } - network_delegate_.reset(delegate_->CreateNetworkDelegate()); - url_request_context_->set_network_delegate(network_delegate_.get()); - storage_.reset( new net::URLRequestContextStorage(url_request_context_.get())); + storage_->set_network_delegate(delegate_->CreateNetworkDelegate()); + auto cookie_path = in_memory_ ? base::FilePath() : base_path_.Append(FILE_PATH_LITERAL("Cookies")); auto cookie_config = content::CookieStoreConfig( diff --git a/brightray/browser/url_request_context_getter.h b/brightray/browser/url_request_context_getter.h index 25c775607010..04033f451e04 100644 --- a/brightray/browser/url_request_context_getter.h +++ b/brightray/browser/url_request_context_getter.h @@ -15,6 +15,7 @@ #include "net/http/http_cache.h" #include "net/http/transport_security_state.h" #include "net/http/url_security_manager.h" +#include "net/url_request/url_request_context.h" #include "net/url_request/url_request_context_getter.h" namespace base { @@ -44,7 +45,9 @@ class URLRequestContextGetter : public net::URLRequestContextGetter { Delegate() {} virtual ~Delegate() {} - virtual net::NetworkDelegate* CreateNetworkDelegate() { return nullptr; } + virtual std::unique_ptr CreateNetworkDelegate() { + return nullptr; + } virtual net::CookieMonsterDelegate* CreateCookieDelegate() { return nullptr; } @@ -77,6 +80,11 @@ 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_; @@ -91,7 +99,6 @@ class URLRequestContextGetter : public net::URLRequestContextGetter { std::unique_ptr ct_delegate_; std::unique_ptr proxy_config_service_; - std::unique_ptr network_delegate_; std::unique_ptr storage_; std::unique_ptr url_request_context_; std::unique_ptr host_mapping_rules_;