diff --git a/app/browser/windows.js b/app/browser/windows.js index 62f0e3b52ba..a77d231c2f6 100644 --- a/app/browser/windows.js +++ b/app/browser/windows.js @@ -4,6 +4,7 @@ const electron = require('electron') const Immutable = require('immutable') +const { EventEmitter } = require('events') const appActions = require('../../js/actions/appActions') const appStore = require('../../js/stores/appStore') const appUrlUtil = require('../../js/lib/appUrlUtil') @@ -12,7 +13,6 @@ const debounce = require('../../js/lib/debounce') const {getSetting} = require('../../js/settings') const locale = require('../locale') const LocalShortcuts = require('../localShortcuts') -const {initWindowCacheState} = require('../sessionStoreShutdown') const {makeImmutable} = require('../common/state/immutableUtil') const {getPinnedTabsByWindowId} = require('../common/state/tabState') const messages = require('../../js/constants/messages') @@ -31,6 +31,8 @@ const {app, BrowserWindow, ipcMain} = electron // TODO(bridiver) - set window uuid let currentWindows = {} const windowPinnedTabStateMemoize = new WeakMap() +const publicEvents = new EventEmitter() +let lastCreatedWindowIsRendererWindow = false const getWindowState = (win) => { if (win.isFullScreen()) { @@ -154,6 +156,12 @@ function showDeferredShowWindow (win) { const api = { init: (state, action) => { app.on('browser-window-created', function (event, win) { + // handle non-renderer windows + if (!lastCreatedWindowIsRendererWindow) { + // nothing to do as yet + return + } + lastCreatedWindowIsRendererWindow = false let windowId = -1 const updateWindowMove = debounce(updateWindow, 100) const updateWindowDebounce = debounce(updateWindow, 5) @@ -423,10 +431,16 @@ const api = { fullscreenWhenRendered = true } // create window with Url to renderer + // new-window action handler is sync, so won't be added to any 'renderer collection' we create here + // by the time the action handler runs. + // Instead, add a flag to indicate the next-created window is an actual + // tabbed renderer window + lastCreatedWindowIsRendererWindow = true const win = new electron.BrowserWindow(windowOptions) win.loadURL(appUrlUtil.getBraveExtIndexHTML()) + // TODO: pass UUID - initWindowCacheState(win.id, immutableState) + publicEvents.emit('new-window-state', win.id, immutableState) // let the windowReady handler know to show the window win.__showWhenRendered = showWhenRendered if (win.__showWhenRendered) { @@ -501,6 +515,20 @@ const api = { return windowState.WINDOW_ID_NONE }, + /** + * Provides an array of all Browser Windows which are actual + * main windows (not background workers), and are not destroyed + */ + getAllRendererWindows: () => { + return Object.keys(currentWindows) + .map(key => currentWindows[key]) + .filter(win => win && !win.isDestroyed()) + }, + + on: (...args) => publicEvents.on(...args), + off: (...args) => publicEvents.off(...args), + once: (...args) => publicEvents.once(...args), + privateMethods: () => { return process.env.NODE_ENV === 'test' ? { diff --git a/app/sessionStoreShutdown.js b/app/sessionStoreShutdown.js index 45ef7d7571f..5a4c40d765e 100644 --- a/app/sessionStoreShutdown.js +++ b/app/sessionStoreShutdown.js @@ -4,8 +4,9 @@ 'use strict' -const {BrowserWindow, app, ipcMain} = require('electron') +const {app, ipcMain} = require('electron') const sessionStore = require('./sessionStore') +const windows = require('./browser/windows') const updateStatus = require('../js/constants/updateStatus') const updater = require('./updater') const appConfig = require('../js/constants/appConfig') @@ -32,6 +33,10 @@ let immutableWindowStateCache let sessionStoreQueue let appStore +windows.on('new-window-state', (windowId, immutableWindowState) => { + immutableWindowStateCache = immutableWindowStateCache.set(windowId, immutableWindowState) +}) + // Useful for automated tests const reset = () => { immutablePerWindowState = Immutable.List() @@ -76,7 +81,7 @@ const saveAppState = (forceSave = false) => { } let immutableAppState = appStore.getState().set('perWindowState', immutablePerWindowState) - const receivedAllWindows = immutablePerWindowState.size === BrowserWindow.getAllWindows().length + const receivedAllWindows = immutablePerWindowState.size === windows.getAllRendererWindows().length if (receivedAllWindows) { clearTimeout(saveAppStateTimeout) } @@ -138,12 +143,14 @@ const initiateSessionStateSave = () => { if (isAllWindowsClosed && immutableLastWindowClosedState) { immutablePerWindowState = immutablePerWindowState.push(immutableLastWindowClosedState) saveAppState(true) - } else if (BrowserWindow.getAllWindows().length > 0) { + } else if (windows.getAllRendererWindows().length > 0) { ++windowCloseRequestId - const windowIds = BrowserWindow.getAllWindows().map((win) => { + const windowIds = windows.getAllRendererWindows().map((win) => { return win.id }) - BrowserWindow.getAllWindows().forEach((win) => win.webContents.send(messages.REQUEST_WINDOW_STATE, windowCloseRequestId)) + windows.getAllRendererWindows().forEach((win) => { + win.webContents.send(messages.REQUEST_WINDOW_STATE, windowCloseRequestId) + }) // 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(() => { @@ -166,10 +173,6 @@ const removeWindowFromCache = (windowId) => { immutableWindowStateCache = immutableWindowStateCache.delete(windowId) } -const initWindowCacheState = (windowId, immutableWindowState) => { - immutableWindowStateCache = immutableWindowStateCache.set(windowId, immutableWindowState) -} - app.on('before-quit', (e) => { if (shuttingDown && sessionStateStoreComplete) { return @@ -241,6 +244,5 @@ module.exports = { startSessionSaveInterval, initiateSessionStateSave, reset, - removeWindowFromCache, - initWindowCacheState + removeWindowFromCache } diff --git a/test/unit/app/sessionStoreShutdownTest.js b/test/unit/app/sessionStoreShutdownTest.js index bdfbd81b544..016cc1e0eb0 100644 --- a/test/unit/app/sessionStoreShutdownTest.js +++ b/test/unit/app/sessionStoreShutdownTest.js @@ -139,6 +139,7 @@ describe('sessionStoreShutdown unit tests', function () { describe('app before-quit event', function () { before(function () { + this.windowsAPI = require('../../../app/browser/windows') this.appStoreGetStateStub = sinon.stub(appStore, 'getState', () => { return Immutable.fromJS({}) }) @@ -160,7 +161,7 @@ describe('sessionStoreShutdown unit tests', function () { describe('with no windows', function () { before(function () { this.requestId = 1 - this.getAllWindowsStub = sinon.stub(fakeElectron.BrowserWindow, 'getAllWindows', () => { + this.getAllWindowsStub = sinon.stub(this.windowsAPI, 'getAllRendererWindows', () => { return [] }) }) @@ -234,7 +235,7 @@ describe('sessionStoreShutdown unit tests', function () { before(function () { this.fakeWindow1 = new FakeWindow(1) this.requestId = 1 - this.getAllWindowsStub = sinon.stub(fakeElectron.BrowserWindow, 'getAllWindows', () => { + this.getAllWindowsStub = sinon.stub(this.windowsAPI, 'getAllRendererWindows', () => { return [this.fakeWindow1] }) }) @@ -286,7 +287,7 @@ describe('sessionStoreShutdown unit tests', function () { this.fakeWindow2 = new FakeWindow(2) this.fakeWindow3 = new FakeWindow(3) this.requestId = 1 - this.getAllWindowsStub = sinon.stub(fakeElectron.BrowserWindow, 'getAllWindows', () => { + this.getAllWindowsStub = sinon.stub(this.windowsAPI, 'getAllRendererWindows', () => { return [this.fakeWindow1, this.fakeWindow2, this.fakeWindow3] }) })