Merge pull request #5406 from deepak1556/protocol_standard_scheme_patch

protocol: fix registerStandardSchemes api

atom/app/atom_content_client.cc

@@ -183,13 +183,6 @@ base::string16 AtomContentClient::GetLocalizedString(int message_id) const {
 void AtomContentClient::AddAdditionalSchemes(
     std::vector<url::SchemeWithType>* standard_schemes,
     std::vector<std::string>* savable_schemes) {
-  std::vector<std::string> schemes;
-  ConvertStringWithSeparatorToVector(&schemes, ",",
-                                     switches::kRegisterStandardSchemes);
-  if (!schemes.empty()) {
-    for (const std::string& scheme : schemes)
-      standard_schemes->push_back({scheme.c_str(), url::SCHEME_WITHOUT_PORT});
-  }
   standard_schemes->push_back({"chrome-extension", url::SCHEME_WITHOUT_PORT});
 }
 

atom/browser/api/atom_api_protocol.cc

@@ -15,6 +15,7 @@
 #include "atom/common/native_mate_converters/net_converter.h"
 #include "atom/common/node_includes.h"
 #include "native_mate/dictionary.h"
+#include "url/url_util.h"
 
 using content::BrowserThread;
 
@@ -29,11 +30,6 @@ Protocol::Protocol(v8::Isolate* isolate, AtomBrowserContext* browser_context)
   Init(isolate);
 }
 
-void Protocol::RegisterStandardSchemes(
-    const std::vector<std::string>& schemes) {
-  atom::AtomBrowserClient::SetCustomSchemes(schemes);
-}
-
 void Protocol::RegisterServiceWorkerSchemes(
     const std::vector<std::string>& schemes) {
   atom::AtomBrowserClient::SetCustomServiceWorkerSchemes(schemes);
@@ -131,7 +127,6 @@ mate::Handle<Protocol> Protocol::Create(
 void Protocol::BuildPrototype(
     v8::Isolate* isolate, v8::Local<v8::ObjectTemplate> prototype) {
   mate::ObjectTemplateBuilder(isolate, prototype)
-      .SetMethod("registerStandardSchemes", &Protocol::RegisterStandardSchemes)
       .SetMethod("registerServiceWorkerSchemes",
                  &Protocol::RegisterServiceWorkerSchemes)
       .SetMethod("registerStringProtocol",
@@ -161,13 +156,24 @@ void Protocol::BuildPrototype(
 
 namespace {
 
+void RegisterStandardSchemes(
+    const std::vector<std::string>& schemes) {
+  for (const auto& scheme : schemes)
+    url::AddStandardScheme(scheme.c_str(), url::SCHEME_WITHOUT_PORT);
+}
+
+mate::Handle<atom::api::Protocol> CreateProtocol(v8::Isolate* isolate) {
+  auto browser_context = static_cast<atom::AtomBrowserContext*>(
+      atom::AtomBrowserMainParts::Get()->browser_context());
+  return atom::api::Protocol::Create(isolate, browser_context);
+}
+
 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.SetMethod("createProtocolObject", base::Bind(&CreateProtocol, isolate));
+  dict.SetMethod("registerStandardSchemes", &RegisterStandardSchemes);
 }
 
 }  // namespace

atom/browser/api/atom_api_protocol.h

@@ -88,9 +88,6 @@ class Protocol : public mate::Wrappable<Protocol> {
     DISALLOW_COPY_AND_ASSIGN(CustomProtocolHandler);
   };
 
-  // Register schemes to standard scheme list.
-  void RegisterStandardSchemes(const std::vector<std::string>& schemes);
-
   // Register schemes that can handle service worker.
   void RegisterServiceWorkerSchemes(const std::vector<std::string>& schemes);
 

atom/browser/atom_browser_client.cc

@@ -49,8 +49,6 @@ namespace {
 // Next navigation should not restart renderer process.
 bool g_suppress_renderer_process_restart = false;
 
-// Custom schemes to be registered to standard.
-std::string g_custom_schemes = "";
 // Custom schemes to be registered to handle service worker.
 std::string g_custom_service_worker_schemes = "";
 
@@ -61,11 +59,6 @@ void AtomBrowserClient::SuppressRendererProcessRestartForOnce() {
   g_suppress_renderer_process_restart = true;
 }
 
-void AtomBrowserClient::SetCustomSchemes(
-    const std::vector<std::string>& schemes) {
-  g_custom_schemes = base::JoinString(schemes, ",");
-}
-
 void AtomBrowserClient::SetCustomServiceWorkerSchemes(
     const std::vector<std::string>& schemes) {
   g_custom_service_worker_schemes = base::JoinString(schemes, ",");
@@ -153,11 +146,6 @@ void AtomBrowserClient::AppendExtraCommandLineSwitches(
   if (process_type != "renderer")
     return;
 
-  // The registered standard schemes.
-  if (!g_custom_schemes.empty())
-    command_line->AppendSwitchASCII(switches::kRegisterStandardSchemes,
-                                    g_custom_schemes);
-
   // The registered service worker schemes.
   if (!g_custom_service_worker_schemes.empty())
     command_line->AppendSwitchASCII(switches::kRegisterServiceWorkerSchemes,

atom/browser/atom_browser_client.h

@@ -36,8 +36,7 @@ class AtomBrowserClient : public brightray::BrowserClient,
 
   // Don't force renderer process to restart for once.
   static void SuppressRendererProcessRestartForOnce();
-  // Custom schemes to be registered to standard.
-  static void SetCustomSchemes(const std::vector<std::string>& schemes);
+
   // Custom schemes to be registered to handle service worker.
   static void SetCustomServiceWorkerSchemes(
       const std::vector<std::string>& schemes);

atom/common/options_switches.cc

@@ -125,9 +125,6 @@ const char kPpapiFlashVersion[] = "ppapi-flash-version";
 // Disable HTTP cache.
 const char kDisableHttpCache[] = "disable-http-cache";
 
-// Register schemes to standard.
-const char kRegisterStandardSchemes[] = "register-standard-schemes";
-
 // Register schemes to handle service worker.
 const char kRegisterServiceWorkerSchemes[] = "register-service-worker-schemes";
 

atom/common/options_switches.h

@@ -70,7 +70,6 @@ extern const char kEnablePlugins[];
 extern const char kPpapiFlashPath[];
 extern const char kPpapiFlashVersion[];
 extern const char kDisableHttpCache[];
-extern const char kRegisterStandardSchemes[];
 extern const char kRegisterServiceWorkerSchemes[];
 extern const char kSSLVersionFallbackMin[];
 extern const char kCipherSuiteBlacklist[];

docs/api/protocol.md

@@ -7,11 +7,10 @@ An example of implementing a protocol that has the same effect as the
 
 ```javascript
 const electron = require('electron');
-const { app } = electron;
+const { app, protocol } = electron;
 const path = require('path');
 
 app.on('ready', function() {
-    const { protocol } = electron;
     protocol.registerFileProtocol('atom', function(request, callback) {
       const url = request.url.substr(7);
       callback({path: path.normalize(__dirname + '/' + url)});
@@ -21,9 +20,8 @@ app.on('ready', function() {
     });
 });
 ```
-
-**Note:** This module can only be used after the `ready` event in the `app`
-module is emitted.
+**Note:** All methods unless specified can only be used after the `ready`
+event in the `app` module is emitted.
 
 ## Methods
 
@@ -35,7 +33,11 @@ The `protocol` module has the following methods:
 
 A standard `scheme` adheres to what RFC 3986 calls
 [generic URI syntax](https://tools.ietf.org/html/rfc3986#section-3). This
-includes `file:` and `filesystem:`.
+includes `file:`, `filesystem:`, `http` etc. Registering a scheme as standard, will
+allow relative and absolute resources to be resolved correctly when served.
+
+**Note:** This method can only be used before the `ready` event in the
+`app` module is emitted.
 
 ### `protocol.registerServiceWorkerSchemes(schemes)`
 

lib/browser/api/protocol.js

@@ -1,10 +1,6 @@
 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
+const {createProtocolObject, registerStandardSchemes} = process.atomBinding('protocol')
+let protocol = null
 
 // Warn about removed APIs.
 var logAndThrow = function (callback, message) {
@@ -16,6 +12,16 @@ var logAndThrow = function (callback, message) {
   }
 }
 
+exports.registerStandardSchemes = function (schemes) {
+  if (app.isReady()) {
+    throw new Error('protocol.registerStandardSchemes should be called before app is ready')
+  }
+  registerStandardSchemes(schemes)
+}
+
+app.once('ready', function () {
+  protocol = createProtocolObject()
+  // Be compatible with old APIs.
   protocol.registerProtocol = function (scheme, handler, callback) {
     return logAndThrow(callback, 'registerProtocol API has been replaced by the register[File/Http/Buffer/String]Protocol API family, please switch to the new APIs.')
   }
@@ -28,4 +34,7 @@ protocol.interceptProtocol = function (scheme, handler, callback) {
     return logAndThrow(callback, 'interceptProtocol API has been replaced by the intercept[File/Http/Buffer/String]Protocol API family, please switch to the new APIs.')
   }
 
-module.exports = protocol
+  for (let method in protocol) {
+    exports[method] = protocol[method].bind(protocol)
+  }
+})

lib/browser/chrome-extension.js

@@ -1,5 +1,4 @@
-const electron = require('electron')
-const app = electron.app
+const {app, protocol, BrowserWindow} = require('electron')
 const fs = require('fs')
 const path = require('path')
 const url = require('url')
@@ -70,9 +69,7 @@ app.on('will-quit', function () {
 
 // We can not use protocol or BrowserWindow until app is ready.
 app.once('ready', function () {
-  var BrowserWindow, chromeExtensionHandler, i, init, len, protocol, srcDirectory
-  protocol = electron.protocol
-  BrowserWindow = electron.BrowserWindow
+  var chromeExtensionHandler, i, init, len, srcDirectory
 
   // Load persisted extensions.
   loadedExtensionsPath = path.join(app.getPath('userData'), 'DevTools Extensions')

spec/api-protocol-spec.js

@@ -3,6 +3,7 @@ const http = require('http')
 const path = require('path')
 const qs = require('querystring')
 const remote = require('electron').remote
+const BrowserWindow = remote.require('electron').BrowserWindow
 const protocol = remote.require('electron').protocol
 
 describe('protocol module', function () {
@@ -344,6 +345,7 @@ describe('protocol module', function () {
         })
       })
     })
+
     it('sends object as response', function (done) {
       var handler = function (request, callback) {
         callback({
@@ -394,7 +396,7 @@ describe('protocol module', function () {
       var handler = function (request, callback) {
         callback(fakeFilePath)
       }
-      protocol.registerBufferProtocol(protocolName, handler, function (error) {
+      protocol.registerFileProtocol(protocolName, handler, function (error) {
         if (error) {
           return done(error)
         }
@@ -415,7 +417,7 @@ describe('protocol module', function () {
       var handler = function (request, callback) {
         callback(new Date())
       }
-      protocol.registerBufferProtocol(protocolName, handler, function (error) {
+      protocol.registerFileProtocol(protocolName, handler, function (error) {
         if (error) {
           return done(error)
         }
@@ -814,4 +816,80 @@ describe('protocol module', function () {
       })
     })
   })
+
+  describe('protocol.registerStandardSchemes', function () {
+    const standardScheme = remote.getGlobal('standardScheme')
+    const origin = standardScheme + '://fake-host'
+    const imageURL = origin + '/test.png'
+    const filePath = path.join(__dirname, 'fixtures', 'pages', 'b.html')
+    const fileContent = '<img src="/test.png" />'
+    var w = null
+    var success = null
+
+    beforeEach(function () {
+      w = new BrowserWindow({show: false})
+      success = false
+    })
+
+    afterEach(function (done) {
+      protocol.unregisterProtocol(standardScheme, function () {
+        if (w != null) {
+          w.destroy()
+        }
+        w = null
+        done()
+      })
+    })
+
+    it('throws when called after ready event', function () {
+      assert.throws(function () {
+        protocol.registerStandardSchemes(['some-scheme'])
+      }, 'protocol.registerStandardSchemes should be called before app is ready')
+    })
+
+    it('resolves relative resources', function (done) {
+      var handler = function (request, callback) {
+        if (request.url === imageURL) {
+          success = true
+          callback()
+        } else {
+          callback(filePath)
+        }
+      }
+      protocol.registerFileProtocol(standardScheme, handler, function (error) {
+        if (error) {
+          return done(error)
+        }
+        w.webContents.on('did-finish-load', function () {
+          assert(success)
+          done()
+        })
+        w.loadURL(origin)
+      })
+    })
+
+    it('resolves absolute resources', function (done) {
+      var handler = function (request, callback) {
+        if (request.url === imageURL) {
+          success = true
+          callback()
+        } else {
+          callback({
+            data: fileContent,
+            mimeType: 'text/html'
+          })
+        }
+      }
+      protocol.registerStringProtocol(standardScheme, handler, function (error) {
+        if (error) {
+          return done(error)
+        }
+        w.webContents.on('did-finish-load', function () {
+          assert(success)
+          done()
+        })
+        w.loadURL(origin)
+      })
+    })
+  })
 })

spec/fixtures/pages/b.html

@@ -1,8 +1,8 @@
 <html>
 <body>
+<img src="./test.png" />
 <script type="text/javascript" charset="utf-8">
   console.log('b');
 </script>
 </body>
 </html>
-

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