From 0f6eadcfceb27784a65f9898b301fa5839c71210 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 7 Sep 2016 11:27:47 +0200 Subject: [PATCH] Wait for windows' state to be saved before closing the app or any window Previously, we used to save the window's state in the renderer process `beforeunload` event handler: because of the synchronous nature of event handlers and the asynchronous design of IndexedDB, this could potentially not save anything if windows close fast enough to prevent IndexedDB from committing the pending transaction containing the state. (Ref.: https://mzl.la/2bXCXDn) With this commit, we will intercept the `before-quit` events on `electron.app` and the `close` event on `BrowserWindow` (which will fire respectively before quitting the application and before closing a window), and prevent them from performing the default action. We will then ask each renderer process to save its state and, finally, close the window and/or the app. --- spec/atom-environment-spec.coffee | 7 ++---- spec/integration/helpers/start-atom.coffee | 9 +------- src/application-delegate.coffee | 11 +++++++++ src/atom-environment.coffee | 26 ++++++++++++---------- src/main-process/atom-application.coffee | 12 ++++++---- src/main-process/atom-window.coffee | 22 +++++++++++++++--- 6 files changed, 55 insertions(+), 32 deletions(-) diff --git a/spec/atom-environment-spec.coffee b/spec/atom-environment-spec.coffee index fa8c3cc5d..cd8408a73 100644 --- a/spec/atom-environment-spec.coffee +++ b/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 diff --git a/spec/integration/helpers/start-atom.coffee b/spec/integration/helpers/start-atom.coffee index 17f11fe2c..1eb610a2f 100644 --- a/spec/integration/helpers/start-atom.coffee +++ b/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) -> diff --git a/src/application-delegate.coffee b/src/application-delegate.coffee index 853ac54f5..f2979ba0d 100644 --- a/src/application-delegate.coffee +++ b/src/application-delegate.coffee @@ -244,6 +244,17 @@ class ApplicationDelegate new Disposable -> ipcRenderer.removeListener('context-command', outerCallback) + onSaveWindowState: (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') diff --git a/src/atom-environment.coffee b/src/atom-environment.coffee index 803e5246a..977f9d70b 100644 --- a/src/atom-environment.coffee +++ b/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.onSaveWindowState => + 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 diff --git a/src/main-process/atom-application.coffee b/src/main-process/atom-application.coffee index 0eebf043f..701710337 100644 --- a/src/main-process/atom-application.coffee +++ b/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) -> diff --git a/src/main-process/atom-window.coffee b/src/main-process/atom-window.coffee index effef48ab..b0490f940 100644 --- a/src/main-process/atom-window.coffee +++ b/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,18 @@ class AtomWindow @browserWindow.on 'blur', => @browserWindow.focusOnWebView() + didCancelWindowUnload: -> + @unloading = false + + saveState: -> + 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') + openPath: (pathToOpen, initialLine, initialColumn) -> @openLocations([{pathToOpen, initialLine, initialColumn}])