From e85b8738d1c530f9b66a05061bd703c0ecc4388b Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 10 Jan 2018 13:11:25 +0100 Subject: [PATCH 1/2] Allow destroying AtomEnvironment instances that haven't been initialized --- src/atom-environment.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/atom-environment.js b/src/atom-environment.js index 159464534..9fe306ea7 100644 --- a/src/atom-environment.js +++ b/src/atom-environment.js @@ -373,7 +373,7 @@ class AtomEnvironment { if (this.project) this.project.destroy() this.project = null this.commands.clear() - this.stylesElement.remove() + if (this.stylesElement) this.stylesElement.remove() this.config.unobserveUserConfig() this.autoUpdater.destroy() this.uriHandlerRegistry.destroy() From a5c0223592ea99ec7b5514b9e5acd2903082f895 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 10 Jan 2018 13:22:09 +0100 Subject: [PATCH 2/2] Fix race condition between opening new files and restoring window state This commit fixes a race condition in the `attemptRestoreProjectStateForPaths` function that could cause a file to be opened more than once within the same workspace pane. In particular, when opening some file into an empty window, Atom tries to recover the state for the project containing the file (if there is one). However, we were previously not waiting until the `AtomEnvironment`'s state had been fully deserialized before trying to load the requested file into the workspace. If the same file also existed in the serialized representation of the workspace, it could end up being opened twice. With this commit we will now wait until the environment has been fully deserialized before honoring the user's request of loading new files into an empty window. Also, tests have been restructured to test more thoroughly this interaction. --- spec/atom-environment-spec.js | 30 ++++++++++++++++++++++-------- src/atom-environment.js | 2 +- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/spec/atom-environment-spec.js b/spec/atom-environment-spec.js index 7dc07dafd..324e9eddf 100644 --- a/spec/atom-environment-spec.js +++ b/spec/atom-environment-spec.js @@ -1,5 +1,6 @@ const {it, fit, ffit, beforeEach, afterEach, conditionPromise} = require('./async-spec-helpers') const _ = require('underscore-plus') +const fs = require('fs') const path = require('path') const temp = require('temp').track() const AtomEnvironment = require('../src/atom-environment') @@ -471,15 +472,28 @@ describe('AtomEnvironment', () => { await atom.workspace.open() }) - it('automatically restores the saved state into the current environment', () => { - const state = {} - spyOn(atom.workspace, 'open') - spyOn(atom, 'restoreStateIntoThisEnvironment') + it('automatically restores the saved state into the current environment', async () => { + const projectPath = temp.mkdirSync() + const filePath1 = path.join(projectPath, 'file-1') + const filePath2 = path.join(projectPath, 'file-2') + const filePath3 = path.join(projectPath, 'file-3') + fs.writeFileSync(filePath1, 'abc') + fs.writeFileSync(filePath2, 'def') + fs.writeFileSync(filePath3, 'ghi') - atom.attemptRestoreProjectStateForPaths(state, [__dirname], [__filename]) - expect(atom.restoreStateIntoThisEnvironment).toHaveBeenCalledWith(state) - expect(atom.workspace.open.callCount).toBe(1) - expect(atom.workspace.open).toHaveBeenCalledWith(__filename) + const env1 = new AtomEnvironment({applicationDelegate: atom.applicationDelegate}) + env1.project.setPaths([projectPath]) + await env1.workspace.open(filePath1) + await env1.workspace.open(filePath2) + await env1.workspace.open(filePath3) + const env1State = env1.serialize() + env1.destroy() + + const env2 = new AtomEnvironment({applicationDelegate: atom.applicationDelegate}) + await env2.attemptRestoreProjectStateForPaths(env1State, [projectPath], [filePath2]) + const restoredURIs = env2.workspace.getPaneItems().map(p => p.getURI()) + expect(restoredURIs).toEqual([filePath1, filePath2, filePath3]) + env2.destroy() }) describe('when a dock has a non-text editor', () => { diff --git a/src/atom-environment.js b/src/atom-environment.js index 9fe306ea7..a80cde66c 100644 --- a/src/atom-environment.js +++ b/src/atom-environment.js @@ -1121,7 +1121,7 @@ class AtomEnvironment { } if (windowIsUnused()) { - this.restoreStateIntoThisEnvironment(state) + await this.restoreStateIntoThisEnvironment(state) return Promise.all(filesToOpen.map(file => this.workspace.open(file))) } else { let resolveDiscardStatePromise = null