diff --git a/app/browser/windows.js b/app/browser/windows.js index 7cd914f1d80..3ee1fa6ef64 100644 --- a/app/browser/windows.js +++ b/app/browser/windows.js @@ -341,7 +341,10 @@ const api = { // if we have a bufferWindow, the 'window-all-closed' // event will not fire once the last window is closed, // so close the buffer window if this is the last closed window - // apart from the buffer window + // apart from the buffer window. + // This would mean that the last window to close is the buffer window, but + // that will not get saved to state as the last-closed window which should be restored + // since we won't save state if there are no frames. if (!platformUtil.isDarwin() && api.getBufferWindow()) { const remainingWindows = api.getAllRendererWindows().filter(win => win !== api.getBufferWindow()) if (!remainingWindows.length) { diff --git a/app/sessionStoreShutdown.js b/app/sessionStoreShutdown.js index 5a4c40d765e..09edf2ca884 100644 --- a/app/sessionStoreShutdown.js +++ b/app/sessionStoreShutdown.js @@ -218,7 +218,10 @@ ipcMain.on(messages.RESPONSE_WINDOW_STATE, (evt, mem) => { }) ipcMain.on(messages.LAST_WINDOW_STATE, (evt, data) => { - if (data) { + // Remember last window (that was not buffer window, i.e. had frames). + // When the last tab of a window closes, the window is closed before the tab closes, so + // a used closing window will almost always have at least 1 frame. + if (data && data.frames && data.frames.length) { immutableLastWindowClosedState = Immutable.fromJS(data) } }) diff --git a/test/unit/app/sessionStoreShutdownTest.js b/test/unit/app/sessionStoreShutdownTest.js index 016cc1e0eb0..156759974db 100644 --- a/test/unit/app/sessionStoreShutdownTest.js +++ b/test/unit/app/sessionStoreShutdownTest.js @@ -36,6 +36,16 @@ describe('sessionStoreShutdown unit tests', function () { const FakeWindow = require('../lib/fakeWindow') const fakeAdBlock = require('../lib/fakeAdBlock') + const testRestorableWindowFrames = [ + { + key: 1, + partitionNumber: 0, + src: 'https://brave.com', + location: 'https://brave.com', + unloaded: true + } + ] + before(function () { this.clock = sinon.useFakeTimers() mockery.enable({ @@ -120,21 +130,32 @@ describe('sessionStoreShutdown unit tests', function () { }) it('works for first closed window', function () { - const windowState = { a: 1, frames: [] } + const windowState = { a: 1, frames: testRestorableWindowFrames } 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].toJS(), windowState) }) it('works for subsequent windows', function () { - const windowState1 = { b: 1, frames: [] } - const windowState2 = { x: 2, frames: [] } + const windowState1 = { b: 1, frames: testRestorableWindowFrames } + const windowState2 = { x: 2, frames: testRestorableWindowFrames } fakeElectron.ipcMain.send(messages.LAST_WINDOW_STATE, {}, windowState1) fakeElectron.ipcMain.send(messages.LAST_WINDOW_STATE, {}, windowState2) process.emit(messages.UNDO_CLOSED_WINDOW) assert(this.newWindowStub.calledOnce) assert.deepEqual(this.newWindowStub.getCall(0).args[2].toJS(), windowState2) }) + // test for buffer window close (only case a window will be closed with no frames, + // since closing the last tab of a window closes the window with tab still there) + it('ignores windows with no frames', function () { + const windowState1 = { b: 1, frames: testRestorableWindowFrames } + const windowState2 = { x: 2, frames: [] } + fakeElectron.ipcMain.send(messages.LAST_WINDOW_STATE, {}, windowState1) + fakeElectron.ipcMain.send(messages.LAST_WINDOW_STATE, {}, windowState2) + process.emit(messages.UNDO_CLOSED_WINDOW) + assert(this.newWindowStub.calledOnce) + assert.deepEqual(this.newWindowStub.getCall(0).args[2].toJS(), windowState1) + }) }) describe('app before-quit event', function () { @@ -182,10 +203,11 @@ describe('sessionStoreShutdown unit tests', function () { }) it('remembers last closed window with no windows (Win32)', function (cb) { isWindows = true - const windowState = Immutable.fromJS({ a: 1 }) + const windowState = { a: 1, frames: testRestorableWindowFrames } fakeElectron.ipcMain.send(messages.LAST_WINDOW_STATE, {}, windowState) fakeElectron.app.emit('window-all-closed') const saveAppStateStub = sinon.stub(sessionStore, 'saveAppState', (state) => { + console.log(state.toJS()) isWindows = false assert.equal(saveAppStateStub.calledOnce, true) saveAppStateStub.restore() @@ -198,7 +220,7 @@ describe('sessionStoreShutdown unit tests', function () { this.clock.tick(1) }) it('remembers last closed window with no windows (Linux)', function (cb) { - const windowState = Immutable.fromJS({ a: 1 }) + const windowState = { a: 1, frames: testRestorableWindowFrames } fakeElectron.ipcMain.send(messages.LAST_WINDOW_STATE, {}, windowState) fakeElectron.app.emit('window-all-closed') const saveAppStateStub = sinon.stub(sessionStore, 'saveAppState', (state) => { @@ -214,7 +236,7 @@ describe('sessionStoreShutdown unit tests', function () { }) it('remembers last closed window with no windows (macOS)', function (cb) { isDarwin = true - const windowState = Immutable.fromJS({ a: 1 }) + const windowState = Immutable.fromJS({ a: 1, frames: testRestorableWindowFrames }) fakeElectron.ipcMain.send(messages.LAST_WINDOW_STATE, {}, windowState) fakeElectron.app.emit('window-all-closed') const saveAppStateStub = sinon.stub(sessionStore, 'saveAppState', (state) => {