From 92e157de3036e801e11f1e4b8a2faab5c48a55b4 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 20 Sep 2013 18:32:05 +0800 Subject: [PATCH 1/3] Fix crash when using protocol module on startup. The job factory was not created before any request was sent, so when the app used the protocol module on startup it would cause a crash. --- browser/atom_browser_main_parts.cc | 5 +++++ spec/main.js | 3 +++ 2 files changed, 8 insertions(+) diff --git a/browser/atom_browser_main_parts.cc b/browser/atom_browser_main_parts.cc index 2ee9c08e4e62..aaec24bea750 100644 --- a/browser/atom_browser_main_parts.cc +++ b/browser/atom_browser_main_parts.cc @@ -75,6 +75,11 @@ void AtomBrowserMainParts::PreMainMessageLoopRun() { node_bindings_->RunMessageLoop(); + // Make sure the url request job factory is created before the + // will-finish-launching event. + static_cast(AtomBrowserContext::Get())-> + GetRequestContext(); + #if !defined(OS_MACOSX) // The corresponding call in OS X is in AtomApplicationDelegate. Browser::Get()->WillFinishLaunching(); diff --git a/spec/main.js b/spec/main.js index 0d021157151e..f2e52627946a 100644 --- a/spec/main.js +++ b/spec/main.js @@ -35,6 +35,9 @@ app.on('window-all-closed', function() { }); app.on('finish-launching', function() { + // Test if using protocol module would crash. + require('protocol').registerProtocol('test-if-crashes', function() {}); + window = new BrowserWindow({ title: 'atom-shell tests', show: false, From e65220adb07dbdb7fb9539c1910d54aa12c59257 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 20 Sep 2013 18:36:16 +0800 Subject: [PATCH 2/3] doc: Mention when protocol module is safe to use. --- docs/api/browser/protocol.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/api/browser/protocol.md b/docs/api/browser/protocol.md index ec6c1f2f0556..7e566ba0ecbb 100644 --- a/docs/api/browser/protocol.md +++ b/docs/api/browser/protocol.md @@ -14,6 +14,9 @@ protocol.registerProtocol('atom', function(request) { }); ``` +**Note:** This module can only be used after the `will-finish-launching` event +was emitted. + ## protocol.registerProtocol(scheme, handler) * `scheme` String From 2be1145a9e3d50c65095b55dd6a7c5c766e19b03 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 20 Sep 2013 18:45:53 +0800 Subject: [PATCH 3/3] Guard against using protocol module too early. --- browser/api/atom_api_protocol.cc | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/browser/api/atom_api_protocol.cc b/browser/api/atom_api_protocol.cc index 5b1919fa6759..6f0af3dfeb75 100644 --- a/browser/api/atom_api_protocol.cc +++ b/browser/api/atom_api_protocol.cc @@ -29,6 +29,9 @@ v8::Persistent g_protocol_object; typedef std::map> HandlersMap; static HandlersMap g_handlers; +static const char* kEarlyUseProtocolError = "This method can only be used" + "after the application has finished launching."; + // Emit an event for the protocol module. void EmitEventInUI(const std::string& event, const std::string& parameter) { v8::HandleScope scope; @@ -185,6 +188,9 @@ v8::Handle Protocol::RegisterProtocol(const v8::Arguments& args) { net::URLRequest::IsHandledProtocol(scheme)) return node::ThrowError("The scheme is already registered"); + if (AtomBrowserContext::Get()->url_request_context_getter() == NULL) + return node::ThrowError(kEarlyUseProtocolError); + // Store the handler in a map. if (!args[1]->IsFunction()) return node::ThrowError("Handler must be a function"); @@ -202,6 +208,9 @@ v8::Handle Protocol::RegisterProtocol(const v8::Arguments& args) { v8::Handle Protocol::UnregisterProtocol(const v8::Arguments& args) { std::string scheme(*v8::String::Utf8Value(args[0])); + if (AtomBrowserContext::Get()->url_request_context_getter() == NULL) + return node::ThrowError(kEarlyUseProtocolError); + // Erase the handler from map. HandlersMap::iterator it(g_handlers.find(scheme)); if (it == g_handlers.end()) @@ -230,6 +239,9 @@ v8::Handle Protocol::InterceptProtocol(const v8::Arguments& args) { if (ContainsKey(g_handlers, scheme)) return node::ThrowError("Cannot intercept custom procotols"); + if (AtomBrowserContext::Get()->url_request_context_getter() == NULL) + return node::ThrowError(kEarlyUseProtocolError); + // Store the handler in a map. if (!args[1]->IsFunction()) return node::ThrowError("Handler must be a function"); @@ -246,6 +258,9 @@ v8::Handle Protocol::InterceptProtocol(const v8::Arguments& args) { v8::Handle Protocol::UninterceptProtocol(const v8::Arguments& args) { std::string scheme(*v8::String::Utf8Value(args[0])); + if (AtomBrowserContext::Get()->url_request_context_getter() == NULL) + return node::ThrowError(kEarlyUseProtocolError); + // Erase the handler from map. HandlersMap::iterator it(g_handlers.find(scheme)); if (it == g_handlers.end())