Browse Source

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.
Cheng Zhao 7 years ago
parent
commit
53229e3d6c

+ 7 - 6
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<AtomNetworkDelegate*>(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<AtomNetworkDelegate*>(context->network_delegate());
+  network_delegate->SetDevToolsNetworkEmulationClientId(client_id);
 }
 
 }  // namespace

+ 24 - 5
atom/browser/api/atom_api_web_request.cc

@@ -37,6 +37,26 @@ namespace atom {
 
 namespace api {
 
+namespace {
+
+template<typename Method, typename Event, typename Listener>
+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<AtomNetworkDelegate*>(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<AtomNetworkDelegate*>(
-                     url_request_context_getter->network_delegate())),
-                 type, patterns, listener));
+      base::Bind(&CallNetworkDelegateMethod<Method, Event, Listener>,
+                 base::RetainedRef(url_request_context_getter),
+                 method, type, std::move(patterns), std::move(listener)));
 }
 
 // static

+ 6 - 6
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(

+ 4 - 4
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);
 

+ 0 - 5
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_;