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

@@ -350,8 +350,8 @@ struct Converter<content::CertificateRequestResultType> {
 namespace atom {
 
 ProcessMetric::ProcessMetric(int type,
-    base::ProcessId pid,
-    std::unique_ptr<base::ProcessMetrics> metrics) {
+                             base::ProcessId pid,
+                             std::unique_ptr<base::ProcessMetrics> 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)

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);

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()`
+
+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 `makeSingleInstance`. This will allow
-multiple instances of the application to once again run side by side.
+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_
 

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()
+```

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)

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')

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

spec/fixtures/api/singleton-old/package.json

@@ -0,0 +1,5 @@
+{
+  "name": "electron-app-singleton",
+  "main": "main.js"
+}
+

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