Skip to content

Commit

Permalink
Windows restore in the correct order to ensure that the previous z-in…
Browse files Browse the repository at this point in the history
…dex is restored and the previously-focused window gets focus

Fix brave#13158
  • Loading branch information
petemill authored and ryanml committed Feb 27, 2018
1 parent 53a414e commit 7fed589
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 19 deletions.
35 changes: 32 additions & 3 deletions app/browser/windows.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,37 @@ const updatePinnedTabs = (win, appState) => {
}
}

function refocusFocusedWindow () {
const focusedWindow = BrowserWindow.getFocusedWindow()
if (focusedWindow) {
if (shouldDebugWindowEvents) {
console.log('focusing on window', focusedWindow.id)
}
focusedWindow.focus()
}
}

function showDeferredShowWindow (win) {
if (shouldDebugWindowEvents) {
console.log(`Window [${win.id}] showDeferredShowWindow`)
// were we asked to make the window active / foreground?
const shouldShowInactive = win.webContents.browserWindowOptions.inactive
if (shouldShowInactive) {
// we were asked NOT to show the window active.
// we should maintain focus on the window which already has it
if (shouldDebugWindowEvents) {
console.log('showing deferred window inactive', win.id)
}
win.showInactive()
// Whilst the window will not have focus, it will potentially be
// on top of the window which already had focus,
// so re-focus the focused window.
setImmediate(refocusFocusedWindow)
} else {
// we were asked to show the window active
if (shouldDebugWindowEvents) {
console.log('showing deferred window active', win.id)
}
win.show()
}
win.show()
if (win.__shouldFullscreen) {
// this timeout helps with an issue that
// when a user is loading from state, and
Expand All @@ -160,6 +186,9 @@ function showDeferredShowWindow (win) {
// spaces because macOS has switched away from the desktop space
setTimeout(() => {
win.setFullScreen(true)
if (shouldShowInactive) {
setImmediate(refocusFocusedWindow)
}
}, 100)
} else if (win.__shouldMaximize) {
win.maximize()
Expand Down
9 changes: 7 additions & 2 deletions app/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ app.on('ready', () => {
appActions.newWindow()
}
} else {
const lastIndex = loadedPerWindowImmutableState.size - 1
loadedPerWindowImmutableState
.sort((a, b) => {
let comparison = 0
Expand All @@ -222,8 +223,12 @@ app.on('ready', () => {

return comparison
})
.forEach((wndState) => {
appActions.newWindow(undefined, undefined, wndState)
.forEach((wndState, i) => {
const isLastWindow = i === lastIndex
if (CmdLine.shouldDebugWindowEvents && isLastWindow) {
console.log(`The restored window which should get focus has ${wndState.get('frames').size} frames`)
}
appActions.newWindow(undefined, isLastWindow ? undefined : { inactive: true }, wndState, true)
})
}
process.emit(messages.APP_INITIALIZED)
Expand Down
3 changes: 3 additions & 0 deletions js/entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ ipc.on(messages.INITIALIZE_WINDOW, (e, mem) => {
const windowValue = message.windowValue

currentWindow.setWindowId(windowValue.id)
if (process.env.NODE_ENV === 'development') {
console.debug(`This Window's ID is:`, windowValue.id)
}
const newState = Immutable.fromJS(message.windowState) || windowStore.getState()

appStoreRenderer.state = Immutable.fromJS(message.appState)
Expand Down
23 changes: 9 additions & 14 deletions js/stores/windowStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
* 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 performance */

const appDispatcher = require('../dispatcher/appDispatcher')
const EventEmitter = require('events').EventEmitter
const appActions = require('../actions/appActions')
Expand Down Expand Up @@ -752,21 +750,18 @@ const doAction = (action) => {
}
break
}
case windowConstants.WINDOW_ON_WINDOW_UPDATE:
case appConstants.APP_WINDOW_READY:
{
const oldInfo = windowState.get('windowInfo', Immutable.Map())
let windowValue = makeImmutable(action.windowValue)

if (windowValue.get('focused')) {
windowValue = windowValue.set('focusTime', performance.timing.navigationStart + performance.now())
}
windowState = windowState.set('windowInfo', oldInfo.merge(windowValue))
break
}
case appConstants.APP_WINDOW_UPDATED:
case appConstants.APP_WINDOW_RESIZED:
windowState = windowState.set('windowInfo', action.windowValue)
let windowValue = makeImmutable(action.windowValue)
const oldInfo = windowState.get('windowInfo', Immutable.Map())
// detect if window is newly focused
if (windowValue.get('focused') && !oldInfo.get('focused')) {
// record time of focus so we can make sure the window
// z-index is restored on app-restart
windowValue = windowValue.set('focusTime', new Date().getTime())
}
windowState = windowState.set('windowInfo', oldInfo.merge(windowValue))
break
case windowConstants.WINDOW_TAB_MOVE_INCREMENTAL_REQUESTED:
const sourceFrame = frameStateUtil.getActiveFrame(windowState)
Expand Down
1 change: 1 addition & 0 deletions test/unit/lib/fakeWindow.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ function FakeWindow (id) {
this.webContents = Object.assign(new EventEmitter())
this.webContents.send = this.webContents.emit
this._isVisible = false
this.webContents.browserWindowOptions = { }
}

util.inherits(FakeWindow, EventEmitter)
Expand Down

0 comments on commit 7fed589

Please sign in to comment.