From 0ff36680025abd8e52dd21898a50247f48249134 Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Fri, 4 Aug 2017 19:31:08 -0400 Subject: [PATCH] Stop beach balling with this one simple trick Fix #10094 The problem is with toJS being called to convert the immutable app state Instead we just keep everything in Immutable --- app/common/state/immutableUtil.js | 9 + app/common/state/tabState.js | 2 +- app/common/state/windowState.js | 3 +- app/index.js | 29 +- app/sessionStore.js | 408 ++++++++++-------- app/sessionStoreShutdown.js | 51 +-- js/constants/appConfig.js | 2 +- js/stores/appStore.js | 61 ++- .../app/common/state/immutableUtilTest.js | 110 +++++ test/unit/app/sessionStoreShutdownTest.js | 30 +- test/unit/app/sessionStoreTest.js | 174 ++++---- 11 files changed, 512 insertions(+), 367 deletions(-) create mode 100644 test/unit/app/common/state/immutableUtilTest.js diff --git a/app/common/state/immutableUtil.js b/app/common/state/immutableUtil.js index 0d3316d61a4..02dcb8fe1ab 100644 --- a/app/common/state/immutableUtil.js +++ b/app/common/state/immutableUtil.js @@ -29,6 +29,15 @@ const api = { makeImmutable: (obj) => { return api.isImmutable(obj) ? obj : Immutable.fromJS(obj) + }, + + deleteImmutablePaths: (obj, paths) => { + return paths.reduce((result, path) => { + if (path.constructor === Array) { + return result.deleteIn(path) + } + return result.delete(path) + }, obj) } } diff --git a/app/common/state/tabState.js b/app/common/state/tabState.js index 965125eaac2..28e77dd8ad7 100644 --- a/app/common/state/tabState.js +++ b/app/common/state/tabState.js @@ -610,7 +610,7 @@ const tabState = { state = tabState.removeTabField(state, 'messageBoxDetail') state = tabState.removeTabField(state, 'frame') state = state.delete('tabsInternal') - return state.delete('tabs') + return state.set('tabs', Immutable.List()) }, setNavigationState: (state, tabId, navigationState) => { diff --git a/app/common/state/windowState.js b/app/common/state/windowState.js index e0d265b9bdb..8b9505b7c78 100644 --- a/app/common/state/windowState.js +++ b/app/common/state/windowState.js @@ -3,6 +3,7 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ const { makeImmutable, isMap, isList } = require('./immutableUtil') +const Immutable = require('immutable') const assert = require('assert') // TODO(bridiver) - make these generic validation functions @@ -154,7 +155,7 @@ const api = { getPersistentState: (state) => { // TODO(bridiver) handle restoring state state = makeImmutable(state) - return state.delete('windows') + return state.set('windows', Immutable.List()) }, // TODO (nejc) we should only pass in one state diff --git a/app/index.js b/app/index.js index 342dca23221..041e25aa4a8 100644 --- a/app/index.js +++ b/app/index.js @@ -94,19 +94,19 @@ const defaultProtocols = ['http', 'https'] let loadAppStatePromise = SessionStore.loadAppState() // Some settings must be set right away on startup, those settings should be handled here. -loadAppStatePromise.then((initialState) => { +loadAppStatePromise.then((initialImmutableState) => { telemetry.setCheckpointAndReport('state-loaded') const {HARDWARE_ACCELERATION_ENABLED, SMOOTH_SCROLL_ENABLED, SEND_CRASH_REPORTS} = require('../js/constants/settings') - if (initialState.settings[HARDWARE_ACCELERATION_ENABLED] === false) { + if (initialImmutableState.getIn('settings', HARDWARE_ACCELERATION_ENABLED) === false) { app.disableHardwareAcceleration() } - if (initialState.settings[SEND_CRASH_REPORTS] !== false) { + if (initialImmutableState.getIn(['settings', SEND_CRASH_REPORTS]) !== false) { console.log('Crash reporting enabled') CrashHerald.init() } else { console.log('Crash reporting disabled') } - if (initialState.settings[SMOOTH_SCROLL_ENABLED] === false) { + if (initialImmutableState.getIn(['settings', SMOOTH_SCROLL_ENABLED]) === false) { app.commandLine.appendSwitch('disable-smooth-scrolling') } }) @@ -158,18 +158,17 @@ app.on('ready', () => { appActions.networkDisconnected() }) - loadAppStatePromise.then((initialState) => { + loadAppStatePromise.then((initialImmutableState) => { // Do this after loading the state // For tests we always want to load default app state - const loadedPerWindowState = initialState.perWindowState - delete initialState.perWindowState + const loadedPerWindowImmutableState = initialImmutableState.get('perWindowState') + initialImmutableState = initialImmutableState.delete('perWindowState') // Restore map order after load - let state = Immutable.fromJS(initialState) - appActions.setState(state) - setImmediate(() => perWindowStateLoaded(loadedPerWindowState)) + appActions.setState(initialImmutableState) + setImmediate(() => perWindowStateLoaded(loadedPerWindowImmutableState)) }) - const perWindowStateLoaded = (loadedPerWindowState) => { + const perWindowStateLoaded = (loadedPerWindowImmutableState) => { // TODO(bridiver) - this shold be refactored into reducers // DO NOT ADD ANYHING TO THIS LIST // See tabsReducer.js for app state init example @@ -184,12 +183,12 @@ app.on('ready', () => { AdInsertion.init() PDFJS.init() - if (!loadedPerWindowState || loadedPerWindowState.length === 0) { + if (!loadedPerWindowImmutableState || loadedPerWindowImmutableState.size === 0) { if (!CmdLine.newWindowURL()) { appActions.newWindow() } } else { - loadedPerWindowState.forEach((wndState) => { + loadedPerWindowImmutableState.forEach((wndState) => { appActions.newWindow(undefined, undefined, wndState) }) } @@ -304,12 +303,12 @@ app.on('ready', () => { // DO NOT TO THIS LIST - see above // We need the initial state to read the UPDATE_TO_PREVIEW_RELEASES preference - loadAppStatePromise.then((initialState) => { + loadAppStatePromise.then((initialImmutableState) => { updater.init( process.platform, process.arch, process.env.BRAVE_UPDATE_VERSION || app.getVersion(), - initialState.settings[settings.UPDATE_TO_PREVIEW_RELEASES] + initialImmutableState.getIn('settings', settings.UPDATE_TO_PREVIEW_RELEASES) ) // This is fired by a menu entry diff --git a/app/sessionStore.js b/app/sessionStore.js index 57969dd27c9..126fa7ef186 100644 --- a/app/sessionStore.js +++ b/app/sessionStore.js @@ -14,6 +14,8 @@ const path = require('path') const electron = require('electron') const os = require('os') +const assert = require('assert') +const Immutable = require('immutable') const app = electron.app // Constants @@ -34,7 +36,7 @@ const filtering = require('./filtering') const autofill = require('./autofill') const {navigatableTypes} = require('../js/lib/appUrlUtil') const Channel = require('./channel') -const {makeImmutable} = require('./common/state/immutableUtil') +const {isImmutable, makeImmutable, deleteImmutablePaths} = require('./common/state/immutableUtil') const {getSetting} = require('../js/settings') const platformUtil = require('./common/lib/platformUtil') const historyUtil = require('./common/lib/historyUtil') @@ -56,38 +58,44 @@ const getStoragePath = () => { /** * Saves the specified immutable browser state to storage. * - * @param {object} payload - Application state as per + * @param {object} immutablePayload - Application immutable state as per * https://github.com/brave/browser/wiki/Application-State * (not immutable data) * @return a promise which resolves when the state is saved */ -module.exports.saveAppState = (payload, isShutdown) => { +module.exports.saveAppState = (immutablePayload, isShutdown) => { + assert(isImmutable(immutablePayload)) + return new Promise((resolve, reject) => { // Don't persist private frames let startupModeSettingValue = getSetting(settings.STARTUP_MODE) const savePerWindowState = startupModeSettingValue == null || startupModeSettingValue === 'lastTime' - if (payload.perWindowState && savePerWindowState) { - payload.perWindowState.forEach((wndPayload) => { - wndPayload.frames = wndPayload.frames.filter((frame) => !frame.isPrivate) + if (immutablePayload.get('perWindowState') && savePerWindowState) { + immutablePayload.get('perWindowState').forEach((immutableWndPayload, i) => { + const frames = immutableWndPayload.get('frames').filter((frame) => !frame.get('isPrivate')) + immutableWndPayload = immutableWndPayload.set('frames', frames) + immutablePayload = immutablePayload.setIn(['perWindowState', i], immutableWndPayload) }) } else { - delete payload.perWindowState + immutablePayload = immutablePayload.delete('perWindowState') } try { - payload = module.exports.cleanAppData(payload, isShutdown) - payload.cleanedOnShutdown = isShutdown + immutablePayload = module.exports.cleanAppData(immutablePayload, isShutdown) + immutablePayload.set('cleanedOnShutdown', isShutdown) } catch (e) { - payload.cleanedOnShutdown = false + immutablePayload.set('cleanedOnShutdown', false) } - payload.lastAppVersion = app.getVersion() + immutablePayload.set('lastAppVersion', app.getVersion()) if (isShutdown) { module.exports.cleanSessionDataOnShutdown() } - muon.file.writeImportant(getStoragePath(), JSON.stringify(payload), (success) => { + const storagePath = getStoragePath() + const json = JSON.stringify(immutablePayload) + muon.file.writeImportant(storagePath, json, (success) => { if (success) { resolve() } else { @@ -99,169 +107,186 @@ module.exports.saveAppState = (payload, isShutdown) => { /** * Cleans session data from unwanted values. + * @param immutablePerWindowData - Per window data in ImmutableJS format + * @return ImmutableJS cleaned window data */ -module.exports.cleanPerWindowData = (perWindowData, isShutdown) => { - if (!perWindowData) { - perWindowData = {} +module.exports.cleanPerWindowData = (immutablePerWindowData, isShutdown) => { + if (!immutablePerWindowData) { + immutablePerWindowData = Immutable.Map() } + + assert(isImmutable(immutablePerWindowData)) + // delete the frame index because tabId is per-session - delete perWindowData.framesInternal - // Hide the context menu when we restore. - delete perWindowData.contextMenuDetail - // Don't save preview frame since they are only related to hovering on a tab - delete perWindowData.previewFrameKey - // Don't save widevine panel detail - delete perWindowData.widevinePanelDetail - // Don't save preview tab pages - if (perWindowData.ui && perWindowData.ui.tabs) { - delete perWindowData.ui.tabs.previewTabPageIndex - } - // Don't restore add/edit dialog - delete perWindowData.bookmarkDetail - // Don't restore bravery panel - delete perWindowData.braveryPanelDetail - // Don't restore drag data and clearBrowsingDataPanel's visibility - if (perWindowData.ui) { + immutablePerWindowData = immutablePerWindowData.delete('framesInternal') + + immutablePerWindowData = deleteImmutablePaths(immutablePerWindowData, [ + // Hide the context menu when we restore. + 'contextMenuDetail', + // Don't save preview frame since they are only related to hovering on a tab + 'previewFrameKey', + // Don't save widevine panel detail + 'widevinePanelDetail', + // Don't save preview tab pages + ['ui', 'tabs', 'previewTabPageIndex'], + // Don't restore add/edit dialog + 'bookmarkDetail', + // Don't restore bravery panel + 'braveryPanelDetail', + // Don't restore drag data and clearBrowsingDataPanel's visibility // This is no longer stored, we can remove this line eventually - delete perWindowData.ui.isFocused - delete perWindowData.ui.mouseInTitlebar - delete perWindowData.ui.mouseInFrame - delete perWindowData.ui.dragging - delete perWindowData.ui.isClearBrowsingDataPanelVisible + ['ui', 'isFocused'], + ['ui', 'mouseInTitlebar'], + ['ui', 'mouseInFrame'], + ['ui', 'dragging'], + ['ui', 'isClearBrowsingDataPanelVisible'] + ]) + + if (!immutablePerWindowData.get('frames')) { + immutablePerWindowData = immutablePerWindowData.set('frames', Immutable.List()) } - perWindowData.frames = perWindowData.frames || [] + let newKey = 0 - const cleanFrame = (frame) => { + const cleanFrame = (immutableFrame) => { newKey++ // Reset the ids back to sequential numbers - if (frame.key === perWindowData.activeFrameKey) { - perWindowData.activeFrameKey = newKey + if (immutableFrame.get('key') === immutablePerWindowData.get('activeFrameKey')) { + immutablePerWindowData = immutablePerWindowData.set('activeFrameKey', newKey) } else { // For now just set everything to unloaded unless it's the active frame - frame.unloaded = true + immutableFrame = immutableFrame.set('unloaded', true) } - frame.key = newKey + immutableFrame = immutableFrame.set('key', newKey) // Set the frame src to the last visited location // or else users will see the first visited URL. // Pinned location always get reset to what they are - frame.src = frame.pinnedLocation || frame.location + immutableFrame = immutableFrame.set('src', immutableFrame.get('pinnedLocation') || immutableFrame.get('location')) // If a blob is present for the thumbnail, create the object URL - if (frame.thumbnailBlob) { + if (immutableFrame.get('thumbnailBlob')) { try { - frame.thumbnailUrl = window.URL.createObjectURL(frame.thumbnailBlob) + immutableFrame.set('thumbnailUrl', window.URL.createObjectURL(immutableFrame.get('thumbnailBlob'))) } catch (e) { - delete frame.thumbnailUrl + immutableFrame = immutableFrame.delete('thumbnailUrl') } } - // Delete lists of blocked sites - delete frame.trackingProtection - delete frame.httpsEverywhere - delete frame.adblock - delete frame.noScript - - // clean up any legacy frame opening props - delete frame.openInForeground - delete frame.disposition - - // Guest instance ID's are not valid after restarting. - // Electron won't know about them. - delete frame.guestInstanceId - - // Tab ids are per-session and should not be persisted - delete frame.tabId - - // Do not show the audio indicator until audio starts playing - delete frame.audioMuted - delete frame.audioPlaybackActive - // Let's not assume wknow anything about loading - delete frame.loading - // Always re-determine the security data - delete frame.security - // Value is only used for local storage - delete frame.isActive - // Hide modal prompts. - delete frame.modalPromptDetail - // Remove HTTP basic authentication requests. - delete frame.basicAuthDetail - // Remove open search details - delete frame.searchDetail - // Remove find in page details - if (frame.findDetail) { - delete frame.findDetail.numberOfMatches - delete frame.findDetail.activeMatchOrdinal - delete frame.findDetail.internalFindStatePresent - } - delete frame.findbarShown - // Don't restore full screen state - delete frame.isFullScreen - delete frame.showFullScreenWarning - // Don't store child tab open ordering since keys - // currently get re-generated when session store is - // restored. We will be able to keep this once we - // don't regenerate new frame keys when opening storage. - delete frame.parentFrameKey - // Delete the active shortcut details - delete frame.activeShortcut - delete frame.activeShortcutDetails - - if (frame.navbar && frame.navbar.urlbar) { - if (frame.navbar.urlbar.suggestions) { - frame.navbar.urlbar.suggestions.selectedIndex = null - frame.navbar.urlbar.suggestions.suggestionList = null + immutableFrame = deleteImmutablePaths(immutableFrame, [ + // Delete lists of blocked sites + 'trackingProtection', + 'httpsEverywhere', + 'adblock', + 'noScript', + // clean up any legacy frame opening props + 'openInForeground', + 'disposition', + // Guest instance ID's are not valid after restarting. + // Electron won't know about them. + 'guestInstanceId', + // Tab ids are per-session and should not be persisted + 'tabId', + // Do not show the audio indicator until audio starts playing + 'audioMuted', + 'audioPlaybackActive', + // Let's not assume wknow anything about loading + 'loading', + // Always re-determine the security data + 'security', + // Value is only used for local storage + 'isActive', + // Hide modal prompts. + 'modalPromptDetail', + // Remove HTTP basic authentication requests. + 'basicAuthDetail', + // Remove open search details + 'searchDetail', + // Remove find in page details + ['findDetail', 'numberOfMatches'], + ['findDetail', 'activeMatchOrdinal'], + ['findDetail', 'internalFindStatePresent'], + 'findbarShown', + // Don't restore full screen state + 'isFullScreen', + 'showFullScreenWarning', + // Don't store child tab open ordering since keys + // currently get re-generated when session store is + // restored. We will be able to keep this once we + // don't regenerate new frame keys when opening storage. + 'parentFrameKey', + // Delete the active shortcut details + 'activeShortcut', + 'activeShortcutDetails' + ]) + + if (immutableFrame.get('navbar') && immutableFrame.getIn(['navbar', 'urlbar'])) { + if (immutableFrame.getIn(['navbar', 'urlbar', 'suggestions'])) { + immutableFrame = immutableFrame.setIn(['navbar', 'urlbar', 'suggestions', 'selectedIndex'], null) + immutableFrame = immutableFrame.setIn(['navbar', 'urlbar', 'suggestions', 'suggestionList'], null) } - delete frame.navbar.urlbar.searchDetail + immutableFrame = immutableFrame.deleteIn(['navbar', 'urlbar', 'searchDetail']) } + return immutableFrame } const clearHistory = isShutdown && getSetting(settings.SHUTDOWN_CLEAR_HISTORY) === true if (clearHistory) { - perWindowData.closedFrames = [] + immutablePerWindowData = immutablePerWindowData.set('closedFrames', Immutable.List()) } // Clean closed frame data before frames because the keys are re-ordered // and the new next key is calculated in windowStore.js based on // the max frame key ID. - if (perWindowData.closedFrames) { - perWindowData.closedFrames.forEach(cleanFrame) - } - if (perWindowData.frames) { + if (immutablePerWindowData.get('closedFrames')) { + immutablePerWindowData = + immutablePerWindowData.get('closedFrames').reduce((immutablePerWindowData, immutableFrame, index) => { + const cleanImmutableFrame = cleanFrame(immutableFrame) + return immutablePerWindowData.setIn(['closedFrames', index], cleanImmutableFrame) + }, immutablePerWindowData) + } + if (immutablePerWindowData.get('frames')) { // Don't restore pinned locations because they will be auto created by the app state change event - perWindowData.frames = perWindowData.frames - .filter((frame) => !frame.pinnedLocation) - perWindowData.frames.forEach(cleanFrame) - } + immutablePerWindowData.set('frames', + immutablePerWindowData.get('frames') + .filter((frame) => !frame.get('pinnedLocation'))) + immutablePerWindowData = + immutablePerWindowData.get('frames').reduce((immutablePerWindowData, immutableFrame, index) => { + const cleanImmutableFrame = cleanFrame(immutableFrame) + return immutablePerWindowData.setIn(['frames', index], cleanImmutableFrame) + }, immutablePerWindowData) + } + return immutablePerWindowData } /** * Cleans app data before it's written to disk. - * @param {Object} data - top-level app data + * @param {Object} data - top-level app data in ImmutableJS format * @param {Object} isShutdown - true if the data is being cleared for a shutdown * WARNING: getPrefs is only available in this function when isShutdown is true + * @return Immutable JS cleaned up data */ -module.exports.cleanAppData = (data, isShutdown) => { - // make a copy - // TODO(bridiver) use immutable - data = makeImmutable(data).toJS() +module.exports.cleanAppData = (immutableData, isShutdown) => { + assert(isImmutable(immutableData)) // Don't show notifications from the last session - data.notifications = [] + immutableData = immutableData.set('notifications', Immutable.List()) // Delete temp site settings - data.temporarySiteSettings = {} + immutableData = immutableData.set('temporarySiteSettings', Immutable.Map()) - if (data.settings && data.settings[settings.CHECK_DEFAULT_ON_STARTUP] === true) { + if (immutableData.getIn(['settings', settings.CHECK_DEFAULT_ON_STARTUP]) === true) { // Delete defaultBrowserCheckComplete state since this is checked on startup - delete data.defaultBrowserCheckComplete + immutableData = immutableData.delete('defaultBrowserCheckComplete') } // Delete Recovery status on shut down try { - delete data.ui.about.preferences.recoverySucceeded + immutableData = immutableData.deleteIn(['ui', 'about', 'preferences', 'recoverySucceeded']) } catch (e) {} - if (data.perWindowState) { - data.perWindowState.forEach((perWindowState) => - module.exports.cleanPerWindowData(perWindowState, isShutdown)) + const perWindowStateList = immutableData.get('perWindowState') + if (perWindowStateList) { + perWindowStateList.forEach((immutablePerWindowState, i) => { + const cleanedImmutablePerWindowState = module.exports.cleanPerWindowData(immutablePerWindowState, isShutdown) + immutableData = immutableData.setIn(['perWindowState', i], cleanedImmutablePerWindowState) + }) } const clearAutocompleteData = isShutdown && getSetting(settings.SHUTDOWN_CLEAR_AUTOCOMPLETE_DATA) === true if (clearAutocompleteData) { @@ -275,7 +300,7 @@ module.exports.cleanAppData = (data, isShutdown) => { if (clearAutofillData) { autofill.clearAutofillData() const date = new Date().getTime() - data.autofill = { + immutableData = immutableData.set('autofill', Immutable.fromJS({ addresses: { guid: [], timestamp: date @@ -284,94 +309,94 @@ module.exports.cleanAppData = (data, isShutdown) => { guid: [], timestamp: date } - } - } - if (data.dragData) { - delete data.dragData + })) } - if (data.sync) { + immutableData = immutableData.delete('dragData') + + if (immutableData.get('sync')) { // clear sync site cache - data.sync.objectsById = {} + immutableData = immutableData.deleteIn(['sync', 'objectsById'], Immutable.Map()) } const clearSiteSettings = isShutdown && getSetting(settings.SHUTDOWN_CLEAR_SITE_SETTINGS) === true if (clearSiteSettings) { - data.siteSettings = {} + immutableData = immutableData.set('siteSettings', Immutable.Map()) } // Delete expired Flash and NoScript allow-once approvals let now = Date.now() - for (var host in data.siteSettings) { - let expireTime = data.siteSettings[host].flash + + immutableData.get('siteSettings', Immutable.Map()).forEach((value, host) => { + let expireTime = value.get('flash') if (typeof expireTime === 'number' && expireTime < now) { - delete data.siteSettings[host].flash + immutableData = immutableData.deleteIn(['siteSettings', host, 'flash']) } - let noScript = data.siteSettings[host].noScript + let noScript = immutableData.getIn(['siteSettings', host, 'noScript']) if (typeof noScript === 'number') { - delete data.siteSettings[host].noScript + immutableData = immutableData.deleteIn(['siteSettings', host, 'noScript']) } // Don't persist any noScript exceptions - delete data.siteSettings[host].noScriptExceptions + immutableData = immutableData.deleteIn(['siteSettings', host, 'noScriptExceptions']) // Don't write runInsecureContent to session - delete data.siteSettings[host].runInsecureContent + immutableData = immutableData.deleteIn(['siteSettings', host, 'runInsecureContent']) // If the site setting is empty, delete it for privacy - if (Object.keys(data.siteSettings[host]).length === 0) { - delete data.siteSettings[host] + if (Array.from(immutableData.getIn(['siteSettings', host]).keys()).length === 0) { + immutableData = immutableData.deleteIn(['siteSettings', host]) } - } - if (data.sites) { + }) + + if (immutableData.get('sites')) { const clearHistory = isShutdown && getSetting(settings.SHUTDOWN_CLEAR_HISTORY) === true if (clearHistory) { - data.historySites = {} - if (data.about) { - delete data.about.history - delete data.about.newtab - } + immutableData = immutableData.set('historySites', Immutable.Map()) + immutableData = deleteImmutablePaths(immutableData, [ + ['about', 'history'], + ['about', 'newtab'] + ]) } } - if (data.downloads) { + + if (immutableData.get('downloads')) { const clearDownloads = isShutdown && getSetting(settings.SHUTDOWN_CLEAR_DOWNLOADS) === true if (clearDownloads) { - delete data.downloads + immutableData = immutableData.delete('downloads') } else { // Always at least delete downloaded items older than a week const dateOffset = 7 * 24 * 60 * 60 * 1000 const lastWeek = new Date().getTime() - dateOffset - Object.keys(data.downloads).forEach((downloadId) => { - if (data.downloads[downloadId].startTime < lastWeek) { - delete data.downloads[downloadId] + Array.from(immutableData.get('downloads').keys()).forEach((downloadId) => { + if (immutableData.getIn(['downloads', downloadId, 'startTime']) < lastWeek) { + immutableData = immutableData.deleteIn(['downloads', downloadId]) } else { - const state = data.downloads[downloadId].state + const state = immutableData.getIn(['downloads', downloadId, 'state']) if (state === downloadStates.IN_PROGRESS || state === downloadStates.PAUSED) { - data.downloads[downloadId].state = downloadStates.INTERRUPTED + immutableData = immutableData.setIn(['downloads', downloadId, 'state'], downloadStates.INTERRUPTED) } } }) } } - if (data.menu) { - delete data.menu - } + immutableData = immutableData.delete('menu') try { - data = tabState.getPersistentState(data).toJS() + immutableData = tabState.getPersistentState(immutableData) } catch (e) { - delete data.tabs console.log('cleanAppData: error calling tabState.getPersistentState: ', e) + immutableData = immutableData.set('tabs', Immutable.List()) } try { - data = windowState.getPersistentState(data).toJS() + immutableData = windowState.getPersistentState(immutableData) } catch (e) { - delete data.windows console.log('cleanAppData: error calling windowState.getPersistentState: ', e) + immutableData = immutableData.set('windows', Immutable.List()) } - if (data.extensions) { - Object.keys(data.extensions).forEach((extensionId) => { - delete data.extensions[extensionId].tabs + if (immutableData.get('extensions')) { + Array.from(immutableData.get('extensions').keys()).forEach((extensionId) => { + immutableData = immutableData.delete('extensions', extensionId, 'tabs') }) } - return data + return immutableData } /** @@ -410,7 +435,7 @@ const safeGetVersion = (fieldName, getFieldVersion) => { /** * version information (shown on about:brave) */ -const setVersionInformation = (data) => { +const setVersionInformation = (immutableData) => { const versionFields = [ ['Brave', app.getVersion], ['rev', Channel.browserLaptopRev], @@ -430,11 +455,11 @@ const setVersionInformation = (data) => { versionInformation[versionField.name] = versionField.version }) - data.about = data.about || {} - data.about.brave = { - versionInformation: versionInformation + if (!immutableData.get('about')) { + immutableData = immutableData.set('about', Immutable.Map()) } - return data + immutableData = immutableData.setIn(['about', 'brave', 'versionInformation'], Immutable.fromJS(versionInformation)) + return immutableData } const sortBookmarkOrder = (bookmarkOrder) => { @@ -673,8 +698,8 @@ module.exports.runPreMigrations = (data) => { return data } -module.exports.runPostMigrations = (data) => { - return data +module.exports.runPostMigrations = (immutableData) => { + return immutableData } module.exports.runImportDefaultSettings = (data) => { @@ -723,28 +748,32 @@ module.exports.loadAppState = () => { if (data) { console.log('could not parse data: ', data, e) } - data = exports.defaultAppState() - data = module.exports.runImportDefaultSettings(data) + data = {} } + data = Object.assign({}, module.exports.defaultAppState(), data) + data = module.exports.runImportDefaultSettings(data) if (loaded) { data = module.exports.runPreMigrations(data) + } + let immutableData = makeImmutable(data) + if (loaded) { // Clean app data here if it wasn't cleared on shutdown - if (data.cleanedOnShutdown !== true || data.lastAppVersion !== app.getVersion()) { - data = module.exports.cleanAppData(data, false) + if (immutableData.get('cleanedOnShutdown') !== true || immutableData.get('lastAppVersion') !== app.getVersion()) { + immutableData = module.exports.cleanAppData(immutableData, false) } - data = Object.assign(module.exports.defaultAppState(), data) - data.cleanedOnShutdown = false + + immutableData = immutableData.set('cleanedOnShutdown', false) // Always recalculate the update status - if (data.updates) { - const updateStatus = data.updates.status - delete data.updates.status + if (immutableData.get('updates')) { + const updateStatus = immutableData.getIn(['updates', 'status']) + immutableData = immutableData.deleteIn(['updates', 'status']) // The process always restarts after an update so if the state // indicates that a restart isn't wanted, close right away. if (updateStatus === UpdateStatus.UPDATE_APPLYING_NO_RESTART) { - module.exports.saveAppState(data, true).then(() => { + module.exports.saveAppState(immutableData, true).then(() => { // Exit immediately without doing the session store saving stuff // since we want the same state saved except for the update status app.exit(0) @@ -753,15 +782,12 @@ module.exports.loadAppState = () => { } } - data = module.exports.runPostMigrations(data) - data = module.exports.runImportDefaultSettings(data) + immutableData = module.exports.runPostMigrations(immutableData) } - - data = setVersionInformation(data) - - locale.init(data.settings[settings.LANGUAGE]).then((locale) => { + immutableData = setVersionInformation(immutableData) + locale.init(immutableData.getIn(['settings', settings.LANGUAGE])).then((locale) => { app.setLocale(locale) - resolve(data) + resolve(immutableData) }) }) } diff --git a/app/sessionStoreShutdown.js b/app/sessionStoreShutdown.js index b0c2fe104d3..d43b9746d5a 100644 --- a/app/sessionStoreShutdown.js +++ b/app/sessionStoreShutdown.js @@ -13,9 +13,11 @@ const async = require('async') const messages = require('../js/constants/messages') const appActions = require('../js/actions/appActions') const platformUtil = require('./common/lib/platformUtil') +const Immutable = require('immutable') +const {makeImmutable} = require('./common/state/immutableUtil') // Used to collect the per window state when shutting down the application -let perWindowState +let immutablePerWindowState let sessionStateStoreComplete let sessionStateStoreCompleteCallback let saveAppStateTimeout @@ -26,13 +28,13 @@ let isAllWindowsClosed let sessionStateSaveInterval // Stores the last window state for each requested window in case a hung window happens, // we'll at least have the last known window state. -let windowStateCache +let immutableWindowStateCache let sessionStoreQueue let appStore // Useful for automated tests const reset = () => { - perWindowState = [] + immutablePerWindowState = Immutable.List() sessionStateStoreComplete = false if (saveAppStateTimeout) { clearTimeout(saveAppStateTimeout) @@ -43,7 +45,7 @@ const reset = () => { lastWindowThatWasClosedState = undefined isAllWindowsClosed = false sessionStateSaveInterval = null - windowStateCache = {} + immutableWindowStateCache = Immutable.Map() if (sessionStateStoreCompleteCallback) { sessionStateStoreCompleteCallback() } @@ -73,10 +75,8 @@ const saveAppState = (forceSave = false) => { app.exit(0) } - const appState = appStore.getState().toJS() - appState.perWindowState = perWindowState - - const receivedAllWindows = perWindowState.length === BrowserWindow.getAllWindows().length + let immutableAppState = appStore.getState().set('perWindowState', immutablePerWindowState) + const receivedAllWindows = immutablePerWindowState.size === BrowserWindow.getAllWindows().length if (receivedAllWindows) { clearTimeout(saveAppStateTimeout) } @@ -85,7 +85,7 @@ const saveAppState = (forceSave = false) => { return } - return sessionStore.saveAppState(appState, shuttingDown).catch((e) => { + return sessionStore.saveAppState(immutableAppState, shuttingDown).catch((e) => { logSaveAppStateError(e) }).then(() => { if (receivedAllWindows || forceSave) { @@ -96,21 +96,21 @@ const saveAppState = (forceSave = false) => { 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)) { + if (immutableAppState.get('updates') && (immutableAppState.getIn(['updates', 'status']) === updateStatus.UPDATE_AVAILABLE || + immutableAppState.getIn(['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 (platformUtil.isWindows()) { - appState.updates.status = updateStatus.UPDATE_APPLYING_RESTART + immutableAppState.setIn(['updates', 'status'], updateStatus.UPDATE_APPLYING_RESTART) } else { - appState.updates.status = updateStatus.UPDATE_APPLYING_NO_RESTART + immutableAppState.setIn(['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)) { + if (immutableAppState.get('updates') && (immutableAppState.getIn(['updates', 'status']) === updateStatus.UPDATE_APPLYING_NO_RESTART || + immutableAppState.getIn(['updates', 'status']) === updateStatus.UPDATE_APPLYING_RESTART)) { updater.quitAndInstall() } else { app.quit() @@ -132,11 +132,11 @@ const initiateSessionStateSave = () => { sessionStoreQueue.push((cb) => { sessionStateStoreComplete = false sessionStateStoreCompleteCallback = cb - perWindowState.length = 0 + immutablePerWindowState = Immutable.List() // quit triggered by window-all-closed should save last window state if (isAllWindowsClosed && lastWindowThatWasClosedState) { - perWindowState.push(lastWindowThatWasClosedState) + immutablePerWindowState = immutablePerWindowState.push(lastWindowThatWasClosedState) saveAppState(true) } else if (BrowserWindow.getAllWindows().length > 0) { ++windowCloseRequestId @@ -148,9 +148,9 @@ const initiateSessionStateSave = () => { // In this case just save session store for the windows that we have already. saveAppStateTimeout = setTimeout(() => { // Rewrite perwindowstate here - perWindowState = windowIds - .filter((windowId) => windowStateCache[windowId]) - .map((windowId) => windowStateCache[windowId]) + immutablePerWindowState = Immutable.fromJS(windowIds + .filter((windowId) => immutableWindowStateCache.get(windowId)) + .map((windowId) => immutableWindowStateCache.get(windowId))) saveAppState(true) }, appConfig.quitTimeout) } else { @@ -163,11 +163,11 @@ const removeWindowFromCache = (windowId) => { if (shuttingDown) { return } - delete windowStateCache[windowId] + immutableWindowStateCache = immutableWindowStateCache.delete(windowId) } -const initWindowCacheState = (windowId, windowState) => { - windowStateCache[windowId] = Object.assign({}, windowState) +const initWindowCacheState = (windowId, immutableWindowState) => { + immutableWindowStateCache = immutableWindowStateCache.set(windowId, immutableWindowState) } app.on('before-quit', (e) => { @@ -202,14 +202,15 @@ ipcMain.on(messages.RESPONSE_WINDOW_STATE, (evt, mem) => { const memory = mem.memory() const data = memory.windowState const id = memory.requestId + const immutableWindowState = makeImmutable(data) const senderWindowId = evt.sender.getOwnerBrowserWindow().id if (id !== windowCloseRequestId) { return } if (data) { - perWindowState.push(data) - windowStateCache[senderWindowId] = data + immutablePerWindowState = immutablePerWindowState.push(immutableWindowState) + immutableWindowStateCache = immutableWindowStateCache.set(senderWindowId, immutableWindowState) } saveAppState() }) diff --git a/js/constants/appConfig.js b/js/constants/appConfig.js index 320de6a5580..f9c98ca6d58 100644 --- a/js/constants/appConfig.js +++ b/js/constants/appConfig.js @@ -18,7 +18,7 @@ module.exports = { name: 'Brave', contactUrl: 'mailto:support+laptop@brave.com', quitTimeout: isTest ? 3 * 1000 : 10 * 1000, - sessionSaveInterval: 1000 * 60 * 5, + sessionSaveInterval: 10000, resourceNames: { ADBLOCK: 'adblock', SAFE_BROWSING: 'safeBrowsing', diff --git a/js/stores/appStore.js b/js/stores/appStore.js index 865b95376b3..43ef21be30b 100644 --- a/js/stores/appStore.js +++ b/js/stores/appStore.js @@ -39,7 +39,7 @@ const {zoomLevel} = require('../../app/common/constants/toolbarUserInterfaceScal const {initWindowCacheState} = require('../../app/sessionStoreShutdown') // state helpers -const {makeImmutable} = require('../../app/common/state/immutableUtil') +const {isImmutable, makeImmutable} = require('../../app/common/state/immutableUtil') const basicAuthState = require('../../app/common/state/basicAuthState') const extensionState = require('../../app/common/state/extensionState') const aboutNewTabState = require('../../app/common/state/aboutNewTabState') @@ -76,10 +76,11 @@ const navbarHeight = () => { /** * Determine window dimensions (width / height) */ -const setWindowDimensions = (browserOpts, defaults, windowState) => { - if (windowState.ui && windowState.ui.size) { - browserOpts.width = firstDefinedValue(browserOpts.width, windowState.ui.size[0]) - browserOpts.height = firstDefinedValue(browserOpts.height, windowState.ui.size[1]) +const setWindowDimensions = (browserOpts, defaults, immutableWindowState) => { + assert(isImmutable(immutableWindowState)) + if (immutableWindowState.getIn(['ui', 'size'])) { + browserOpts.width = firstDefinedValue(browserOpts.width, immutableWindowState.getIn(['ui', 'size', 0])) + browserOpts.height = firstDefinedValue(browserOpts.height, immutableWindowState.getIn(['ui', 'size', 1])) } browserOpts.width = firstDefinedValue(browserOpts.width, browserOpts.innerWidth, defaults.width) // height and innerHeight are the frame webview size @@ -97,15 +98,15 @@ const setWindowDimensions = (browserOpts, defaults, windowState) => { /** * Determine window position (x / y) */ -const setWindowPosition = (browserOpts, defaults, windowState) => { +const setWindowPosition = (browserOpts, defaults, immutableWindowState) => { if (browserOpts.positionByMouseCursor) { const screenPos = electron.screen.getCursorScreenPoint() browserOpts.x = screenPos.x browserOpts.y = screenPos.y - } else if (windowState.ui && windowState.ui.position) { + } else if (immutableWindowState.getIn(['ui', 'position'])) { // Position comes from window state - browserOpts.x = firstDefinedValue(browserOpts.x, windowState.ui.position[0]) - browserOpts.y = firstDefinedValue(browserOpts.y, windowState.ui.position[1]) + browserOpts.x = firstDefinedValue(browserOpts.x, immutableWindowState.getIn(['ui', 'position', 0])) + browserOpts.y = firstDefinedValue(browserOpts.y, immutableWindowState.getIn(['ui', 'position', 1])) } else if (typeof defaults.x === 'number' && typeof defaults.y === 'number') { // Position comes from the default position browserOpts.x = firstDefinedValue(browserOpts.x, defaults.x) @@ -121,11 +122,11 @@ const setWindowPosition = (browserOpts, defaults, windowState) => { const createWindow = (action) => { const frameOpts = (action.frameOpts && action.frameOpts.toJS()) || {} let browserOpts = (action.browserOpts && action.browserOpts.toJS()) || {} - const windowState = action.restoredState || {} + const immutableWindowState = action.restoredState || Immutable.Map() const defaults = windowDefaults() - browserOpts = setWindowDimensions(browserOpts, defaults, windowState) - browserOpts = setWindowPosition(browserOpts, defaults, windowState) + browserOpts = setWindowDimensions(browserOpts, defaults, immutableWindowState) + browserOpts = setWindowPosition(browserOpts, defaults, immutableWindowState) delete browserOpts.left delete browserOpts.top @@ -203,39 +204,40 @@ const createWindow = (action) => { setImmediate(() => { let mainWindow = new BrowserWindow(Object.assign(windowProps, browserOpts, {disposition: frameOpts.disposition})) - initWindowCacheState(mainWindow.id, action.restoredState) + let restoredImmutableWindowState = action.restoredState + initWindowCacheState(mainWindow.id, restoredImmutableWindowState) // initialize frames state - let frames = [] - if (action.restoredState) { - frames = action.restoredState.frames - action.restoredState.frames = [] - action.restoredState.tabs = [] + let frames = Immutable.List() + if (restoredImmutableWindowState) { + frames = restoredImmutableWindowState.get('frames') + restoredImmutableWindowState = restoredImmutableWindowState.set('frames', Immutable.List()) + restoredImmutableWindowState = restoredImmutableWindowState.set('tabs', Immutable.List()) } else { if (frameOpts && Object.keys(frameOpts).length > 0) { if (frameOpts.forEach) { - frames = frameOpts + frames = Immutable.toJS(frameOpts) } else { frames.push(frameOpts) } } else if (startupSetting === 'homePage' && homepageSetting) { - frames = homepageSetting.split('|').map((homepage) => { + frames = Immutable.fromJS(homepageSetting.split('|').map((homepage) => { return { location: homepage } - }) + })) } } - if (frames.length === 0) { - frames = [{}] + if (frames.size === 0) { + frames = Immutable.fromJS([{}]) } - if (windowState.ui && windowState.ui.isMaximized) { + if (immutableWindowState.getIn(['ui', 'isMaximized'])) { mainWindow.maximize() } - if (windowState.ui && windowState.ui.isFullScreen) { + if (immutableWindowState.getIn(['ui', 'isFullScreen'])) { mainWindow.setFullScreen(true) } @@ -249,8 +251,8 @@ const createWindow = (action) => { id: mainWindow.id }, appState: appState.toJS(), - frames, - windowState: action.restoredState}) + frames: frames.toJS(), + windowState: (restoredImmutableWindowState && restoredImmutableWindowState.toJS()) || undefined}) e.sender.sendShared(messages.INITIALIZE_WINDOW, mem) if (action.cb) { @@ -271,11 +273,6 @@ class AppStore extends EventEmitter { return appState } - emitFullWindowState (wnd) { - wnd.webContents.send(messages.APP_STATE_CHANGE, { state: appState.toJS() }) - lastEmittedState = appState - } - emitChanges (emitFullState) { if (lastEmittedState) { const d = diff(lastEmittedState, appState) diff --git a/test/unit/app/common/state/immutableUtilTest.js b/test/unit/app/common/state/immutableUtilTest.js new file mode 100644 index 00000000000..1164838318e --- /dev/null +++ b/test/unit/app/common/state/immutableUtilTest.js @@ -0,0 +1,110 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +/* global describe, it */ +const assert = require('assert') +require('../../../braveUnit') +const Immutable = require('immutable') +const immutableUtil = require('../../../../../app/common/state/immutableUtil') + +describe('immutableUtil unit test', function () { + describe('isImmutable', function () { + it('false when Array', function () { + assert(!immutableUtil.isImmutable([])) + }) + it('falsy when non-array Object', function () { + assert(!immutableUtil.isImmutable({})) + }) + it('falsy when null', function () { + assert(!immutableUtil.isImmutable(null)) + }) + it('falsy when undewfined', function () { + assert(!immutableUtil.isImmutable(undefined)) + }) + it('falsy when boolean', function () { + assert(!immutableUtil.isImmutable(true)) + }) + it('truthy when Immutable.Map', function () { + assert(immutableUtil.isImmutable(Immutable.Map())) + }) + it('truthy when Immutable.List', function () { + assert(immutableUtil.isImmutable(Immutable.List())) + }) + }) + describe('isMap', function () { + it('Immutable.Map returns true', function () { + assert.equal(immutableUtil.isMap(Immutable.Map()), true) + }) + it('Immutable.List returns false', function () { + assert.equal(immutableUtil.isMap(Immutable.List()), false) + }) + it('Array returns false', function () { + assert.equal(immutableUtil.isMap([]), false) + }) + it('Object returns false', function () { + assert.equal(immutableUtil.isMap({}), false) + }) + it('null returns false', function () { + assert.equal(immutableUtil.isMap(null), false) + }) + it('undefined returns false', function () { + assert.equal(immutableUtil.isMap(undefined), false) + }) + }) + describe('isList', function () { + it('Immutable.List returns true', function () { + assert.equal(immutableUtil.isList(Immutable.List()), true) + }) + it('Immutable.Map returns false', function () { + assert.equal(immutableUtil.isList(Immutable.Map()), false) + }) + it('Array returns false', function () { + assert.equal(immutableUtil.isList([]), false) + }) + it('Object returns false', function () { + assert.equal(immutableUtil.isList({}), false) + }) + it('null returns false', function () { + assert.equal(immutableUtil.isList(null), false) + }) + it('undefined returns false', function () { + assert.equal(immutableUtil.isList(undefined), false) + }) + }) + describe('isSameHashCode', function () { + it('returns true if both undefined or null', function () { + assert.deepEqual(immutableUtil.isSameHashCode(undefined, null), true) + }) + it('returns true for 2 identical but different Immutable objects', function () { + assert.deepEqual(immutableUtil.isSameHashCode(Immutable.fromJS({a: 1, b: [1, 2, 3]}), Immutable.fromJS({a: 1, b: [1, 2, 3]})), true) + }) + it('returns false for 2 different Immutable objects', function () { + assert.deepEqual(immutableUtil.isSameHashCode(Immutable.fromJS({a: 1, b: [1, 2]}), Immutable.fromJS({a: 1, b: [1, 2, 3]})), false) + }) + }) + describe('makeImmutable', function () { + it('converts an Object Map to Immutable.Map', function () { + assert.deepEqual(immutableUtil.makeImmutable({a: 1}).constructor, Immutable.Map) + }) + it('converts an Array Immutable.List', function () { + assert.deepEqual(immutableUtil.makeImmutable([1]).constructor, Immutable.List) + }) + it('converts an Object Immutable.Map to Immutable.Map', function () { + assert.deepEqual(immutableUtil.makeImmutable(Immutable.Map()).constructor, Immutable.Map) + }) + it('converts an Array Immutable.List', function () { + assert.deepEqual(immutableUtil.makeImmutable(Immutable.List()).constructor, Immutable.List) + }) + }) + describe('deleteImmutablePaths', function () { + it('removes properties with simple strings', function () { + const data = Immutable.fromJS({a: 'Cezar is a black belt in ameri-do-te', b: 2, c: 3}) + assert.deepEqual(immutableUtil.deleteImmutablePaths(data, ['a', 'b']).toJS(), {c: 3}) + }) + it('removes properties with array string paths', function () { + const data = Immutable.fromJS({a: {a1: 4, a2: 8}, c: 'Cezar learnt directly from master ken', d: 5}) + assert.deepEqual(immutableUtil.deleteImmutablePaths(data, [['a', 'a1'], 'c']).toJS(), {a: {a2: 8}, d: 5}) + }) + }) +}) diff --git a/test/unit/app/sessionStoreShutdownTest.js b/test/unit/app/sessionStoreShutdownTest.js index 76b25fb87f1..b7bf80c84be 100644 --- a/test/unit/app/sessionStoreShutdownTest.js +++ b/test/unit/app/sessionStoreShutdownTest.js @@ -121,15 +121,15 @@ describe('sessionStoreShutdown unit tests', function () { }) it('works for first closed window', function () { - const windowState = { a: 1 } + const windowState = Immutable.fromJS({ a: 1 }) fakeElectron.ipcMain.send(messages.LAST_WINDOW_STATE, {}, windowState) process.emit(messages.UNDO_CLOSED_WINDOW) assert(this.newWindowStub.calledOnce) assert.deepEqual(this.newWindowStub.getCall(0).args[2], windowState) }) it('works for subsequent windows', function () { - const windowState1 = { b: 1 } - const windowState2 = { x: 2 } + const windowState1 = Immutable.fromJS({ b: 1 }) + const windowState2 = Immutable.fromJS({ x: 2 }) fakeElectron.ipcMain.send(messages.LAST_WINDOW_STATE, {}, windowState1) fakeElectron.ipcMain.send(messages.LAST_WINDOW_STATE, {}, windowState2) process.emit(messages.UNDO_CLOSED_WINDOW) @@ -172,7 +172,7 @@ describe('sessionStoreShutdown unit tests', function () { const saveAppStateStub = sinon.stub(sessionStore, 'saveAppState', (state) => { assert.equal(saveAppStateStub.calledOnce, true) saveAppStateStub.restore() - assert.equal(state.perWindowState.length, 0) + assert.equal(state.get('perWindowState').size, 0) cb() return Promise.resolve() }) @@ -182,14 +182,14 @@ describe('sessionStoreShutdown unit tests', function () { }) it('remembers last closed window with no windows (Win32)', function (cb) { isWindows = true - const windowState = { a: 1 } + const windowState = Immutable.fromJS({ a: 1 }) fakeElectron.ipcMain.send(messages.LAST_WINDOW_STATE, {}, windowState) fakeElectron.app.emit('window-all-closed') const saveAppStateStub = sinon.stub(sessionStore, 'saveAppState', (state) => { isWindows = false assert.equal(saveAppStateStub.calledOnce, true) saveAppStateStub.restore() - assert.equal(state.perWindowState.length, 1) + assert.equal(state.get('perWindowState').size, 1) cb() return Promise.resolve() }) @@ -198,13 +198,13 @@ describe('sessionStoreShutdown unit tests', function () { this.clock.tick(1) }) it('remembers last closed window with no windows (Linux)', function (cb) { - const windowState = { a: 1 } + const windowState = Immutable.fromJS({ a: 1 }) fakeElectron.ipcMain.send(messages.LAST_WINDOW_STATE, {}, windowState) fakeElectron.app.emit('window-all-closed') const saveAppStateStub = sinon.stub(sessionStore, 'saveAppState', (state) => { assert.equal(saveAppStateStub.calledOnce, true) saveAppStateStub.restore() - assert.equal(state.perWindowState.length, 1) + assert.equal(state.get('perWindowState').size, 1) cb() return Promise.resolve() }) @@ -214,14 +214,14 @@ describe('sessionStoreShutdown unit tests', function () { }) it('remembers last closed window with no windows (macOS)', function (cb) { isDarwin = true - const windowState = { a: 1 } + const windowState = Immutable.fromJS({ a: 1 }) fakeElectron.ipcMain.send(messages.LAST_WINDOW_STATE, {}, windowState) fakeElectron.app.emit('window-all-closed') const saveAppStateStub = sinon.stub(sessionStore, 'saveAppState', (state) => { isDarwin = false assert.equal(saveAppStateStub.calledOnce, true) saveAppStateStub.restore() - assert.equal(state.perWindowState.length, 0) + assert.equal(state.get('perWindowState').size, 0) cb() return Promise.resolve() }) @@ -256,7 +256,7 @@ describe('sessionStoreShutdown unit tests', function () { const saveAppStateStub = sinon.stub(sessionStore, 'saveAppState', (state) => { assert.equal(saveAppStateStub.calledOnce, true) saveAppStateStub.restore() - assert.equal(state.perWindowState.length, 1) + assert.equal(state.get('perWindowState').size, 1) cb() return Promise.resolve() }) @@ -268,7 +268,7 @@ describe('sessionStoreShutdown unit tests', function () { const saveAppStateStub = sinon.stub(sessionStore, 'saveAppState', (state) => { assert.equal(saveAppStateStub.calledOnce, true) saveAppStateStub.restore() - assert.deepEqual(state.perWindowState, [{a: 1}]) + assert.deepEqual(state.get('perWindowState').toJS(), [{a: 1}]) cb() return Promise.resolve() }) @@ -321,7 +321,7 @@ describe('sessionStoreShutdown unit tests', function () { const saveAppStateStub = sinon.stub(sessionStore, 'saveAppState', (state) => { assert.equal(saveAppStateStub.calledOnce, true) saveAppStateStub.restore() - assert.equal(state.perWindowState.length, 3) + assert.equal(state.get('perWindowState').size, 3) cb() return Promise.resolve() }) @@ -333,7 +333,7 @@ describe('sessionStoreShutdown unit tests', function () { const saveAppStateStub = sinon.stub(sessionStore, 'saveAppState', (state) => { assert.equal(saveAppStateStub.calledOnce, true) saveAppStateStub.restore() - assert.deepEqual(state.perWindowState, [{a: 1}, {b: 2}, {c: 3}]) + assert.deepEqual(state.get('perWindowState').toJS(), [{a: 1}, {b: 2}, {c: 3}]) cb() return Promise.resolve() }) @@ -349,7 +349,7 @@ describe('sessionStoreShutdown unit tests', function () { const saveAppStateStub = sinon.stub(sessionStore, 'saveAppState', (state) => { assert.equal(saveAppStateStub.called, true) saveAppStateStub.restore() - assert.deepEqual(state.perWindowState, [{a: 5}, {b: 2}]) + assert.deepEqual(state.get('perWindowState').toJS(), [{a: 5}, {b: 2}]) cb() return Promise.resolve() }) diff --git a/test/unit/app/sessionStoreTest.js b/test/unit/app/sessionStoreTest.js index 8ed67e803a4..f71d41729e8 100644 --- a/test/unit/app/sessionStoreTest.js +++ b/test/unit/app/sessionStoreTest.js @@ -2,6 +2,7 @@ const mockery = require('mockery') const assert = require('assert') const sinon = require('sinon') +const Immutable = require('immutable') const settings = require('../../../js/constants/settings') const {makeImmutable} = require('../../../app/common/state/immutableUtil') const downloadStates = require('../../../js/constants/downloadStates') @@ -102,7 +103,7 @@ describe('sessionStore unit tests', function () { let cleanSessionDataOnShutdownStub before(function () { - cleanAppDataStub = sinon.stub(sessionStore, 'cleanAppData').returns({}) + cleanAppDataStub = sinon.stub(sessionStore, 'cleanAppData').returns(Immutable.Map()) cleanSessionDataOnShutdownStub = sinon.stub(sessionStore, 'cleanSessionDataOnShutdown') }) @@ -113,7 +114,7 @@ describe('sessionStore unit tests', function () { it('calls cleanAppData', function () { cleanAppDataStub.reset() - return sessionStore.saveAppState({}) + return sessionStore.saveAppState(Immutable.Map()) .then(function (result) { assert.equal(cleanAppDataStub.calledOnce, true) }, function (err) { @@ -124,7 +125,7 @@ describe('sessionStore unit tests', function () { describe('with isShutdown', function () { it('calls cleanSessionDataOnShutdown if true', function () { cleanSessionDataOnShutdownStub.reset() - return sessionStore.saveAppState({}, true) + return sessionStore.saveAppState(Immutable.Map(), true) .then(() => { assert.equal(cleanSessionDataOnShutdownStub.calledOnce, true) }, function (err) { @@ -134,7 +135,7 @@ describe('sessionStore unit tests', function () { it('does not call cleanSessionDataOnShutdown if false', function () { cleanSessionDataOnShutdownStub.reset() - return sessionStore.saveAppState({}, false) + return sessionStore.saveAppState(Immutable.Map(), false) .then(() => { assert.equal(cleanSessionDataOnShutdownStub.notCalled, true) }, function (err) { @@ -149,58 +150,59 @@ describe('sessionStore unit tests', function () { describe('cleanAppData', function () { it('clears notifications from the last session', function () { - const data = {notifications: ['message 1', 'message 2']} + const data = Immutable.fromJS({notifications: ['message 1', 'message 2']}) const result = sessionStore.cleanAppData(data) - assert.deepEqual(result.notifications, []) + assert.deepEqual(result.get('notifications').toJS(), []) }) it('deletes temp site settings', function () { - const data = {temporarySiteSettings: {site1: {setting1: 'value1'}}} + const data = Immutable.fromJS({temporarySiteSettings: {site1: {setting1: 'value1'}}}) const result = sessionStore.cleanAppData(data) - assert.deepEqual(result.temporarySiteSettings, {}) + assert.deepEqual(result.get('temporarySiteSettings').toJS(), {}) }) describe('when CHECK_DEFAULT_ON_STARTUP is true', function () { it('clears defaultBrowserCheckComplete', function () { - const data = { - settings: {}, + const data = Immutable.fromJS({ + settings: { + [settings.CHECK_DEFAULT_ON_STARTUP]: true + }, defaultBrowserCheckComplete: 'test_value' - } - data.settings[settings.CHECK_DEFAULT_ON_STARTUP] = true + }) const result = sessionStore.cleanAppData(data) - assert.equal(result.defaultBrowserCheckComplete, undefined) + assert.equal(result.get('defaultBrowserCheckComplete'), undefined) }) }) describe('with recovery status', function () { it('deletes status if present', function () { - const data = { + const data = Immutable.fromJS({ ui: { about: { preferences: { recoverySucceeded: true } } } - } + }) const result = sessionStore.cleanAppData(data) - assert.deepEqual(result.ui.about.preferences.recoverySucceeded, undefined) - assert.deepEqual(result.ui.about.preferences, {}) + assert.deepEqual(result.getIn(['ui', 'about', 'preferences', 'recoverySucceeded']), undefined) + assert.deepEqual(result.getIn(['ui', 'about', 'preferences']).toJS(), {}) }) it('does not throw an exception if not present', function () { - const data = { + const data = Immutable.fromJS({ ui: {} - } + }) const result = sessionStore.cleanAppData(data) - assert.deepEqual(result.ui, {}) + assert.deepEqual(result.get('ui').toJS(), {}) }) }) describe('if perWindowState is present', function () { it('calls cleanPerWindowData for each item', function () { const cleanPerWindowDataStub = sinon.stub(sessionStore, 'cleanPerWindowData') - const data = { + const data = Immutable.fromJS({ perWindowState: ['window1', 'window2'] - } + }) sessionStore.cleanAppData(data, 'IS_SHUTDOWN_VALUE') assert.equal(cleanPerWindowDataStub.withArgs('window1', 'IS_SHUTDOWN_VALUE').calledOnce, true) assert.equal(cleanPerWindowDataStub.withArgs('window2', 'IS_SHUTDOWN_VALUE').calledOnce, true) @@ -211,7 +213,7 @@ describe('sessionStore unit tests', function () { describe('when clearAutocompleteData is true', function () { it('calls autofill.clearAutocompleteData', function () { const clearAutocompleteDataSpy = sinon.spy(fakeAutofill, 'clearAutocompleteData') - const data = {} + const data = Immutable.Map() sessionStore.cleanAppData(data, true) assert.equal(clearAutocompleteDataSpy.calledOnce, true) clearAutocompleteDataSpy.restore() @@ -227,7 +229,7 @@ describe('sessionStore unit tests', function () { }) it('swallows exception', function () { - const data = {} + const data = Immutable.Map() sessionStore.cleanAppData(data, true) assert.ok(true) }) @@ -245,7 +247,7 @@ describe('sessionStore unit tests', function () { clearAutofillDataSpy = sinon.spy(fakeAutofill, 'clearAutofillData') clock = sinon.useFakeTimers() now = new Date(0) - const data = { + const data = Immutable.fromJS({ autofill: { addresses: { guid: ['value1', 'value2'], @@ -256,7 +258,7 @@ describe('sessionStore unit tests', function () { timestamp: 'time2' } } - } + }) result = sessionStore.cleanAppData(data, true) }) @@ -270,189 +272,189 @@ describe('sessionStore unit tests', function () { }) it('sets the guid for addresses to []', function () { - assert.deepEqual(result.autofill.addresses.guid, []) + assert.deepEqual(result.getIn(['autofill', 'addresses', 'guid']).toJS(), []) }) it('sets the timestamp for addresses to now', function () { - assert.equal(result.autofill.addresses.timestamp, now.getTime()) + assert.equal(result.getIn(['autofill', 'addresses', 'timestamp']), now.getTime()) }) it('sets the guid for creditCards to []', function () { - assert.deepEqual(result.autofill.creditCards.guid, []) + assert.deepEqual(result.getIn(['autofill', 'creditCards', 'guid']).toJS(), []) }) it('sets the timestamp for creditCards to now', function () { - assert.equal(result.autofill.creditCards.timestamp, now.getTime()) + assert.equal(result.getIn(['autofill', 'creditCards', 'timestamp']), now.getTime()) }) }) describe('malformed input', function () { it('does not throw an exception', function () { - sessionStore.cleanAppData({}, true) - sessionStore.cleanAppData({autofill: 'stringValue'}, true) - sessionStore.cleanAppData({autofill: {}}, true) - sessionStore.cleanAppData({autofill: {addresses: 'stringValue'}}, true) - sessionStore.cleanAppData({autofill: {creditCards: 'stringValue'}}, true) + sessionStore.cleanAppData(Immutable.Map(), true) + sessionStore.cleanAppData(Immutable.fromJS({autofill: 'stringValue'}), true) + sessionStore.cleanAppData(Immutable.fromJS({autofill: {}}), true) + sessionStore.cleanAppData(Immutable.fromJS({autofill: {addresses: 'stringValue'}}), true) + sessionStore.cleanAppData(Immutable.fromJS({autofill: {creditCards: 'stringValue'}}), true) }) }) }) describe('when clearSiteSettings is true', function () { it('clears siteSettings', function () { - const data = {siteSettings: {site1: {setting1: 'value1'}}} + const data = Immutable.fromJS({siteSettings: {site1: {setting1: 'value1'}}}) const result = sessionStore.cleanAppData(data, true) - assert.deepEqual(result.siteSettings, {}) + assert.deepEqual(result.get('siteSettings').toJS(), {}) }) }) describe('with siteSettings', function () { it('deletes Flash approval if expired', function () { - const data = { + const data = Immutable.fromJS({ siteSettings: { site1: {flash: 1, test: 2} } - } + }) const result = sessionStore.cleanAppData(data, false) - assert.equal(result.siteSettings.site1.flash, undefined) + assert.equal(result.getIn(['siteSettings', 'site1', 'flash']), undefined) }) it('leaves Flash approval alone if not expired', function () { - const data = { + const data = Immutable.fromJS({ siteSettings: { site1: {flash: Infinity, test: 2} } - } + }) const result = sessionStore.cleanAppData(data, false) - assert.equal(result.siteSettings.site1.flash, Infinity) + assert.equal(result.getIn(['siteSettings', 'site1', 'flash']), Infinity) }) it('deletes NoScript approval if set', function () { - const data = { + const data = Immutable.fromJS({ siteSettings: { site1: {noScript: 1, test: 2} } - } + }) const result = sessionStore.cleanAppData(data, false) - assert.equal(result.siteSettings.noScript, undefined) + assert.equal(result.getIn(['siteSettings', 'noScript']), undefined) }) it('deletes NoScript exceptions', function () { - const data = { + const data = Immutable.fromJS({ siteSettings: { site1: {noScriptExceptions: true, test: 2} } - } + }) const result = sessionStore.cleanAppData(data, false) - assert.equal(result.siteSettings.site1.noScriptExceptions, undefined) + assert.equal(result.getIn(['siteSettings', 'site1', 'noScriptExceptions']), undefined) }) it('deletes runInsecureContent', function () { - const data = { + const data = Immutable.fromJS({ siteSettings: { site1: {runInsecureContent: true, test: 2} } - } + }) const result = sessionStore.cleanAppData(data, false) - assert.equal(result.siteSettings.site1.runInsecureContent, undefined) + assert.equal(result.getIn(['siteSettings', 'site1', 'runInsecureContent']), undefined) }) it('deletes entry if empty', function () { - const data = { + const data = Immutable.fromJS({ siteSettings: { site1: {} } - } + }) const result = sessionStore.cleanAppData(data, false) - assert.equal(result.siteSettings.site1, undefined) + assert.equal(result.getIn(['siteSettings', 'site1']), undefined) }) }) describe('when sites and clearHistory are truthy', function () { it('deletes temporary entries used in about:history', function () { - const data = { + const data = Immutable.fromJS({ about: {history: true}, sites: {entry1: {}} - } + }) const result = sessionStore.cleanAppData(data, true) - assert.equal(result.about.history, undefined) + assert.equal(result.getIn(['about', 'history']), undefined) }) it('deletes top site entries used in about:newtab', function () { - const data = { + const data = Immutable.fromJS({ about: {newtab: true}, sites: {entry1: {}} - } + }) const result = sessionStore.cleanAppData(data, true) - assert.equal(result.about.newtab, undefined) + assert.equal(result.getIn(['about', 'newtab']), undefined) }) }) describe('when downloads is truthy', function () { describe('when clearDownloads is true', function () { it('deletes downloads', function () { - const data = { + const data = Immutable.fromJS({ downloads: { entry1: {} } - } + }) const result = sessionStore.cleanAppData(data, true) - assert.equal(result.downloads, undefined) + assert.equal(result.get('downloads'), undefined) }) }) describe('when clearDownloads is falsey', function () { it('deletes entries which are more than a week old', function () { - const data = { + const data = Immutable.fromJS({ downloads: { entry1: {startTime: 1} } - } + }) const result = sessionStore.cleanAppData(data, false) - assert.deepEqual(result.downloads, {}) + assert.deepEqual(result.get('downloads').toJS(), {}) }) it('leaves entries which are less than a week old', function () { - const data = { + const data = Immutable.fromJS({ downloads: { entry1: {startTime: new Date().getTime()} } - } + }) const result = sessionStore.cleanAppData(data, false) - assert.deepEqual(result.downloads, data.downloads) + assert.deepEqual(result.get('downloads').toJS(), data.get('downloads').toJS()) }) describe('with download state', function () { const getEntry = (state) => { - return { + return Immutable.fromJS({ downloads: { - entry1: {startTime: new Date().getTime(), state: state} + entry1: {startTime: new Date().getTime(), state} } - } + }) } it('sets IN_PROGRESS to INTERRUPTED', function () { const data = getEntry(downloadStates.IN_PROGRESS) const result = sessionStore.cleanAppData(data, false) - assert.equal(result.downloads.entry1.state, downloadStates.INTERRUPTED) + assert.equal(result.getIn(['downloads', 'entry1', 'state']), downloadStates.INTERRUPTED) }) it('sets PAUSED to INTERRUPTED', function () { const data = getEntry(downloadStates.PAUSED) const result = sessionStore.cleanAppData(data, false) - assert.equal(result.downloads.entry1.state, downloadStates.INTERRUPTED) + assert.equal(result.getIn(['downloads', 'entry1', 'state']), downloadStates.INTERRUPTED) }) it('leaves other states alone', function () { let data = getEntry(downloadStates.COMPLETED) let result = sessionStore.cleanAppData(data, false) - assert.equal(result.downloads.entry1.state, downloadStates.COMPLETED) + assert.equal(result.getIn(['downloads', 'entry1', 'state']), downloadStates.COMPLETED) data = getEntry(downloadStates.CANCELLED) result = sessionStore.cleanAppData(data, false) - assert.equal(result.downloads.entry1.state, downloadStates.CANCELLED) + assert.equal(result.getIn(['downloads', 'entry1', 'state']), downloadStates.CANCELLED) data = getEntry(downloadStates.PENDING) result = sessionStore.cleanAppData(data, false) - assert.equal(result.downloads.entry1.state, downloadStates.PENDING) + assert.equal(result.getIn(['downloads', 'entry1', 'state']), downloadStates.PENDING) }) }) }) @@ -461,7 +463,7 @@ describe('sessionStore unit tests', function () { describe('with tabState', function () { it('calls getPersistentState', function () { const getPersistentStateSpy = sinon.spy(fakeTabState, 'getPersistentState') - const data = {} + const data = Immutable.Map() sessionStore.cleanAppData(data) assert.equal(getPersistentStateSpy.calledOnce, true) getPersistentStateSpy.restore() @@ -469,9 +471,9 @@ describe('sessionStore unit tests', function () { it('deletes tabState if an exception is thrown', function () { const getPersistentStateSpy = sinon.stub(fakeTabState, 'getPersistentState').throws('oh noes') - const data = {tabs: true} + const data = Immutable.fromJS({tabs: true}) const result = sessionStore.cleanAppData(data) - assert.equal(result.tabs, undefined) + assert.deepEqual(result.get('tabs').toJS(), []) getPersistentStateSpy.restore() }) }) @@ -479,7 +481,7 @@ describe('sessionStore unit tests', function () { describe('with windowState', function () { it('calls getPersistentState', function () { const getPersistentStateSpy = sinon.spy(fakeWindowState, 'getPersistentState') - const data = {} + const data = Immutable.Map() sessionStore.cleanAppData(data) assert.equal(getPersistentStateSpy.calledOnce, true) getPersistentStateSpy.restore() @@ -487,7 +489,7 @@ describe('sessionStore unit tests', function () { it('deletes windowState if an exception is thrown', function () { const getPersistentStateSpy = sinon.stub(fakeWindowState, 'getPersistentState').throws('oh noes') - const data = {windows: true} + const data = Immutable.fromJS({windows: true}) const result = sessionStore.cleanAppData(data) assert.equal(result.windows, undefined) getPersistentStateSpy.restore() @@ -512,7 +514,7 @@ describe('sessionStore unit tests', function () { before(function () { runPreMigrationsSpy = sinon.spy(sessionStore, 'runPreMigrations') - cleanAppDataStub = sinon.stub(sessionStore, 'cleanAppData') + cleanAppDataStub = sinon.stub(sessionStore, 'cleanAppData', (data) => data) defaultAppStateSpy = sinon.spy(sessionStore, 'defaultAppState') runPostMigrationsSpy = sinon.spy(sessionStore, 'runPostMigrations') localeInitSpy = sinon.spy(fakeLocale, 'init') @@ -565,7 +567,7 @@ describe('sessionStore unit tests', function () { it('does not crash when exception thrown during read', function () { return sessionStore.loadAppState() .then(function (result) { - assert.ok(result.firstRunTimestamp) + assert.ok(result.get('firstRunTimestamp')) }, function (result) { assert.ok(false, 'promise was rejected: ' + JSON.stringify(result)) }) @@ -653,7 +655,7 @@ describe('sessionStore unit tests', function () { .then(function (result) { assert.equal(runPreMigrationsSpy.calledOnce, true) }, function (result) { - assert.ok(false, 'promise was rejected: ' + JSON.stringify(result)) + assert.ok(false, 'promise was rejected: ' + result) }) })