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()