Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

use write.writeImportant for session store #8593

Merged
merged 1 commit into from
May 2, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions app/filtering.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -727,25 +726,21 @@ 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
}

/**
* Clears all session cache.
* @return a promise that always resolves (called on app shutdon so must always)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be updated; doesn't return a promise anymore

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, that's true for several of these now

*/
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))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there are docs on the Muon side, I think it would be helpful to put a link to the docs as a comment; ex:
https://github.com/brave/muon/blob/master/docs/api/session.md#sesclearcachecallback

(and if needed, update the docs)

}
return p
}

module.exports.setDefaultZoomLevel = (zoom) => {
Expand Down
142 changes: 77 additions & 65 deletions app/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -99,77 +101,87 @@ 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()
}
})
}

/**
* 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()
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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
}
Expand Down
30 changes: 13 additions & 17 deletions app/sessionStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => {
Expand Down Expand Up @@ -72,11 +71,8 @@ const getStoragePath = () => {
module.exports.saveAppState = (payload, isShutdown) => {
return new Promise((resolve, reject) => {
// Don't persist private frames
let startupModeSettingValue
if (require('../js/stores/appStore').getState()) {
startupModeSettingValue = getSetting(settings.STARTUP_MODE)
}
const savePerWindowState = startupModeSettingValue === undefined ||
let startupModeSettingValue = getSetting(settings.STARTUP_MODE)
const savePerWindowState = startupModeSettingValue == null ||
startupModeSettingValue === 'lastTime'
if (payload.perWindowState && savePerWindowState) {
payload.perWindowState.forEach((wndPayload) => {
Expand All @@ -99,15 +95,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()))
}
})
})
}

Expand Down Expand Up @@ -381,14 +379,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) => {
Expand Down
4 changes: 2 additions & 2 deletions js/entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand Down
7 changes: 4 additions & 3 deletions js/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,10 @@ const getDefaultSetting = (settingKey, settingsCollection) => {
}

const resolveValue = (settingKey, settingsCollection) => {
const appSettings = (process.type === 'browser'
? require('./stores/appStore').getState().get('settings')
: require('./stores/appStoreRenderer').state.get('settings')) || Immutable.Map()
const appStore = (process.type === 'browser'
? require('./stores/appStore').getState()
: require('./stores/appStoreRenderer').state) || Immutable.Map()
const appSettings = appStore.get('settings') || Immutable.Map()
if (settingsCollection && settingsCollection.constructor === Immutable.Map &&
settingsCollection.get(settingKey) !== undefined) {
return settingsCollection.get(settingKey)
Expand Down
4 changes: 4 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,10 @@
"electron-installer-redhat": "^0.3.0"
},
"standard": {
"global": [
"chrome",
"muon"
],
"ignore": [
"app/extensions/**",
"app/browser/ads/adDivCandidates.js",
Expand Down
Loading