From 673f4f4d0c9167bc03a54e73bad4b0cab78708eb Mon Sep 17 00:00:00 2001 From: Ryohei Ikegami Date: Mon, 3 Apr 2017 19:46:24 +0900 Subject: [PATCH 01/10] Add test for `require` to search under app dir --- spec/modules-spec.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/spec/modules-spec.js b/spec/modules-spec.js index 5f82717527..26cafc7ccb 100644 --- a/spec/modules-spec.js +++ b/spec/modules-spec.js @@ -2,6 +2,8 @@ const assert = require('assert') const Module = require('module') const path = require('path') const temp = require('temp') +const {remote} = require('electron') +const {BrowserWindow} = remote describe('third-party module', function () { var fixtures = path.join(__dirname, 'fixtures') @@ -129,3 +131,17 @@ describe('Module._nodeModulePaths', function () { }) }) }) + +describe('require', () => { + describe('when loaded URL is not file: protocol', () => { + it('searches for module under app directory', async () => { + const w = new BrowserWindow({ + show: false, + }) + w.loadURL('about:blank') + const result = await w.webContents.executeJavaScript('typeof require("q").when') + assert.equal(result, 'function') + w.destroy() + }) + }) +}) From 50c99e4507d3a9f06c66d52754c977d24eea347c Mon Sep 17 00:00:00 2001 From: Ryohei Ikegami Date: Mon, 3 Apr 2017 20:12:02 +0900 Subject: [PATCH 02/10] Search for module under the app directory --- lib/renderer/init.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/renderer/init.js b/lib/renderer/init.js index 38441c9ec1..c6fd6a2e24 100644 --- a/lib/renderer/init.js +++ b/lib/renderer/init.js @@ -116,6 +116,9 @@ if (nodeIntegration === 'true') { } else { global.__filename = __filename global.__dirname = __dirname + + // Search for module under the app directory + module.paths = module.paths.concat(Module._nodeModulePaths(electron.remote.app.getAppPath())) } // Redirect window.onerror to uncaughtException. From d1212d4a43a15dc1d501365cda513e7dcee51353 Mon Sep 17 00:00:00 2001 From: Ryohei Ikegami Date: Mon, 3 Apr 2017 20:23:36 +0900 Subject: [PATCH 03/10] Fix JS style --- spec/modules-spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/modules-spec.js b/spec/modules-spec.js index 26cafc7ccb..c918d377dd 100644 --- a/spec/modules-spec.js +++ b/spec/modules-spec.js @@ -136,7 +136,7 @@ describe('require', () => { describe('when loaded URL is not file: protocol', () => { it('searches for module under app directory', async () => { const w = new BrowserWindow({ - show: false, + show: false }) w.loadURL('about:blank') const result = await w.webContents.executeJavaScript('typeof require("q").when') From 001d03c859be5e68267c6b59d3a73d7f2fe7a9fd Mon Sep 17 00:00:00 2001 From: Ryohei Ikegami Date: Mon, 3 Apr 2017 22:11:29 +0900 Subject: [PATCH 04/10] Do not add search paths in devtools --- lib/renderer/init.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/renderer/init.js b/lib/renderer/init.js index c6fd6a2e24..72f7c9ebab 100644 --- a/lib/renderer/init.js +++ b/lib/renderer/init.js @@ -117,8 +117,11 @@ if (nodeIntegration === 'true') { global.__filename = __filename global.__dirname = __dirname - // Search for module under the app directory - module.paths = module.paths.concat(Module._nodeModulePaths(electron.remote.app.getAppPath())) + if (window.location.protocol !== 'chrome-devtools:') { + // Search for module under the app directory + // (remote.app doesn't work in devtools) + module.paths = module.paths.concat(Module._nodeModulePaths(electron.remote.app.getAppPath())) + } } // Redirect window.onerror to uncaughtException. From 9cb6bc098de5a9df75c6afda0be6f5c697a4ed3f Mon Sep 17 00:00:00 2001 From: Ryohei Ikegami Date: Tue, 4 Apr 2017 09:08:27 +0900 Subject: [PATCH 05/10] Use beforeEach/afterEach --- spec/modules-spec.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/spec/modules-spec.js b/spec/modules-spec.js index c918d377dd..02fec41444 100644 --- a/spec/modules-spec.js +++ b/spec/modules-spec.js @@ -134,13 +134,21 @@ describe('Module._nodeModulePaths', function () { describe('require', () => { describe('when loaded URL is not file: protocol', () => { - it('searches for module under app directory', async () => { - const w = new BrowserWindow({ + let w + + beforeEach(() => { + w = new BrowserWindow({ show: false }) + }) + + it('searches for module under app directory', async () => { w.loadURL('about:blank') const result = await w.webContents.executeJavaScript('typeof require("q").when') assert.equal(result, 'function') + }) + + afterEach(() => { w.destroy() }) }) From 4a7eec8f2dd74217aa99d8da4ed0cc61038eee3a Mon Sep 17 00:00:00 2001 From: Ryohei Ikegami Date: Tue, 4 Apr 2017 09:36:01 +0900 Subject: [PATCH 06/10] Pass app path as command line argument --- atom/browser/api/atom_api_app.cc | 10 ++++++++++ atom/browser/api/atom_api_app.h | 6 ++++++ atom/browser/atom_browser_client.cc | 6 ++++++ atom/common/options_switches.cc | 3 +++ atom/common/options_switches.h | 1 + lib/browser/api/app.js | 4 ---- lib/renderer/init.js | 8 +++++--- 7 files changed, 31 insertions(+), 7 deletions(-) diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index 56a11daf69..a53e957660 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -655,6 +655,14 @@ void App::OnGpuProcessCrashed(base::TerminationStatus status) { status == base::TERMINATION_STATUS_PROCESS_WAS_KILLED); } +std::string App::GetAppPath() { + return app_path_; +} + +void App::SetAppPath(const std::string& app_path) { + app_path_ = app_path; +} + base::FilePath App::GetPath(mate::Arguments* args, const std::string& name) { bool succeed = false; base::FilePath path; @@ -959,6 +967,8 @@ void App::BuildPrototype( .SetMethod("isUnityRunning", base::Bind(&Browser::IsUnityRunning, browser)) #endif + .SetMethod("setAppPath", &App::SetAppPath) + .SetMethod("getAppPath", &App::GetAppPath) .SetMethod("setPath", &App::SetPath) .SetMethod("getPath", &App::GetPath) .SetMethod("setDesktopName", &App::SetDesktopName) diff --git a/atom/browser/api/atom_api_app.h b/atom/browser/api/atom_api_app.h index 8b276f334d..4b09d77031 100644 --- a/atom/browser/api/atom_api_app.h +++ b/atom/browser/api/atom_api_app.h @@ -70,6 +70,8 @@ class App : public AtomBrowserClient::Delegate, std::unique_ptr model); #endif + std::string GetAppPath(); + protected: explicit App(v8::Isolate* isolate); ~App() override; @@ -115,6 +117,8 @@ class App : public AtomBrowserClient::Delegate, void OnGpuProcessCrashed(base::TerminationStatus status) override; private: + void SetAppPath(const std::string& app_path); + // Get/Set the pre-defined path in PathService. base::FilePath GetPath(mate::Arguments* args, const std::string& name); void SetPath(mate::Arguments* args, @@ -154,6 +158,8 @@ class App : public AtomBrowserClient::Delegate, // Tracks tasks requesting file icons. base::CancelableTaskTracker cancelable_task_tracker_; + std::string app_path_; + DISALLOW_COPY_AND_ASSIGN(App); }; diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index d0bbf4ad53..9894e5e928 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/browser/atom_browser_client.cc @@ -234,6 +234,12 @@ void AtomBrowserClient::AppendExtraCommandLineSwitches( } #endif + if (delegate_) { + auto app_path = static_cast(delegate_)->GetAppPath(); + command_line->AppendSwitchASCII(switches::kAppPath, + app_path); + } + content::WebContents* web_contents = GetWebContentsFromProcessID(process_id); if (!web_contents) return; diff --git a/atom/common/options_switches.cc b/atom/common/options_switches.cc index 288fcd3a07..2f1c0368f3 100644 --- a/atom/common/options_switches.cc +++ b/atom/common/options_switches.cc @@ -159,6 +159,9 @@ const char kSecureSchemes[] = "secure-schemes"; // The browser process app model ID const char kAppUserModelId[] = "app-user-model-id"; +// The application path +const char kAppPath[] = "app-path"; + // The command line switch versions of the options. const char kBackgroundColor[] = "background-color"; const char kPreloadScript[] = "preload"; diff --git a/atom/common/options_switches.h b/atom/common/options_switches.h index 9e1a71ca5b..69e7af029e 100644 --- a/atom/common/options_switches.h +++ b/atom/common/options_switches.h @@ -81,6 +81,7 @@ extern const char kStandardSchemes[]; extern const char kRegisterServiceWorkerSchemes[]; extern const char kSecureSchemes[]; extern const char kAppUserModelId[]; +extern const char kAppPath[]; extern const char kBackgroundColor[]; extern const char kPreloadScript[]; diff --git a/lib/browser/api/app.js b/lib/browser/api/app.js index c5865f2242..d1ca60280e 100644 --- a/lib/browser/api/app.js +++ b/lib/browser/api/app.js @@ -12,11 +12,7 @@ const {EventEmitter} = require('events') Object.setPrototypeOf(App.prototype, EventEmitter.prototype) -let appPath = null - Object.assign(app, { - getAppPath () { return appPath }, - setAppPath (path) { appPath = path }, setApplicationMenu (menu) { return Menu.setApplicationMenu(menu) }, diff --git a/lib/renderer/init.js b/lib/renderer/init.js index 72f7c9ebab..0306463bc4 100644 --- a/lib/renderer/init.js +++ b/lib/renderer/init.js @@ -56,6 +56,7 @@ electron.ipcRenderer.on('ELECTRON_INTERNAL_RENDERER_ASYNC_WEB_FRAME_METHOD', (ev let nodeIntegration = 'false' let preloadScript = null let isBackgroundPage = false +let appPath = null for (let arg of process.argv) { if (arg.indexOf('--guest-instance-id=') === 0) { // This is a guest web view. @@ -69,6 +70,8 @@ for (let arg of process.argv) { preloadScript = arg.substr(arg.indexOf('=') + 1) } else if (arg === '--background-page') { isBackgroundPage = true + } else if (arg.indexOf('--app-path=') === 0) { + appPath = arg.substr(arg.indexOf('=') + 1) } } @@ -117,10 +120,9 @@ if (nodeIntegration === 'true') { global.__filename = __filename global.__dirname = __dirname - if (window.location.protocol !== 'chrome-devtools:') { + if (appPath) { // Search for module under the app directory - // (remote.app doesn't work in devtools) - module.paths = module.paths.concat(Module._nodeModulePaths(electron.remote.app.getAppPath())) + module.paths = module.paths.concat(Module._nodeModulePaths(appPath)) } } From 24fedb2e20e9468f9b12a03198fe9c649039a9e5 Mon Sep 17 00:00:00 2001 From: Ryohei Ikegami Date: Tue, 4 Apr 2017 09:40:38 +0900 Subject: [PATCH 07/10] No extra linebreak --- atom/browser/atom_browser_client.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index 9894e5e928..3908c9613a 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/browser/atom_browser_client.cc @@ -236,8 +236,7 @@ void AtomBrowserClient::AppendExtraCommandLineSwitches( if (delegate_) { auto app_path = static_cast(delegate_)->GetAppPath(); - command_line->AppendSwitchASCII(switches::kAppPath, - app_path); + command_line->AppendSwitchASCII(switches::kAppPath, app_path); } content::WebContents* web_contents = GetWebContentsFromProcessID(process_id); From c77e07bc1539e3d395047a03adbfb1ef0e2e249d Mon Sep 17 00:00:00 2001 From: Ryohei Ikegami Date: Wed, 12 Apr 2017 11:55:41 +0900 Subject: [PATCH 08/10] Fix afterEach --- spec/modules-spec.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/spec/modules-spec.js b/spec/modules-spec.js index 02fec41444..7cc5d34fe6 100644 --- a/spec/modules-spec.js +++ b/spec/modules-spec.js @@ -4,6 +4,7 @@ const path = require('path') const temp = require('temp') const {remote} = require('electron') const {BrowserWindow} = remote +const {closeWindow} = require('./window-helpers') describe('third-party module', function () { var fixtures = path.join(__dirname, 'fixtures') @@ -142,14 +143,15 @@ describe('require', () => { }) }) + afterEach(async () => { + await closeWindow(w) + w = null + }) + it('searches for module under app directory', async () => { w.loadURL('about:blank') const result = await w.webContents.executeJavaScript('typeof require("q").when') assert.equal(result, 'function') }) - - afterEach(() => { - w.destroy() - }) }) }) From 9d62b196d3676cfe43963ad7dab0917aa2992ad9 Mon Sep 17 00:00:00 2001 From: Ryohei Ikegami Date: Thu, 13 Apr 2017 10:59:12 +0900 Subject: [PATCH 09/10] Use base::FilePath --- atom/browser/api/atom_api_app.cc | 4 ++-- atom/browser/api/atom_api_app.h | 6 +++--- atom/browser/atom_browser_client.cc | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index a53e957660..d8e79171f8 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -655,11 +655,11 @@ void App::OnGpuProcessCrashed(base::TerminationStatus status) { status == base::TERMINATION_STATUS_PROCESS_WAS_KILLED); } -std::string App::GetAppPath() { +base::FilePath App::GetAppPath() { return app_path_; } -void App::SetAppPath(const std::string& app_path) { +void App::SetAppPath(const base::FilePath& app_path) { app_path_ = app_path; } diff --git a/atom/browser/api/atom_api_app.h b/atom/browser/api/atom_api_app.h index 4b09d77031..78871f6920 100644 --- a/atom/browser/api/atom_api_app.h +++ b/atom/browser/api/atom_api_app.h @@ -70,7 +70,7 @@ class App : public AtomBrowserClient::Delegate, std::unique_ptr model); #endif - std::string GetAppPath(); + base::FilePath GetAppPath(); protected: explicit App(v8::Isolate* isolate); @@ -117,7 +117,7 @@ class App : public AtomBrowserClient::Delegate, void OnGpuProcessCrashed(base::TerminationStatus status) override; private: - void SetAppPath(const std::string& app_path); + void SetAppPath(const base::FilePath& app_path); // Get/Set the pre-defined path in PathService. base::FilePath GetPath(mate::Arguments* args, const std::string& name); @@ -158,7 +158,7 @@ class App : public AtomBrowserClient::Delegate, // Tracks tasks requesting file icons. base::CancelableTaskTracker cancelable_task_tracker_; - std::string app_path_; + base::FilePath app_path_; DISALLOW_COPY_AND_ASSIGN(App); }; diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index 3908c9613a..db33e2e38f 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/browser/atom_browser_client.cc @@ -236,7 +236,7 @@ void AtomBrowserClient::AppendExtraCommandLineSwitches( if (delegate_) { auto app_path = static_cast(delegate_)->GetAppPath(); - command_line->AppendSwitchASCII(switches::kAppPath, app_path); + command_line->AppendSwitchPath(switches::kAppPath, app_path); } content::WebContents* web_contents = GetWebContentsFromProcessID(process_id); From ea6890aa5ce178ec993ab805b4e29f174dcb090b Mon Sep 17 00:00:00 2001 From: Ryohei Ikegami Date: Thu, 13 Apr 2017 23:26:42 +0900 Subject: [PATCH 10/10] Use const --- atom/browser/api/atom_api_app.cc | 2 +- atom/browser/api/atom_api_app.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index d8e79171f8..acf7ce8c74 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -655,7 +655,7 @@ void App::OnGpuProcessCrashed(base::TerminationStatus status) { status == base::TERMINATION_STATUS_PROCESS_WAS_KILLED); } -base::FilePath App::GetAppPath() { +base::FilePath App::GetAppPath() const { return app_path_; } diff --git a/atom/browser/api/atom_api_app.h b/atom/browser/api/atom_api_app.h index 78871f6920..a87b88bc46 100644 --- a/atom/browser/api/atom_api_app.h +++ b/atom/browser/api/atom_api_app.h @@ -70,7 +70,7 @@ class App : public AtomBrowserClient::Delegate, std::unique_ptr model); #endif - base::FilePath GetAppPath(); + base::FilePath GetAppPath() const; protected: explicit App(v8::Isolate* isolate);