Make api::Protocol thread safe

diff --git a/atom/browser/api/atom_api_protocol.cc b/atom/browser/api/atom_api_protocol.cc
index b53fd6c..c762559 100644 (file)
--- a/atom/browser/api/atom_api_protocol.cc
+++ b/atom/browser/api/atom_api_protocol.cc
@@ -27,7 +27,8 @@ namespace atom {
 namespace api {
 
 Protocol::Protocol(v8::Isolate* isolate, AtomBrowserContext* browser_context)
-    : request_context_getter_(browser_context->GetRequestContext()),
+    : request_context_getter_(static_cast<brightray::URLRequestContextGetter*>(
+          browser_context->GetRequestContext())),
       weak_factory_(this) {
   Init(isolate);
 }
@@ -44,16 +45,20 @@ void Protocol::UnregisterProtocol(
   content::BrowserThread::PostTaskAndReplyWithResult(
       content::BrowserThread::IO, FROM_HERE,
       base::Bind(&Protocol::UnregisterProtocolInIO,
-                 base::Unretained(this), scheme),
+                 request_context_getter_, scheme),
       base::Bind(&Protocol::OnIOCompleted,
                  GetWeakPtr(), callback));
 }
 
+// static
 Protocol::ProtocolError Protocol::UnregisterProtocolInIO(
+    scoped_refptr<brightray::URLRequestContextGetter> request_context_getter,
     const std::string& scheme) {
-  if (!job_factory()->HasProtocolHandler(scheme))
+  auto job_factory = static_cast<AtomURLRequestJobFactory*>(
+      request_context_getter->job_factory());
+  if (!job_factory->HasProtocolHandler(scheme))
     return PROTOCOL_NOT_REGISTERED;
-  job_factory()->SetProtocolHandler(scheme, nullptr);
+  job_factory->SetProtocolHandler(scheme, nullptr);
   return PROTOCOL_OK;
 }
 
@@ -62,12 +67,15 @@ void Protocol::IsProtocolHandled(const std::string& scheme,
   content::BrowserThread::PostTaskAndReplyWithResult(
       content::BrowserThread::IO, FROM_HERE,
       base::Bind(&Protocol::IsProtocolHandledInIO,
-                 base::Unretained(this), scheme),
+                 request_context_getter_, scheme),
       callback);
 }
 
-bool Protocol::IsProtocolHandledInIO(const std::string& scheme) {
-  return job_factory()->IsHandledProtocol(scheme);
+// static
+bool Protocol::IsProtocolHandledInIO(
+    scoped_refptr<brightray::URLRequestContextGetter> request_context_getter,
+    const std::string& scheme) {
+  return request_context_getter->job_factory()->IsHandledProtocol(scheme);
 }
 
 void Protocol::UninterceptProtocol(
@@ -77,18 +85,18 @@ void Protocol::UninterceptProtocol(
   content::BrowserThread::PostTaskAndReplyWithResult(
       content::BrowserThread::IO, FROM_HERE,
       base::Bind(&Protocol::UninterceptProtocolInIO,
-                 base::Unretained(this), scheme),
+                 request_context_getter_, scheme),
       base::Bind(&Protocol::OnIOCompleted,
                  GetWeakPtr(), callback));
 }
 
+// static
 Protocol::ProtocolError Protocol::UninterceptProtocolInIO(
+    scoped_refptr<brightray::URLRequestContextGetter> request_context_getter,
     const std::string& scheme) {
-  if (!original_protocols_.contains(scheme))
-    return PROTOCOL_NOT_INTERCEPTED;
-  job_factory()->ReplaceProtocol(scheme,
-                                 original_protocols_.take_and_erase(scheme));
-  return PROTOCOL_OK;
+  return static_cast<AtomURLRequestJobFactory*>(
+      request_context_getter->job_factory())->UninterceptProtocol(scheme) ?
+          PROTOCOL_OK : PROTOCOL_NOT_INTERCEPTED;
 }
 
 void Protocol::OnIOCompleted(
@@ -119,6 +127,13 @@ std::string Protocol::ErrorCodeToString(ProtocolError error) {
   }
 }
 
+AtomURLRequestJobFactory* Protocol::GetJobFactoryInIO() const {
+  request_context_getter_->GetURLRequestContext();  // Force init.
+  return static_cast<AtomURLRequestJobFactory*>(
+      static_cast<brightray::URLRequestContextGetter*>(
+          request_context_getter_.get())->job_factory());
+}
+
 // static
 mate::Handle<Protocol> Protocol::Create(
     v8::Isolate* isolate, AtomBrowserContext* browser_context) {

diff --git a/atom/browser/api/atom_api_protocol.h b/atom/browser/api/atom_api_protocol.h
index 3c262d2..379c43b 100644 (file)
--- a/atom/browser/api/atom_api_protocol.h
+++ b/atom/browser/api/atom_api_protocol.h
@@ -13,7 +13,6 @@
 #include "atom/browser/atom_browser_context.h"
 #include "atom/browser/net/atom_url_request_job_factory.h"
 #include "base/callback.h"
-#include "base/containers/scoped_ptr_hash_map.h"
 #include "base/memory/weak_ptr.h"
 #include "content/public/browser/browser_thread.h"
 #include "native_mate/arguments.h"
@@ -45,17 +44,6 @@ class Protocol : public mate::TrackableObject<Protocol> {
  protected:
   Protocol(v8::Isolate* isolate, AtomBrowserContext* browser_context);
 
-  AtomURLRequestJobFactory* job_factory() const {
-    request_context_getter_->GetURLRequestContext();  // Force init.
-    return static_cast<AtomURLRequestJobFactory*>(
-        static_cast<brightray::URLRequestContextGetter*>(
-            request_context_getter_.get())->job_factory());
-  }
-
-  base::WeakPtr<Protocol> GetWeakPtr() {
-    return weak_factory_.GetWeakPtr();
-  }
-
  private:
   // Possible errors.
   enum ProtocolError {
@@ -86,13 +74,13 @@ class Protocol : public mate::TrackableObject<Protocol> {
         net::URLRequest* request,
         net::NetworkDelegate* network_delegate) const override {
       RequestJob* request_job = new RequestJob(request, network_delegate);
-      request_job->SetHandlerInfo(isolate_, request_context_, handler_);
+      request_job->SetHandlerInfo(isolate_, request_context_.get(), handler_);
       return request_job;
     }
 
    private:
     v8::Isolate* isolate_;
-    net::URLRequestContextGetter* request_context_;
+    scoped_refptr<net::URLRequestContextGetter> request_context_;
     Protocol::Handler handler_;
 
     DISALLOW_COPY_AND_ASSIGN(CustomProtocolHandler);
@@ -111,19 +99,24 @@ class Protocol : public mate::TrackableObject<Protocol> {
     content::BrowserThread::PostTaskAndReplyWithResult(
         content::BrowserThread::IO, FROM_HERE,
         base::Bind(&Protocol::RegisterProtocolInIO<RequestJob>,
-                   base::Unretained(this), scheme, handler),
+                   request_context_getter_, isolate(), scheme, handler),
         base::Bind(&Protocol::OnIOCompleted,
                    GetWeakPtr(), callback));
   }
   template<typename RequestJob>
-  ProtocolError RegisterProtocolInIO(const std::string& scheme,
-                                     const Handler& handler) {
-    if (job_factory()->IsHandledProtocol(scheme))
+  static ProtocolError RegisterProtocolInIO(
+      scoped_refptr<brightray::URLRequestContextGetter> request_context_getter,
+      v8::Isolate* isolate,
+      const std::string& scheme,
+      const Handler& handler) {
+    auto job_factory = static_cast<AtomURLRequestJobFactory*>(
+        request_context_getter->job_factory());
+    if (job_factory->IsHandledProtocol(scheme))
       return PROTOCOL_REGISTERED;
     std::unique_ptr<CustomProtocolHandler<RequestJob>> protocol_handler(
         new CustomProtocolHandler<RequestJob>(
-            isolate(), request_context_getter_.get(), handler));
-    if (job_factory()->SetProtocolHandler(scheme, std::move(protocol_handler)))
+            isolate, request_context_getter.get(), handler));
+    if (job_factory->SetProtocolHandler(scheme, std::move(protocol_handler)))
       return PROTOCOL_OK;
     else
       return PROTOCOL_FAIL;
@@ -131,12 +124,16 @@ class Protocol : public mate::TrackableObject<Protocol> {
 
   // Unregister the protocol handler that handles |scheme|.
   void UnregisterProtocol(const std::string& scheme, mate::Arguments* args);
-  ProtocolError UnregisterProtocolInIO(const std::string& scheme);
+  static ProtocolError UnregisterProtocolInIO(
+      scoped_refptr<brightray::URLRequestContextGetter> request_context_getter,
+      const std::string& scheme);
 
   // Whether the protocol has handler registered.
   void IsProtocolHandled(const std::string& scheme,
                          const BooleanCallback& callback);
-  bool IsProtocolHandledInIO(const std::string& scheme);
+  static bool IsProtocolHandledInIO(
+      scoped_refptr<brightray::URLRequestContextGetter> request_context_getter,
+      const std::string& scheme);
 
   // Replace the protocol handler with a new one.
   template<typename RequestJob>
@@ -148,32 +145,36 @@ class Protocol : public mate::TrackableObject<Protocol> {
     content::BrowserThread::PostTaskAndReplyWithResult(
         content::BrowserThread::IO, FROM_HERE,
         base::Bind(&Protocol::InterceptProtocolInIO<RequestJob>,
-                   base::Unretained(this), scheme, handler),
+                   request_context_getter_, isolate(), scheme, handler),
         base::Bind(&Protocol::OnIOCompleted,
                    GetWeakPtr(), callback));
   }
   template<typename RequestJob>
-  ProtocolError InterceptProtocolInIO(const std::string& scheme,
-                                      const Handler& handler) {
-    if (!job_factory()->IsHandledProtocol(scheme))
+  static ProtocolError InterceptProtocolInIO(
+      scoped_refptr<brightray::URLRequestContextGetter> request_context_getter,
+      v8::Isolate* isolate,
+      const std::string& scheme,
+      const Handler& handler) {
+    auto job_factory = static_cast<AtomURLRequestJobFactory*>(
+        request_context_getter->job_factory());
+    if (!job_factory->IsHandledProtocol(scheme))
       return PROTOCOL_NOT_REGISTERED;
     // It is possible a protocol is handled but can not be intercepted.
-    if (!job_factory()->HasProtocolHandler(scheme))
+    if (!job_factory->HasProtocolHandler(scheme))
       return PROTOCOL_FAIL;
-    if (ContainsKey(original_protocols_, scheme))
-      return PROTOCOL_INTERCEPTED;
     std::unique_ptr<CustomProtocolHandler<RequestJob>> protocol_handler(
         new CustomProtocolHandler<RequestJob>(
-            isolate(), request_context_getter_.get(), handler));
-    original_protocols_.set(
-        scheme,
-        job_factory()->ReplaceProtocol(scheme, std::move(protocol_handler)));
+            isolate, request_context_getter.get(), handler));
+    if (!job_factory->InterceptProtocol(scheme, std::move(protocol_handler)))
+      return PROTOCOL_INTERCEPTED;
     return PROTOCOL_OK;
   }
 
   // Restore the |scheme| to its original protocol handler.
   void UninterceptProtocol(const std::string& scheme, mate::Arguments* args);
-  ProtocolError UninterceptProtocolInIO(const std::string& scheme);
+  static ProtocolError UninterceptProtocolInIO(
+      scoped_refptr<brightray::URLRequestContextGetter> request_context_getter,
+      const std::string& scheme);
 
   // Convert error code to JS exception and call the callback.
   void OnIOCompleted(const CompletionCallback& callback, ProtocolError error);
@@ -181,14 +182,13 @@ class Protocol : public mate::TrackableObject<Protocol> {
   // Convert error code to string.
   std::string ErrorCodeToString(ProtocolError error);
 
-  scoped_refptr<net::URLRequestContextGetter> request_context_getter_;
+  AtomURLRequestJobFactory* GetJobFactoryInIO() const;
 
-  // Map that stores the original protocols of schemes.
-  using OriginalProtocolsMap = base::ScopedPtrHashMap<
-      std::string,
-      std::unique_ptr<net::URLRequestJobFactory::ProtocolHandler>>;
-  OriginalProtocolsMap original_protocols_;
+  base::WeakPtr<Protocol> GetWeakPtr() {
+    return weak_factory_.GetWeakPtr();
+  }
 
+  scoped_refptr<brightray::URLRequestContextGetter> request_context_getter_;
   base::WeakPtrFactory<Protocol> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(Protocol);

diff --git a/atom/browser/net/atom_url_request_job_factory.cc b/atom/browser/net/atom_url_request_job_factory.cc
index aff2565..d78b702 100644 (file)
--- a/atom/browser/net/atom_url_request_job_factory.cc
+++ b/atom/browser/net/atom_url_request_job_factory.cc
@@ -5,6 +5,7 @@
 
 #include "atom/browser/net/atom_url_request_job_factory.h"
 
+#include "base/memory/ptr_util.h"
 #include "base/stl_util.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/base/load_flags.h"
@@ -41,14 +42,24 @@ bool AtomURLRequestJobFactory::SetProtocolHandler(
   return true;
 }
 
-std::unique_ptr<ProtocolHandler> AtomURLRequestJobFactory::ReplaceProtocol(
+bool AtomURLRequestJobFactory::InterceptProtocol(
     const std::string& scheme,
     std::unique_ptr<ProtocolHandler> protocol_handler) {
-  if (!ContainsKey(protocol_handler_map_, scheme))
-    return nullptr;
+  if (!ContainsKey(protocol_handler_map_, scheme) ||
+      ContainsKey(original_protocols_, scheme))
+    return false;
   ProtocolHandler* original_protocol_handler = protocol_handler_map_[scheme];
   protocol_handler_map_[scheme] = protocol_handler.release();
-  return make_scoped_ptr(original_protocol_handler);
+  original_protocols_.set(scheme, base::WrapUnique(original_protocol_handler));
+  return true;
+}
+
+bool AtomURLRequestJobFactory::UninterceptProtocol(const std::string& scheme) {
+  if (!original_protocols_.contains(scheme))
+    return false;
+  protocol_handler_map_[scheme] =
+      original_protocols_.take_and_erase(scheme).release();
+  return true;
 }
 
 ProtocolHandler* AtomURLRequestJobFactory::GetProtocolHandler(

diff --git a/atom/browser/net/atom_url_request_job_factory.h b/atom/browser/net/atom_url_request_job_factory.h
index e3dbd77..ea2710b 100644 (file)
--- a/atom/browser/net/atom_url_request_job_factory.h
+++ b/atom/browser/net/atom_url_request_job_factory.h
@@ -7,11 +7,11 @@
 #define ATOM_BROWSER_NET_ATOM_URL_REQUEST_JOB_FACTORY_H_
 
 #include <map>
+#include <memory>
 #include <string>
 #include <vector>
 
-#include "base/memory/scoped_ptr.h"
-#include "base/synchronization/lock.h"
+#include "base/containers/scoped_ptr_hash_map.h"
 #include "net/url_request/url_request_job_factory.h"
 
 namespace atom {
@@ -27,11 +27,11 @@ class AtomURLRequestJobFactory : public net::URLRequestJobFactory {
   bool SetProtocolHandler(const std::string& scheme,
                           std::unique_ptr<ProtocolHandler> protocol_handler);
 
-  // Intercepts the ProtocolHandler for a scheme. Returns the original protocol
-  // handler on success, otherwise returns NULL.
-  std::unique_ptr<ProtocolHandler> ReplaceProtocol(
+  // Intercepts the ProtocolHandler for a scheme.
+  bool InterceptProtocol(
       const std::string& scheme,
       std::unique_ptr<ProtocolHandler> protocol_handler);
+  bool UninterceptProtocol(const std::string& scheme);
 
   // Returns the protocol handler registered with scheme.
   ProtocolHandler* GetProtocolHandler(const std::string& scheme) const;
@@ -60,6 +60,12 @@ class AtomURLRequestJobFactory : public net::URLRequestJobFactory {
 
   ProtocolHandlerMap protocol_handler_map_;
 
+  // Map that stores the original protocols of schemes.
+  using OriginalProtocolsMap = base::ScopedPtrHashMap<
+      std::string, std::unique_ptr<ProtocolHandler>>;
+  // Can only be accessed in IO thread.
+  OriginalProtocolsMap original_protocols_;
+
   DISALLOW_COPY_AND_ASSIGN(AtomURLRequestJobFactory);
 };