From 18f302c166ffb90aca9ef94bae0bc6abf1598866 Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Wed, 9 Aug 2017 22:20:15 -0400 Subject: [PATCH] Store changes for all immutable sets Fix #10374 Auditors: @bsclifton --- app/sessionStore.js | 10 +++--- app/sessionStoreShutdown.js | 4 +-- test/unit/app/sessionStoreTest.js | 55 +++++++++++++++++++++++++++++-- 3 files changed, 59 insertions(+), 10 deletions(-) diff --git a/app/sessionStore.js b/app/sessionStore.js index 9d026d084aa..d4d508677cc 100644 --- a/app/sessionStore.js +++ b/app/sessionStore.js @@ -90,11 +90,11 @@ module.exports.saveAppState = (immutablePayload, isShutdown) => { try { immutablePayload = module.exports.cleanAppData(immutablePayload, isShutdown) - immutablePayload.set('cleanedOnShutdown', isShutdown) + immutablePayload = immutablePayload.set('cleanedOnShutdown', isShutdown) } catch (e) { - immutablePayload.set('cleanedOnShutdown', false) + immutablePayload = immutablePayload.set('cleanedOnShutdown', false) } - immutablePayload.set('lastAppVersion', app.getVersion()) + immutablePayload = immutablePayload.set('lastAppVersion', app.getVersion()) if (isShutdown) { module.exports.cleanSessionDataOnShutdown() @@ -173,7 +173,7 @@ module.exports.cleanPerWindowData = (immutablePerWindowData, isShutdown) => { // If a blob is present for the thumbnail, create the object URL if (immutableFrame.get('thumbnailBlob')) { try { - immutableFrame.set('thumbnailUrl', window.URL.createObjectURL(immutableFrame.get('thumbnailBlob'))) + immutableFrame = immutableFrame.set('thumbnailUrl', window.URL.createObjectURL(immutableFrame.get('thumbnailBlob'))) } catch (e) { immutableFrame = immutableFrame.delete('thumbnailUrl') } @@ -252,7 +252,7 @@ module.exports.cleanPerWindowData = (immutablePerWindowData, isShutdown) => { } if (immutablePerWindowData.get('frames')) { // Don't restore pinned locations because they will be auto created by the app state change event - immutablePerWindowData.set('frames', + immutablePerWindowData = immutablePerWindowData.set('frames', immutablePerWindowData.get('frames') .filter((frame) => !frame.get('pinnedLocation'))) immutablePerWindowData = diff --git a/app/sessionStoreShutdown.js b/app/sessionStoreShutdown.js index d70a7cc54fd..6643383fb2c 100644 --- a/app/sessionStoreShutdown.js +++ b/app/sessionStoreShutdown.js @@ -101,9 +101,9 @@ const saveAppState = (forceSave = false) => { // In this case on win32, the process doesn't try to auto restart, so avoid the user // having to open the app twice. Maybe squirrel detects the app is already shutting down. if (platformUtil.isWindows()) { - immutableAppState.setIn(['updates', 'status'], updateStatus.UPDATE_APPLYING_RESTART) + immutableAppState = immutableAppState.setIn(['updates', 'status'], updateStatus.UPDATE_APPLYING_RESTART) } else { - immutableAppState.setIn(['updates', 'status'], updateStatus.UPDATE_APPLYING_NO_RESTART) + immutableAppState = immutableAppState.setIn(['updates', 'status'], updateStatus.UPDATE_APPLYING_NO_RESTART) } } diff --git a/test/unit/app/sessionStoreTest.js b/test/unit/app/sessionStoreTest.js index 9aeafa60d0b..3624aef47b6 100644 --- a/test/unit/app/sessionStoreTest.js +++ b/test/unit/app/sessionStoreTest.js @@ -122,32 +122,61 @@ describe('sessionStore unit tests', function () { cleanSessionDataOnShutdownStub.restore() }) - it('calls cleanAppData', function () { + it('calls cleanAppData', function (cb) { cleanAppDataStub.reset() return sessionStore.saveAppState(Immutable.Map()) .then(function (result) { assert.equal(cleanAppDataStub.calledOnce, true) + cb() }, function (err) { assert(!err) }) }) describe('with isShutdown', function () { - it('calls cleanSessionDataOnShutdown if true', function () { + before(function () { + this.writeImportantSpy = sinon.spy(muon.file, 'writeImportant') + }) + after(function () { + this.writeImportantSpy.restore() + }) + it('calls cleanSessionDataOnShutdown if true', function (cb) { cleanSessionDataOnShutdownStub.reset() return sessionStore.saveAppState(Immutable.Map(), true) .then(() => { assert.equal(cleanSessionDataOnShutdownStub.calledOnce, true) + cb() }, function (err) { assert(!err) }) }) - it('does not call cleanSessionDataOnShutdown if false', function () { + it('does not call cleanSessionDataOnShutdown if false', function (cb) { cleanSessionDataOnShutdownStub.reset() return sessionStore.saveAppState(Immutable.Map(), false) .then(() => { assert.equal(cleanSessionDataOnShutdownStub.notCalled, true) + cb() + }, function (err) { + assert(!err) + }) + }) + + it('sets cleanedOnShutdown for saveAppState', function (cb) { + sessionStore.saveAppState(Immutable.Map(), true) + .then(() => { + assert.equal(JSON.parse(this.writeImportantSpy.getCall(0).args[1]).cleanedOnShutdown, true) + cb() + }, function (err) { + assert(!err) + }) + }) + + it('sets lastAppVersion for saveAppState', function (cb) { + sessionStore.saveAppState(Immutable.Map(), true) + .then(() => { + assert.equal(JSON.parse(this.writeImportantSpy.getCall(0).args[1]).lastAppVersion, '0.14.0') + cb() }, function (err) { assert(!err) }) @@ -156,6 +185,26 @@ describe('sessionStore unit tests', function () { }) describe('cleanPerWindowData', function () { + it('clears pinned frames', function () { + const data = Immutable.fromJS({frames: [ + { + location: 'https://brave.com/cezar/master/ken/fight', + pinnedLocation: 'https://brave.com/cezar/master/ken/fight' + }, { + key: 1, + location: 'https://brave.com/cezar/monkey/fights/dragon', + src: 'https://brave.com/cezar/monkey/fights/dragon', + unloaded: true + } + ]}) + const result = sessionStore.cleanPerWindowData(data, true) + assert.deepEqual(result.get('frames').toJS(), [{ + key: 1, + location: 'https://brave.com/cezar/monkey/fights/dragon', + src: 'https://brave.com/cezar/monkey/fights/dragon', + unloaded: true + }]) + }) }) describe('cleanAppData', function () {