diff --git a/spec/main-process/atom-application.test.js b/spec/main-process/atom-application.test.js index b73a57fab..7818314db 100644 --- a/spec/main-process/atom-application.test.js +++ b/spec/main-process/atom-application.test.js @@ -1,3 +1,4 @@ +const temp = require('temp').track() const season = require('season') const dedent = require('dedent') const electron = require('electron') @@ -389,6 +390,110 @@ describe('AtomApplication', function () { assert.deepEqual(app2Window.representedDirectoryPaths, []) }) + describe('when the `--wait` flag is passed', () => { + let killedPids, atomApplication, onDidKillProcess + + beforeEach(() => { + killedPids = [] + onDidKillProcess = null + atomApplication = buildAtomApplication({ + killProcess (pid) { + killedPids.push(pid) + if (onDidKillProcess) onDidKillProcess() + } + }) + }) + + it('kills the specified pid after a newly-opened window is closed', async () => { + const window1 = atomApplication.launch(parseCommandLine(['--wait', '--pid', '101'])) + await focusWindow(window1) + + const [window2] = atomApplication.launch(parseCommandLine(['--new-window', '--wait', '--pid', '102'])) + await focusWindow(window2) + assert.deepEqual(killedPids, []) + + let processKillPromise = new Promise(resolve => { onDidKillProcess = resolve }) + window1.close() + await processKillPromise + assert.deepEqual(killedPids, [101]) + + processKillPromise = new Promise(resolve => { onDidKillProcess = resolve }) + window2.close() + await processKillPromise + assert.deepEqual(killedPids, [101, 102]) + }) + + it('kills the specified pid after a newly-opened file in an existing window is closed', async () => { + const window = atomApplication.launch(parseCommandLine(['--wait', '--pid', '101'])) + await focusWindow(window) + + const filePath1 = temp.openSync('test').path + const filePath2 = temp.openSync('test').path + fs.writeFileSync(filePath1, 'File 1') + fs.writeFileSync(filePath2, 'File 2') + + const reusedWindow = atomApplication.launch(parseCommandLine(['--wait', '--pid', '102', filePath1, filePath2])) + assert.equal(reusedWindow, window) + + const activeEditorPath = await evalInWebContents(window.browserWindow.webContents, send => { + const subscription = atom.workspace.onDidChangeActivePaneItem(editor => { + send(editor.getPath()) + subscription.dispose() + }) + }) + + assert([filePath1, filePath2].includes(activeEditorPath)) + assert.deepEqual(killedPids, []) + + await evalInWebContents(window.browserWindow.webContents, send => { + atom.workspace.getActivePaneItem().destroy() + send() + }) + await timeoutPromise(100) + assert.deepEqual(killedPids, []) + + let processKillPromise = new Promise(resolve => { onDidKillProcess = resolve }) + await evalInWebContents(window.browserWindow.webContents, send => { + atom.workspace.getActivePaneItem().destroy() + send() + }) + await processKillPromise + assert.deepEqual(killedPids, [102]) + + processKillPromise = new Promise(resolve => { onDidKillProcess = resolve }) + window.close() + await processKillPromise + assert.deepEqual(killedPids, [102, 101]) + }) + + it('kills the specified pid after a newly-opened directory in an existing window is closed', async () => { + const window = atomApplication.launch(parseCommandLine([])) + await focusWindow(window) + + const dirPath1 = makeTempDir() + const reusedWindow = atomApplication.launch(parseCommandLine(['--wait', '--pid', '101', dirPath1])) + assert.equal(reusedWindow, window) + assert.deepEqual(await getTreeViewRootDirectories(window), [dirPath1]) + assert.deepEqual(killedPids, []) + + const dirPath2 = makeTempDir() + await evalInWebContents(window.browserWindow.webContents, (send, dirPath1, dirPath2) => { + atom.project.setPaths([dirPath1, dirPath2]) + send() + }, dirPath1, dirPath2) + await timeoutPromise(100) + assert.deepEqual(killedPids, []) + + let processKillPromise = new Promise(resolve => { onDidKillProcess = resolve }) + await evalInWebContents(window.browserWindow.webContents, (send, dirPath2) => { + atom.project.setPaths([dirPath2]) + send() + }, dirPath2) + await processKillPromise + assert.deepEqual(killedPids, [101]) + }) + }) + describe('when closing the last window', () => { if (process.platform === 'linux' || process.platform === 'win32') { it('quits the application', async () => { @@ -529,11 +634,11 @@ describe('AtomApplication', function () { assert(electron.app.didQuit()) }) - function buildAtomApplication () { - const atomApplication = new AtomApplication({ + function buildAtomApplication (params = {}) { + const atomApplication = new AtomApplication(Object.assign({ resourcePath: ATOM_RESOURCE_PATH, - atomHomeDirPath: process.env.ATOM_HOME - }) + atomHomeDirPath: process.env.ATOM_HOME, + }, params)) atomApplicationsToDestroy.push(atomApplication) return atomApplication } @@ -566,7 +671,6 @@ describe('AtomApplication', function () { } function makeTempDir (name) { - const temp = require('temp').track() return fs.realpathSync(temp.mkdirSync(name)) } @@ -585,7 +689,7 @@ describe('AtomApplication', function () { function sendBackToMainProcess (result) { require('electron').ipcRenderer.send('${channelId}', result) } - (${source})(sendBackToMainProcess) + (${source})(sendBackToMainProcess, ${args.map(JSON.stringify).join(', ')}) `) }) } diff --git a/src/application-delegate.js b/src/application-delegate.js index 87e531b85..1b1dd1e9c 100644 --- a/src/application-delegate.js +++ b/src/application-delegate.js @@ -139,6 +139,10 @@ class ApplicationDelegate { return ipcRenderer.send('execute-javascript-in-dev-tools', code) } + didClosePathWithWaitSession (path) { + return ipcHelpers.call('window-method', 'didClosePathWithWaitSession', path) + } + setWindowDocumentEdited (edited) { return ipcHelpers.call('window-method', 'setDocumentEdited', edited) } diff --git a/src/atom-environment.js b/src/atom-environment.js index fc3201dfc..70fb352e2 100644 --- a/src/atom-environment.js +++ b/src/atom-environment.js @@ -70,6 +70,7 @@ class AtomEnvironment { this.loadTime = null this.emitter = new Emitter() this.disposables = new CompositeDisposable() + this.pathsWithWaitSessions = new Set() // Public: A {DeserializerManager} instance this.deserializers = new DeserializerManager(this) @@ -359,6 +360,7 @@ class AtomEnvironment { this.grammars.clear() this.textEditors.clear() this.views.clear() + this.pathsWithWaitSessions.clear() } destroy () { @@ -822,7 +824,22 @@ class AtomEnvironment { this.document.body.appendChild(this.workspace.getElement()) if (this.backgroundStylesheet) this.backgroundStylesheet.remove() - this.watchProjectPaths() + let previousProjectPaths = this.project.getPaths() + this.disposables.add(this.project.onDidChangePaths(newPaths => { + for (let path of previousProjectPaths) { + if (this.pathsWithWaitSessions.has(path) && !newPaths.includes(path)) { + this.applicationDelegate.didClosePathWithWaitSession(path) + } + } + previousProjectPaths = newPaths + this.applicationDelegate.setRepresentedDirectoryPaths(newPaths) + })) + this.disposables.add(this.workspace.onDidDestroyPaneItem(({item}) => { + const path = item.getPath && item.getPath() + if (this.pathsWithWaitSessions.has(path)) { + this.applicationDelegate.didClosePathWithWaitSession(path) + } + })) this.packages.activate() this.keymaps.loadUserKeymap() @@ -1025,13 +1042,6 @@ class AtomEnvironment { return this.themes.load() } - // Notify the browser project of the window's current project path - watchProjectPaths () { - this.disposables.add(this.project.onDidChangePaths(() => { - this.applicationDelegate.setRepresentedDirectoryPaths(this.project.getPaths()) - })) - } - setDocumentEdited (edited) { if (typeof this.applicationDelegate.setWindowDocumentEdited === 'function') { this.applicationDelegate.setWindowDocumentEdited(edited) @@ -1300,8 +1310,9 @@ class AtomEnvironment { } } - for (var {pathToOpen, initialLine, initialColumn, forceAddToWindow} of locations) { - if (pathToOpen && (needsProjectPaths || forceAddToWindow)) { + for (const location of locations) { + const {pathToOpen} = location + if (pathToOpen && (needsProjectPaths || location.forceAddToWindow)) { if (fs.existsSync(pathToOpen)) { pushFolderToOpen(this.project.getDirectoryForProjectPath(pathToOpen).getPath()) } else if (fs.existsSync(path.dirname(pathToOpen))) { @@ -1312,8 +1323,10 @@ class AtomEnvironment { } if (!fs.isDirectorySync(pathToOpen)) { - fileLocationsToOpen.push({pathToOpen, initialLine, initialColumn}) + fileLocationsToOpen.push(location) } + + if (location.hasWaitSession) this.pathsWithWaitSessions.add(pathToOpen) } let restoredState = false @@ -1334,7 +1347,7 @@ class AtomEnvironment { if (!restoredState) { const fileOpenPromises = [] - for ({pathToOpen, initialLine, initialColumn} of fileLocationsToOpen) { + for (const {pathToOpen, initialLine, initialColumn} of fileLocationsToOpen) { fileOpenPromises.push(this.workspace && this.workspace.open(pathToOpen, {initialLine, initialColumn})) } await Promise.all(fileOpenPromises) diff --git a/src/main-process/atom-application.js b/src/main-process/atom-application.js index 459520722..372bd537c 100644 --- a/src/main-process/atom-application.js +++ b/src/main-process/atom-application.js @@ -101,9 +101,10 @@ class AtomApplication extends EventEmitter { this.socketPath = options.socketPath this.logFile = options.logFile this.userDataDir = options.userDataDir + this._killProcess = options.killProcess || process.kill.bind(process) if (options.test || options.benchmark || options.benchmarkTest) this.socketPath = null - this.pidsToOpenWindows = {} + this.waitSessionsByWindow = new Map() this.windowStack = new WindowStack() this.config = new Config({enablePersistence: true}) @@ -788,13 +789,17 @@ class AtomApplication extends EventEmitter { safeMode = Boolean(safeMode) clearWindowState = Boolean(clearWindowState) - const locationsToOpen = pathsToOpen.map(pathToOpen => - this.locationForPathToOpen(pathToOpen, executedFrom, addToLastWindow) - ) - pathsToOpen = locationsToOpen.map(locationToOpen => locationToOpen.pathToOpen) + const locationsToOpen = [] + for (let i = 0; i < pathsToOpen.length; i++) { + const location = this.parsePathToOpen(pathsToOpen[i], executedFrom, addToLastWindow) + location.forceAddToWindow = addToLastWindow + location.hasWaitSession = pidToKillWhenClosed != null + locationsToOpen.push(location) + pathsToOpen[i] = location.pathToOpen + } let existingWindow - if (!pidToKillWhenClosed && !newWindow) { + if (!newWindow) { existingWindow = this.windowForPaths(pathsToOpen, devMode) const stats = pathsToOpen.map(pathToOpen => fs.statSyncNoException(pathToOpen)) if (!existingWindow) { @@ -852,26 +857,44 @@ class AtomApplication extends EventEmitter { } if (pidToKillWhenClosed != null) { - this.pidsToOpenWindows[pidToKillWhenClosed] = openedWindow + if (!this.waitSessionsByWindow.has(openedWindow)) { + this.waitSessionsByWindow.set(openedWindow, []) + } + this.waitSessionsByWindow.get(openedWindow).push({ + pid: pidToKillWhenClosed, + remainingPaths: new Set(pathsToOpen) + }) } - openedWindow.browserWindow.once('closed', () => this.killProcessForWindow(openedWindow)) + openedWindow.browserWindow.once('closed', () => this.killProcessesForWindow(openedWindow)) return openedWindow } // Kill all processes associated with opened windows. killAllProcesses () { - for (let pid in this.pidsToOpenWindows) { - this.killProcess(pid) + for (let window of this.waitSessionsByWindow.keys()) { + this.killProcessesForWindow(window) } } - // Kill process associated with the given opened window. - killProcessForWindow (openedWindow) { - for (let pid in this.pidsToOpenWindows) { - const trackedWindow = this.pidsToOpenWindows[pid] - if (trackedWindow === openedWindow) { - this.killProcess(pid) + killProcessesForWindow (window) { + const sessions = this.waitSessionsByWindow.get(window) + if (!sessions) return + for (const session of sessions) { + this.killProcess(session.pid) + } + this.waitSessionsByWindow.delete(window) + } + + windowDidClosePathWithWaitSession (window, initialPath) { + const waitSessions = this.waitSessionsByWindow.get(window) + if (!waitSessions) return + for (let i = waitSessions.length - 1; i >= 0; i--) { + const session = waitSessions[i] + session.remainingPaths.delete(initialPath) + if (session.remainingPaths.size === 0) { + this.killProcess(session.pid) + waitSessions.splice(i, 1) } } } @@ -880,13 +903,12 @@ class AtomApplication extends EventEmitter { killProcess (pid) { try { const parsedPid = parseInt(pid) - if (isFinite(parsedPid)) process.kill(parsedPid) + if (isFinite(parsedPid)) this._killProcess(parsedPid) } catch (error) { if (error.code !== 'ESRCH') { console.log(`Killing process ${pid} failed: ${error.code != null ? error.code : error.message}`) } } - delete this.pidsToOpenWindows[pid] } saveState (allowEmpty = false) { @@ -1192,7 +1214,7 @@ class AtomApplication extends EventEmitter { } } - locationForPathToOpen (pathToOpen, executedFrom = '', forceAddToWindow) { + parsePathToOpen (pathToOpen, executedFrom = '') { let initialColumn, initialLine if (!pathToOpen) { return {pathToOpen} @@ -1217,7 +1239,7 @@ class AtomApplication extends EventEmitter { pathToOpen = path.resolve(executedFrom, fs.normalize(pathToOpen)) } - return {pathToOpen, initialLine, initialColumn, forceAddToWindow} + return {pathToOpen, initialLine, initialColumn} } // Opens a native dialog to prompt the user for a path. diff --git a/src/main-process/atom-window.js b/src/main-process/atom-window.js index 582852ad4..0492b5f8f 100644 --- a/src/main-process/atom-window.js +++ b/src/main-process/atom-window.js @@ -411,6 +411,10 @@ class AtomWindow extends EventEmitter { return this.atomApplication.saveState() } + didClosePathWithWaitSession (path) { + this.atomApplication.windowDidClosePathWithWaitSession(this, path) + } + copy () { return this.browserWindow.copy() }