From 95455e729f62747391162ce044acfb115ef967f0 Mon Sep 17 00:00:00 2001
From: "Brian R. Bondy" <netzen@gmail.com>
Date: Sat, 22 Jul 2017 22:55:18 -0400
Subject: [PATCH] Use muon's tab_strip_model for driving index and active

Fix #10436
Fix #9385
Fix #9722
Fix #10561
Fix #9083
Fix #9671
Fix #10038
Fix #10384
Fix #10532

and probably several others.
---
 app/browser/reducers/tabsReducer.js           | 103 +++++-------
 app/browser/reducers/windowsReducer.js        |   9 +-
 app/browser/tabs.js                           | 148 ++++++++++++++++--
 app/browser/windows.js                        |  15 +-
 app/common/state/tabState.js                  |  31 +---
 app/renderer/components/frame/frame.js        |   8 +-
 app/renderer/components/tabs/tab.js           |  12 +-
 app/renderer/reducers/frameReducer.js         |   9 +-
 app/sessionStore.js                           |  29 ++--
 js/actions/appActions.js                      |  48 +++++-
 js/actions/windowActions.js                   |   7 -
 js/constants/appConstants.js                  |   4 +
 js/contextMenus.js                            |   2 +-
 js/state/frameStateUtil.js                    |  32 ++--
 js/stores/windowStore.js                      |  46 ++----
 test/lib/brave.js                             |   2 +-
 .../app/browser/reducers/tabsReducerTest.js   |  47 +++---
 test/unit/app/browser/tabsTest.js             |   5 +-
 test/unit/js/stores/windowStoreTest.js        |  25 ++-
 19 files changed, 356 insertions(+), 226 deletions(-)

diff --git a/app/browser/reducers/tabsReducer.js b/app/browser/reducers/tabsReducer.js
index 47f608205c3..c70f1c3f101 100644
--- a/app/browser/reducers/tabsReducer.js
+++ b/app/browser/reducers/tabsReducer.js
@@ -11,7 +11,6 @@ const windows = require('../windows')
 const {getWebContents} = require('../webContentsCache')
 const {BrowserWindow} = require('electron')
 const tabState = require('../../common/state/tabState')
-const windowState = require('../../common/state/windowState')
 const tabActions = require('../../common/actions/tabActions')
 const siteSettings = require('../../../js/state/siteSettings')
 const siteSettingsState = require('../../common/state/siteSettingsState')
@@ -22,62 +21,8 @@ const {getFlashResourceId} = require('../../../js/flash')
 const {l10nErrorText} = require('../../common/lib/httpUtil')
 const Immutable = require('immutable')
 const dragTypes = require('../../../js/constants/dragTypes')
-const getSetting = require('../../../js/settings').getSetting
-const settings = require('../../../js/constants/settings')
-const {tabCloseAction} = require('../../common/constants/settingsEnums')
 const {frameOptsFromFrame} = require('../../../js/state/frameStateUtil')
 
-const updateActiveTab = (state, closeTabId) => {
-  if (!tabState.getByTabId(state, closeTabId)) {
-    return
-  }
-
-  const index = tabState.getIndex(state, closeTabId)
-  if (index === -1) {
-    return
-  }
-
-  const windowId = tabState.getWindowId(state, closeTabId)
-  if (windowId === windowState.WINDOW_ID_NONE) {
-    return
-  }
-
-  const lastActiveTabId = tabState.getTabsByLastActivated(state, windowId).last()
-  if (lastActiveTabId !== closeTabId && !tabState.isActive(state, closeTabId)) {
-    return
-  }
-
-  let nextTabId = tabState.TAB_ID_NONE
-  switch (getSetting(settings.TAB_CLOSE_ACTION)) {
-    case tabCloseAction.LAST_ACTIVE:
-      nextTabId = tabState.getLastActiveTabId(state, windowId)
-      break
-    case tabCloseAction.PARENT:
-      {
-        const openerTabId = tabState.getOpenerTabId(state, closeTabId)
-        if (openerTabId !== tabState.TAB_ID_NONE) {
-          nextTabId = openerTabId
-        }
-        break
-      }
-  }
-
-  // DEFAULT: always fall back to NEXT
-  if (nextTabId === tabState.TAB_ID_NONE) {
-    nextTabId = tabState.getNextTabIdByIndex(state, windowId, index)
-    if (nextTabId === tabState.TAB_ID_NONE) {
-      // no unpinned tabs so find the next pinned tab
-      nextTabId = tabState.getNextTabIdByIndex(state, windowId, index, true)
-    }
-  }
-
-  if (nextTabId !== tabState.TAB_ID_NONE) {
-    setImmediate(() => {
-      tabs.setActive(nextTabId)
-    })
-  }
-}
-
 const WEBRTC_DEFAULT = 'default'
 const WEBRTC_DISABLE_NON_PROXY = 'disable_non_proxied_udp'
 
@@ -125,7 +70,23 @@ const tabsReducer = (state, action, immutableAction) => {
     case appConstants.APP_TAB_CREATED:
       state = tabState.maybeCreateTab(state, action)
       break
-    case appConstants.APP_TAB_MOVED: {
+    case appConstants.APP_TAB_ATTACHED:
+      tabs.updateTabsStateForAttachedTab(state, action.get('tabId'))
+      break
+    case appConstants.APP_TAB_WILL_ATTACH: {
+      const tabId = action.get('tabId')
+      const tabValue = tabState.getByTabId(state, tabId)
+      if (!tabValue) {
+        break
+      }
+      const oldWindowId = tabState.getWindowId(state, tabId)
+      tabs.updateTabsStateForWindow(state, oldWindowId)
+      break
+    }
+    case appConstants.APP_TAB_MOVED:
+      tabs.updateTabsStateForAttachedTab(state, action.get('tabId'))
+      break
+    case appConstants.APP_TAB_DETACH_MENU_ITEM_CLICKED: {
       setImmediate(() => {
         const tabId = action.get('tabId')
         const frameOpts = frameOptsFromFrame(action.get('frameOpts'))
@@ -153,6 +114,7 @@ const tabsReducer = (state, action, immutableAction) => {
       break
     case appConstants.APP_TAB_UPDATED:
       state = tabState.maybeCreateTab(state, action)
+      // tabs.debugTabs(state)
       break
     case appConstants.APP_TAB_CLOSE_REQUESTED:
       {
@@ -189,7 +151,7 @@ const tabsReducer = (state, action, immutableAction) => {
                 tabs.closeTab(tabId, action.get('forceClosePinned'))
               })
             } else {
-              state = windows.closeWindow(state, windowId)
+              windows.closeWindow(windowId)
             }
           }
         }
@@ -201,8 +163,21 @@ const tabsReducer = (state, action, immutableAction) => {
         if (tabId === tabState.TAB_ID_NONE) {
           break
         }
-        updateActiveTab(state, tabId)
+        const nextActiveTabId = tabs.getNextActiveTab(state, tabId)
+
+        // Must be called before tab is removed
+        // But still check for no tabId because on tab detach there's a dummy tabId
+        const tabValue = tabState.getByTabId(state, tabId)
+        if (tabValue) {
+          const windowIdOfTabBeingRemoved = tabState.getWindowId(state, tabId)
+          tabs.updateTabsStateForWindow(state, windowIdOfTabBeingRemoved)
+        }
         state = tabState.removeTabByTabId(state, tabId)
+        setImmediate(() => {
+          if (nextActiveTabId !== tabState.TAB_ID_NONE) {
+            tabs.setActive(nextActiveTabId)
+          }
+        })
       }
       break
     case appConstants.APP_ALLOW_FLASH_ONCE:
@@ -362,10 +337,16 @@ const tabsReducer = (state, action, immutableAction) => {
       const dragData = state.get('dragData')
       if (dragData && dragData.get('type') === dragTypes.TAB) {
         const frame = dragData.get('data')
-        const frameOpts = frameOptsFromFrame(frame).toJS()
+        let frameOpts = frameOptsFromFrame(frame)
         const browserOpts = { positionByMouseCursor: true }
-        frameOpts.indexByFrameKey = dragData.getIn(['dragOverData', 'draggingOverKey'])
-        frameOpts.prependIndexByFrameKey = dragData.getIn(['dragOverData', 'draggingOverLeftHalf'])
+        const tabIdForIndex = dragData.getIn(['dragOverData', 'draggingOverKey'])
+        const tabForIndex = tabState.getByTabId(state, tabIdForIndex)
+        const dropWindowId = dragData.get('dropWindowId')
+        if (dropWindowId != null && dropWindowId !== -1 && tabForIndex) {
+          const prependIndexByTabId = dragData.getIn(['dragOverData', 'draggingOverLeftHalf'])
+          frameOpts = frameOpts.set('index', tabForIndex.get('index') + (prependIndexByTabId ? 0 : 1))
+        }
+
         tabs.moveTo(state, frame.get('tabId'), frameOpts, browserOpts, dragData.get('dropWindowId'))
       }
       break
diff --git a/app/browser/reducers/windowsReducer.js b/app/browser/reducers/windowsReducer.js
index 358ca04b709..f830d61136e 100644
--- a/app/browser/reducers/windowsReducer.js
+++ b/app/browser/reducers/windowsReducer.js
@@ -28,11 +28,13 @@ const windowsReducer = (state, action, immutableAction) => {
       }
       break
     case appConstants.APP_CLOSE_WINDOW:
-      state = windows.closeWindow(state, action.get('windowId'))
+      windows.closeWindow(action.get('windowId'))
       break
     case appConstants.APP_WINDOW_CLOSED:
       state = windowState.removeWindow(state, action)
-      sessionStoreShutdown.removeWindowFromCache(action.getIn(['windowValue', 'windowId']))
+      const windowId = action.getIn(['windowValue', 'windowId'])
+      sessionStoreShutdown.removeWindowFromCache(windowId)
+      windows.cleanupWindow(windowId)
       break
     case appConstants.APP_WINDOW_CREATED:
       state = windowState.maybeCreateWindow(state, action)
@@ -40,6 +42,9 @@ const windowsReducer = (state, action, immutableAction) => {
     case appConstants.APP_WINDOW_UPDATED:
       state = windowState.maybeCreateWindow(state, action)
       break
+    case appConstants.APP_TAB_STRIP_EMPTY:
+      windows.closeWindow(action.get('windowId'))
+      break
     case appConstants.APP_DEFAULT_WINDOW_PARAMS_CHANGED:
       if (action.get('size')) {
         state = state.setIn(['defaultWindowParams', 'width'], action.getIn(['size', 0]))
diff --git a/app/browser/tabs.js b/app/browser/tabs.js
index a2d897312e3..51b41226ac7 100644
--- a/app/browser/tabs.js
+++ b/app/browser/tabs.js
@@ -8,6 +8,7 @@ const tabActions = require('../common/actions/tabActions')
 const config = require('../../js/constants/config')
 const Immutable = require('immutable')
 const tabState = require('../common/state/tabState')
+const windowState = require('../common/state/windowState')
 const {app, BrowserWindow, extensions, session, ipcMain} = require('electron')
 const {makeImmutable} = require('../common/state/immutableUtil')
 const {getTargetAboutUrl, getSourceAboutUrl, isSourceAboutUrl, newFrameUrl, isTargetAboutUrl, isIntermediateAboutPage, isTargetMagnetUrl, getSourceMagnetUrl} = require('../../js/lib/appUrlUtil')
@@ -24,6 +25,7 @@ const appStore = require('../../js/stores/appStore')
 const appConfig = require('../../js/constants/appConfig')
 const siteTags = require('../../js/constants/siteTags')
 const {newTabMode} = require('../common/constants/settingsEnums')
+const {tabCloseAction} = require('../common/constants/settingsEnums')
 const {cleanupWebContents, currentWebContents, getWebContents, updateWebContents} = require('./webContentsCache')
 const {FilterOptions} = require('ad-block')
 const {isResourceEnabled} = require('../filtering')
@@ -436,8 +438,7 @@ const api = {
       if (tab.isBackgroundPage() || !tab.isGuest()) {
         return
       }
-      let tabId = tab.getId()
-
+      const tabId = tab.getId()
       tab.on('did-start-navigation', (e, navigationHandle) => {
         if (!tab.isDestroyed() && navigationHandle.isValid() && navigationHandle.isInMainFrame()) {
           const controller = tab.controller()
@@ -473,10 +474,35 @@ const api = {
       tab.on('unresponsive', () => {
         console.log('unresponsive')
       })
+
       tab.on('responsive', () => {
         console.log('responsive')
       })
 
+      tab.on('tab-changed-at', () => {
+        updateTab(tabId)
+      })
+
+      tab.on('tab-moved', () => {
+        appActions.tabMoved(tabId)
+      })
+
+      tab.on('will-attach', () => {
+        appActions.tabWillAttach(tab.getId())
+      })
+
+      tab.on('tab-strip-empty', () => {
+        tab.once('will-attach', () => {
+          const tabValue = getTabValue(tabId)
+          const windowId = tabValue.get('windowId')
+          appActions.tabStripEmpty(windowId)
+        })
+      })
+
+      tab.on('did-attach', () => {
+        appActions.tabAttached(tab.getId())
+      })
+
       tab.on('save-password', (e, username, origin) => {
         appActions.savePassword(username, origin, tabId)
       })
@@ -611,8 +637,11 @@ const api = {
     const tabId = action.get('tabId')
     let options = action.get('options') || Immutable.Map()
     const tabValue = getTabValue(tabId)
-    if (tabValue && tabValue.get('index') !== undefined) {
-      options = options.set('index', tabValue.get('index') + 1)
+    if (tabValue) {
+      const index = tabValue.get('index')
+      if (index !== undefined) {
+        options = options.set('index', index + 1)
+      }
     }
     const tab = getWebContents(tabId)
     if (tab && !tab.isDestroyed()) {
@@ -723,7 +752,7 @@ const api = {
     win.loadURL('about:blank')
   },
 
-  moveTo: (state, tabId, frameOpts, browserOpts, windowId) => {
+  moveTo: (state, tabId, frameOpts, browserOpts, toWindowId) => {
     frameOpts = makeImmutable(frameOpts)
     browserOpts = makeImmutable(browserOpts)
     const tab = getWebContents(tabId)
@@ -735,11 +764,11 @@ const api = {
       }
 
       const currentWindowId = tabValue && tabValue.get('windowId')
-      if (windowId != null && currentWindowId === windowId) {
+      if (toWindowId != null && currentWindowId === toWindowId) {
         return
       }
 
-      if (windowId == null || windowId === -1) {
+      if (toWindowId == null || toWindowId === -1) {
         // If there's only one tab and we're dragging outside the window, then disallow
         // a new window to be created.
         const windowTabCount = tabState.getTabsByWindowId(state, currentWindowId).size
@@ -752,13 +781,27 @@ const api = {
         // If the current tab is pinned, then don't allow to drag out
         return
       }
-
+      const nextActiveTabIdForOldWindow = api.getNextActiveTab(state, tabId)
       tab.detach(() => {
-        if (windowId == null || windowId === -1) {
-          appActions.newWindow(makeImmutable(frameOpts), browserOpts)
+        if (toWindowId == null || toWindowId === -1) {
+          frameOpts = frameOpts.set('index', 0)
+          appActions.newWindow(frameOpts, browserOpts)
         } else {
-          appActions.newWebContentsAdded(windowId, frameOpts, tabValue)
+          appActions.newWebContentsAdded(toWindowId, frameOpts, tabValue)
         }
+
+        // Setting the next active tab for the old window must happen after re-attach of the new tab.
+        // This is because muon's tab_strip index for the tab would not be consistent with browser-laptop's
+        // expectation and it would try to set an invalid index as active, possibly leaivng nothing active.
+        tab.once('did-attach', () => {
+          if (nextActiveTabIdForOldWindow !== tabState.TAB_ID_NONE) {
+            api.setActive(nextActiveTabIdForOldWindow)
+          }
+          const index = frameOpts.get('index')
+          if (index !== undefined) {
+            api.setTabIndex(tabId, frameOpts.get('index'))
+          }
+        })
       })
     }
   },
@@ -857,6 +900,89 @@ const api = {
     }
 
     return null
+  },
+
+  getNextActiveTab: (state, closeTabId) => {
+    if (!tabState.getByTabId(state, closeTabId)) {
+      return
+    }
+
+    const index = tabState.getIndex(state, closeTabId)
+    if (index === -1) {
+      return
+    }
+
+    const windowId = tabState.getWindowId(state, closeTabId)
+    if (windowId === windowState.WINDOW_ID_NONE) {
+      return
+    }
+
+    const lastActiveTabId = tabState.getTabsByLastActivated(state, windowId).last()
+    if (lastActiveTabId !== closeTabId && !tabState.isActive(state, closeTabId)) {
+      return
+    }
+
+    let nextTabId = tabState.TAB_ID_NONE
+    switch (getSetting(settings.TAB_CLOSE_ACTION)) {
+      case tabCloseAction.LAST_ACTIVE:
+        nextTabId = tabState.getLastActiveTabId(state, windowId)
+        break
+      case tabCloseAction.PARENT:
+        {
+          const openerTabId = tabState.getOpenerTabId(state, closeTabId)
+          if (openerTabId !== tabState.TAB_ID_NONE) {
+            nextTabId = openerTabId
+          }
+          break
+        }
+    }
+
+    // DEFAULT: always fall back to NEXT
+    if (nextTabId === tabState.TAB_ID_NONE) {
+      nextTabId = tabState.getNextTabIdByIndex(state, windowId, index)
+      if (nextTabId === tabState.TAB_ID_NONE) {
+        // no unpinned tabs so find the next pinned tab
+        nextTabId = tabState.getNextTabIdByIndex(state, windowId, index, true)
+      }
+    }
+
+    return nextTabId
+  },
+  debugTabs: (state) => {
+    console.log(tabState.getTabs(state)
+      .toJS()
+      .map((tab) => {
+        return {
+          tabId: tab.tabId,
+          index: tab.index,
+          windowId: tab.windowId,
+          active: tab.active
+        }
+      })
+      .sort((tab1, tab2) => {
+        if (tab1.windowId !== tab2.windowId) {
+          return tab1.windowId - tab2.windowId
+        }
+        if (tab1.index !== tab2.index) {
+          return tab1.index - tab2.index
+        }
+        return 0
+      }))
+  },
+  updateTabsStateForAttachedTab: (state, tabId) => {
+    const tabValue = getTabValue(tabId)
+    if (!tabValue) {
+      return
+    }
+    api.updateTabsStateForWindow(state, tabValue.get('windowId'))
+  },
+  updateTabsStateForWindow: (state, windowId) => {
+    tabState.getTabsByWindowId(state, windowId).forEach((tabValue) => {
+      const newTabValue = getTabValue(tabValue.get('tabId'))
+      if (newTabValue) {
+        state = tabState.updateTabValue(state, newTabValue, false)
+      }
+    })
   }
 }
 
diff --git a/app/browser/windows.js b/app/browser/windows.js
index 809faac8858..962c900f7c8 100644
--- a/app/browser/windows.js
+++ b/app/browser/windows.js
@@ -21,11 +21,6 @@ const Immutable = require('immutable')
 // TODO(bridiver) - set window uuid
 let currentWindows = {}
 
-const cleanupWindow = (windowId) => {
-  delete currentWindows[windowId]
-  appActions.windowClosed({ windowId })
-}
-
 const getWindowState = (win) => {
   if (win.isFullScreen()) {
     return 'fullscreen'
@@ -134,6 +129,7 @@ const api = {
         win.webContents.once('close', () => {
           LocalShortcuts.unregister(win)
         })
+
         win.once('close', () => {
           LocalShortcuts.unregister(win)
         })
@@ -197,7 +193,6 @@ const api = {
         appActions.windowCreated(windowValue)
       })
       win.once('closed', () => {
-        cleanupWindow(windowId)
       })
       win.on('blur', () => {
         appActions.windowBlurred(windowId)
@@ -329,7 +324,7 @@ const api = {
     })
   },
 
-  closeWindow: (state, windowId) => {
+  closeWindow: (windowId) => {
     let win = api.getWindow(windowId)
     try {
       setImmediate(() => {
@@ -340,7 +335,6 @@ const api = {
     } catch (e) {
       // ignore
     }
-    return windowState.removeWindowByWindowId(state, windowId)
   },
 
   getWindow: (windowId) => {
@@ -351,8 +345,11 @@ const api = {
     if (BrowserWindow.getFocusedWindow()) {
       return BrowserWindow.getFocusedWindow().id
     }
-
     return windowState.WINDOW_ID_NONE
+  },
+
+  cleanupWindow: (windowId) => {
+    delete currentWindows[windowId]
   }
 }
 
diff --git a/app/common/state/tabState.js b/app/common/state/tabState.js
index 3938cf8ccb5..28632076929 100644
--- a/app/common/state/tabState.js
+++ b/app/common/state/tabState.js
@@ -97,27 +97,11 @@ const updateLastActive = (state, oldTabValue, newTabValue) => {
 const getTabIdByDisplayIndex = (state, windowId, index) => {
   index = validateIndex(index)
   windowId = validateId('windowId', windowId)
-
-  const tabId = state.getIn(['tabsInternal', 'displayIndex', windowId.toString(), index.toString()])
-  return tabId == null ? tabState.TAB_ID_NONE : tabId
-}
-
-const updateTabsDisplayIndex = (state, oldTabValue, newTabValue) => {
-  const oldTabId = validateId('tabId', oldTabValue.get('tabId'))
-  const oldWindowId = validateId('windowId', oldTabValue.get('windowId'))
-  const oldIndex = validateIndex(oldTabValue.get('index'))
-  if (oldIndex !== -1 && oldTabId !== tabState.TAB_ID_NONE && oldWindowId !== windowState.WINDOW_ID_NONE) {
-    state = state.deleteIn(['tabsInternal', 'displayIndex', oldWindowId.toString(), oldIndex.toString()])
-  }
-
-  const tabId = validateId('tabId', newTabValue.get('tabId'))
-  const windowId = validateId('windowId', newTabValue.get('windowId'))
-  const index = validateIndex(newTabValue.get('index'))
-  if (index !== -1 && tabId !== tabState.TAB_ID_NONE && windowId !== windowState.WINDOW_ID_NONE) {
-    return state.setIn(['tabsInternal', 'displayIndex', windowId.toString(), index.toString()], tabId)
+  const tabValue = tabState.queryTab(state, {windowId, index})
+  if (!tabValue) {
+    return tabState.TAB_ID_NONE
   }
-
-  return state
+  return tabValue.get('tabId')
 }
 
 // The internal index is the location of the tabId in the tabs array
@@ -134,12 +118,7 @@ const deleteTabsInternalIndex = (state, tabValue) => {
   if (tabId === tabState.TAB_ID_NONE) {
     return state
   }
-
   const windowId = validateId('windowId', tabValue.get('windowId'))
-  const displayIndex = validateIndex(tabValue.get('index'))
-  if (displayIndex !== -1 && windowId !== windowState.WINDOW_ID_NONE) {
-    state = state.deleteIn(['tabsInternal', 'displayIndex', windowId.toString(), displayIndex.toString()])
-  }
   if (windowId !== windowState.WINDOW_ID_NONE) {
     let activeList = state.getIn(['tabsInternal', 'lastActive', windowId.toString()], Immutable.OrderedSet())
     activeList = activeList.remove(tabId)
@@ -211,7 +190,6 @@ const tabState = {
     let tabValue = validateTabValue(action.get('tabValue'))
     assert.ok(!tabState.getTab(state, tabValue), 'Tab already exists')
     state = state.set('tabs', state.get('tabs').push(tabValue))
-    state = updateTabsDisplayIndex(state, tabValue, tabValue)
     state = updateLastActive(state, tabValue, tabValue)
     return updateTabsInternalIndex(state, state.get('tabs').size - 1)
   },
@@ -416,7 +394,6 @@ const tabState = {
       tabValue = currentTabValue.mergeDeep(tabValue)
     }
 
-    state = updateTabsDisplayIndex(state, currentTabValue, tabValue)
     state = updateLastActive(state, currentTabValue, tabValue)
     return state.set('tabs', tabs.delete(index).insert(index, tabValue))
   },
diff --git a/app/renderer/components/frame/frame.js b/app/renderer/components/frame/frame.js
index 499222e18e0..2043cfd518a 100644
--- a/app/renderer/components/frame/frame.js
+++ b/app/renderer/components/frame/frame.js
@@ -161,12 +161,14 @@ class Frame extends React.Component {
       this.addEventListeners()
       if (cb) {
         this.runOnDomReady = cb
-        let eventCallback = (e) => {
-          this.webview.removeEventListener(e.type, eventCallback)
+        let didAttachCallback = (e) => {
+          this.webview.removeEventListener(e.type, didAttachCallback)
           this.runOnDomReady()
           delete this.runOnDomReady
         }
-        this.webview.addEventListener('did-attach', eventCallback, { passive: true })
+        this.webview.addEventListener('will-attach', () => {
+        })
+        this.webview.addEventListener('did-attach', didAttachCallback, { passive: true })
       }
 
       if (!this.props.guestInstanceId || !this.webview.attachGuest(this.props.guestInstanceId)) {
diff --git a/app/renderer/components/tabs/tab.js b/app/renderer/components/tabs/tab.js
index 311f89924d6..392862aebd1 100644
--- a/app/renderer/components/tabs/tab.js
+++ b/app/renderer/components/tabs/tab.js
@@ -66,7 +66,7 @@ class Tab extends React.Component {
   get draggingOverData () {
     const draggingOverData = this.props.dragData && this.props.dragData.get('dragOverData')
     if (!draggingOverData ||
-        draggingOverData.get('draggingOverKey') !== this.props.frameKey ||
+        draggingOverData.get('draggingOverKey') !== this.props.tabId ||
         draggingOverData.get('draggingOverWindowId') !== getCurrentWindowId()) {
       return
     }
@@ -76,8 +76,8 @@ class Tab extends React.Component {
       return
     }
     const location = sourceDragData.get('location')
-    const key = draggingOverData.get('draggingOverKey')
-    const draggingOverFrame = windowStore.getFrame(key)
+    const tabId = draggingOverData.get('draggingOverKey')
+    const draggingOverFrame = windowStore.getFrameByTabId(tabId)
     if ((location === 'about:blank' || location === 'about:newtab' || isIntermediateAboutPage(location)) &&
         (draggingOverFrame && draggingOverFrame.get('pinnedLocation'))) {
       return
@@ -89,7 +89,7 @@ class Tab extends React.Component {
   get isDragging () {
     const sourceDragData = dnd.getInterBraveDragData()
     return sourceDragData &&
-      sourceDragData.get('key') === this.props.frameKey &&
+      sourceDragData.get('tabId') === this.props.tabId &&
       sourceDragData.get('draggingOverWindowId') === getCurrentWindowId()
   }
 
@@ -99,7 +99,7 @@ class Tab extends React.Component {
     if (!draggingOverData || !sourceDragData) {
       return false
     }
-    return draggingOverData.get('draggingOverKey') === sourceDragData.get('key')
+    return draggingOverData.get('draggingOverKey') === sourceDragData.get('tabId')
   }
 
   get isDraggingOverLeft () {
@@ -125,7 +125,7 @@ class Tab extends React.Component {
   }
 
   onDragOver (e) {
-    dnd.onDragOver(dragTypes.TAB, this.tabNode.getBoundingClientRect(), this.props.frameKey, this.draggingOverData, e)
+    dnd.onDragOver(dragTypes.TAB, this.tabNode.getBoundingClientRect(), this.props.tabId, this.draggingOverData, e)
   }
 
   onMouseLeave () {
diff --git a/app/renderer/reducers/frameReducer.js b/app/renderer/reducers/frameReducer.js
index cec4b5ab2ab..831df7d4f9c 100644
--- a/app/renderer/reducers/frameReducer.js
+++ b/app/renderer/reducers/frameReducer.js
@@ -16,7 +16,6 @@ const appActions = require('../../../js/actions/appActions')
 
 // Utils
 const frameStateUtil = require('../../../js/state/frameStateUtil')
-const {getCurrentWindowId} = require('../currentWindow')
 const {getSourceAboutUrl, getSourceMagnetUrl} = require('../../../js/lib/appUrlUtil')
 const {isURL, isPotentialPhishingUrl, getUrlFromInput} = require('../../../js/lib/urlutil')
 
@@ -39,16 +38,14 @@ const closeFrame = (state, action) => {
 
   state = state.merge(frameStateUtil.removeFrame(
     state,
-    frameProps.set('closedAtIndex', index),
+    frameProps
+      .set('closedAtIndex', index)
+      .delete('openerTabId'),
     index
   ))
   state = frameStateUtil.deleteFrameInternalIndex(state, frameProps)
   state = frameStateUtil.updateFramesInternalIndex(state, index)
 
-  if (state.get('frames', Immutable.List()).size === 0) {
-    appActions.closeWindow(getCurrentWindowId())
-  }
-
   const nextFrame = frameStateUtil.getFrameByIndex(state, index)
 
   if (nextFrame) {
diff --git a/app/sessionStore.js b/app/sessionStore.js
index df1415c1339..c1bb195120f 100644
--- a/app/sessionStore.js
+++ b/app/sessionStore.js
@@ -154,11 +154,15 @@ module.exports.cleanPerWindowData = (immutablePerWindowData, isShutdown) => {
   }
 
   let newKey = 0
-  const cleanFrame = (immutableFrame) => {
+  let activeFrameKey = immutablePerWindowData.get('activeFrameKey')
+  // If adjustActive is set to true then activeFrameKey will be set to the new frame key.
+  // We re-use this function for both closedFrames and frames, and we only want to adjust the active for frames.
+  const cleanFrame = (immutableFrame, adjustActive) => {
     newKey++
     // Reset the ids back to sequential numbers
-    if (immutableFrame.get('key') === immutablePerWindowData.get('activeFrameKey')) {
-      immutablePerWindowData = immutablePerWindowData.set('activeFrameKey', newKey)
+    if (adjustActive &&
+        immutableFrame.get('key') === immutablePerWindowData.get('activeFrameKey')) {
+      activeFrameKey = newKey
     } else {
       // For now just set everything to unloaded unless it's the active frame
       immutableFrame = immutableFrame.set('unloaded', true)
@@ -193,6 +197,7 @@ module.exports.cleanPerWindowData = (immutablePerWindowData, isShutdown) => {
       'guestInstanceId',
       // Tab ids are per-session and should not be persisted
       'tabId',
+      'openerTabId',
       // Do not show the audio indicator until audio starts playing
       'audioMuted',
       'audioPlaybackActive',
@@ -246,20 +251,24 @@ module.exports.cleanPerWindowData = (immutablePerWindowData, isShutdown) => {
   if (immutablePerWindowData.get('closedFrames')) {
     immutablePerWindowData =
       immutablePerWindowData.get('closedFrames').reduce((immutablePerWindowData, immutableFrame, index) => {
-        const cleanImmutableFrame = cleanFrame(immutableFrame)
+        const cleanImmutableFrame = cleanFrame(immutableFrame, false)
         return immutablePerWindowData.setIn(['closedFrames', index], cleanImmutableFrame)
       }, immutablePerWindowData)
   }
-  if (immutablePerWindowData.get('frames')) {
+  let immutableFrames = immutablePerWindowData.get('frames')
+  if (immutableFrames) {
     // Don't restore pinned locations because they will be auto created by the app state change event
-    immutablePerWindowData = immutablePerWindowData.set('frames',
-      immutablePerWindowData.get('frames')
-        .filter((frame) => !frame.get('pinnedLocation')))
+    immutableFrames = immutableFrames
+        .filter((frame) => !frame.get('pinnedLocation'))
+    immutablePerWindowData = immutablePerWindowData.set('frames', immutableFrames)
     immutablePerWindowData =
-      immutablePerWindowData.get('frames').reduce((immutablePerWindowData, immutableFrame, index) => {
-        const cleanImmutableFrame = cleanFrame(immutableFrame)
+      immutableFrames.reduce((immutablePerWindowData, immutableFrame, index) => {
+        const cleanImmutableFrame = cleanFrame(immutableFrame, true)
         return immutablePerWindowData.setIn(['frames', index], cleanImmutableFrame)
       }, immutablePerWindowData)
+    if (activeFrameKey !== undefined) {
+      immutablePerWindowData = immutablePerWindowData.set('activeFrameKey', activeFrameKey)
+    }
   }
   return immutablePerWindowData
 }
diff --git a/js/actions/appActions.js b/js/actions/appActions.js
index 280171f9491..70acfa6fde6 100644
--- a/js/actions/appActions.js
+++ b/js/actions/appActions.js
@@ -83,6 +83,17 @@ const appActions = {
     })
   },
 
+  /**
+   * The tab strip is empty
+   * @param {Number} windowId
+   */
+  tabStripEmpty: function (windowId) {
+    dispatch({
+      actionType: appConstants.APP_TAB_STRIP_EMPTY,
+      windowId
+    })
+  },
+
   /**
    * A new tab has been created
    * @param {Object} tabValue
@@ -94,6 +105,39 @@ const appActions = {
     })
   },
 
+  /**
+   * Tab moved event fired from muon
+   * @param {Object} tabValue
+   */
+  tabMoved: function (tabId) {
+    dispatch({
+      actionType: appConstants.APP_TAB_MOVED,
+      tabId
+    })
+  },
+
+  /**
+   * A tab has been attached
+   * @param {Object} tabValue
+   */
+  tabAttached: function (tabId) {
+    dispatch({
+      actionType: appConstants.APP_TAB_ATTACHED,
+      tabId
+    })
+  },
+
+  /**
+   * A tab will be attached
+   * @param {Object} tabValue
+   */
+  tabWillAttach: function (tabId) {
+    dispatch({
+      actionType: appConstants.APP_TAB_WILL_ATTACH,
+      tabId
+    })
+  },
+
   /**
    * A tab has been moved to another window
    * @param {Number} tabId
@@ -101,9 +145,9 @@ const appActions = {
    * @param {Object} browserOpts
    * @param {Number} windowId
    */
-  tabMoved: function (tabId, frameOpts, browserOpts, windowId) {
+  tabDetachMenuItemClicked: function (tabId, frameOpts, browserOpts, windowId) {
     dispatch({
-      actionType: appConstants.APP_TAB_MOVED,
+      actionType: appConstants.APP_TAB_DETACH_MENU_ITEM_CLICKED,
       tabId,
       frameOpts,
       browserOpts,
diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js
index c4168474368..dbf7e8802b2 100644
--- a/js/actions/windowActions.js
+++ b/js/actions/windowActions.js
@@ -206,13 +206,6 @@ const windowActions = {
     })
   },
 
-  activeFrameChanged: function (frameProps) {
-    dispatch({
-      actionType: windowConstants.WINDOW_ACTIVE_FRAME_CHANGED,
-      frameProps: frameProps
-    })
-  },
-
   /**
    * Dispatches a message to the store when the frame is active and the window is focused
    *
diff --git a/js/constants/appConstants.js b/js/constants/appConstants.js
index 75549b9bd44..68c7beb12ea 100644
--- a/js/constants/appConstants.js
+++ b/js/constants/appConstants.js
@@ -11,6 +11,7 @@ const appConstants = {
   APP_WINDOW_CLOSED: _,
   APP_WINDOW_CREATED: _,
   APP_WINDOW_UPDATED: _,
+  APP_TAB_STRIP_EMPTY: _,
   APP_TAB_CLOSED: _,
   APP_TAB_UPDATED: _,
   APP_ADD_SITE: _,
@@ -58,6 +59,9 @@ const appConstants = {
   APP_FRAME_CHANGED: _,
   APP_TAB_CREATED: _,
   APP_TAB_MOVED: _,
+  APP_TAB_DETACH_MENU_ITEM_CLICKED: _,
+  APP_TAB_ATTACHED: _,
+  APP_TAB_WILL_ATTACH: _,
   APP_TAB_ACTIVATE_REQUESTED: _,
   APP_TAB_INDEX_CHANGED: _,
   APP_TAB_CLOSE_REQUESTED: _,
diff --git a/js/contextMenus.js b/js/contextMenus.js
index 2bca66486c9..3d8c51dd311 100644
--- a/js/contextMenus.js
+++ b/js/contextMenus.js
@@ -547,7 +547,7 @@ function tabTemplateInit (frameProps) {
       label: locale.translation('detach'),
       click: (item) => {
         const browserOpts = { positionByMouseCursor: true }
-        appActions.tabMoved(tabId, frameProps.toJS(), browserOpts, -1)
+        appActions.tabDetachMenuItemClicked(tabId, frameProps.toJS(), browserOpts, -1)
       }
     })
   }
diff --git a/js/state/frameStateUtil.js b/js/state/frameStateUtil.js
index 33f793ff44a..4bea7eee8a5 100644
--- a/js/state/frameStateUtil.js
+++ b/js/state/frameStateUtil.js
@@ -11,7 +11,6 @@ const settings = require('../constants/settings')
 // Actions
 const windowActions = require('../actions/windowActions')
 const webviewActions = require('../actions/webviewActions')
-const appActions = require('../actions/appActions')
 
 // State
 const {makeImmutable} = require('../../app/common/state/immutableUtil')
@@ -313,7 +312,7 @@ const frameOptsFromFrame = (frame) => {
  * Adds a frame specified by frameOpts and newKey and sets the activeFrameKey
  * @return Immutable top level application state ready to merge back in
  */
-function addFrame (state, frameOpts, newKey, partitionNumber, openInForeground, insertionIndex) {
+function addFrame (state, frameOpts, newKey, partitionNumber, openInForeground, insertionIndex, active) {
   const frames = state.get('frames')
 
   const location = frameOpts.location // page url
@@ -509,18 +508,30 @@ const deleteFrameInternalIndex = (state, frame) => {
 
 const updateFramesInternalIndex = (state, fromIndex) => {
   let framesInternal = state.get('framesInternal') || Immutable.Map()
-  state.get('frames').slice(fromIndex).forEach((frame, idx) => {
+  state.get('frames').slice(fromIndex).reduceRight((result, frame, idx) => {
+    const tabId = frame.get('tabId')
+    const frameKey = frame.get('key')
     const realIndex = idx + fromIndex
-    if (frame.get('key')) {
-      framesInternal = framesInternal.setIn(['index', frame.get('key').toString()], realIndex)
+    if (frameKey) {
+      framesInternal = framesInternal.setIn(['index', frameKey.toString()], realIndex)
     }
-    if (frame.get('tabId') !== -1) {
-      framesInternal = framesInternal.setIn(['tabIndex', frame.get('tabId').toString()], realIndex)
+    if (tabId !== -1) {
+      framesInternal = framesInternal.setIn(['tabIndex', tabId.toString()], realIndex)
     }
+  }, 0)
+  return state.set('framesInternal', framesInternal)
+}
 
-    appActions.tabIndexChanged(frame.get('tabId'), realIndex)
-  })
-
+const moveFrame = (state, tabId, index) => {
+  let framesInternal = state.get('framesInternal') || Immutable.Map()
+  const frame = getFrameByTabId(state, tabId)
+  const frameKey = frame.get('key')
+  if (frameKey) {
+    framesInternal = framesInternal.setIn(['index', frameKey.toString()], index)
+  }
+  if (tabId !== -1) {
+    framesInternal = framesInternal.setIn(['tabIndex', tabId.toString()], index)
+  }
   return state.set('framesInternal', framesInternal)
 }
 
@@ -691,6 +702,7 @@ module.exports = {
   deleteTabInternalIndex,
   deleteFrameInternalIndex,
   updateFramesInternalIndex,
+  moveFrame,
   query,
   find,
   isAncestorFrameKey,
diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js
index 6b75240dda7..4624f42071b 100644
--- a/js/stores/windowStore.js
+++ b/js/stores/windowStore.js
@@ -71,6 +71,10 @@ class WindowStore extends EventEmitter {
     return frameStateUtil.getFrameByKey(windowState, key)
   }
 
+  getFrameByTabId (tabId) {
+    return frameStateUtil.getFrameByTabId(windowState, tabId)
+  }
+
   emitChanges () {
     if (lastEmittedState !== windowState) {
       lastEmittedState = windowState
@@ -104,7 +108,7 @@ const newFrame = (state, frameOpts) => {
   // handle tabs.create properties
   let insertionIndex = frameOpts.index !== undefined
     ? frameOpts.index
-    : undefined
+    : 0
 
   if (frameOpts.partition) {
     frameOpts.isPrivate = frameStateUtil.isPrivatePartition(frameOpts.partition)
@@ -116,7 +120,6 @@ const newFrame = (state, frameOpts) => {
 
   const active = frameOpts.active
   delete frameOpts.active
-  delete frameOpts.openInForeground // clean up any legacy openInForeground props
   let openInForeground = active
 
   if (openInForeground == null && frameOpts.disposition) {
@@ -143,45 +146,11 @@ const newFrame = (state, frameOpts) => {
       frameOpts.location = ''
     }
   }
-
-  // TODO: longer term get rid of parentFrameKey completely instead of
-  // calculating it here.
-  let parentFrameKey = frameOpts.parentFrameKey
-  if (frameOpts.openerTabId) {
-    parentFrameKey = frameStateUtil.getFrameKeyByTabId(state, frameOpts.openerTabId)
-  }
-
-  // Find the closest index to the current frame's index which has
-  // a different ancestor frame key.
-  const frames = frameStateUtil.getFrames(state)
-  if (insertionIndex === undefined) {
-    insertionIndex = frameStateUtil.getFrameIndex(state, frameOpts.indexByFrameKey || parentFrameKey)
-    if (frameOpts.prependIndexByFrameKey === false) {
-      insertionIndex++
-    }
-    if (insertionIndex === -1) {
-      insertionIndex = frames.size
-    // frameOpts.indexByFrameKey is used when the insertionIndex should be used exactly
-    } else if (!frameOpts.indexByFrameKey) {
-      while (insertionIndex < frames.size) {
-        ++insertionIndex
-        if (!frameStateUtil.isAncestorFrameKey(state, frames.get(insertionIndex), parentFrameKey)) {
-          break
-        }
-      }
-    }
-  }
-  if (frameStateUtil.isFrameKeyPinned(state, parentFrameKey)) {
-    insertionIndex = 0
-  }
-
   const nextKey = incrementNextKey()
-
   state = state.merge(
     frameStateUtil.addFrame(
       state, frameOpts,
       nextKey, frameOpts.partitionNumber, openInForeground, insertionIndex))
-
   state = frameStateUtil.updateFramesInternalIndex(state, insertionIndex)
 
   if (openInForeground) {
@@ -212,7 +181,8 @@ const frameTabIdChanged = (state, action) => {
   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)
+  state = frameStateUtil.updateFramesInternalIndex(state, index)
+  return state
 }
 
 const frameGuestInstanceIdChanged = (state, action) => {
@@ -418,7 +388,9 @@ const doAction = (action) => {
         windowState = windowState.set('frames', frames)
         // Since the tab could have changed pages, update the tab page as well
         windowState = frameStateUtil.updateFramesInternalIndex(windowState, Math.min(sourceFrameIndex, newIndex))
+        windowState = frameStateUtil.moveFrame(windowState, sourceFrameProps.get('tabId'), newIndex)
         windowState = frameStateUtil.updateTabPageIndex(windowState, activeFrame.get('tabId'))
+        appActions.tabIndexChanged(activeFrame.get('tabId'), newIndex)
         break
       }
     case windowConstants.WINDOW_SET_LINK_HOVER_PREVIEW:
diff --git a/test/lib/brave.js b/test/lib/brave.js
index bd02552ad73..2cda31b0a69 100644
--- a/test/lib/brave.js
+++ b/test/lib/brave.js
@@ -613,7 +613,7 @@ var exports = {
         const frame = val.value.frames[index]
         return this.execute(function (tabId, windowId, frame) {
           const browserOpts = { positionByMouseCursor: true }
-          devTools('appActions').tabMoved(tabId, frame, browserOpts, windowId)
+          devTools('appActions').tabDetachMenuItemClicked(tabId, frame, browserOpts, windowId)
         }, frame.tabId, windowId, frame)
       })
     })
diff --git a/test/unit/app/browser/reducers/tabsReducerTest.js b/test/unit/app/browser/reducers/tabsReducerTest.js
index 3daf8c7444b..2f0cd1fbac9 100644
--- a/test/unit/app/browser/reducers/tabsReducerTest.js
+++ b/test/unit/app/browser/reducers/tabsReducerTest.js
@@ -63,17 +63,6 @@ describe('tabsReducer unit tests', function () {
           4: 3,
           5: 4
         },
-        displayIndex: {
-          1: {
-            0: 1,
-            1: 2
-          },
-          2: {
-            2: 3,
-            3: 4,
-            4: 5
-          }
-        },
         lastActive: {
           1: [0, 1],
           2: [4, 3, 2]
@@ -101,6 +90,7 @@ describe('tabsReducer unit tests', function () {
       setActive: sinon.spy(),
       moveTo: sinon.mock(),
       reload: sinon.mock(),
+      updateTabsStateForWindow: sinon.mock(),
       create: sinon.mock()
     }
 
@@ -113,13 +103,16 @@ describe('tabsReducer unit tests', function () {
     mockery.registerMock('../tabs', this.tabsAPI)
     mockery.registerMock('../windows', this.windowsAPI)
     mockery.registerMock('../../common/state/tabState', this.tabStateAPI)
-    mockery.registerMock('../../../js/settings', { getSetting: (settingKey, settingsCollection, value) => {
+    mockery.registerMock('../../js/settings', { getSetting: (settingKey, settingsCollection, value) => {
       if (settingKey === settings.TAB_CLOSE_ACTION) {
         return tabCloseSetting
       }
       return false
     }})
     tabsReducer = require('../../../../../app/browser/reducers/tabsReducer')
+
+    this.realTabsAPI = require('../../../../../app/browser/tabs')
+    this.tabsAPI.getNextActiveTab = this.realTabsAPI.getNextActiveTab
   })
 
   after(function () {
@@ -224,8 +217,14 @@ describe('tabsReducer unit tests', function () {
     })
   })
 
+  describe.skip('APP_TAB_DETACH_MENU_ITEM_CLICKED', function () {
+    it('Someone clicked the detach menu item', function () {
+      // TODO
+    })
+  })
+
   describe.skip('APP_TAB_MOVED', function () {
-    it('moves a tab', function () {
+    it('A tab has moved', function () {
       // TODO
     })
   })
@@ -273,11 +272,13 @@ describe('tabsReducer unit tests', function () {
 
     afterEach(function () {
       this.removeTabByTabIdSpy.restore()
+      this.tabsAPI.updateTabsStateForWindow.reset()
     })
 
     it('calls tabState.removeTabByTabId', function () {
       tabsReducer(this.state, action)
       assert(this.tabStateAPI.removeTabByTabId.withArgs(this.state, action.tabId).calledOnce)
+      assert.equal(this.tabsAPI.updateTabsStateForWindow.getCall(0).args[1], 2)
     })
 
     it('does nothing if tabId is TAB_ID_NONE', function () {
@@ -302,6 +303,7 @@ describe('tabsReducer unit tests', function () {
           tabsReducer(this.state, action)
           this.clock.tick(1510)
           assert(this.tabsAPI.setActive.withArgs(3).calledOnce)
+          assert.equal(this.tabsAPI.updateTabsStateForWindow.getCall(0).args[1], 2)
         })
       })
 
@@ -325,6 +327,7 @@ describe('tabsReducer unit tests', function () {
           tabsReducer(testState, pickNextAction)
           this.clock.tick(1510)
           assert(this.tabsAPI.setActive.withArgs(4).calledOnce)
+          assert.equal(this.tabsAPI.updateTabsStateForWindow.getCall(0).args[1], 2)
         })
 
         it('chooses next unpinned tab', function () {
@@ -339,6 +342,7 @@ describe('tabsReducer unit tests', function () {
           tabsReducer(testState, pickNextAction)
           this.clock.tick(1510)
           assert(this.tabsAPI.setActive.withArgs(5).calledOnce)
+          assert.equal(this.tabsAPI.updateTabsStateForWindow.getCall(0).args[1], 2)
         })
 
         it('chooses previous unpinned tab', function () {
@@ -349,6 +353,7 @@ describe('tabsReducer unit tests', function () {
           tabsReducer(testState, action)
           this.clock.tick(1510)
           assert(this.tabsAPI.setActive.withArgs(3).calledOnce)
+          assert.equal(this.tabsAPI.updateTabsStateForWindow.getCall(0).args[1], 2)
         })
 
         describe('if no unpinned tabs come after this', function () {
@@ -365,6 +370,7 @@ describe('tabsReducer unit tests', function () {
             tabsReducer(testState, pickNextAction)
             this.clock.tick(1510)
             assert(this.tabsAPI.setActive.withArgs(4).calledOnce)
+            assert.equal(this.tabsAPI.updateTabsStateForWindow.getCall(0).args[1], 2)
           })
         })
       })
@@ -374,6 +380,7 @@ describe('tabsReducer unit tests', function () {
           tabsReducer(this.state, action)
           this.clock.tick(1510)
           assert(this.tabsAPI.setActive.withArgs(4).calledOnce)
+          assert.equal(this.tabsAPI.updateTabsStateForWindow.getCall(0).args[1], 2)
         })
 
         it('chooses parent tab id (even if parent tab was NOT last active)', function () {
@@ -383,6 +390,7 @@ describe('tabsReducer unit tests', function () {
           tabsReducer(testState, action)
           this.clock.tick(1510)
           assert(this.tabsAPI.setActive.withArgs(3).calledOnce)
+          assert.equal(this.tabsAPI.updateTabsStateForWindow.getCall(0).args[1], 2)
         })
       })
     })
@@ -578,13 +586,6 @@ describe('tabsReducer unit tests', function () {
             beforeEach(function () {
               this.pinnedTabs = sinon.stub(this.tabStateAPI, 'getPinnedTabsByWindowId', (state, windowId) => Immutable.fromJS([]))
             })
-
-            it('closes window when there are no tabs left', function () {
-              tabsReducer(this.state, action)
-              this.clock.tick(1510)
-              assert(this.tabsAPI.toggleDevTools.notCalled)
-              assert(this.windowsAPI.closeWindow.withArgs(this.state, 1).calledOnce)
-            })
           })
         })
       })
@@ -707,12 +708,10 @@ describe('tabsReducer unit tests', function () {
       assert.equal(args[0], state)  // State is passed in as first arg
       assert.equal(args[1], 1)  // tabId is 1 for first tab
       // frameOpts being dragged is for the first tab
-      assert.deepEqual(args[2], { tabId: 1,
+      assert.deepEqual(args[2].toJS(), { tabId: 1,
         windowId: 1,
         pinned: false,
-        active: true,
-        indexByFrameKey: undefined,
-        prependIndexByFrameKey: undefined
+        active: true
       })
       // Passes browser options for position by mouse cursor
       assert.deepEqual(args[3], {
diff --git a/test/unit/app/browser/tabsTest.js b/test/unit/app/browser/tabsTest.js
index 03d2eaa7aed..6397573f5d8 100644
--- a/test/unit/app/browser/tabsTest.js
+++ b/test/unit/app/browser/tabsTest.js
@@ -82,7 +82,10 @@ describe('tabs API unit tests', function () {
           tabValue: () =>
             this.state.get('tabs').find((tab) => tab.get('tabId') === tabId),
           isDestroyed: () => false,
-          detach: (cb) => cb()
+          detach: (cb) => cb(),
+          once: (event, cb) => {
+            setImmediate(cb)
+          }
         }
         if (tabId === 1) {
           Object.assign(webContents, this.tabWithDevToolsClosed)
diff --git a/test/unit/js/stores/windowStoreTest.js b/test/unit/js/stores/windowStoreTest.js
index f161f63186d..f3b258e0ee7 100644
--- a/test/unit/js/stores/windowStoreTest.js
+++ b/test/unit/js/stores/windowStoreTest.js
@@ -136,7 +136,7 @@ describe('Window store unit tests', function () {
 
     describe('APP_NEW_WEB_CONTENTS_ADDED', function () {
       let windowState
-      let tabIndexChangedStub
+      let tabDetachMenuItemClickedStub
       const demoWindowState = {
         frames: [{
           security: {
@@ -183,7 +183,15 @@ describe('Window store unit tests', function () {
           loading: false,
           unloaded: true,
           key: 2
-        }]
+        }],
+        framesInternal: {
+          index: {
+            8: 0
+          },
+          tabIndex: {
+            8: 0
+          }
+        }
       }
       const demoAction = {
         actionType: appConstants.APP_NEW_WEB_CONTENTS_ADDED,
@@ -194,7 +202,8 @@ describe('Window store unit tests', function () {
           location: 'about:blank',
           partition: 'persist:default',
           active: true,
-          guestInstanceId: 8,
+          guestInstanceId: 2,
+          tabId: 8,
           isPinned: false,
           openerTabId: 8,
           disposition: 'foreground-tab',
@@ -205,14 +214,14 @@ describe('Window store unit tests', function () {
           width: 300,
           active: true,
           height: 300,
-          guestInstanceId: 8,
+          guestInstanceId: 2,
           autoDiscardable: true,
           partition: 'persist:default',
           windowId: -1,
           incognito: false,
           canGoForward: false,
           url: '',
-          tabId: 33,
+          tabId: 8,
           index: -1,
           status: 'complete',
           highlighted: false,
@@ -223,7 +232,7 @@ describe('Window store unit tests', function () {
             muted: false,
             reason: 'user'
           },
-          id: 33,
+          id: 8,
           selected: true,
           discarded: false,
           canGoBack: false
@@ -231,7 +240,7 @@ describe('Window store unit tests', function () {
       }
 
       beforeEach(function () {
-        tabIndexChangedStub = sinon.stub(appActions, 'tabIndexChanged')
+        tabDetachMenuItemClickedStub = sinon.stub(appActions, 'tabDetachMenuItemClicked')
       })
 
       afterEach(function () {
@@ -239,7 +248,7 @@ describe('Window store unit tests', function () {
           mockery.deregisterMock(reducer)
         })
 
-        tabIndexChangedStub.restore()
+        tabDetachMenuItemClickedStub.restore()
       })
 
       describe('when tab being opened is active', function () {