Merge pull request #12422 from petemill/fix/issue-11859

Reinstate call to cleanup window data
brave · Dec 28, 2017 · 119e4e7 · 119e4e7
2 parents 358641a + 947c213
commit 119e4e7
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 18 deletions.
diff --git a/app/browser/reducers/windowsReducer.js b/app/browser/reducers/windowsReducer.js
@@ -329,9 +329,8 @@ const windowsReducer = (state, action, immutableAction) => {
       break
     case appConstants.APP_WINDOW_CLOSED:
       state = windowState.removeWindow(state, action)
-      const windowId = action.getIn(['windowValue', 'windowId'])
+      const windowId = action.get('windowId')
       sessionStoreShutdown.removeWindowFromCache(windowId)
-      windows.cleanupWindow(windowId)
       break
     case appConstants.APP_WINDOW_CREATED:
     case appConstants.APP_WINDOW_RESIZED:

diff --git a/app/browser/windows.js b/app/browser/windows.js
@@ -236,6 +236,8 @@ const api = {
         appActions.windowCreated(windowValue, windowId)
       })
       win.once('closed', () => {
+        appActions.windowClosed(windowId)
+        cleanupWindow(windowId)
       })
       win.on('blur', () => {
         appActions.windowBlurred(windowId)

diff --git a/app/common/state/windowState.js b/app/common/state/windowState.js
@@ -115,8 +115,7 @@ const api = {
   removeWindow: (state, action) => {
     action = validateAction(action)
     state = validateState(state)
-    let windowValue = validateWindowValue(action.get('windowValue'))
-    let windowId = validateId('windowId', windowValue.get('windowId'))
+    const windowId = action.get('windowId')
     return api.removeWindowByWindowId(state, windowId)
   },
 

diff --git a/js/actions/appActions.js b/js/actions/appActions.js
@@ -65,10 +65,10 @@ const appActions = {
     })
   },
 
-  windowClosed: function (windowValue) {
+  windowClosed: function (windowId) {
     dispatch({
       actionType: appConstants.APP_WINDOW_CLOSED,
-      windowValue
+      windowId
     })
   },
 

diff --git a/test/unit/common/state/windowStateTest.js b/test/unit/common/state/windowStateTest.js
@@ -116,12 +116,12 @@ const shouldValidateWindowValue = function (check) {
   })
 }
 
-const shouldValidateAction = function (check) {
+const shouldValidateAction = function (check, payload = { windowValue: { windowId: 1 } }) {
   it('throws an AssertionError if action does not contain a `windowValue` that is convertable to an Immutable.Map', function () {
     assert.doesNotThrow(
       () => {
-        check(Immutable.fromJS({ windowValue: { windowId: 1 } }))
-        check({ windowValue: { windowId: 1 } })
+        check(Immutable.fromJS(payload))
+        check(payload)
       },
       AssertionError
     )
@@ -142,7 +142,7 @@ const shouldValidateAction = function (check) {
   it('throws an AssertionError if `action` is not convertable to an Immutable.Map', function () {
     assert.doesNotThrow(
       () => {
-        check({ windowValue: { windowId: 1 } })
+        check(payload)
       },
       AssertionError
     )
@@ -327,24 +327,20 @@ describe('windowState', function () {
 
     it('returns a new immutable state with the window removed by `windowId`', function () {
       assert.deepEqual(
-        windowState.removeWindow(this.appState, { windowValue: { windowId: 2 } }).get('windows').toJS(),
+        windowState.removeWindow(this.appState, { windowId: 2 }).get('windows').toJS(),
         [{ windowId: 1 }])
     })
 
     shouldValidateAction((action) => {
       windowState.removeWindow(defaultAppState, action)
-    })
-
-    shouldValidateWindowValue((windowValue) => {
-      windowState.removeWindow(defaultAppState, { windowValue })
-    })
+    }, { windowId: 1 })
 
     shouldValidateId((windowId) => {
-      windowState.removeWindow(defaultAppState, { windowValue: { windowId } })
+      windowState.removeWindow(defaultAppState, { windowId })
     })
 
     shouldValidateWindowState((state) => {
-      windowState.removeWindow(state, { windowValue: { windowId: 1 } })
+      windowState.removeWindow(state, { windowId: 1 })
     })
   })