From da00329d78869bab7d3d8be08dd1d87893ede0ff Mon Sep 17 00:00:00 2001 From: Robo Date: Sun, 5 Jul 2015 23:23:07 +0530 Subject: [PATCH 1/3] protocol: cleanup --- atom/browser/api/atom_api_protocol.cc | 111 ++++++++++++------ atom/browser/api/atom_api_protocol.h | 20 +++- .../net/atom_url_request_job_factory.cc | 20 ++-- .../net/atom_url_request_job_factory.h | 4 +- docs/api/protocol.md | 7 +- spec/api-protocol-spec.coffee | 50 +++++--- 6 files changed, 138 insertions(+), 74 deletions(-) diff --git a/atom/browser/api/atom_api_protocol.cc b/atom/browser/api/atom_api_protocol.cc index e298ec790940..a6d37237632e 100644 --- a/atom/browser/api/atom_api_protocol.cc +++ b/atom/browser/api/atom_api_protocol.cc @@ -25,12 +25,16 @@ namespace mate { template<> struct Converter { static v8::Local ToV8(v8::Isolate* isolate, - const net::URLRequest* val) { - return mate::ObjectTemplateBuilder(isolate) - .SetValue("method", val->method()) - .SetValue("url", val->url().spec()) - .SetValue("referrer", val->referrer()) - .Build()->NewInstance(); + const net::URLRequest* val) { + if (val) { + return mate::ObjectTemplateBuilder(isolate) + .SetValue("method", val->method()) + .SetValue("url", val->url().spec()) + .SetValue("referrer", val->referrer()) + .Build()->NewInstance(); + } else { + return v8::Null(isolate); + } } }; @@ -65,7 +69,7 @@ class CustomProtocolRequestJob : public AdapterRequestJob { // AdapterRequestJob: void GetJobTypeInUI() override { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + DCHECK_CURRENTLY_ON(BrowserThread::UI); v8::Locker locker(registry_->isolate()); v8::HandleScope handle_scope(registry_->isolate()); @@ -73,7 +77,8 @@ class CustomProtocolRequestJob : public AdapterRequestJob { // Call the JS handler. Protocol::JsProtocolHandler callback = registry_->GetProtocolHandler(request()->url().scheme()); - v8::Local result = callback.Run(request()); + v8::Local result = + callback.Run(v8::Null(registry_->isolate()), request()); // Determine the type of the job we are going to create. if (result->IsString()) { @@ -202,6 +207,51 @@ Protocol::JsProtocolHandler Protocol::GetProtocolHandler( return protocol_handlers_[scheme]; } +void Protocol::OnRegisterProtocol(const std::string& scheme, + const JsProtocolHandler& callback, + bool is_handled) { + DCHECK_CURRENTLY_ON(BrowserThread::UI); + v8::Locker locker(isolate()); + v8::HandleScope handle_scope(isolate()); + + if (is_handled || ContainsKey(protocol_handlers_, scheme)) { + callback.Run(v8::Exception::Error( + mate::StringToV8(isolate(), "The Scheme is already registered")), + nullptr); + return; + } + + protocol_handlers_[scheme] = callback; + BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, + base::Bind(&Protocol::RegisterProtocolInIO, + base::Unretained(this), scheme)); +} + +void Protocol::OnInterceptProtocol(const std::string& scheme, + const JsProtocolHandler& callback, + bool 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.")), nullptr); + return; + } + + if (ContainsKey(protocol_handlers_, scheme)) { + callback.Run(v8::Exception::Error( + mate::StringToV8(isolate(), "Cannot intercept custom protocols.")), + nullptr); + return; + } + + protocol_handlers_[scheme] = callback; + BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, + base::Bind(&Protocol::InterceptProtocolInIO, + base::Unretained(this), scheme)); +} + mate::ObjectTemplateBuilder Protocol::GetObjectTemplateBuilder( v8::Isolate* isolate) { return mate::ObjectTemplateBuilder(isolate) @@ -216,15 +266,10 @@ mate::ObjectTemplateBuilder Protocol::GetObjectTemplateBuilder( void Protocol::RegisterProtocol(v8::Isolate* isolate, const std::string& scheme, const JsProtocolHandler& callback) { - if (ContainsKey(protocol_handlers_, scheme) || - job_factory_->IsHandledProtocol(scheme)) - return node::ThrowError(isolate, "The scheme is already registered"); - - protocol_handlers_[scheme] = callback; - BrowserThread::PostTask(BrowserThread::IO, - FROM_HERE, - base::Bind(&Protocol::RegisterProtocolInIO, - base::Unretained(this), scheme)); + IsHandledProtocol(scheme, + base::Bind(&Protocol::OnRegisterProtocol, + base::Unretained(this), + scheme, callback)); } void Protocol::UnregisterProtocol(v8::Isolate* isolate, @@ -245,24 +290,22 @@ void Protocol::RegisterStandardSchemes( atom::AtomBrowserClient::SetCustomSchemes(schemes); } -bool Protocol::IsHandledProtocol(const std::string& scheme) { - return job_factory_->IsHandledProtocol(scheme); +void Protocol::IsHandledProtocol(const std::string& scheme, + const CompletionCallback& callback) { + BrowserThread::PostTaskAndReplyWithResult(BrowserThread::IO, FROM_HERE, + base::Bind(&AtomURLRequestJobFactory::IsHandledProtocol, + base::Unretained(job_factory_), scheme), + callback); } void Protocol::InterceptProtocol(v8::Isolate* isolate, const std::string& scheme, const JsProtocolHandler& callback) { - if (!job_factory_->HasProtocolHandler(scheme)) - return node::ThrowError(isolate, "Scheme does not exist."); - - if (ContainsKey(protocol_handlers_, scheme)) - return node::ThrowError(isolate, "Cannot intercept custom procotols"); - - protocol_handlers_[scheme] = callback; - BrowserThread::PostTask(BrowserThread::IO, - FROM_HERE, - base::Bind(&Protocol::InterceptProtocolInIO, - base::Unretained(this), scheme)); + BrowserThread::PostTaskAndReplyWithResult(BrowserThread::IO, FROM_HERE, + base::Bind(&AtomURLRequestJobFactory::HasProtocolHandler, + base::Unretained(job_factory_), scheme), + base::Bind(&Protocol::OnInterceptProtocol, + base::Unretained(this), scheme, callback)); } void Protocol::UninterceptProtocol(v8::Isolate* isolate, @@ -279,7 +322,7 @@ void Protocol::UninterceptProtocol(v8::Isolate* isolate, } void Protocol::RegisterProtocolInIO(const std::string& scheme) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + DCHECK_CURRENTLY_ON(BrowserThread::IO); job_factory_->SetProtocolHandler(scheme, new CustomProtocolHandler(this)); BrowserThread::PostTask(BrowserThread::UI, @@ -290,7 +333,7 @@ void Protocol::RegisterProtocolInIO(const std::string& scheme) { } void Protocol::UnregisterProtocolInIO(const std::string& scheme) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + DCHECK_CURRENTLY_ON(BrowserThread::IO); job_factory_->SetProtocolHandler(scheme, NULL); BrowserThread::PostTask(BrowserThread::UI, @@ -301,7 +344,7 @@ void Protocol::UnregisterProtocolInIO(const std::string& scheme) { } void Protocol::InterceptProtocolInIO(const std::string& scheme) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + DCHECK_CURRENTLY_ON(BrowserThread::IO); ProtocolHandler* original_handler = job_factory_->GetProtocolHandler(scheme); if (original_handler == NULL) { @@ -322,7 +365,7 @@ void Protocol::InterceptProtocolInIO(const std::string& scheme) { } void Protocol::UninterceptProtocolInIO(const std::string& scheme) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + DCHECK_CURRENTLY_ON(BrowserThread::IO); CustomProtocolHandler* handler = static_cast( job_factory_->GetProtocolHandler(scheme)); diff --git a/atom/browser/api/atom_api_protocol.h b/atom/browser/api/atom_api_protocol.h index 1b17b630c5e6..147b9e1912a9 100644 --- a/atom/browser/api/atom_api_protocol.h +++ b/atom/browser/api/atom_api_protocol.h @@ -26,8 +26,10 @@ namespace api { class Protocol : public mate::EventEmitter { public: - typedef base::Callback(const net::URLRequest*)> - JsProtocolHandler; + using JsProtocolHandler = + base::Callback(v8::Local, + const net::URLRequest*)>; + using CompletionCallback = base::Callback; static mate::Handle Create( v8::Isolate* isolate, AtomBrowserContext* browser_context); @@ -46,6 +48,15 @@ class Protocol : public mate::EventEmitter { private: typedef std::map ProtocolHandlersMap; + // Callback called if protocol can be registered. + void OnRegisterProtocol(const std::string& scheme, + const JsProtocolHandler& callback, + bool is_handled); + // Callback called if protocol can be intercepted. + void OnInterceptProtocol(const std::string& scheme, + const JsProtocolHandler& callback, + bool is_handled); + // Register schemes to standard scheme list. void RegisterStandardSchemes(const std::vector& schemes); @@ -57,9 +68,8 @@ class Protocol : public mate::EventEmitter { void UnregisterProtocol(v8::Isolate* isolate, const std::string& scheme); // Returns whether a scheme has been registered. - // FIXME Should accept a callback and be asynchronous so we do not have to use - // locks. - bool IsHandledProtocol(const std::string& scheme); + void IsHandledProtocol(const std::string& scheme, + const CompletionCallback& callback); // Intercept/unintercept an existing protocol handler. void InterceptProtocol(v8::Isolate* isolate, diff --git a/atom/browser/net/atom_url_request_job_factory.cc b/atom/browser/net/atom_url_request_job_factory.cc index ed1fb97e2c1c..00942b2ad057 100644 --- a/atom/browser/net/atom_url_request_job_factory.cc +++ b/atom/browser/net/atom_url_request_job_factory.cc @@ -6,9 +6,12 @@ #include "atom/browser/net/atom_url_request_job_factory.h" #include "base/stl_util.h" +#include "content/public/browser/browser_thread.h" #include "net/base/load_flags.h" #include "net/url_request/url_request.h" +using content::BrowserThread; + namespace atom { typedef net::URLRequestJobFactory::ProtocolHandler ProtocolHandler; @@ -22,9 +25,7 @@ AtomURLRequestJobFactory::~AtomURLRequestJobFactory() { bool AtomURLRequestJobFactory::SetProtocolHandler( const std::string& scheme, ProtocolHandler* protocol_handler) { - DCHECK(CalledOnValidThread()); - - base::AutoLock locked(lock_); + DCHECK_CURRENTLY_ON(BrowserThread::IO); if (!protocol_handler) { ProtocolHandlerMap::iterator it = protocol_handler_map_.find(scheme); @@ -45,10 +46,9 @@ bool AtomURLRequestJobFactory::SetProtocolHandler( ProtocolHandler* AtomURLRequestJobFactory::ReplaceProtocol( const std::string& scheme, ProtocolHandler* protocol_handler) { - DCHECK(CalledOnValidThread()); + DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK(protocol_handler); - base::AutoLock locked(lock_); if (!ContainsKey(protocol_handler_map_, scheme)) return nullptr; ProtocolHandler* original_protocol_handler = protocol_handler_map_[scheme]; @@ -58,9 +58,8 @@ ProtocolHandler* AtomURLRequestJobFactory::ReplaceProtocol( ProtocolHandler* AtomURLRequestJobFactory::GetProtocolHandler( const std::string& scheme) const { - DCHECK(CalledOnValidThread()); + DCHECK_CURRENTLY_ON(BrowserThread::IO); - base::AutoLock locked(lock_); ProtocolHandlerMap::const_iterator it = protocol_handler_map_.find(scheme); if (it == protocol_handler_map_.end()) return nullptr; @@ -69,7 +68,6 @@ ProtocolHandler* AtomURLRequestJobFactory::GetProtocolHandler( bool AtomURLRequestJobFactory::HasProtocolHandler( const std::string& scheme) const { - base::AutoLock locked(lock_); return ContainsKey(protocol_handler_map_, scheme); } @@ -77,9 +75,8 @@ net::URLRequestJob* AtomURLRequestJobFactory::MaybeCreateJobWithProtocolHandler( const std::string& scheme, net::URLRequest* request, net::NetworkDelegate* network_delegate) const { - DCHECK(CalledOnValidThread()); + DCHECK_CURRENTLY_ON(BrowserThread::IO); - base::AutoLock locked(lock_); ProtocolHandlerMap::const_iterator it = protocol_handler_map_.find(scheme); if (it == protocol_handler_map_.end()) return nullptr; @@ -101,7 +98,8 @@ net::URLRequestJob* AtomURLRequestJobFactory::MaybeInterceptResponse( bool AtomURLRequestJobFactory::IsHandledProtocol( const std::string& scheme) const { - DCHECK(CalledOnValidThread()); + DCHECK_CURRENTLY_ON(BrowserThread::IO); + return HasProtocolHandler(scheme) || net::URLRequest::IsHandledProtocol(scheme); } diff --git a/atom/browser/net/atom_url_request_job_factory.h b/atom/browser/net/atom_url_request_job_factory.h index a083e51483c7..ce2a18a85e66 100644 --- a/atom/browser/net/atom_url_request_job_factory.h +++ b/atom/browser/net/atom_url_request_job_factory.h @@ -56,12 +56,10 @@ class AtomURLRequestJobFactory : public net::URLRequestJobFactory { bool IsSafeRedirectTarget(const GURL& location) const override; private: - typedef std::map ProtocolHandlerMap; + using ProtocolHandlerMap = std::map; ProtocolHandlerMap protocol_handler_map_; - mutable base::Lock lock_; - DISALLOW_COPY_AND_ASSIGN(AtomURLRequestJobFactory); }; diff --git a/docs/api/protocol.md b/docs/api/protocol.md index c6dd373b586c..dbcdb577ca9c 100644 --- a/docs/api/protocol.md +++ b/docs/api/protocol.md @@ -28,7 +28,7 @@ was emitted. * `handler` Function Registers a custom protocol of `scheme`, the `handler` would be called with -`handler(request)` when the a request with registered `scheme` is made. +`handler(error, request)` when the a request with registered `scheme` is made. You need to return a request job in the `handler` to specify which type of response you would like to send. @@ -45,11 +45,12 @@ Unregisters the custom protocol of `scheme`. `value` is an array of custom schemes to be registered to the standard. -## protocol.isHandledProtocol(scheme) +## protocol.isHandledProtocol(scheme, callback) * `scheme` String +* `callback` Function -Returns whether the `scheme` can be handled already. +`callback` returns a boolean whether the `scheme` can be handled already. ## protocol.interceptProtocol(scheme, handler) diff --git a/spec/api-protocol-spec.coffee b/spec/api-protocol-spec.coffee index 3ad1cbd91fad..4fb00b9a2d4f 100644 --- a/spec/api-protocol-spec.coffee +++ b/spec/api-protocol-spec.coffee @@ -7,17 +7,19 @@ protocol = remote.require 'protocol' describe 'protocol module', -> describe 'protocol.registerProtocol', -> - it 'throws error when scheme is already registered', (done) -> - register = -> protocol.registerProtocol('test1', ->) + it 'error when scheme is already registered', (done) -> + register = -> + protocol.registerProtocol 'test1', (error, request) -> + assert error instanceof Error + protocol.unregisterProtocol 'test1' + done() protocol.once 'registered', (event, scheme) -> assert.equal scheme, 'test1' - assert.throws register, /The scheme is already registered/ - protocol.unregisterProtocol 'test1' - done() + register() register() it 'calls the callback when scheme is visited', (done) -> - protocol.registerProtocol 'test2', (request) -> + protocol.registerProtocol 'test2', (error, request) -> assert.equal request.url, 'test2://test2' protocol.unregisterProtocol 'test2' done() @@ -169,16 +171,28 @@ describe 'protocol module', -> protocol.unregisterProtocol 'atom-file-job' describe 'protocol.isHandledProtocol', -> - it 'returns true if the scheme can be handled', -> - assert.equal protocol.isHandledProtocol('file'), true - assert.equal protocol.isHandledProtocol('http'), true - assert.equal protocol.isHandledProtocol('https'), true - assert.equal protocol.isHandledProtocol('atom'), false + it 'returns true if the file scheme can be handled', (done) -> + protocol.isHandledProtocol 'file', (result) -> + assert.equal result, true + done() + it 'returns true if the http scheme can be handled', (done) -> + protocol.isHandledProtocol 'http', (result) -> + assert.equal result, true + done() + it 'returns true if the https scheme can be handled', (done) -> + protocol.isHandledProtocol 'https', (result) -> + assert.equal result, true + done() + it 'returns false if the atom scheme cannot be handled', (done) -> + protocol.isHandledProtocol 'atom', (result) -> + assert.equal result, false + done() describe 'protocol.interceptProtocol', -> - it 'throws error when scheme is not a registered one', -> - register = -> protocol.interceptProtocol('test-intercept', ->) - assert.throws register, /Scheme does not exist/ + it 'throws error when scheme is not a registered one', (done) -> + protocol.interceptProtocol 'test-intercept', (error, request) -> + assert error instanceof Error + done() it 'throws error when scheme is a custom protocol', (done) -> protocol.once 'unregistered', (event, scheme) -> @@ -186,9 +200,9 @@ describe 'protocol module', -> done() protocol.once 'registered', (event, scheme) -> assert.equal scheme, 'atom' - register = -> protocol.interceptProtocol('test-intercept', ->) - assert.throws register, /Scheme does not exist/ - protocol.unregisterProtocol scheme + protocol.interceptProtocol 'test-intercept', (error, request) -> + assert error instanceof Error + protocol.unregisterProtocol scheme protocol.registerProtocol('atom', ->) it 'returns original job when callback returns nothing', (done) -> @@ -206,7 +220,7 @@ describe 'protocol module', -> error: (xhr, errorType, error) -> free() assert false, 'Got error: ' + errorType + ' ' + error - protocol.interceptProtocol targetScheme, (request) -> + protocol.interceptProtocol targetScheme, (error, request) -> if process.platform is 'win32' pathInUrl = path.normalize request.url.substr(8) assert.equal pathInUrl.toLowerCase(), __filename.toLowerCase() From 2cd5fb5694cf0ab50955c3a7b40cc4e07ad64c40 Mon Sep 17 00:00:00 2001 From: Robo Date: Tue, 7 Jul 2015 20:13:13 +0530 Subject: [PATCH 2/3] add compatibility will old api --- atom/browser/api/atom_api_protocol.cc | 152 +++++++++++--------------- atom/browser/api/atom_api_protocol.h | 33 +++--- atom/browser/api/lib/protocol.coffee | 23 ++++ docs/api/protocol.md | 17 ++- spec/api-protocol-spec.coffee | 43 ++++---- 5 files changed, 139 insertions(+), 129 deletions(-) diff --git a/atom/browser/api/atom_api_protocol.cc b/atom/browser/api/atom_api_protocol.cc index a6d37237632e..fa498a7fa99c 100644 --- a/atom/browser/api/atom_api_protocol.cc +++ b/atom/browser/api/atom_api_protocol.cc @@ -25,16 +25,12 @@ namespace mate { template<> struct Converter { static v8::Local ToV8(v8::Isolate* isolate, - const net::URLRequest* val) { - if (val) { - return mate::ObjectTemplateBuilder(isolate) - .SetValue("method", val->method()) - .SetValue("url", val->url().spec()) - .SetValue("referrer", val->referrer()) - .Build()->NewInstance(); - } else { - return v8::Null(isolate); - } + const net::URLRequest* val) { + return mate::ObjectTemplateBuilder(isolate) + .SetValue("method", val->method()) + .SetValue("url", val->url().spec()) + .SetValue("referrer", val->referrer()) + .Build()->NewInstance(); } }; @@ -77,8 +73,7 @@ class CustomProtocolRequestJob : public AdapterRequestJob { // Call the JS handler. Protocol::JsProtocolHandler callback = registry_->GetProtocolHandler(request()->url().scheme()); - v8::Local result = - callback.Run(v8::Null(registry_->isolate()), request()); + v8::Local result = callback.Run(request()); // Determine the type of the job we are going to create. if (result->IsString()) { @@ -208,81 +203,88 @@ Protocol::JsProtocolHandler Protocol::GetProtocolHandler( } void Protocol::OnRegisterProtocol(const std::string& scheme, - const JsProtocolHandler& callback, - bool is_handled) { + 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 || ContainsKey(protocol_handlers_, scheme)) { callback.Run(v8::Exception::Error( - mate::StringToV8(isolate(), "The Scheme is already registered")), - nullptr); + mate::StringToV8(isolate(), "The Scheme is already registered"))); return; } - protocol_handlers_[scheme] = callback; - BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, - base::Bind(&Protocol::RegisterProtocolInIO, - base::Unretained(this), scheme)); + protocol_handlers_[scheme] = handler; + 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& callback, - bool is_handled) { + 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.")), nullptr); + 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.")), - nullptr); + mate::StringToV8(isolate(), "Cannot intercept custom protocols."))); return; } - protocol_handlers_[scheme] = callback; - BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, - base::Bind(&Protocol::InterceptProtocolInIO, - base::Unretained(this), scheme)); + 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("_registerProtocol", &Protocol::RegisterProtocol) + .SetMethod("_unregisterProtocol", &Protocol::UnregisterProtocol) .SetMethod("registerStandardSchemes", &Protocol::RegisterStandardSchemes) .SetMethod("isHandledProtocol", &Protocol::IsHandledProtocol) - .SetMethod("interceptProtocol", &Protocol::InterceptProtocol) - .SetMethod("uninterceptProtocol", &Protocol::UninterceptProtocol); + .SetMethod("_interceptProtocol", &Protocol::InterceptProtocol) + .SetMethod("_uninterceptProtocol", &Protocol::UninterceptProtocol); } void Protocol::RegisterProtocol(v8::Isolate* isolate, const std::string& scheme, - const JsProtocolHandler& callback) { + const JsProtocolHandler& handler, + const JsCompletionCallback& callback) { IsHandledProtocol(scheme, base::Bind(&Protocol::OnRegisterProtocol, base::Unretained(this), - scheme, callback)); + scheme, handler, callback)); } void Protocol::UnregisterProtocol(v8::Isolate* isolate, - const std::string& scheme) { + const std::string& scheme, + const JsCompletionCallback& callback) { ProtocolHandlersMap::iterator it(protocol_handlers_.find(scheme)); - if (it == protocol_handlers_.end()) - return node::ThrowError(isolate, "The scheme has not been registered"); + 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::PostTask(BrowserThread::IO, - FROM_HERE, - base::Bind(&Protocol::UnregisterProtocolInIO, - base::Unretained(this), scheme)); + BrowserThread::PostTaskAndReply(BrowserThread::IO, FROM_HERE, + base::Bind(&Protocol::UnregisterProtocolInIO, + base::Unretained(this), scheme), + base::Bind(callback, v8::Null(isolate))); } void Protocol::RegisterStandardSchemes( @@ -291,7 +293,7 @@ void Protocol::RegisterStandardSchemes( } void Protocol::IsHandledProtocol(const std::string& scheme, - const CompletionCallback& callback) { + const net::CompletionCallback& callback) { BrowserThread::PostTaskAndReplyWithResult(BrowserThread::IO, FROM_HERE, base::Bind(&AtomURLRequestJobFactory::IsHandledProtocol, base::Unretained(job_factory_), scheme), @@ -300,68 +302,54 @@ void Protocol::IsHandledProtocol(const std::string& scheme, void Protocol::InterceptProtocol(v8::Isolate* isolate, const std::string& scheme, - const JsProtocolHandler& callback) { + const JsProtocolHandler& handler, + const JsCompletionCallback& callback) { BrowserThread::PostTaskAndReplyWithResult(BrowserThread::IO, FROM_HERE, base::Bind(&AtomURLRequestJobFactory::HasProtocolHandler, base::Unretained(job_factory_), scheme), base::Bind(&Protocol::OnInterceptProtocol, - base::Unretained(this), scheme, callback)); + base::Unretained(this), scheme, handler, callback)); } void Protocol::UninterceptProtocol(v8::Isolate* isolate, - const std::string& scheme) { + const std::string& scheme, + const JsCompletionCallback& callback) { ProtocolHandlersMap::iterator it(protocol_handlers_.find(scheme)); - if (it == protocol_handlers_.end()) - return node::ThrowError(isolate, "The scheme has not been registered"); + 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::PostTask(BrowserThread::IO, - FROM_HERE, - base::Bind(&Protocol::UninterceptProtocolInIO, - base::Unretained(this), scheme)); + 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)); - BrowserThread::PostTask(BrowserThread::UI, - FROM_HERE, - base::Bind(&Protocol::EmitEventInUI, - base::Unretained(this), - "registered", scheme)); } void Protocol::UnregisterProtocolInIO(const std::string& scheme) { DCHECK_CURRENTLY_ON(BrowserThread::IO); job_factory_->SetProtocolHandler(scheme, NULL); - BrowserThread::PostTask(BrowserThread::UI, - FROM_HERE, - base::Bind(&Protocol::EmitEventInUI, - base::Unretained(this), - "unregistered", scheme)); } void Protocol::InterceptProtocolInIO(const std::string& scheme) { DCHECK_CURRENTLY_ON(BrowserThread::IO); ProtocolHandler* original_handler = job_factory_->GetProtocolHandler(scheme); - if (original_handler == NULL) { - BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, base::Bind( - &Protocol::EmitEventInUI, - base::Unretained(this), - "error", "There is no protocol handler to intercpet")); - return; + if (original_handler == nullptr) { + NOTREACHED(); } job_factory_->ReplaceProtocol( scheme, new CustomProtocolHandler(this, original_handler)); - BrowserThread::PostTask(BrowserThread::UI, - FROM_HERE, - base::Bind(&Protocol::EmitEventInUI, - base::Unretained(this), - "intercepted", scheme)); } void Protocol::UninterceptProtocolInIO(const std::string& scheme) { @@ -369,28 +357,14 @@ void Protocol::UninterceptProtocolInIO(const std::string& scheme) { CustomProtocolHandler* handler = static_cast( job_factory_->GetProtocolHandler(scheme)); - if (handler->original_handler() == NULL) { - BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, base::Bind( - &Protocol::EmitEventInUI, - base::Unretained(this), - "error", "The protocol is not intercpeted")); - return; + if (handler->original_handler() == nullptr) { + NOTREACHED(); } // 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); - BrowserThread::PostTask(BrowserThread::UI, - FROM_HERE, - base::Bind(&Protocol::EmitEventInUI, - base::Unretained(this), - "unintercepted", scheme)); -} - -void Protocol::EmitEventInUI(const std::string& event, - const std::string& parameter) { - Emit(event, parameter); } // static diff --git a/atom/browser/api/atom_api_protocol.h b/atom/browser/api/atom_api_protocol.h index 147b9e1912a9..148620a0fef7 100644 --- a/atom/browser/api/atom_api_protocol.h +++ b/atom/browser/api/atom_api_protocol.h @@ -12,6 +12,7 @@ #include "atom/browser/api/event_emitter.h" #include "base/callback.h" #include "native_mate/handle.h" +#include "net/base/completion_callback.h" namespace net { class URLRequest; @@ -27,9 +28,8 @@ namespace api { class Protocol : public mate::EventEmitter { public: using JsProtocolHandler = - base::Callback(v8::Local, - const net::URLRequest*)>; - using CompletionCallback = base::Callback; + base::Callback(const net::URLRequest*)>; + using JsCompletionCallback = base::Callback)>; static mate::Handle Create( v8::Isolate* isolate, AtomBrowserContext* browser_context); @@ -50,12 +50,14 @@ class Protocol : public mate::EventEmitter { // Callback called if protocol can be registered. void OnRegisterProtocol(const std::string& scheme, - const JsProtocolHandler& callback, - bool is_handled); + const JsProtocolHandler& handler, + const JsCompletionCallback& callback, + int is_handled); // Callback called if protocol can be intercepted. void OnInterceptProtocol(const std::string& scheme, - const JsProtocolHandler& callback, - bool is_handled); + const JsProtocolHandler& handler, + const JsCompletionCallback& callback, + int is_handled); // Register schemes to standard scheme list. void RegisterStandardSchemes(const std::vector& schemes); @@ -64,18 +66,22 @@ class Protocol : public mate::EventEmitter { // |callback|. void RegisterProtocol(v8::Isolate* isolate, const std::string& scheme, - const JsProtocolHandler& callback); - void UnregisterProtocol(v8::Isolate* isolate, const std::string& scheme); + const JsProtocolHandler& handler, + const JsCompletionCallback& callback); + 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 CompletionCallback& callback); + const net::CompletionCallback& callback); // Intercept/unintercept an existing protocol handler. void InterceptProtocol(v8::Isolate* isolate, const std::string& scheme, - const JsProtocolHandler& callback); - void UninterceptProtocol(v8::Isolate* isolate, const std::string& scheme); + const JsProtocolHandler& handler, + const JsCompletionCallback& callback); + void UninterceptProtocol(v8::Isolate* isolate, const std::string& scheme, + const JsCompletionCallback& callback); // The networking related operations have to be done in IO thread. void RegisterProtocolInIO(const std::string& scheme); @@ -83,9 +89,6 @@ class Protocol : public mate::EventEmitter { void InterceptProtocolInIO(const std::string& scheme); void UninterceptProtocolInIO(const std::string& scheme); - // Do protocol.emit(event, parameter) under UI thread. - void EmitEventInUI(const std::string& event, const std::string& parameter); - AtomBrowserContext* browser_context_; AtomURLRequestJobFactory* job_factory_; ProtocolHandlersMap protocol_handlers_; diff --git a/atom/browser/api/lib/protocol.coffee b/atom/browser/api/lib/protocol.coffee index ff4bc5ba9a66..6e22329318b4 100644 --- a/atom/browser/api/lib/protocol.coffee +++ b/atom/browser/api/lib/protocol.coffee @@ -6,6 +6,29 @@ EventEmitter = require('events').EventEmitter protocol.__proto__ = EventEmitter.prototype +GetWrappedCallback = (scheme, callback, notification) -> + wrappedCallback = (error) -> + if not callback? + if error + throw error + else + protocol.emit notification, scheme + else + callback error, scheme + +# Compatibility with old api. +protocol.registerProtocol = (scheme, handler, callback) -> + protocol._registerProtocol scheme, handler, GetWrappedCallback(scheme, callback, 'registered') + +protocol.unregisterProtocol = (scheme, callback) -> + protocol._unregisterProtocol scheme, GetWrappedCallback(scheme, callback, 'unregistered') + +protocol.interceptProtocol = (scheme, handler, callback) -> + protocol._interceptProtocol scheme, handler, GetWrappedCallback(scheme, callback, 'intercepted') + +protocol.uninterceptProtocol = (scheme, callback) -> + protocol._uninterceptProtocol scheme, GetWrappedCallback(scheme, callback, 'unintercepted') + protocol.RequestStringJob = class RequestStringJob constructor: ({mimeType, charset, data}) -> diff --git a/docs/api/protocol.md b/docs/api/protocol.md index dbcdb577ca9c..a076e3640daa 100644 --- a/docs/api/protocol.md +++ b/docs/api/protocol.md @@ -15,6 +15,9 @@ app.on('ready', function() { protocol.registerProtocol('atom', function(request) { var url = request.url.substr(7) return new protocol.RequestFileJob(path.normalize(__dirname + '/' + url)); + }, function (error, scheme) { + if (!error) + console.log(scheme, ' registered successfully') }); }); ``` @@ -22,20 +25,22 @@ app.on('ready', function() { **Note:** This module can only be used after the `ready` event was emitted. -## protocol.registerProtocol(scheme, handler) +## protocol.registerProtocol(scheme, handler, callback) * `scheme` String * `handler` Function +* `callback` Function Registers a custom protocol of `scheme`, the `handler` would be called with -`handler(error, request)` when the a request with registered `scheme` is made. +`handler(request)` when the a request with registered `scheme` is made. You need to return a request job in the `handler` to specify which type of response you would like to send. -## protocol.unregisterProtocol(scheme) +## protocol.unregisterProtocol(scheme, callback) * `scheme` String +* `callback` Function Unregisters the custom protocol of `scheme`. @@ -52,17 +57,19 @@ Unregisters the custom protocol of `scheme`. `callback` returns a boolean whether the `scheme` can be handled already. -## protocol.interceptProtocol(scheme, handler) +## protocol.interceptProtocol(scheme, handler, callback) * `scheme` String * `handler` Function +* `callback` Function Intercepts an existing protocol with `scheme`, returning `null` or `undefined` in `handler` would use the original protocol handler to handle the request. -## protocol.uninterceptProtocol(scheme) +## protocol.uninterceptProtocol(scheme, callback) * `scheme` String +* `callback` Function Unintercepts a protocol. diff --git a/spec/api-protocol-spec.coffee b/spec/api-protocol-spec.coffee index 4fb00b9a2d4f..3ddcdf62ca4f 100644 --- a/spec/api-protocol-spec.coffee +++ b/spec/api-protocol-spec.coffee @@ -9,17 +9,18 @@ describe 'protocol module', -> describe 'protocol.registerProtocol', -> it 'error when scheme is already registered', (done) -> register = -> - protocol.registerProtocol 'test1', (error, request) -> - assert error instanceof Error - protocol.unregisterProtocol 'test1' - done() - protocol.once 'registered', (event, scheme) -> - assert.equal scheme, 'test1' - register() + protocol.registerProtocol 'test1', ((request) ->), (error, scheme) -> + if error? + protocol.unregisterProtocol 'test1', (error, scheme) -> + assert.equal scheme, 'test1' + done() + else + assert.equal scheme, 'test1' + register() register() it 'calls the callback when scheme is visited', (done) -> - protocol.registerProtocol 'test2', (error, request) -> + protocol.registerProtocol 'test2', (request) -> assert.equal request.url, 'test2://test2' protocol.unregisterProtocol 'test2' done() @@ -28,7 +29,7 @@ describe 'protocol module', -> describe 'protocol.unregisterProtocol', -> it 'throws error when scheme does not exist', -> unregister = -> protocol.unregisterProtocol 'test3' - assert.throws unregister, /The scheme has not been registered/ + assert.throws unregister, /The Scheme has not been registered/ describe 'registered protocol callback', -> it 'returns string should send the string as request content', (done) -> @@ -190,37 +191,39 @@ describe 'protocol module', -> describe 'protocol.interceptProtocol', -> it 'throws error when scheme is not a registered one', (done) -> - protocol.interceptProtocol 'test-intercept', (error, request) -> - assert error instanceof Error - done() + protocol.interceptProtocol 'test-intercept', ( ->), (error, scheme) -> + if error? + assert.equal scheme, 'test-intercept' + done() it 'throws error when scheme is a custom protocol', (done) -> - protocol.once 'unregistered', (event, scheme) -> + protocol.once 'unregistered', (scheme) -> assert.equal scheme, 'atom' done() - protocol.once 'registered', (event, scheme) -> + protocol.once 'registered', (scheme) -> assert.equal scheme, 'atom' - protocol.interceptProtocol 'test-intercept', (error, request) -> - assert error instanceof Error - protocol.unregisterProtocol scheme + protocol.interceptProtocol 'test-intercept', (->), (error, newScheme) -> + if error? + assert.equal newScheme, 'test-intercept' + protocol.unregisterProtocol scheme protocol.registerProtocol('atom', ->) it 'returns original job when callback returns nothing', (done) -> targetScheme = 'file' - protocol.once 'intercepted', (event, scheme) -> + protocol.once 'intercepted', (scheme) -> assert.equal scheme, targetScheme free = -> protocol.uninterceptProtocol targetScheme $.ajax url: "#{targetScheme}://#{__filename}", success: -> - protocol.once 'unintercepted', (event, scheme) -> + protocol.once 'unintercepted', (scheme) -> assert.equal scheme, targetScheme done() free() error: (xhr, errorType, error) -> free() assert false, 'Got error: ' + errorType + ' ' + error - protocol.interceptProtocol targetScheme, (error, request) -> + protocol.interceptProtocol targetScheme, (request) -> if process.platform is 'win32' pathInUrl = path.normalize request.url.substr(8) assert.equal pathInUrl.toLowerCase(), __filename.toLowerCase() From c56b3425a919254b4821dfbce1282bcb66d60551 Mon Sep 17 00:00:00 2001 From: Robo Date: Thu, 9 Jul 2015 14:48:45 +0530 Subject: [PATCH 3/3] read/write protocol handler map in IO --- atom/browser/api/atom_api_protocol.cc | 194 ++++++++++++++------------ atom/browser/api/atom_api_protocol.h | 41 +++--- spec/api-protocol-spec.coffee | 6 +- 3 files changed, 128 insertions(+), 113 deletions(-) diff --git a/atom/browser/api/atom_api_protocol.cc b/atom/browser/api/atom_api_protocol.cc index fa498a7fa99c..65ba3039d153 100644 --- a/atom/browser/api/atom_api_protocol.cc +++ b/atom/browser/api/atom_api_protocol.cc @@ -36,7 +36,6 @@ struct Converter { } // 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, - const JsProtocolHandler& handler, - const JsCompletionCallback& callback, - int is_handled) { +void Protocol::OnIOActionCompleted(const JsCompletionCallback& callback, + int error) { 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; - 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()))); + callback.Run(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& 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::Unretained(job_factory_), scheme), - base::Bind(&Protocol::OnInterceptProtocol, - base::Unretained(this), scheme, handler, callback)); + base::Bind(&Protocol::InterceptProtocolInIO, + base::Unretained(this), scheme, handler), + base::Bind(&Protocol::OnIOActionCompleted, + 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( - mate::StringToV8(isolate, "The Scheme has not been registered"))); - return; + return ERR_SCHEME_UNREGISTERED; } 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( 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 diff --git a/atom/browser/api/atom_api_protocol.h b/atom/browser/api/atom_api_protocol.h index 148620a0fef7..b4d56018baf3 100644 --- a/atom/browser/api/atom_api_protocol.h +++ b/atom/browser/api/atom_api_protocol.h @@ -31,6 +31,16 @@ class Protocol : public mate::EventEmitter { base::Callback(const net::URLRequest*)>; using JsCompletionCallback = base::Callback)>; + enum { + OK = 0, + ERR_SCHEME_REGISTERED, + ERR_SCHEME_UNREGISTERED, + ERR_SCHEME_INTERCEPTED, + ERR_SCHEME_UNINTERCEPTED, + ERR_NO_SCHEME, + ERR_SCHEME + }; + static mate::Handle Create( v8::Isolate* isolate, AtomBrowserContext* browser_context); @@ -48,20 +58,17 @@ class Protocol : public mate::EventEmitter { private: typedef std::map ProtocolHandlersMap; - // Callback called if protocol can be registered. - void OnRegisterProtocol(const std::string& scheme, - const JsProtocolHandler& handler, - 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); + // Callback called after performing action on IO thread. + void OnIOActionCompleted(const JsCompletionCallback& callback, + int error); // Register schemes to standard scheme list. void RegisterStandardSchemes(const std::vector& 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); - void UnregisterProtocolInIO(const std::string& scheme); - void InterceptProtocolInIO(const std::string& scheme); - void UninterceptProtocolInIO(const std::string& scheme); + int RegisterProtocolInIO(const std::string& scheme, + const JsProtocolHandler& handler); + int UnregisterProtocolInIO(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_; diff --git a/spec/api-protocol-spec.coffee b/spec/api-protocol-spec.coffee index 3ddcdf62ca4f..b65002a63189 100644 --- a/spec/api-protocol-spec.coffee +++ b/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' - assert.throws unregister, /The Scheme has not been registered/ + protocol.unregisterProtocol 'test3', (->), (error, scheme) -> + if (error) + assert.equal scheme, 'test3' + done() describe 'registered protocol callback', -> it 'returns string should send the string as request content', (done) ->