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

Commit

Permalink
Fix chance of having a lost window
Browse files Browse the repository at this point in the history 
Fix #9806
  • Loading branch information
@bbondy
bbondy committed Jul 12, 2017
1 parent bebfb2e commit 38280c3
Show file tree
Hide file tree
Showing 15 changed files with 788 additions and 210 deletions.
12 changes: 9 additions & 3 deletions app/browser/menu.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ const createDebugSubmenu = () => {
// The console will print a message like: [84790:0710/201431:ERROR:child_process.cc(136)] Renderer (84790) paused waiting for debugger to attach. Send SIGUSR1 to unpause.
// And this means you should attach Xcode or whatever your debugger is to PID 84790 to unpause.
// To debug all renderer processes then add the appendSwitch call to app/index.js
label: 'append wait renderer switch',
label: 'Append wait renderer switch',
click: function () {
app.commandLine.appendSwitch('renderer-startup-dialog')
}
Expand Down Expand Up @@ -512,11 +512,17 @@ const createDebugSubmenu = () => {
}
}
}, {
label: 'Toggle React Profiling',
label: 'Toggle React profiling',
accelerator: 'Alt+P',
click: function (item, focusedWindow) {
CommonMenu.sendToFocusedWindow(focusedWindow, [messages.DEBUG_REACT_PROFILE])
}
}, {
label: 'Stop reporting window state',
click: function () {
const win = BrowserWindow.getActiveWindow()
appActions.noReportStateModeClicked(win.id)
}
}
]
}
Expand Down Expand Up @@ -553,7 +559,7 @@ const createMenu = (state) => {
{ label: locale.translation('help'), submenu: createHelpSubmenu(), role: 'help' }
]

if (process.env.NODE_ENV === 'development') {
if (process.env.NODE_ENV === 'development' || process.env.BRAVE_ENABLE_DEBUG_MENU !== undefined) {
template.push({ label: 'Debug', submenu: createDebugSubmenu() })
}

Expand Down
2 changes: 2 additions & 0 deletions app/browser/reducers/windowsReducer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const appConstants = require('../../../js/constants/appConstants')
const windowConstants = require('../../../js/constants/windowConstants')
const windowState = require('../../common/state/windowState')
const windows = require('../windows')
const sessionStoreShutdown = require('../../sessionStoreShutdown')
const {makeImmutable} = require('../../common/state/immutableUtil')

const windowsReducer = (state, action, immutableAction) => {
Expand All @@ -31,6 +32,7 @@ const windowsReducer = (state, action, immutableAction) => {
break
case appConstants.APP_WINDOW_CLOSED:
state = windowState.removeWindow(state, action)
sessionStoreShutdown.removeWindowFromCache(action.getIn(['windowValue', 'windowId']))
break
case appConstants.APP_WINDOW_CREATED:
state = windowState.maybeCreateWindow(state, action)
Expand Down
186 changes: 10 additions & 176 deletions app/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,15 @@ if (process.platform === 'win32') {

const electron = require('electron')
const app = electron.app
const BrowserWindow = electron.BrowserWindow
const ipcMain = electron.ipcMain
const Immutable = require('immutable')
const Updater = require('./updater')
const updater = require('./updater')
const Importer = require('./importer')
const messages = require('../js/constants/messages')
const appConfig = require('../js/constants/appConfig')
const appActions = require('../js/actions/appActions')
const SessionStore = require('./sessionStore')
const AppStore = require('../js/stores/appStore')
const {startSessionSaveInterval} = require('./sessionStoreShutdown')
const appStore = require('../js/stores/appStore')
const Autofill = require('./autofill')
const Extensions = require('./extensions')
const TrackingProtection = require('./trackingProtection')
Expand All @@ -67,29 +66,17 @@ const HttpsEverywhere = require('./httpsEverywhere')
const PDFJS = require('./pdfJS')
const SiteHacks = require('./siteHacks')
const CmdLine = require('./cmdLine')
const UpdateStatus = require('../js/constants/updateStatus')
const urlParse = require('./common/urlParse')
const spellCheck = require('./spellCheck')
const locale = require('./locale')
const contentSettings = require('../js/state/contentSettings')
const privacy = require('../js/state/privacy')
const async = require('async')
const settings = require('../js/constants/settings')
const BookmarksExporter = require('./browser/bookmarksExporter')
const siteUtil = require('../js/state/siteUtil')

app.commandLine.appendSwitch('enable-features', 'BlockSmallPluginContent,PreferHtmlOverPlugins')

// Used to collect the per window state when shutting down the application
let perWindowState = []
let sessionStateStoreComplete = false
let sessionStateStoreCompleteCallback = null
let saveAppStateTimeout = null
let requestId = 0
let shuttingDown = false
let lastWindowState
let lastWindowClosed = false

// Domains to accept bad certs for. TODO: Save the accepted cert fingerprints.
let acceptCertDomains = {}
let errorCerts = {}
Expand All @@ -99,105 +86,8 @@ const prefsRestartLastValue = {}

const defaultProtocols = ['http', 'https']

const sessionStoreQueue = async.queue((task, callback) => {
task(callback)
}, 1)

const logSaveAppStateError = (e) => {
console.error('Error saving app state: ', e)
}

const saveAppState = (forceSave = false) => {
if (!sessionStateStoreCompleteCallback) {
return
}

// 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)
}

const appState = AppStore.getState().toJS()
appState.perWindowState = perWindowState

const receivedAllWindows = perWindowState.length === BrowserWindow.getAllWindows().length
if (receivedAllWindows) {
clearTimeout(saveAppStateTimeout)
}

if (!forceSave && !receivedAllWindows) {
return
}

return SessionStore.saveAppState(appState, shuttingDown).catch((e) => {
logSaveAppStateError(e)
}).then(() => {
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 {
const cb = sessionStateStoreCompleteCallback
sessionStateStoreCompleteCallback = null
cb()
}
}
})
}

/**
* Saves the session storage for all windows
*/
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)
saveAppState(true)
} 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.
saveAppStateTimeout = setTimeout(() => {
saveAppState(true)
}, appConfig.quitTimeout)
} else {
saveAppState()
}
})
}

// exit cleanly on signals
['SIGTERM', 'SIGHUP', 'SIGINT', 'SIGBREAK'].forEach((signal) => {
;['SIGTERM', 'SIGHUP', 'SIGINT', 'SIGBREAK'].forEach((signal) => {
process.on(signal, () => {
app.quit()
})
Expand Down Expand Up @@ -245,7 +135,6 @@ const notifyCertError = (webContents, url, error, cert) => {
}

app.on('ready', () => {
let sessionStateSaveInterval = null
app.on('certificate-error', (e, webContents, url, error, cert, resourceType, overridable, strictEnforcement, expiredPreviousDecision, cb) => {
let host = urlParse(url).host
if (host && acceptCertDomains[host] === true) {
Expand All @@ -263,34 +152,6 @@ app.on('ready', () => {
notifyCertError(webContents, url, error, cert)
})

app.on('window-all-closed', () => {
// On macOS it is common for applications and their menu bar
// to stay active until the user quits explicitly with Cmd + Q
if (process.platform !== 'darwin') {
lastWindowClosed = true
app.quit()
}
})

app.on('before-quit', (e) => {
if (shuttingDown && sessionStateStoreComplete) {
return
}

e.preventDefault()

// before-quit can be triggered multiple times because of the preventDefault call
if (shuttingDown) {
return
} else {
shuttingDown = true
}

appActions.shuttingDown()
clearInterval(sessionStateSaveInterval)
initiateSessionStateSave()
})

app.on('network-connected', () => {
appActions.networkConnected()
})
Expand All @@ -299,32 +160,6 @@ app.on('ready', () => {
appActions.networkDisconnected()
})

// User initiated exit using File->Quit
ipcMain.on(messages.RESPONSE_WINDOW_STATE, (evt, data, id) => {
if (id !== requestId) {
return
}

if (data) {
perWindowState.push(data)
}
saveAppState()
})

ipcMain.on(messages.LAST_WINDOW_STATE, (evt, data) => {
if (data) {
lastWindowState = data
}
})

process.on(messages.UNDO_CLOSED_WINDOW, () => {
if (lastWindowState) {
SessionStore.cleanPerWindowData(lastWindowState)
appActions.newWindow(undefined, undefined, lastWindowState)
lastWindowState = undefined
}
})

loadAppStatePromise.then((initialState) => {
// Do this after loading the state
// For tests we always want to load default app state
Expand Down Expand Up @@ -455,8 +290,7 @@ app.on('ready', () => {
}
})

// save app state every 5 minutes regardless of update frequency
sessionStateSaveInterval = setInterval(initiateSessionStateSave, 1000 * 60 * 5)
startSessionSaveInterval()

ipcMain.on(messages.NOTIFICATION_RESPONSE, (e, message, buttonIndex, persist) => {
if (prefsRestartCallbacks[message]) {
Expand All @@ -469,22 +303,22 @@ app.on('ready', () => {
})

ipcMain.on(messages.EXPORT_BOOKMARKS, () => {
BookmarksExporter.showDialog(AppStore.getState().get('sites'))
BookmarksExporter.showDialog(appStore.getState().get('sites'))
})
// DO NOT TO THIS LIST - see above

// We need the initial state to read the UPDATE_TO_PREVIEW_RELEASES preference
loadAppStatePromise.then((initialState) => {
Updater.init(
updater.init(
process.platform,
process.arch,
process.env.BRAVE_UPDATE_VERSION || app.getVersion(),
initialState.settings[settings.UPDATE_TO_PREVIEW_RELEASES]
)

// This is fired by a menu entry
process.on(messages.CHECK_FOR_UPDATE, () => Updater.checkForUpdate(true))
ipcMain.on(messages.CHECK_FOR_UPDATE, () => Updater.checkForUpdate(true))
process.on(messages.CHECK_FOR_UPDATE, () => updater.checkForUpdate(true))
ipcMain.on(messages.CHECK_FOR_UPDATE, () => updater.checkForUpdate(true))

// This is fired from a auto-update metadata call
process.on(messages.UPDATE_META_DATA_RETRIEVED, (metadata) => {
Expand All @@ -498,7 +332,7 @@ app.on('ready', () => {
})

process.on(messages.EXPORT_BOOKMARKS, () => {
BookmarksExporter.showDialog(AppStore.getState().get('sites'))
BookmarksExporter.showDialog(appStore.getState().get('sites'))
})

ready = true
Expand Down
19 changes: 19 additions & 0 deletions app/renderer/reducers/debugReducer.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/* 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/. */

const appConstants = require('../../../js/constants/appConstants')
const messages = require('../../../js/constants/messages')
const {ipcRenderer} = require('electron')

const debugReducer = (windowState, action) => {
switch (action.actionType) {
case appConstants.APP_DEBUG_NO_REPORT_STATE_MODE_CLICKED:
ipcRenderer.removeAllListeners(messages.REQUEST_WINDOW_STATE)
console.error('Disabled window state updates')
break
}
return windowState
}

module.exports = debugReducer
Loading

0 comments on commit 38280c3

Please sign in to comment.