allow protocol module initialization before app ready.

 * ensure registerStandardSchemes can only be called before app ready
 * ensure other protocol methods can only be used after app ready

diff --git a/atom/browser/api/atom_api_protocol.cc b/atom/browser/api/atom_api_protocol.cc
index 115a3c5..341baf2 100644 (file)
--- a/atom/browser/api/atom_api_protocol.cc
+++ b/atom/browser/api/atom_api_protocol.cc
@@ -7,6 +7,7 @@
 #include "atom/browser/atom_browser_client.h"
 #include "atom/browser/atom_browser_context.h"
 #include "atom/browser/atom_browser_main_parts.h"
+#include "atom/browser/browser.h"
 #include "atom/browser/net/url_request_async_asar_job.h"
 #include "atom/browser/net/url_request_buffer_job.h"
 #include "atom/browser/net/url_request_fetch_job.h"
@@ -23,15 +24,38 @@ namespace atom {
 
 namespace api {
 
-Protocol::Protocol(v8::Isolate* isolate, AtomBrowserContext* browser_context)
-    : request_context_getter_(browser_context->GetRequestContext()),
-      job_factory_(browser_context->job_factory()) {
-  CHECK(job_factory_);
+Protocol::Protocol(v8::Isolate* isolate)
+    : request_context_getter_(nullptr),
+      job_factory_(nullptr) {
+  if (Browser::Get()->is_ready()) {
+    OnWillFinishLaunching();
+  } else {
+    Browser::Get()->AddObserver(this);
+  }
   Init(isolate);
 }
 
+Protocol::~Protocol() {
+  Browser::Get()->RemoveObserver(this);
+}
+
+void Protocol::OnWillFinishLaunching() {
+  auto browser_context = static_cast<atom::AtomBrowserContext*>(
+      atom::AtomBrowserMainParts::Get()->browser_context());
+  request_context_getter_ = browser_context->GetRequestContext();
+  job_factory_ = browser_context->job_factory();
+  CHECK(job_factory_);
+}
+
 void Protocol::RegisterStandardSchemes(
     const std::vector<std::string>& schemes) {
+  if (Browser::Get()->is_ready()) {
+    isolate()->ThrowException(v8::Exception::Error(mate::StringToV8(
+        isolate(),
+        "\"protocol.registerStandardSchemes\" should be called before"
+        "app is ready")));
+    return;
+  }
   for (const auto& scheme : schemes)
     url::AddStandardScheme(scheme.c_str(), url::SCHEME_WITHOUT_PORT);
 }
@@ -45,6 +69,10 @@ void Protocol::UnregisterProtocol(
     const std::string& scheme, mate::Arguments* args) {
   CompletionCallback callback;
   args->GetNext(&callback);
+  if (!job_factory_) {
+    OnIOCompleted(callback, PROTOCOL_FAIL);
+    return;
+  }
   content::BrowserThread::PostTaskAndReplyWithResult(
       content::BrowserThread::IO, FROM_HERE,
       base::Bind(&Protocol::UnregisterProtocolInIO,
@@ -63,6 +91,10 @@ Protocol::ProtocolError Protocol::UnregisterProtocolInIO(
 
 void Protocol::IsProtocolHandled(const std::string& scheme,
                                     const BooleanCallback& callback) {
+  if (!job_factory_) {
+    callback.Run(false);
+    return;
+  }
   content::BrowserThread::PostTaskAndReplyWithResult(
       content::BrowserThread::IO, FROM_HERE,
       base::Bind(&Protocol::IsProtocolHandledInIO,
@@ -78,6 +110,10 @@ void Protocol::UninterceptProtocol(
     const std::string& scheme, mate::Arguments* args) {
   CompletionCallback callback;
   args->GetNext(&callback);
+  if (!job_factory_) {
+    OnIOCompleted(callback, PROTOCOL_FAIL);
+    return;
+  }
   content::BrowserThread::PostTaskAndReplyWithResult(
       content::BrowserThread::IO, FROM_HERE,
       base::Bind(&Protocol::UninterceptProtocolInIO,
@@ -124,9 +160,8 @@ std::string Protocol::ErrorCodeToString(ProtocolError error) {
 }
 
 // static
-mate::Handle<Protocol> Protocol::Create(
-    v8::Isolate* isolate, AtomBrowserContext* browser_context) {
-  return mate::CreateHandle(isolate, new Protocol(isolate, browser_context));
+mate::Handle<Protocol> Protocol::Create(v8::Isolate* isolate) {
+  return mate::CreateHandle(isolate, new Protocol(isolate));
 }
 
 // static
@@ -167,9 +202,7 @@ void Initialize(v8::Local<v8::Object> exports, v8::Local<v8::Value> unused,
                 v8::Local<v8::Context> context, void* priv) {
   v8::Isolate* isolate = context->GetIsolate();
   mate::Dictionary dict(isolate, exports);
-  auto browser_context = static_cast<atom::AtomBrowserContext*>(
-      atom::AtomBrowserMainParts::Get()->browser_context());
-  dict.Set("protocol", atom::api::Protocol::Create(isolate, browser_context));
+  dict.Set("protocol", atom::api::Protocol::Create(isolate));
 }
 
 }  // namespace

diff --git a/atom/browser/api/atom_api_protocol.h b/atom/browser/api/atom_api_protocol.h
index d7fa7f5..80b7006 100644 (file)
--- a/atom/browser/api/atom_api_protocol.h
+++ b/atom/browser/api/atom_api_protocol.h
@@ -9,6 +9,7 @@
 #include <map>
 #include <vector>
 
+#include "atom/browser/browser_observer.h"
 #include "atom/browser/net/atom_url_request_job_factory.h"
 #include "base/callback.h"
 #include "base/containers/scoped_ptr_hash_map.h"
@@ -30,21 +31,25 @@ class AtomURLRequestJobFactory;
 
 namespace api {
 
-class Protocol : public mate::Wrappable<Protocol> {
+class Protocol : public mate::Wrappable<Protocol>,
+                 public BrowserObserver {
  public:
   using Handler =
       base::Callback<void(const net::URLRequest*, v8::Local<v8::Value>)>;
   using CompletionCallback = base::Callback<void(v8::Local<v8::Value>)>;
   using BooleanCallback = base::Callback<void(bool)>;
 
-  static mate::Handle<Protocol> Create(
-      v8::Isolate* isolate, AtomBrowserContext* browser_context);
+  static mate::Handle<Protocol> Create(v8::Isolate* isolate);
 
   static void BuildPrototype(v8::Isolate* isolate,
                              v8::Local<v8::ObjectTemplate> prototype);
 
  protected:
-  Protocol(v8::Isolate* isolate, AtomBrowserContext* browser_context);
+  explicit Protocol(v8::Isolate* isolate);
+  ~Protocol();
+
+  // BrowserObserver:
+  void OnWillFinishLaunching() override;
 
  private:
   // Possible errors.
@@ -101,6 +106,10 @@ class Protocol : public mate::Wrappable<Protocol> {
                         mate::Arguments* args) {
     CompletionCallback callback;
     args->GetNext(&callback);
+    if (!job_factory_) {
+      OnIOCompleted(callback, PROTOCOL_FAIL);
+      return;
+    }
     content::BrowserThread::PostTaskAndReplyWithResult(
         content::BrowserThread::IO, FROM_HERE,
         base::Bind(&Protocol::RegisterProtocolInIO<RequestJob>,
@@ -138,6 +147,10 @@ class Protocol : public mate::Wrappable<Protocol> {
                          mate::Arguments* args) {
     CompletionCallback callback;
     args->GetNext(&callback);
+    if (!job_factory_) {
+      OnIOCompleted(callback, PROTOCOL_FAIL);
+      return;
+    }
     content::BrowserThread::PostTaskAndReplyWithResult(
         content::BrowserThread::IO, FROM_HERE,
         base::Bind(&Protocol::InterceptProtocolInIO<RequestJob>,

diff --git a/lib/browser/api/protocol.js b/lib/browser/api/protocol.js
index dac679f..3c7bed5 100644 (file)
--- a/lib/browser/api/protocol.js
+++ b/lib/browser/api/protocol.js
@@ -1,9 +1,3 @@
-const app = require('electron').app
-
-if (!app.isReady()) {
-  throw new Error('Can not initialize protocol module before app is ready')
-}
-
 const protocol = process.atomBinding('protocol').protocol
 
 // Warn about removed APIs.

diff --git a/spec/api-protocol-spec.js b/spec/api-protocol-spec.js
index c84d5be..2a1d158 100644 (file)
--- a/spec/api-protocol-spec.js
+++ b/spec/api-protocol-spec.js
@@ -818,7 +818,7 @@ describe('protocol module', function () {
   })
 
   describe('protocol.registerStandardSchemes', function () {
-    const standardScheme = 'app'
+    const standardScheme = remote.getGlobal('standardScheme')
     const origin = standardScheme + '://fake-host'
     const imageURL = origin + '/test.png'
     const filePath = path.join(__dirname, 'fixtures', 'pages', 'b.html')
@@ -826,10 +826,6 @@ describe('protocol module', function () {
     var w = null
     var success = null
 
-    before(function () {
-      protocol.registerStandardSchemes([standardScheme])
-    })
-
     beforeEach(function () {
       w = new BrowserWindow({show: false})
       success = false

diff --git a/spec/static/main.js b/spec/static/main.js
index 025ff39..33c3bb4 100644 (file)
--- a/spec/static/main.js
+++ b/spec/static/main.js
@@ -6,6 +6,7 @@ const app = electron.app
 const ipcMain = electron.ipcMain
 const dialog = electron.dialog
 const BrowserWindow = electron.BrowserWindow
+const protocol = electron.protocol
 
 const fs = require('fs')
 const path = require('path')
@@ -71,6 +72,10 @@ if (global.isCi) {
   })
 }
 
+// Register app as standard scheme.
+global.standardScheme = 'app'
+protocol.registerStandardSchemes([global.standardScheme])
+
 app.on('window-all-closed', function () {
   app.quit()
 })