Trying to fix root cause for #8974

brave · May 23, 2017 · 6ba3863 · 6ba3863
1 parent 97fb76e
commit 6ba3863
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 56 deletions.
diff --git a/app/renderer/reducers/frameReducer.js b/app/renderer/reducers/frameReducer.js
@@ -25,37 +25,61 @@ const setFullScreen = (state, action) => {
 }
 
 const closeFrame = (state, action) => {
-  const index = frameStateUtil.getFrameIndex(state, action.frameKey)
-  if (index === -1) {
+  // TODO(bridiver) - why does `will-destroy` fire twice?
+  if (action.frameKey < 0 || !frameStateUtil.getFrameByKey(state, action.frameKey)) {
     return state
   }
-  const frameProps = frameStateUtil.getFrameByKey(state, action.frameKey)
-
-  const hoverState = state.getIn(['frames', index, 'hoverState'])
-  const activeFrameKey = frameStateUtil.getActiveFrame(state).get('key')
-  state = state.merge(frameStateUtil.removeFrame(
-    state,
-    frameProps.set('closedAtIndex', index),
-    activeFrameKey,
-    index,
-    getSetting(settings.TAB_CLOSE_ACTION)
-  ))
-  state = frameStateUtil.deleteFrameInternalIndex(state, frameProps)
-  state = frameStateUtil.updateFramesInternalIndex(state, index)
-
-  // If we reach the limit of opened tabs per page while closing tabs, switch to
-  // the active tab's page otherwise the user will hang on empty page
-  if (frameStateUtil.getNonPinnedFrameCount(state) % getSetting(settings.TABS_PER_PAGE) === 0) {
-    state = updateTabPageIndex(state, frameStateUtil.getActiveFrame(state))
-    state = state.deleteIn(['ui', 'tabs', 'fixTabWidth'])
-  }
-
-  const nextFrame = frameStateUtil.getFrameByIndex(state, index)
 
-  // Copy the hover state if tab closed with mouse as long as we have a next frame
-  // This allow us to have closeTab button visible  for sequential frames closing, until onMouseLeave event happens.
-  if (hoverState && nextFrame) {
-    windowActions.setTabHoverState(nextFrame.get('key'), hoverState)
+  // Unless a caller explicitly specifies to close a pinned frame, then
+  // ignore the call.
+  const nonPinnedFrames = frameStateUtil.getNonPinnedFrames(state)
+  const pinnedFrames = frameStateUtil.getPinnedFrames(state)
+
+  // If there is at least 1 pinned frame don't close the window until subsequent
+  // close attempts
+  if (nonPinnedFrames.size > 1 || pinnedFrames.size > 0) {
+    const index = frameStateUtil.getFrameIndex(state, action.frameKey)
+    if (index === -1) {
+      return state
+    }
+    const frameProps = frameStateUtil.getFrameByKey(state, action.frameKey)
+    const hoverState = state.getIn(['frames', index, 'hoverState'])
+    const activeFrameKey = frameStateUtil.getActiveFrame(state).get('key')
+    const result = frameStateUtil.removeFrame(
+      state,
+      frameProps.set('closedAtIndex', index),
+      activeFrameKey,
+      index,
+      getSetting(settings.TAB_CLOSE_ACTION)
+    )
+
+    state = state.set('previewFrameKey', result.previewFrameKey)
+    state = state.set('closedFrames', result.closedFrames)
+    state = state.set('frames', result.frames)
+    state = frameStateUtil.deleteFrameInternalIndex(state, frameProps)
+    state = frameStateUtil.updateFramesInternalIndex(state, index)
+
+    const activeFrame = frameStateUtil.getFrameByKey(state, result.activeFrameKey)
+
+    // If we reach the limit of opened tabs per page while closing tabs, switch to
+    // the active tab's page otherwise the user will hang on empty page
+    if (frameStateUtil.getNonPinnedFrameCount(state) % getSetting(settings.TABS_PER_PAGE) === 0) {
+      state = updateTabPageIndex(state, activeFrame)
+      state = state.deleteIn(['ui', 'tabs', 'fixTabWidth'])
+    }
+
+    // Copy the hover state if tab closed with mouse as long as we have a next frame
+    // This allow us to have closeTab button visible  for sequential frames closing, until onMouseLeave event happens.
+    const nextFrame = frameStateUtil.getFrameByIndex(state, index)
+    if (hoverState && nextFrame) {
+      windowActions.setTabHoverState(nextFrame.get('key'), hoverState)
+    }
+
+    if (activeFrame) {
+      appActions.tabActivateRequested(activeFrame.get('tabId'))
+    }
+  } else {
+    appActions.closeWindow(getCurrentWindowId())
   }
 
   return state
@@ -127,26 +151,7 @@ const frameReducer = (state, action, immutableAction) => {
       })
       break
     case windowConstants.WINDOW_CLOSE_FRAME:
-      // 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
-      // ignore the call.
-      const nonPinnedFrames = frameStateUtil.getNonPinnedFrames(state)
-      const pinnedFrames = frameStateUtil.getPinnedFrames(state)
-      // If there is at least 1 pinned frame don't close the window until subsequent
-      // close attempts
-      if (nonPinnedFrames.size > 1 || pinnedFrames.size > 0) {
-        state = closeFrame(state, action)
-
-        const frame = frameStateUtil.getActiveFrame(state)
-        if (frame) {
-          appActions.tabActivateRequested(frame.get('tabId'))
-        }
-      } else {
-        appActions.closeWindow(getCurrentWindowId())
-      }
+      state = closeFrame(state, action)
       break
 
     case windowConstants.WINDOW_SET_FULL_SCREEN:

diff --git a/js/state/frameStateUtil.js b/js/state/frameStateUtil.js
@@ -201,13 +201,6 @@ function getActiveFrameKey (state) {
   return state.get('activeFrameKey')
 }
 
-function setActiveFrameKey (state, activeFrameKey) {
-  return state.merge({
-    activeFrameKey: activeFrameKey,
-    previewFrameKey: null
-  })
-}
-
 function getNextFrame (state) {
   const activeFrameIndex = findDisplayIndexForFrameKey(state, getActiveFrameKey(state))
   const index = (activeFrameIndex + 1) % state.get('frames').size
@@ -599,7 +592,6 @@ module.exports = {
   getIndexByTabId,
   getPartitionNumber,
   getActiveFrame,
-  setActiveFrameKey,
   getNextFrame,
   getPreviousFrame,
   findDisplayIndexForFrameKey,