From 5b5c161601810be9c223244dbf20c9d4f7d1dd6d Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Tue, 8 May 2018 01:29:18 +1000 Subject: [PATCH] feat: new makeSingleInstance API (#12782) * Refactor app.makeSingleInstance * new API `app.isPrimaryInstance()` * new API `app.isSingleInstance()` * new event `app.on('second-instance')` * deprecated old syntax `app.makeSingleInstance(cb)` * deprecated old syntax of `app.makeSingleInstance() --> bool` in favor of `app.isPrimaryInstance()` * Fix spec, we don't need process.nextTick hacks any more * Make deprecation TODO for the return value of makeSingleInstance * Refactor makeSingleInstance to requestSingleInstanceLock and add appropriate deprecation comments * I swear this isn't tricking the linter * Make const * Add deprecation warnings for release, and add to planned-breaking-changes BREAKING CHANGE --- atom/browser/api/atom_api_app.cc | 40 ++++++--- atom/browser/api/atom_api_app.h | 8 +- docs/api/app.md | 85 ++++++++++++-------- docs/tutorial/planned-breaking-changes.md | 32 +++++++- lib/browser/api/app.js | 15 ++++ spec/api-app-spec.js | 22 +++++ spec/fixtures/api/singleton-old/main.js | 13 +++ spec/fixtures/api/singleton-old/package.json | 5 ++ spec/fixtures/api/singleton/main.js | 8 +- 9 files changed, 176 insertions(+), 52 deletions(-) create mode 100644 spec/fixtures/api/singleton-old/main.js create mode 100644 spec/fixtures/api/singleton-old/package.json diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index c2fdf716cf98..69d8aaf4825b 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -350,8 +350,8 @@ struct Converter { namespace atom { ProcessMetric::ProcessMetric(int type, - base::ProcessId pid, - std::unique_ptr metrics) { + base::ProcessId pid, + std::unique_ptr metrics) { this->type = type; this->pid = pid; this->metrics = std::move(metrics); @@ -422,7 +422,9 @@ int GetPathConstant(const std::string& name) { } bool NotificationCallbackWrapper( - const ProcessSingleton::NotificationCallback& callback, + const base::Callback< + void(const base::CommandLine::StringVector& command_line, + const base::FilePath& current_directory)>& callback, const base::CommandLine::StringVector& cmd, const base::FilePath& cwd) { // Make sure the callback is called after app gets ready. @@ -864,30 +866,43 @@ std::string App::GetLocale() { return g_browser_process->GetApplicationLocale(); } -bool App::MakeSingleInstance( - const ProcessSingleton::NotificationCallback& callback) { +void App::OnSecondInstance(const base::CommandLine::StringVector& cmd, + const base::FilePath& cwd) { + Emit("second-instance", cmd, cwd); +} + +bool App::HasSingleInstanceLock() const { if (process_singleton_) - return false; + return true; + return false; +} + +bool App::RequestSingleInstanceLock() { + if (HasSingleInstanceLock()) + return true; base::FilePath user_dir; PathService::Get(brightray::DIR_USER_DATA, &user_dir); + + auto cb = base::Bind(&App::OnSecondInstance, base::Unretained(this)); + process_singleton_.reset(new ProcessSingleton( - user_dir, base::Bind(NotificationCallbackWrapper, callback))); + user_dir, base::Bind(NotificationCallbackWrapper, cb))); switch (process_singleton_->NotifyOtherProcessOrCreate()) { case ProcessSingleton::NotifyResult::LOCK_ERROR: case ProcessSingleton::NotifyResult::PROFILE_IN_USE: case ProcessSingleton::NotifyResult::PROCESS_NOTIFIED: { process_singleton_.reset(); - return true; + return false; } case ProcessSingleton::NotifyResult::PROCESS_NONE: default: // Shouldn't be needed, but VS warns if it is not there. - return false; + return true; } } -void App::ReleaseSingleInstance() { +void App::ReleaseSingleInstanceLock() { if (process_singleton_) { process_singleton_->Cleanup(); process_singleton_.reset(); @@ -1252,8 +1267,9 @@ void App::BuildPrototype(v8::Isolate* isolate, #if defined(USE_NSS_CERTS) .SetMethod("importCertificate", &App::ImportCertificate) #endif - .SetMethod("makeSingleInstance", &App::MakeSingleInstance) - .SetMethod("releaseSingleInstance", &App::ReleaseSingleInstance) + .SetMethod("hasSingleInstanceLock", &App::HasSingleInstanceLock) + .SetMethod("requestSingleInstanceLock", &App::RequestSingleInstanceLock) + .SetMethod("releaseSingleInstanceLock", &App::ReleaseSingleInstanceLock) .SetMethod("relaunch", &App::Relaunch) .SetMethod("isAccessibilitySupportEnabled", &App::IsAccessibilitySupportEnabled) diff --git a/atom/browser/api/atom_api_app.h b/atom/browser/api/atom_api_app.h index ac7c256928d6..f4f5b325a4da 100644 --- a/atom/browser/api/atom_api_app.h +++ b/atom/browser/api/atom_api_app.h @@ -178,9 +178,11 @@ class App : public AtomBrowserClient::Delegate, void SetDesktopName(const std::string& desktop_name); std::string GetLocale(); - bool MakeSingleInstance( - const ProcessSingleton::NotificationCallback& callback); - void ReleaseSingleInstance(); + void OnSecondInstance(const base::CommandLine::StringVector& cmd, + const base::FilePath& cwd); + bool HasSingleInstanceLock() const; + bool RequestSingleInstanceLock(); + void ReleaseSingleInstanceLock(); bool Relaunch(mate::Arguments* args); void DisableHardwareAcceleration(mate::Arguments* args); void DisableDomainBlockingFor3DAPIs(mate::Arguments* args); diff --git a/docs/api/app.md b/docs/api/app.md index 4e5066970b8e..54a164f832fa 100644 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -373,6 +373,22 @@ app.on('session-created', (event, session) => { }) ``` +### Event: 'second-instance' + +Returns: + +* `argv` String[] - An array of the second instance's command line arguments +* `workingDirectory` String - The second instance's working directory + +This event will be emitted inside the primary instance of your application +when a second instance has been executed. `argv` is an Array of the second instance's +command line arguments, and `workingDirectory` is its current working directory. Usually +applications respond to this by making their primary window focused and +non-minimized. + +This event is guaranteed to be emitted after the `ready` event of `app` +gets emitted. + ## Methods The `app` object has the following methods: @@ -742,32 +758,24 @@ app.setJumpList([ ]) ``` -### `app.makeSingleInstance(callback)` +### `app.requestSingleInstanceLock()` -* `callback` Function - * `argv` String[] - An array of the second instance's command line arguments - * `workingDirectory` String - The second instance's working directory - -Returns `Boolean`. +Returns `Boolean` This method makes your application a Single Instance Application - instead of allowing multiple instances of your app to run, this will ensure that only a single instance of your app is running, and other instances signal this instance and exit. -`callback` will be called by the first instance with `callback(argv, workingDirectory)` -when a second instance has been executed. `argv` is an Array of the second instance's -command line arguments, and `workingDirectory` is its current working directory. Usually -applications respond to this by making their primary window focused and -non-minimized. +The return value of this method indicates whether or not this instance of your +application successfully obtained the lock. If it failed to obtain the lock +you can assume that another instance of your application is already running with +the lock and exit immediately. -The `callback` is guaranteed to be executed after the `ready` event of `app` -gets emitted. - -This method returns `false` if your process is the primary instance of the -application and your app should continue loading. And returns `true` if your -process has sent its parameters to another instance, and you should immediately -quit. +I.e. This method returns `true` if your process is the primary instance of your +application and your app should continue loading. It returns `false` if your +process should immediately quit as it has sent its parameters to another +instance that has already acquired the lock. On macOS the system enforces single instance automatically when users try to open a second instance of your app in Finder, and the `open-file` and `open-url` @@ -782,27 +790,38 @@ starts: const {app} = require('electron') let myWindow = null -const isSecondInstance = app.makeSingleInstance((commandLine, workingDirectory) => { - // Someone tried to run a second instance, we should focus our window. - if (myWindow) { - if (myWindow.isMinimized()) myWindow.restore() - myWindow.focus() - } -}) +const gotTheLock = app.requestSingleInstanceLock() -if (isSecondInstance) { +if (!gotTheLock) { app.quit() -} +} else { + app.on('second-instance', (commandLine, workingDirectory) => { + // Someone tried to run a second instance, we should focus our window. + if (myWindow) { + if (myWindow.isMinimized()) myWindow.restore() + myWindow.focus() + } + }) -// Create myWindow, load the rest of the app, etc... -app.on('ready', () => { -}) + // Create myWindow, load the rest of the app, etc... + app.on('ready', () => { + }) +} ``` -### `app.releaseSingleInstance()` +### `app.hasSingleInstanceLock()` -Releases all locks that were created by `makeSingleInstance`. This will allow -multiple instances of the application to once again run side by side. +Returns `Boolean` + +This method returns whether or not this instance of your app is currently +holding the single instance lock. You can request the lock with +`app.requestSingleInstanceLock()` and release with +`app.releaseSingleInstanceLock()` + +### `app.releaseSingleInstanceLock()` + +Releases all locks that were created by `requestSingleInstanceLock`. This will +allow multiple instances of the application to once again run side by side. ### `app.setUserActivity(type, userInfo[, webpageURL])` _macOS_ diff --git a/docs/tutorial/planned-breaking-changes.md b/docs/tutorial/planned-breaking-changes.md index 00e01f4f8c3a..727d4ce5b350 100644 --- a/docs/tutorial/planned-breaking-changes.md +++ b/docs/tutorial/planned-breaking-changes.md @@ -1,4 +1,4 @@ -# Planned Breaking API Changes +# Planned Breaking API Changes (3.0) The following list includes the APIs that will be removed in Electron 3.0. @@ -150,3 +150,33 @@ Replace with: https://atom.io/download/electron The `FIXME` string is used in code comments to denote things that should be fixed for the 3.0 release. See https://github.com/electron/electron/search?q=fixme + +# Planned Breaking API Changes (4.0) + +The following list includes the APIs that will be removed in Electron 4.0. + +There is no timetable for when this release will occur but deprecation +warnings will be added at least [one major version](electron-versioning.md#semver) beforehand. + +## `app.makeSingleInstance` + +```js +// Deprecated +app.makeSingleInstance(function (argv, cwd) { + +}) +// Replace with +app.requestSingleInstanceLock() +app.on('second-instance', function (argv, cwd) { + +}) +``` + +## `app.releaseSingleInstance` + +```js +// Deprecated +app.releaseSingleInstance() +// Replace with +app.releaseSingleInstanceLock() +``` \ No newline at end of file diff --git a/lib/browser/api/app.js b/lib/browser/api/app.js index 6aaeadbe8f8c..789419a082d7 100644 --- a/lib/browser/api/app.js +++ b/lib/browser/api/app.js @@ -109,6 +109,21 @@ for (let name of events) { }) } +// TODO(MarshallOfSound): Remove in 4.0 +app.releaseSingleInstance = () => { + deprecate.warn('app.releaseSingleInstance(cb)', 'app.releaseSingleInstanceLock()') + app.releaseSingleInstanceLock() +} + +// TODO(MarshallOfSound): Remove in 4.0 +app.makeSingleInstance = (oldStyleFn) => { + deprecate.warn('app.makeSingleInstance(cb)', 'app.requestSingleInstanceLock() and app.on(\'second-instance\', cb)') + if (oldStyleFn && typeof oldStyleFn === 'function') { + app.on('second-instance', (event, ...args) => oldStyleFn(...args)) + } + return !app.requestSingleInstanceLock() +} + // Wrappers for native classes. const {DownloadItem} = process.atomBinding('download_item') Object.setPrototypeOf(DownloadItem.prototype, EventEmitter.prototype) diff --git a/spec/api-app-spec.js b/spec/api-app-spec.js index 1de8a99e1955..6a13543717d0 100644 --- a/spec/api-app-spec.js +++ b/spec/api-app-spec.js @@ -185,7 +185,29 @@ describe('app module', () => { }) }) + // TODO(MarshallOfSound) - Remove in 4.0.0 describe('app.makeSingleInstance', () => { + it('prevents the second launch of app', function (done) { + this.timeout(120000) + const appPath = path.join(__dirname, 'fixtures', 'api', 'singleton-old') + // First launch should exit with 0. + const first = ChildProcess.spawn(remote.process.execPath, [appPath]) + first.once('exit', (code) => { + assert.equal(code, 0) + }) + // Start second app when received output. + first.stdout.once('data', () => { + // Second launch should exit with 1. + const second = ChildProcess.spawn(remote.process.execPath, [appPath]) + second.once('exit', (code) => { + assert.equal(code, 1) + done() + }) + }) + }) + }) + + describe('app.requestSingleInstanceLock', () => { it('prevents the second launch of app', function (done) { this.timeout(120000) const appPath = path.join(__dirname, 'fixtures', 'api', 'singleton') diff --git a/spec/fixtures/api/singleton-old/main.js b/spec/fixtures/api/singleton-old/main.js new file mode 100644 index 000000000000..ef7fa7b05f55 --- /dev/null +++ b/spec/fixtures/api/singleton-old/main.js @@ -0,0 +1,13 @@ +const {app} = require('electron') + +app.once('ready', () => { + console.log('started') // ping parent +}) + +const shouldExit = app.makeSingleInstance(() => { + setImmediate(() => app.exit(0)) +}) + +if (shouldExit) { + app.exit(1) +} diff --git a/spec/fixtures/api/singleton-old/package.json b/spec/fixtures/api/singleton-old/package.json new file mode 100644 index 000000000000..ebf7c8f4892e --- /dev/null +++ b/spec/fixtures/api/singleton-old/package.json @@ -0,0 +1,5 @@ +{ + "name": "electron-app-singleton", + "main": "main.js" +} + diff --git a/spec/fixtures/api/singleton/main.js b/spec/fixtures/api/singleton/main.js index cbf3da1117d9..3202a7a180d6 100644 --- a/spec/fixtures/api/singleton/main.js +++ b/spec/fixtures/api/singleton/main.js @@ -4,10 +4,12 @@ app.once('ready', () => { console.log('started') // ping parent }) -const shouldExit = app.makeSingleInstance(() => { - process.nextTick(() => app.exit(0)) +const gotTheLock = app.requestSingleInstanceLock() + +app.on('second-instance', () => { + setImmediate(() => app.exit(0)) }) -if (shouldExit) { +if (!gotTheLock) { app.exit(1) }