Skip to content

Commit

Permalink
Save window appState into windowState when closing a window
Browse files Browse the repository at this point in the history
Resolves brave#3754
Resolves brave#6602
Resolves brave#8600
Resolves brave#8925
Resolves brave#9709

Auditors:

Test Plan:
  • Loading branch information
NejcZdovc committed Aug 25, 2017
1 parent 7e97199 commit 921c203
Show file tree
Hide file tree
Showing 18 changed files with 379 additions and 99 deletions.
2 changes: 1 addition & 1 deletion app/browser/reducers/tabsReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ const tabsReducer = (state, action, immutableAction) => {
if (dragData && dragData.get('type') === dragTypes.TAB) {
const frame = dragData.get('data')
const frameOpts = frameOptsFromFrame(frame).toJS()
const browserOpts = { positionByMouseCursor: true }
const browserOpts = { positionByMouseCursor: true, checkMaximized: true }
frameOpts.indexByFrameKey = dragData.getIn(['dragOverData', 'draggingOverKey'])
frameOpts.prependIndexByFrameKey = dragData.getIn(['dragOverData', 'draggingOverLeftHalf'])
tabs.moveTo(state, frame.get('tabId'), frameOpts, browserOpts, dragData.get('dropWindowId'))
Expand Down
87 changes: 55 additions & 32 deletions app/browser/reducers/windowsReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,20 +44,22 @@ const navbarHeight = () => {
*/
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
browserOpts.height = firstDefinedValue(browserOpts.height, browserOpts.innerHeight)
if (typeof browserOpts.height === 'number') {
// add navbar height to get total height for BrowserWindow
browserOpts.height = browserOpts.height + navbarHeight()
if (immutableWindowState.getIn(['windowInfo'])) {
browserOpts.width = firstDefinedValue(browserOpts.width, immutableWindowState.getIn(['windowInfo', 'width']))
browserOpts.height = firstDefinedValue(browserOpts.height, immutableWindowState.getIn(['windowInfo', 'height']))
} else {
// no inner height so check outer height or use default
browserOpts.height = firstDefinedValue(browserOpts.outerHeight, defaults.height)
browserOpts.width = firstDefinedValue(browserOpts.width, browserOpts.innerWidth, defaults.width)
// height and innerHeight are the frame webview size
browserOpts.height = firstDefinedValue(browserOpts.height, browserOpts.innerHeight)
if (typeof browserOpts.height === 'number') {
// add navbar height to get total height for BrowserWindow
browserOpts.height = browserOpts.height + navbarHeight()
} else {
// no inner height so check outer height or use default
browserOpts.height = firstDefinedValue(browserOpts.outerHeight, defaults.height)
}
}

return browserOpts
}

Expand All @@ -69,10 +71,10 @@ const setWindowPosition = (browserOpts, defaults, immutableWindowState) => {
const screenPos = electron.screen.getCursorScreenPoint()
browserOpts.x = screenPos.x
browserOpts.y = screenPos.y
} else if (immutableWindowState.getIn(['ui', 'position'])) {
} else if (immutableWindowState.getIn(['windowInfo'])) {
// Position comes from window state
browserOpts.x = firstDefinedValue(browserOpts.x, immutableWindowState.getIn(['ui', 'position', 0]))
browserOpts.y = firstDefinedValue(browserOpts.y, immutableWindowState.getIn(['ui', 'position', 1]))
browserOpts.x = firstDefinedValue(browserOpts.x, immutableWindowState.getIn(['windowInfo', 'left']))
browserOpts.y = firstDefinedValue(browserOpts.y, immutableWindowState.getIn(['windowInfo', 'top']))
} else if (typeof defaults.x === 'number' && typeof defaults.y === 'number') {
// Position comes from the default position
browserOpts.x = firstDefinedValue(browserOpts.x, defaults.x)
Expand All @@ -85,6 +87,18 @@ const setWindowPosition = (browserOpts, defaults, immutableWindowState) => {
return browserOpts
}

const setMaximized = (state, browserOpts, immutableWindowState) => {
if (Object.keys(browserOpts).length > 0 && !browserOpts.checkMaximized) {
return false
}

if (immutableWindowState.getIn(['windowInfo'])) {
return immutableWindowState.getIn(['windowInfo', 'state']) === 'maximized'
}

return state.getIn(['defaultWindowParams', 'maximized']) || false
}

function windowDefaults (state) {
return {
show: false,
Expand Down Expand Up @@ -116,6 +130,7 @@ function setDefaultWindowSize (state) {
if (!state) {
return
}

const screen = electron.screen
const primaryDisplay = screen.getPrimaryDisplay()
if (!state.getIn(['defaultWindowParams', 'width']) && !state.get('defaultWindowWidth') &&
Expand All @@ -133,6 +148,7 @@ const createWindow = (state, action) => {
const immutableWindowState = action.get('restoredState') || Immutable.Map()
state = setDefaultWindowSize(state)
const defaults = windowDefaults(state)
const isMaximized = setMaximized(state, browserOpts, immutableWindowState)

browserOpts = setWindowDimensions(browserOpts, defaults, immutableWindowState)
browserOpts = setWindowPosition(browserOpts, defaults, immutableWindowState)
Expand All @@ -141,9 +157,18 @@ const createWindow = (state, action) => {
delete browserOpts.top

const screen = electron.screen
const primaryDisplay = screen.getPrimaryDisplay()
let primaryDisplay = screen.getPrimaryDisplay()
const parentWindowKey = browserOpts.parentWindowKey
const parentWindow = parentWindowKey ? BrowserWindow.fromId(parentWindowKey) : BrowserWindow.getFocusedWindow()
if (browserOpts.x != null && browserOpts.y != null) {
const matchingDisplay = screen.getDisplayMatching(browserOpts)
if (matchingDisplay != null) {
primaryDisplay = matchingDisplay
}
}

const parentWindow = parentWindowKey
? BrowserWindow.fromId(parentWindowKey)
: BrowserWindow.getFocusedWindow()
const bounds = parentWindow ? parentWindow.getBounds() : primaryDisplay.bounds

// position on screen should be relative to focused window
Expand Down Expand Up @@ -207,6 +232,10 @@ const createWindow = (state, action) => {
windowProps.icon = path.join(__dirname, '..', '..', 'res', 'app.png')
}

if (immutableWindowState.getIn(['windowInfo', 'state']) === 'fullscreen') {
windowProps.fullscreen = true
}

const homepageSetting = getSetting(settings.HOMEPAGE)
const startupSetting = getSetting(settings.STARTUP_MODE)
const toolbarUserInterfaceScale = getSetting(settings.TOOLBAR_UI_SCALE)
Expand All @@ -218,7 +247,7 @@ const createWindow = (state, action) => {

// initialize frames state
let frames = Immutable.List()
if (restoredImmutableWindowState) {
if (restoredImmutableWindowState && restoredImmutableWindowState.get('frames', Immutable.List()).size > 0) {
frames = restoredImmutableWindowState.get('frames')
restoredImmutableWindowState = restoredImmutableWindowState.set('frames', Immutable.List())
restoredImmutableWindowState = restoredImmutableWindowState.set('tabs', Immutable.List())
Expand All @@ -242,14 +271,10 @@ const createWindow = (state, action) => {
frames = Immutable.fromJS([{}])
}

if (immutableWindowState.getIn(['ui', 'isMaximized'])) {
if (isMaximized) {
mainWindow.maximize()
}

if (immutableWindowState.getIn(['ui', 'isFullScreen'])) {
mainWindow.setFullScreen(true)
}

mainWindow.webContents.on('did-finish-load', (e) => {
const appStore = require('../../../js/stores/appStore')
mainWindow.webContents.setZoomLevel(zoomLevel[toolbarUserInterfaceScale] || 0.0)
Expand Down Expand Up @@ -310,15 +335,13 @@ const windowsReducer = (state, action, immutableAction) => {
break
case appConstants.APP_WINDOW_UPDATED:
state = windowState.maybeCreateWindow(state, action)
break
case appConstants.APP_DEFAULT_WINDOW_PARAMS_CHANGED:
if (action.get('size')) {
state = state.setIn(['defaultWindowParams', 'width'], action.getIn(['size', 0]))
state = state.setIn(['defaultWindowParams', 'height'], action.getIn(['size', 1]))
}
if (action.get('position')) {
state = state.setIn(['defaultWindowParams', 'x'], action.getIn(['position', 0]))
state = state.setIn(['defaultWindowParams', 'y'], action.getIn(['position', 1]))
if (action.get('updateDefault')) {
state = state
.setIn(['defaultWindowParams', 'width'], action.getIn(['windowValue', 'width']))
.setIn(['defaultWindowParams', 'height'], action.getIn(['windowValue', 'height']))
.setIn(['defaultWindowParams', 'x'], action.getIn(['windowValue', 'x']))
.setIn(['defaultWindowParams', 'y'], action.getIn(['windowValue', 'y']))
.setIn(['defaultWindowParams', 'maximized'], action.getIn(['windowValue', 'state']) === 'maximized')
}
break
case windowConstants.WINDOW_SHOULD_SET_TITLE:
Expand Down
23 changes: 9 additions & 14 deletions app/browser/windows.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const windowState = require('../common/state/windowState')
const Immutable = require('immutable')
const pinnedSitesState = require('../common/state/pinnedSitesState')
const pinnedSitesUtil = require('../common/lib/pinnedSitesUtil')
const windowActions = require('../../js/actions/windowActions')

// TODO(bridiver) - set window uuid
let currentWindows = {}
Expand Down Expand Up @@ -55,10 +56,11 @@ const getWindowValue = (windowId) => {
}
}

const updateWindow = (windowId) => {
const updateWindow = (windowId, updateDefault = false) => {
const windowValue = getWindowValue(windowId)
if (windowValue) {
appActions.windowUpdated(windowValue)
appActions.windowUpdated(windowValue, updateDefault)
windowActions.onWindowUpdate(windowId, windowValue)
}
}

Expand Down Expand Up @@ -189,6 +191,7 @@ const api = {
LocalShortcuts.register(win)

appActions.windowCreated(windowValue)
windowActions.onWindowUpdate(windowId, windowValue)
})
win.once('closed', () => {
cleanupWindow(windowId)
Expand All @@ -198,7 +201,7 @@ const api = {
updateWindowDebounce(windowId)
})
win.on('focus', () => {
updateWindowDebounce(windowId)
updateWindowDebounce(windowId, true)
})
win.on('show', () => {
updateWindowDebounce(windowId)
Expand All @@ -207,7 +210,7 @@ const api = {
updateWindowDebounce(windowId)
})
win.on('maximize', () => {
updateWindowDebounce(windowId)
updateWindowDebounce(windowId, true)
})
win.on('unmaximize', () => {
updateWindowDebounce(windowId)
Expand All @@ -219,18 +222,10 @@ const api = {
updateWindowDebounce(windowId)
})
win.on('resize', () => {
updateWindowDebounce(windowId)
const size = win.getSize()
const position = win.getPosition()
// NOTE: the default window size is whatever the last window resize was
appActions.defaultWindowParamsChanged(size, position)
updateWindowDebounce(windowId, true)
})
win.on('move', () => {
updateWindowMove(windowId)
const size = win.getSize()
const position = win.getPosition()
// NOTE: the default window position is whatever the last window move was
appActions.defaultWindowParamsChanged(size, position)
updateWindowMove(windowId, true)
})
win.on('enter-full-screen', () => {
updateWindowDebounce(windowId)
Expand Down
1 change: 0 additions & 1 deletion app/renderer/components/main/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,6 @@ class Main extends React.Component {
}
}, { passive: true })


// disable dnd by default
window.addEventListener('dragover', function (event) {
// allow webviews to handle dnd
Expand Down
26 changes: 17 additions & 9 deletions app/sessionStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,18 +67,26 @@ 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 (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 {
immutablePayload = immutablePayload.delete('perWindowState')

// Don't persist private frames
if (immutablePayload.get('perWindowState')) {
if (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 {
// we still need to preserve window position/size info
immutablePayload.get('perWindowState').forEach((immutableWndPayload, i) => {
let windowInfo = Immutable.Map()
windowInfo = windowInfo.set('windowInfo', immutableWndPayload.get('windowInfo'))
immutablePayload = immutablePayload.setIn(['perWindowState', i], windowInfo)
})
}
}

try {
Expand Down
14 changes: 9 additions & 5 deletions docs/state.md
Original file line number Diff line number Diff line change
Expand Up @@ -677,10 +677,7 @@ WindowStore
downloadsToolbar: {
isVisible: boolean // whether or not the downloads toolbar is visible
},
isFocused: boolean, // true if window has focus
isClearBrowsingDataPanelVisible: boolean, // true if the Clear Browsing Data panel is visible
isFullScreen: boolean, // true if window is fullscreen
isMaximized: boolean, // true if window is maximized
menubar: {
isVisible: boolean, // true if Menubar control is visible
lastFocusedSelector: string, // selector for the last selected element (browser ui, not frame content)
Expand All @@ -690,14 +687,12 @@ WindowStore
noScriptInfo: {
isVisible: boolean // Whether the noscript infobox is visible
},
position: array, // last known window position [x, y]
releaseNotes: {
isVisible: boolean // whether or not to show release notes
},
siteInfo: {
isVisible: boolean // whether or not to show site info like # of blocked ads
},
size: array, // last known window size [x, y]
tabs: {
hoverTabIndex: number, // index of the current hovered tab
previewMode: boolean, // whether or not tab preview should be fired based on mouse idle time
Expand All @@ -709,6 +704,15 @@ WindowStore
alsoAddRememberSiteSetting: boolean, // true if an allow always rule should be added for the acitve frame as well if installed
location: string, // location this dialog is for
shown: boolean // true if the panel is shown
},
windowInfo: {
focused: boolean,
height: number,
left: number,
state: string // "normal", "minimized", "maximized", or "fullscreen"
top: number,
type: string, // "normal", "popup", or "devtools"
width: number,
}
}
```
18 changes: 3 additions & 15 deletions js/actions/appActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,11 @@ const appActions = {
})
},

windowUpdated: function (windowValue) {
windowUpdated: function (windowValue, updateDefault) {
dispatch({
actionType: appConstants.APP_WINDOW_UPDATED,
windowValue
windowValue,
updateDefault
})
},

Expand Down Expand Up @@ -304,19 +305,6 @@ const appActions = {
})
},

/**
* Sets the default window size / position
* @param {Array} size - [width, height]
* @param {Array} position - [x, y]
*/
defaultWindowParamsChanged: function (size, position) {
dispatch({
actionType: appConstants.APP_DEFAULT_WINDOW_PARAMS_CHANGED,
size,
position
})
},

/**
* Sets the etag value for a downloaded data file.
* This is used for keeping track of when to re-download adblock and tracking
Expand Down
10 changes: 10 additions & 0 deletions js/actions/windowActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -1201,6 +1201,16 @@ const windowActions = {
bookmarkKey,
type
})
},

onWindowUpdate: function (windowId, windowValue) {
dispatch({
actionType: windowConstants.WINDOW_ON_WINDOW_UPDATE,
queryInfo: {
windowId
},
windowValue
})
}
}

Expand Down
1 change: 0 additions & 1 deletion js/constants/appConstants.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ const appConstants = {
APP_REMOVE_HISTORY_SITE: _,
APP_MERGE_DOWNLOAD_DETAIL: _, // Sets an individual download detail
APP_CLEAR_COMPLETED_DOWNLOADS: _, // Removes all completed downloads
APP_DEFAULT_WINDOW_PARAMS_CHANGED: _,
APP_SET_DATA_FILE_ETAG: _,
APP_SET_DATA_FILE_LAST_CHECK: _,
APP_SET_RESOURCE_ENABLED: _,
Expand Down
3 changes: 2 additions & 1 deletion js/constants/windowConstants.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ const windowConstants = {
WINDOW_ON_BOOKMARK_ADDED: _,
WINDOW_ON_ADD_BOOKMARK_FOLDER: _,
WINDOW_ON_EDIT_BOOKMARK_FOLDER: _,
WINDOW_ON_BOOKMARK_FOLDER_CLOSE: _
WINDOW_ON_BOOKMARK_FOLDER_CLOSE: _,
WINDOW_ON_WINDOW_UPDATE: _
}

module.exports = mapValuesByKeys(windowConstants)
Loading

0 comments on commit 921c203

Please sign in to comment.