Merge pull request #12619 from atom/as-save-window-state-before-closing-window-or-app

Wait for windows' state to be saved before closing the app or any window

spec/atom-environment-spec.coffee

@@ -228,7 +228,7 @@ describe "AtomEnvironment", ->
       expect(atom.saveState).toHaveBeenCalledWith({isUnloading: false})
       expect(atom.saveState).not.toHaveBeenCalledWith({isUnloading: true})
 
-    it "saves state immediately when unloading the editor window, ignoring pending and successive mousedown/keydown events", ->
+    it "ignores mousedown/keydown events happening after calling unloadEditorWindow", ->
       spyOn(atom, 'saveState')
       idleCallbacks = []
       spyOn(window, 'requestIdleCallback').andCallFake (callback) -> idleCallbacks.push(callback)
@@ -236,15 +236,12 @@ describe "AtomEnvironment", ->
       mousedown = new MouseEvent('mousedown')
       atom.document.dispatchEvent(mousedown)
       atom.unloadEditorWindow()
-      expect(atom.saveState).toHaveBeenCalledWith({isUnloading: true})
-      expect(atom.saveState).not.toHaveBeenCalledWith({isUnloading: false})
+      expect(atom.saveState).not.toHaveBeenCalled()
 
-      atom.saveState.reset()
       advanceClock atom.saveStateDebounceInterval
       idleCallbacks.shift()()
       expect(atom.saveState).not.toHaveBeenCalled()
 
-      atom.saveState.reset()
       mousedown = new MouseEvent('mousedown')
       atom.document.dispatchEvent(mousedown)
       advanceClock atom.saveStateDebounceInterval

spec/integration/helpers/start-atom.coffee

@@ -125,11 +125,6 @@ buildAtomClient = (args, env) ->
       @execute "atom.commands.dispatch(document.activeElement, '#{command}')"
       .call(done)
 
-    .addCommand "simulateQuit", (done) ->
-      @execute -> atom.unloadEditorWindow()
-      .execute -> require("electron").remote.app.emit("before-quit")
-      .call(done)
-
 module.exports = (args, env, fn) ->
   [chromedriver, chromedriverLogs, chromedriverExit] = []
 
@@ -154,9 +149,7 @@ module.exports = (args, env, fn) ->
 
   waitsFor("tests to run", (done) ->
     finish = once ->
-      client
-        .simulateQuit()
-        .end()
+      client.end()
         .then(-> chromedriver.kill())
         .then(chromedriverExit.then(
           (errorCode) ->

spec/main-process/atom-application.test.js

@@ -14,9 +14,10 @@ const ATOM_RESOURCE_PATH = path.resolve(__dirname, '..', '..')
 describe('AtomApplication', function () {
   this.timeout(60 * 1000)
 
-  let originalAtomHome, atomApplicationsToDestroy
+  let originalAppQuit, originalAtomHome, atomApplicationsToDestroy
 
   beforeEach(function () {
+    originalAppQuit = electron.app.quit
     originalAtomHome = process.env.ATOM_HOME
     process.env.ATOM_HOME = makeTempDir('atom-home')
     // Symlinking the compile cache into the temporary home dir makes the windows load much faster
@@ -31,6 +32,7 @@ describe('AtomApplication', function () {
   })
 
   afterEach(async function () {
+    electron.app.quit = originalAppQuit
     process.env.ATOM_HOME = originalAtomHome
     for (let atomApplication of atomApplicationsToDestroy) {
       await atomApplication.destroy()
@@ -368,6 +370,23 @@ describe('AtomApplication', function () {
     })
   })
 
+  describe('before quitting', function () {
+    it('waits until all the windows have saved their state and then quits', async function () {
+      mockElectronAppQuit()
+      const dirAPath = makeTempDir("a")
+      const dirBPath = makeTempDir("b")
+      const atomApplication = buildAtomApplication()
+      const window1 = atomApplication.launch(parseCommandLine([path.join(dirAPath, 'file-a')]))
+      await focusWindow(window1)
+      const window2 = atomApplication.launch(parseCommandLine([path.join(dirBPath, 'file-b')]))
+      await focusWindow(window2)
+      electron.app.quit()
+      assert(!electron.app.hasQuitted())
+      await Promise.all([window1.lastSaveStatePromise, window2.lastSaveStatePromise])
+      assert(electron.app.hasQuitted())
+    })
+  })
+
   function buildAtomApplication () {
     const atomApplication = new AtomApplication({
       resourcePath: ATOM_RESOURCE_PATH,
@@ -383,6 +402,20 @@ describe('AtomApplication', function () {
     await conditionPromise(() => window.atomApplication.lastFocusedWindow === window)
   }
 
+  function mockElectronAppQuit () {
+    let quitted = false
+    electron.app.quit = function () {
+      let shouldQuit = true
+      electron.app.emit('before-quit', {preventDefault: () => { shouldQuit = false }})
+      if (shouldQuit) {
+        quitted = true
+      }
+    }
+    electron.app.hasQuitted = function () {
+      return quitted
+    }
+  }
+
   function makeTempDir (name) {
     return fs.realpathSync(require('temp').mkdirSync(name))
   }

src/application-delegate.coffee

@@ -244,6 +244,17 @@ class ApplicationDelegate
     new Disposable ->
       ipcRenderer.removeListener('context-command', outerCallback)
 
+  onSaveWindowStateRequest: (callback) ->
+    outerCallback = (event, message) ->
+      callback(event)
+
+    ipcRenderer.on('save-window-state', outerCallback)
+    new Disposable ->
+      ipcRenderer.removeListener('save-window-state', outerCallback)
+
+  didSaveWindowState: ->
+    ipcRenderer.send('did-save-window-state')
+
   didCancelWindowUnload: ->
     ipcRenderer.send('did-cancel-window-unload')
 

src/atom-environment.coffee

@@ -674,6 +674,10 @@ class AtomEnvironment extends Model
         @disposables.add(@applicationDelegate.onDidOpenLocations(@openLocations.bind(this)))
         @disposables.add(@applicationDelegate.onApplicationMenuCommand(@dispatchApplicationMenuCommand.bind(this)))
         @disposables.add(@applicationDelegate.onContextMenuCommand(@dispatchContextMenuCommand.bind(this)))
+        @disposables.add @applicationDelegate.onSaveWindowStateRequest =>
+          callback = => @applicationDelegate.didSaveWindowState()
+          @saveState({isUnloading: true}).catch(callback).then(callback)
+
         @listenForUpdates()
 
         @registerDefaultTargetForKeymaps()
@@ -713,7 +717,6 @@ class AtomEnvironment extends Model
   unloadEditorWindow: ->
     return if not @project
 
-    @saveState({isUnloading: true})
     @storeWindowBackground()
     @packages.deactivatePackages()
     @saveBlobStoreSync()
@@ -851,18 +854,17 @@ class AtomEnvironment extends Model
     @blobStore.save()
 
   saveState: (options) ->
-    return Promise.resolve() unless @enablePersistence
-
     new Promise (resolve, reject) =>
-      return if not @project
-
-      state = @serialize(options)
-      savePromise =
-        if storageKey = @getStateKey(@project?.getPaths())
-          @stateStore.save(storageKey, state)
-        else
-          @applicationDelegate.setTemporaryWindowState(state)
-      savePromise.catch(reject).then(resolve)
+      if @enablePersistence and @project
+        state = @serialize(options)
+        savePromise =
+          if storageKey = @getStateKey(@project?.getPaths())
+            @stateStore.save(storageKey, state)
+          else
+            @applicationDelegate.setTemporaryWindowState(state)
+        savePromise.catch(reject).then(resolve)
+      else
+        resolve()
 
   loadState: ->
     if @enablePersistence

src/main-process/atom-application.coffee

@@ -96,11 +96,10 @@ class AtomApplication
     @launch(options)
 
   destroy: ->
-    @disposable.dispose()
     windowsClosePromises = @windows.map (window) ->
       window.close()
       window.closedPromise
-    Promise.all(windowsClosePromises)
+    Promise.all(windowsClosePromises).then(=> @disposable.dispose())
 
   launch: (options) ->
     if options.pathsToOpen?.length > 0 or options.urlsToOpen?.length > 0 or options.test
@@ -233,8 +232,11 @@ class AtomApplication
     @openPathOnEvent('application:open-your-stylesheet', 'atom://.atom/stylesheet')
     @openPathOnEvent('application:open-license', path.join(process.resourcesPath, 'LICENSE.md'))
 
-    @disposable.add ipcHelpers.on app, 'before-quit', =>
-      @quitting = true
+    @disposable.add ipcHelpers.on app, 'before-quit', (event) =>
+      unless @quitting
+        event.preventDefault()
+        @quitting = true
+        Promise.all(@windows.map((window) -> window.saveState())).then(-> app.quit())
 
     @disposable.add ipcHelpers.on app, 'will-quit', =>
       @killAllProcesses()
@@ -322,6 +324,8 @@ class AtomApplication
 
     @disposable.add ipcHelpers.on ipcMain, 'did-cancel-window-unload', =>
       @quitting = false
+      for window in @windows
+        window.didCancelWindowUnload()
 
     clipboard = require '../safe-clipboard'
     @disposable.add ipcHelpers.on ipcMain, 'write-text-to-selection-clipboard', (event, selectedText) ->

src/main-process/atom-window.coffee

@@ -1,4 +1,4 @@
-{BrowserWindow, app, dialog} = require 'electron'
+{BrowserWindow, app, dialog, ipcMain} = require 'electron'
 path = require 'path'
 fs = require 'fs'
 url = require 'url'
@@ -128,8 +128,12 @@ class AtomWindow
         false
 
   handleEvents: ->
-    @browserWindow.on 'close', =>
-      @atomApplication.saveState(false)
+    @browserWindow.on 'close', (event) =>
+      unless @atomApplication.quitting or @unloading
+        event.preventDefault()
+        @unloading = true
+        @atomApplication.saveState(false)
+        @saveState().then(=> @close())
 
     @browserWindow.on 'closed', =>
       @fileRecoveryService.didCloseWindow(this)
@@ -170,6 +174,19 @@ class AtomWindow
       @browserWindow.on 'blur', =>
         @browserWindow.focusOnWebView()
 
+  didCancelWindowUnload: ->
+    @unloading = false
+
+  saveState: ->
+    @lastSaveStatePromise = new Promise (resolve) =>
+      callback = (event) =>
+        if BrowserWindow.fromWebContents(event.sender) is @browserWindow
+          ipcMain.removeListener('did-save-window-state', callback)
+          resolve()
+      ipcMain.on('did-save-window-state', callback)
+      @browserWindow.webContents.send('save-window-state')
+    @lastSaveStatePromise
+
   openPath: (pathToOpen, initialLine, initialColumn) ->
     @openLocations([{pathToOpen, initialLine, initialColumn}])