read/write protocol handler map in IO

atom/browser/api/atom_api_protocol.cc

@@ -36,7 +36,6 @@ struct Converter<const net::URLRequest*> {
 
 }  // namespace mate
 
-
 namespace atom {
 
 namespace api {
@@ -189,6 +188,26 @@ class CustomProtocolHandler : public ProtocolHandler {
   DISALLOW_COPY_AND_ASSIGN(CustomProtocolHandler);
 };
 
+std::string ConvertErrorCode(int error_code) {
+  switch (error_code) {
+    case Protocol::ERR_SCHEME_REGISTERED:
+      return "The Scheme is already registered";
+    case Protocol::ERR_SCHEME_UNREGISTERED:
+      return "The Scheme has not been registered";
+    case Protocol::ERR_SCHEME_INTERCEPTED:
+      return "There is no protocol handler to intercept";
+    case Protocol::ERR_SCHEME_UNINTERCEPTED:
+      return "The protocol is not intercepted";
+    case Protocol::ERR_NO_SCHEME:
+      return "The Scheme does not exist.";
+    case Protocol::ERR_SCHEME:
+      return "Cannot intercept custom protocols";
+    default:
+      NOTREACHED();
+      return std::string();
+  }
+}
+
 }  // namespace
 
 Protocol::Protocol(AtomBrowserContext* browser_context)
@@ -202,91 +221,32 @@ Protocol::JsProtocolHandler Protocol::GetProtocolHandler(
   return protocol_handlers_[scheme];
 }
 
-void Protocol::OnRegisterProtocol(const std::string& scheme,
+void Protocol::OnIOActionCompleted(const JsCompletionCallback& callback,
-                                  const JsProtocolHandler& handler,
+                                   int error) {
-                                  const JsCompletionCallback& callback,
-                                  int is_handled) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   v8::Locker locker(isolate());
   v8::HandleScope handle_scope(isolate());
 
-  if (is_handled || ContainsKey(protocol_handlers_, scheme)) {
+  if (error) {
     callback.Run(v8::Exception::Error(
-        mate::StringToV8(isolate(), "The Scheme is already registered")));
+        mate::StringToV8(isolate(), ConvertErrorCode(error))));
     return;
   }
 
-  protocol_handlers_[scheme] = handler;
+  callback.Run(v8::Null(isolate()));
-  BrowserThread::PostTaskAndReply(BrowserThread::IO, FROM_HERE,
-                                  base::Bind(&Protocol::RegisterProtocolInIO,
-                                             base::Unretained(this), scheme),
-                                  base::Bind(callback, v8::Null(isolate())));
-}
-
-void Protocol::OnInterceptProtocol(const std::string& scheme,
-                                   const JsProtocolHandler& handler,
-                                   const JsCompletionCallback& callback,
-                                   int is_handled) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  v8::Locker locker(isolate());
-  v8::HandleScope handle_scope(isolate());
-  if (!is_handled) {
-    callback.Run(v8::Exception::Error(
-        mate::StringToV8(isolate(), "Scheme does not exist.")));
-    return;
-  }
-
-  if (ContainsKey(protocol_handlers_, scheme)) {
-    callback.Run(v8::Exception::Error(
-        mate::StringToV8(isolate(), "Cannot intercept custom protocols.")));
-    return;
-  }
-
-  protocol_handlers_[scheme] = handler;
-  BrowserThread::PostTaskAndReply(BrowserThread::IO, FROM_HERE,
-                                  base::Bind(&Protocol::InterceptProtocolInIO,
-                                             base::Unretained(this), scheme),
-                                  base::Bind(callback, v8::Null(isolate())));
 }
 
 mate::ObjectTemplateBuilder Protocol::GetObjectTemplateBuilder(
     v8::Isolate* isolate) {
   return mate::ObjectTemplateBuilder(isolate)
-      .SetMethod("_registerProtocol", &Protocol::RegisterProtocol)
-      .SetMethod("_unregisterProtocol", &Protocol::UnregisterProtocol)
       .SetMethod("registerStandardSchemes", &Protocol::RegisterStandardSchemes)
       .SetMethod("isHandledProtocol", &Protocol::IsHandledProtocol)
+      .SetMethod("_registerProtocol", &Protocol::RegisterProtocol)
+      .SetMethod("_unregisterProtocol", &Protocol::UnregisterProtocol)
       .SetMethod("_interceptProtocol", &Protocol::InterceptProtocol)
       .SetMethod("_uninterceptProtocol", &Protocol::UninterceptProtocol);
 }
 
-void Protocol::RegisterProtocol(v8::Isolate* isolate,
-                                const std::string& scheme,
-                                const JsProtocolHandler& handler,
-                                const JsCompletionCallback& callback) {
-  IsHandledProtocol(scheme,
-                    base::Bind(&Protocol::OnRegisterProtocol,
-                               base::Unretained(this),
-                               scheme, handler, callback));
-}
-
-void Protocol::UnregisterProtocol(v8::Isolate* isolate,
-                                  const std::string& scheme,
-                                  const JsCompletionCallback& callback) {
-  ProtocolHandlersMap::iterator it(protocol_handlers_.find(scheme));
-  if (it == protocol_handlers_.end()) {
-    callback.Run(v8::Exception::Error(
-        mate::StringToV8(isolate, "The Scheme has not been registered")));
-    return;
-  }
-
-  protocol_handlers_.erase(it);
-  BrowserThread::PostTaskAndReply(BrowserThread::IO, FROM_HERE,
-      base::Bind(&Protocol::UnregisterProtocolInIO,
-                 base::Unretained(this), scheme),
-      base::Bind(callback, v8::Null(isolate)));
-}
-
 void Protocol::RegisterStandardSchemes(
     const std::vector<std::string>& schemes) {
   atom::AtomBrowserClient::SetCustomSchemes(schemes);
@@ -300,71 +260,119 @@ void Protocol::IsHandledProtocol(const std::string& scheme,
       callback);
 }
 
+void Protocol::RegisterProtocol(v8::Isolate* isolate,
+                                const std::string& scheme,
+                                const JsProtocolHandler& handler,
+                                const JsCompletionCallback& callback) {
+  BrowserThread::PostTaskAndReplyWithResult(BrowserThread::IO, FROM_HERE,
+      base::Bind(&Protocol::RegisterProtocolInIO,
+                 base::Unretained(this), scheme, handler),
+      base::Bind(&Protocol::OnIOActionCompleted,
+                 base::Unretained(this), callback));
+}
+
+void Protocol::UnregisterProtocol(v8::Isolate* isolate,
+                                  const std::string& scheme,
+                                  const JsCompletionCallback& callback) {
+  BrowserThread::PostTaskAndReplyWithResult(BrowserThread::IO, FROM_HERE,
+      base::Bind(&Protocol::UnregisterProtocolInIO,
+                 base::Unretained(this), scheme),
+      base::Bind(&Protocol::OnIOActionCompleted,
+                 base::Unretained(this), callback));
+}
+
 void Protocol::InterceptProtocol(v8::Isolate* isolate,
                                  const std::string& scheme,
                                  const JsProtocolHandler& handler,
                                  const JsCompletionCallback& callback) {
   BrowserThread::PostTaskAndReplyWithResult(BrowserThread::IO, FROM_HERE,
-      base::Bind(&AtomURLRequestJobFactory::HasProtocolHandler,
+      base::Bind(&Protocol::InterceptProtocolInIO,
-                 base::Unretained(job_factory_), scheme),
+                 base::Unretained(this), scheme, handler),
-      base::Bind(&Protocol::OnInterceptProtocol,
+      base::Bind(&Protocol::OnIOActionCompleted,
-                 base::Unretained(this), scheme, handler, callback));
+                 base::Unretained(this), callback));
 }
 
 void Protocol::UninterceptProtocol(v8::Isolate* isolate,
                                    const std::string& scheme,
                                    const JsCompletionCallback& callback) {
+  BrowserThread::PostTaskAndReplyWithResult(BrowserThread::IO, FROM_HERE,
+      base::Bind(&Protocol::UninterceptProtocolInIO,
+                 base::Unretained(this), scheme),
+      base::Bind(&Protocol::OnIOActionCompleted,
+                 base::Unretained(this), callback));
+}
+
+int Protocol::RegisterProtocolInIO(const std::string& scheme,
+                                   const JsProtocolHandler& handler) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  if (ContainsKey(protocol_handlers_, scheme) ||
+      job_factory_->IsHandledProtocol(scheme)) {
+    return ERR_SCHEME_REGISTERED;
+  }
+
+  protocol_handlers_[scheme] = handler;
+  job_factory_->SetProtocolHandler(scheme, new CustomProtocolHandler(this));
+
+  return OK;
+}
+
+int Protocol::UnregisterProtocolInIO(const std::string& scheme) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
   ProtocolHandlersMap::iterator it(protocol_handlers_.find(scheme));
   if (it == protocol_handlers_.end()) {
-    callback.Run(v8::Exception::Error(
+    return ERR_SCHEME_UNREGISTERED;
-        mate::StringToV8(isolate, "The Scheme has not been registered")));
-    return;
   }
 
   protocol_handlers_.erase(it);
-  BrowserThread::PostTaskAndReply(BrowserThread::IO, FROM_HERE,
-      base::Bind(&Protocol::UninterceptProtocolInIO,
-                 base::Unretained(this), scheme),
-      base::Bind(callback, v8::Null(isolate)));
-}
-
-void Protocol::RegisterProtocolInIO(const std::string& scheme) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  job_factory_->SetProtocolHandler(scheme, new CustomProtocolHandler(this));
-}
-
-void Protocol::UnregisterProtocolInIO(const std::string& scheme) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
   job_factory_->SetProtocolHandler(scheme, NULL);
+
+  return OK;
 }
 
-void Protocol::InterceptProtocolInIO(const std::string& scheme) {
+int Protocol::InterceptProtocolInIO(const std::string& scheme,
+                                    const JsProtocolHandler& handler) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
+  if (!job_factory_->HasProtocolHandler(scheme))
+    return ERR_NO_SCHEME;
+
+  if (ContainsKey(protocol_handlers_, scheme))
+    return ERR_SCHEME;
+
+  protocol_handlers_[scheme] = handler;
   ProtocolHandler* original_handler = job_factory_->GetProtocolHandler(scheme);
   if (original_handler == nullptr) {
-    NOTREACHED();
+    return ERR_SCHEME_INTERCEPTED;
   }
 
   job_factory_->ReplaceProtocol(
       scheme, new CustomProtocolHandler(this, original_handler));
+
+  return OK;
 }
 
-void Protocol::UninterceptProtocolInIO(const std::string& scheme) {
+int Protocol::UninterceptProtocolInIO(const std::string& scheme) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
+  ProtocolHandlersMap::iterator it(protocol_handlers_.find(scheme));
+  if (it == protocol_handlers_.end())
+    return ERR_SCHEME_UNREGISTERED;
+
+  protocol_handlers_.erase(it);
   CustomProtocolHandler* handler = static_cast<CustomProtocolHandler*>(
       job_factory_->GetProtocolHandler(scheme));
   if (handler->original_handler() == nullptr) {
-    NOTREACHED();
+    return ERR_SCHEME_UNINTERCEPTED;
   }
 
   // Reset the protocol handler to the orignal one and delete current protocol
   // handler.
   ProtocolHandler* original_handler = handler->ReleaseDefaultProtocolHandler();
   delete job_factory_->ReplaceProtocol(scheme, original_handler);
+
+  return OK;
 }
 
 // static

atom/browser/api/atom_api_protocol.h

@@ -31,6 +31,16 @@ class Protocol : public mate::EventEmitter {
       base::Callback<v8::Local<v8::Value>(const net::URLRequest*)>;
   using JsCompletionCallback = base::Callback<void(v8::Local<v8::Value>)>;
 
+  enum {
+    OK = 0,
+    ERR_SCHEME_REGISTERED,
+    ERR_SCHEME_UNREGISTERED,
+    ERR_SCHEME_INTERCEPTED,
+    ERR_SCHEME_UNINTERCEPTED,
+    ERR_NO_SCHEME,
+    ERR_SCHEME
+  };
+
   static mate::Handle<Protocol> Create(
       v8::Isolate* isolate, AtomBrowserContext* browser_context);
 
@@ -48,20 +58,17 @@ class Protocol : public mate::EventEmitter {
  private:
   typedef std::map<std::string, JsProtocolHandler> ProtocolHandlersMap;
 
-  // Callback called if protocol can be registered.
+  // Callback called after performing action on IO thread.
-  void OnRegisterProtocol(const std::string& scheme,
+  void OnIOActionCompleted(const JsCompletionCallback& callback,
-                          const JsProtocolHandler& handler,
+                           int error);
-                          const JsCompletionCallback& callback,
-                          int is_handled);
-  // Callback called if protocol can be intercepted.
-  void OnInterceptProtocol(const std::string& scheme,
-                           const JsProtocolHandler& handler,
-                           const JsCompletionCallback& callback,
-                           int is_handled);
 
   // Register schemes to standard scheme list.
   void RegisterStandardSchemes(const std::vector<std::string>& schemes);
 
+  // Returns whether a scheme has been registered.
+  void IsHandledProtocol(const std::string& scheme,
+                         const net::CompletionCallback& callback);
+
   // Register/unregister an networking |scheme| which would be handled by
   // |callback|.
   void RegisterProtocol(v8::Isolate* isolate,
@@ -71,10 +78,6 @@ class Protocol : public mate::EventEmitter {
   void UnregisterProtocol(v8::Isolate* isolate, const std::string& scheme,
                           const JsCompletionCallback& callback);
 
-  // Returns whether a scheme has been registered.
-  void IsHandledProtocol(const std::string& scheme,
-                         const net::CompletionCallback& callback);
-
   // Intercept/unintercept an existing protocol handler.
   void InterceptProtocol(v8::Isolate* isolate,
                          const std::string& scheme,
@@ -84,10 +87,12 @@ class Protocol : public mate::EventEmitter {
                            const JsCompletionCallback& callback);
 
   // The networking related operations have to be done in IO thread.
-  void RegisterProtocolInIO(const std::string& scheme);
+  int RegisterProtocolInIO(const std::string& scheme,
-  void UnregisterProtocolInIO(const std::string& scheme);
+                           const JsProtocolHandler& handler);
-  void InterceptProtocolInIO(const std::string& scheme);
+  int UnregisterProtocolInIO(const std::string& scheme);
-  void UninterceptProtocolInIO(const std::string& scheme);
+  int InterceptProtocolInIO(const std::string& scheme,
+                            const JsProtocolHandler& handler);
+  int UninterceptProtocolInIO(const std::string& scheme);
 
   AtomBrowserContext* browser_context_;
   AtomURLRequestJobFactory* job_factory_;

spec/api-protocol-spec.coffee

@@ -28,8 +28,10 @@ describe 'protocol module', ->
 
   describe 'protocol.unregisterProtocol', ->
     it 'throws error when scheme does not exist', ->
-      unregister = -> protocol.unregisterProtocol 'test3'
+      protocol.unregisterProtocol 'test3', (->), (error, scheme) ->
-      assert.throws unregister, /The Scheme has not been registered/
+        if (error)
+          assert.equal scheme, 'test3'
+          done()
 
   describe 'registered protocol callback', ->
     it 'returns string should send the string as request content', (done) ->