fix tab move

auditors @bbondy
brave · May 16, 2017 · 0b851ed · 0b851ed
1 parent a34acee
commit 0b851ed
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 7 deletions.
diff --git a/app/renderer/reducers/frameReducer.js b/app/renderer/reducers/frameReducer.js
@@ -126,7 +126,8 @@ const frameReducer = (state, action, immutableAction) => {
       })
       break
     case windowConstants.WINDOW_CLOSE_FRAME:
-      if (action.frameKey < 0) {
+      // TODO(bridiver) - why does `will-destroy` fire twice?
+      if (action.frameKey < 0 || !frameStateUtil.getFrameByKey(state, action.frameKey)) {
         break
       }
       // Unless a caller explicitly specifies to close a pinned frame, then

diff --git a/js/components/frame.js b/js/components/frame.js
@@ -526,10 +526,7 @@ class Frame extends React.Component {
       windowActions.onFrameMouseLeave(this.props.tabId)
     })
     this.webview.addEventListener('will-destroy', (e) => {
-      if (this.frame.isEmpty()) {
-        return
-      }
-      this.onCloseFrame(this.frame, true)
+      this.onCloseFrame()
     })
     this.webview.addEventListener('page-favicon-updated', (e) => {
       if (this.frame.isEmpty()) {

diff --git a/js/state/frameStateUtil.js b/js/state/frameStateUtil.js
@@ -102,6 +102,9 @@ function getFrameByDisplayIndex (state, i) {
 
 function getFrameByKey (state, key) {
   const index = getFrameIndex(state, key)
+  if (index === -1) {
+    return null
+  }
   return state.getIn(['frames', index])
 }
 
@@ -520,14 +523,18 @@ const getFramesInternalIndexByTabId = (state, tabId) => {
   return index == null ? -1 : index
 }
 
+const deleteTabInternalIndex = (state, tabId) => {
+  state = state.deleteIn(['framesInternal', 'tabIndex', tabId.toString()])
+  return state.deleteIn(['framesInternal', 'tabIndex', tabId])
+}
+
 const deleteFrameInternalIndex = (state, frame) => {
   if (!frame) {
     return state
   }
   state = state.deleteIn(['framesInternal', 'index', frame.get('key').toString()])
   state = state.deleteIn(['framesInternal', 'index', frame.get('key')])
-  state = state.deleteIn(['framesInternal', 'tabIndex', frame.get('tabId').toString()])
-  return state.deleteIn(['framesInternal', 'tabIndex', frame.get('tabId')])
+  return deleteTabInternalIndex(state, frame.get('tabId'))
 }
 
 const updateFramesInternalIndex = (state, fromIndex) => {
@@ -548,6 +555,7 @@ const updateFramesInternalIndex = (state, fromIndex) => {
 }
 
 module.exports = {
+  deleteTabInternalIndex,
   deleteFrameInternalIndex,
   updateFramesInternalIndex,
   query,

diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js
@@ -185,6 +185,9 @@ const newFrame = (state, frameOpts, openInForeground, insertionIndex, nextKey) =
   if (openInForeground) {
     const activeFrame = frameStateUtil.getActiveFrame(state)
     state = updateTabPageIndex(state, activeFrame)
+    if (activeFrame.get('tabId')) {
+      appActions.tabActivateRequested(activeFrame.get('tabId'))
+    }
   }
 
   return state
@@ -202,6 +205,7 @@ const frameTabIdChanged = (state, action) => {
   newFrameProps = newFrameProps.set('tabId', newTabId)
   const index = frameStateUtil.getFrameIndex(state, action.getIn(['frameProps', 'key']))
   state = state.mergeIn(['frames', index], newFrameProps)
+  state = frameStateUtil.deleteTabInternalIndex(state, oldTabId)
   return frameStateUtil.updateFramesInternalIndex(state, index)
 }