Ensure buffer window (or any window with no frames) is not the last r…

…emembered window to restore on next startup (windows and linux)

Fix #13278

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) {

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

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) => {