From a7655179c5bf10e2d40b1e8d94445f3126bcd79c Mon Sep 17 00:00:00 2001 From: bridiver Date: Sun, 30 Apr 2017 23:17:06 -0700 Subject: [PATCH] use write.writeImportant for session store fix #7876 --- app/filtering.js | 9 +-- app/index.js | 142 ++++++++++++++++++++++++-------------------- app/sessionStore.js | 23 ++++--- js/entry.js | 4 +- package.json | 4 ++ 5 files changed, 96 insertions(+), 86 deletions(-) diff --git a/app/filtering.js b/app/filtering.js index 40a63612a9a..3676930c5bf 100644 --- a/app/filtering.js +++ b/app/filtering.js @@ -16,7 +16,6 @@ const downloadStates = require('../js/constants/downloadStates') const urlParse = require('./common/urlParse') const getSetting = require('../js/settings').getSetting const appUrlUtil = require('../js/lib/appUrlUtil') -const promisify = require('../js/lib/promisify') const siteSettings = require('../js/state/siteSettings') const settings = require('../js/constants/settings') const userPrefs = require('../js/state/userPrefs') @@ -727,12 +726,10 @@ module.exports.isResourceEnabled = (resourceName, url, isPrivate) => { * @return a promise that always resolves (called on app shutdon so must always) */ module.exports.clearStorageData = () => { - let p = Promise.resolve() for (let partition in registeredSessions) { let ses = registeredSessions[partition] - p = p.then(promisify(ses.clearStorageData.bind(ses)).catch(() => {})) + setImmediate(ses.clearStorageData.bind(ses)) } - return p } /** @@ -740,12 +737,10 @@ module.exports.clearStorageData = () => { * @return a promise that always resolves (called on app shutdon so must always) */ module.exports.clearCache = () => { - let p = Promise.resolve() for (let partition in registeredSessions) { let ses = registeredSessions[partition] - p = p.then(promisify(ses.clearCache.bind(ses)).catch(() => {})) + setImmediate(ses.clearCache.bind(ses)) } - return p } module.exports.setDefaultZoomLevel = (zoom) => { diff --git a/app/index.js b/app/index.js index b1b217f60fc..4e700099c89 100644 --- a/app/index.js +++ b/app/index.js @@ -81,7 +81,9 @@ app.commandLine.appendSwitch('enable-features', 'BlockSmallPluginContent,PreferH // Used to collect the per window state when shutting down the application let perWindowState = [] -let sessionStateStoreCompleteOnQuit = false +let sessionStateStoreComplete = false +let sessionStateStoreCompleteCallback = null +let requestId = 0 let shuttingDown = false let lastWindowState let lastWindowClosed = false @@ -99,56 +101,59 @@ const sessionStoreQueue = async.queue((task, callback) => { task(callback) }, 1) -const saveIfAllCollected = (forceSave) => { +const logSaveAppStateError = (e) => { + console.error('Error saving app state: ', e) +} + +const saveAppState = (forceSave = false) => { // If we're shutting down early and can't access the state, it's better // to not try to save anything at all and just quit. if (shuttingDown && !AppStore.getState()) { app.exit(0) } - if (forceSave || perWindowState.length === BrowserWindow.getAllWindows().length) { - const appState = AppStore.getState().toJS() - appState.perWindowState = perWindowState - if (shuttingDown) { - // If the status is still UPDATE_AVAILABLE then the user wants to quit - // and not restart - if (appState.updates && (appState.updates.status === UpdateStatus.UPDATE_AVAILABLE || - appState.updates.status === UpdateStatus.UPDATE_AVAILABLE_DEFERRED)) { - // In this case on win32, the process doesn't try to auto restart, so avoid the user - // having to open the app twice. Maybe squirrel detects the app is already shutting down. - if (process.platform === 'win32') { - appState.updates.status = UpdateStatus.UPDATE_APPLYING_RESTART - } else { - appState.updates.status = UpdateStatus.UPDATE_APPLYING_NO_RESTART - } - } - } - sessionStoreQueue.push(saveAppState.bind(null, appState)) - } -} + const appState = AppStore.getState().toJS() + appState.perWindowState = perWindowState -const logSaveAppStateError = (e) => { - console.error('Error saving app state: ', e) -} + const receivedAllWindows = perWindowState.length === BrowserWindow.getAllWindows().length + if (!forceSave && !receivedAllWindows) { + return + } -const saveAppState = (appState, cb) => { - SessionStore.saveAppState(appState, shuttingDown).catch((e) => { + return SessionStore.saveAppState(appState, shuttingDown).catch((e) => { logSaveAppStateError(e) - cb() }).then(() => { - if (shuttingDown) { - sessionStateStoreCompleteOnQuit = true - // If there's an update to apply, then do it here. - // Otherwise just quit. - if (appState.updates && (appState.updates.status === UpdateStatus.UPDATE_APPLYING_NO_RESTART || - appState.updates.status === UpdateStatus.UPDATE_APPLYING_RESTART)) { - Updater.quitAndInstall() + if (receivedAllWindows || forceSave) { + sessionStateStoreComplete = true + } + + if (sessionStateStoreComplete) { + if (shuttingDown) { + // If the status is still UPDATE_AVAILABLE then the user wants to quit + // and not restart + if (appState.updates && (appState.updates.status === UpdateStatus.UPDATE_AVAILABLE || + appState.updates.status === UpdateStatus.UPDATE_AVAILABLE_DEFERRED)) { + // In this case on win32, the process doesn't try to auto restart, so avoid the user + // having to open the app twice. Maybe squirrel detects the app is already shutting down. + if (process.platform === 'win32') { + appState.updates.status = UpdateStatus.UPDATE_APPLYING_RESTART + } else { + appState.updates.status = UpdateStatus.UPDATE_APPLYING_NO_RESTART + } + } + + // If there's an update to apply, then do it here. + // Otherwise just quit. + if (appState.updates && (appState.updates.status === UpdateStatus.UPDATE_APPLYING_NO_RESTART || + appState.updates.status === UpdateStatus.UPDATE_APPLYING_RESTART)) { + Updater.quitAndInstall() + } else { + app.quit() + } } else { - app.quit() + sessionStateStoreCompleteCallback() + sessionStateStoreCompleteCallback = null } - // no callback here because we don't want to get a partial write during shutdown - } else { - cb() } }) } @@ -156,20 +161,27 @@ const saveAppState = (appState, cb) => { /** * Saves the session storage for all windows */ -const initiateSessionStateSave = (beforeQuit) => { - if (shuttingDown && !beforeQuit) { - return - } - - perWindowState.length = 0 - // quit triggered by window-all-closed should save last window state - if (lastWindowClosed && lastWindowState) { - perWindowState.push(lastWindowState) - saveIfAllCollected(true) - } else { - BrowserWindow.getAllWindows().forEach((win) => win.webContents.send(messages.REQUEST_WINDOW_STATE)) - saveIfAllCollected() - } +const initiateSessionStateSave = () => { + sessionStoreQueue.push((cb) => { + sessionStateStoreComplete = false + sessionStateStoreCompleteCallback = cb + + perWindowState.length = 0 + // quit triggered by window-all-closed should save last window state + if (lastWindowClosed && lastWindowState) { + perWindowState.push(lastWindowState) + } else if (BrowserWindow.getAllWindows().length > 0) { + ++requestId + BrowserWindow.getAllWindows().forEach((win) => win.webContents.send(messages.REQUEST_WINDOW_STATE, requestId)) + // Just in case a window is not responsive, we don't want to wait forever. + // In this case just save session store for the windows that we have already. + setTimeout(() => { + saveAppState(true) + }, appConfig.quitTimeout) + } else { + saveAppState() + } + }) } let loadAppStatePromise = SessionStore.loadAppState() @@ -242,26 +254,22 @@ app.on('ready', () => { }) app.on('before-quit', (e) => { - if (sessionStateStoreCompleteOnQuit) { + if (shuttingDown && sessionStateStoreComplete) { return } + e.preventDefault() // before-quit can be triggered multiple times because of the preventDefault call if (shuttingDown) { return + } else { + shuttingDown = true } - shuttingDown = true appActions.shuttingDown() clearInterval(sessionStateSaveInterval) - initiateSessionStateSave(true) - - // Just in case a window is not responsive, we don't want to wait forever. - // In this case just save session store for the windows that we have already. - setTimeout(() => { - saveIfAllCollected(true) - }, appConfig.quitTimeout) + initiateSessionStateSave() }) app.on('network-connected', () => { @@ -273,14 +281,18 @@ app.on('ready', () => { }) // User initiated exit using File->Quit - ipcMain.on(messages.RESPONSE_WINDOW_STATE, (wnd, data) => { + ipcMain.on(messages.RESPONSE_WINDOW_STATE, (evt, data, id) => { + if (id !== requestId) { + return + } + if (data) { perWindowState.push(data) } - saveIfAllCollected() + saveAppState() }) - ipcMain.on(messages.LAST_WINDOW_STATE, (wnd, data) => { + ipcMain.on(messages.LAST_WINDOW_STATE, (evt, data) => { if (data) { lastWindowState = data } diff --git a/app/sessionStore.js b/app/sessionStore.js index a83d83eb50b..648dba49465 100644 --- a/app/sessionStore.js +++ b/app/sessionStore.js @@ -33,7 +33,6 @@ const tabState = require('./common/state/tabState') const windowState = require('./common/state/windowState') const getSetting = require('../js/settings').getSetting -const promisify = require('../js/lib/promisify') const sessionStorageName = `session-store-${sessionStorageVersion}` const getTopSiteMap = () => { @@ -99,15 +98,17 @@ module.exports.saveAppState = (payload, isShutdown) => { } payload.lastAppVersion = app.getVersion() - const tmpStoragePath = getTempStoragePath() - - let p = promisify(fs.writeFile, tmpStoragePath, JSON.stringify(payload)) - .then(() => promisify(fs.rename, tmpStoragePath, getStoragePath())) if (isShutdown) { - p = p.then(module.exports.cleanSessionDataOnShutdown()) + module.exports.cleanSessionDataOnShutdown() } - p.then(resolve) - .catch(reject) + + muon.file.writeImportant(getStoragePath(), JSON.stringify(payload), (success) => { + if (success) { + resolve() + } else { + reject(new Error('Could not save app state to ' + getStoragePath())) + } + }) }) } @@ -381,14 +382,12 @@ module.exports.cleanAppData = (data, isShutdown) => { * @return a promise which resolve when the work is done. */ module.exports.cleanSessionDataOnShutdown = () => { - let p = Promise.resolve() if (getSetting(settings.SHUTDOWN_CLEAR_ALL_SITE_COOKIES) === true) { - p = p.then(filtering.clearStorageData()) + filtering.clearStorageData() } if (getSetting(settings.SHUTDOWN_CLEAR_CACHE) === true) { - p = p.then(filtering.clearCache()) + filtering.clearCache() } - return p } const safeGetVersion = (fieldName, getFieldVersion) => { diff --git a/js/entry.js b/js/entry.js index 351e640d8ac..cc0c0e1984b 100644 --- a/js/entry.js +++ b/js/entry.js @@ -38,8 +38,8 @@ webFrame.setPageScaleLimits(1, 1) l10n.init() -ipc.on(messages.REQUEST_WINDOW_STATE, () => { - ipc.send(messages.RESPONSE_WINDOW_STATE, windowStore.getState().toJS()) +ipc.on(messages.REQUEST_WINDOW_STATE, (evt, requestId) => { + ipc.send(messages.RESPONSE_WINDOW_STATE, windowStore.getState().toJS(), requestId) }) if (process.env.NODE_ENV === 'test') { diff --git a/package.json b/package.json index d1a954a8d3b..8ef96040cfb 100644 --- a/package.json +++ b/package.json @@ -197,6 +197,10 @@ "electron-installer-redhat": "^0.3.0" }, "standard": { + "global": [ + "chrome", + "muon" + ], "ignore": [ "app/extensions/**", "app/browser/ads/adDivCandidates.js",