Don’t close or change tabs in the renderer. The renderer should reque…

app/browser/menu.js

@@ -75,7 +75,7 @@ const createFileSubmenu = () => {
       label: locale.translation('closeTab'),
       accelerator: 'CmdOrCtrl+W',
       click: function (item, focusedWindow) {
-        appActions.activeWebContentsClosed()
+        appActions.tabCloseRequested(tabState.TAB_ID_ACTIVE)
       }
     }, {
       // This should be disabled when no windows are active.
@@ -629,10 +629,10 @@ const doAction = (action) => {
         createMenu()
       })
       break
-    case appConstants.APP_TAB_CLOSED:
+    case appConstants.APP_TAB_CLOSE_REQUESTED:
       appDispatcher.waitFor([appStore.dispatchToken], () => {
         action = makeImmutable(action)
-        const tab = getByTabId(appStore.getState(), action.getIn(['tabValue', 'tabId']))
+        const tab = getByTabId(appStore.getState(), action.get('tabId'))
         if (tab && !tab.get('incognito') && tab.get('url') !== 'about:newtab') {
           if (tab.get('frame')) {
             lastClosedUrl = tab.get('url')

app/browser/reducers/sitesReducer.js

@@ -11,7 +11,6 @@ const siteUtil = require('../../../js/state/siteUtil')
 const syncActions = require('../../../js/actions/syncActions')
 const syncUtil = require('../../../js/state/syncUtil')
 const Immutable = require('immutable')
-const {makeImmutable} = require('../../common/state/immutableUtil')
 const settings = require('../../../js/constants/settings')
 const {getSetting} = require('../../../js/settings')
 const writeActions = require('../../../js/constants/sync/proto').actions
@@ -20,11 +19,10 @@ const syncEnabled = () => {
   return getSetting(settings.SYNC_ENABLED) === true
 }
 
-const sitesReducer = (state, action, emitChanges) => {
+const sitesReducer = (state, action, immutableAction) => {
   switch (action.actionType) {
     case appConstants.APP_ON_CLEAR_BROWSING_DATA:
-      action = makeImmutable(action)
-      if (action.getIn(['clearDataDetail', 'browserHistory'])) {
+      if (immutableAction.getIn(['clearDataDetail', 'browserHistory'])) {
         state = state.set('sites', siteUtil.clearHistory(state.get('sites')))
         filtering.clearHistory()
       }
@@ -99,28 +97,29 @@ const sitesReducer = (state, action, emitChanges) => {
         state = syncUtil.updateSiteCache(state, siteDetail)
       })
       break
-    case appConstants.APP_TAB_PINNED:
-      const tab = state.get('tabs').find((tab) => tab.get('tabId') === action.tabId)
-      if (!tab) {
-        console.warn('Trying to pin a tabId which does not exist:', action.tabId, 'tabs: ', state.get('tabs').toJS())
-        break
-      }
-      const sites = state.get('sites')
-      const siteDetail = siteUtil.getDetailFromTab(tab, siteTags.PINNED, sites)
-      if (action.pinned) {
-        state = state.set('sites', siteUtil.addSite(sites, siteDetail, siteTags.PINNED))
-      } else {
-        state = state.set('sites', siteUtil.removeSite(sites, siteDetail, siteTags.PINNED))
-      }
-      if (syncEnabled()) {
-        state = syncUtil.updateSiteCache(state, siteDetail)
+    case appConstants.APP_TAB_UPDATED:
+      if (immutableAction.getIn(['changeInfo', 'pinned']) != null) {
+        const pinned = immutableAction.getIn(['changeInfo', 'pinned'])
+        const tabId = immutableAction.getIn(['tabValue', 'tabId'])
+        const tab = state.get('tabs').find((tab) => tab.get('tabId') === tabId)
+        if (!tab) {
+          console.warn('Trying to pin a tabId which does not exist:', tabId, 'tabs: ', state.get('tabs').toJS())
+          break
+        }
+        const sites = state.get('sites')
+        const siteDetail = siteUtil.getDetailFromTab(tab, siteTags.PINNED, sites)
+        if (pinned) {
+          state = state.set('sites', siteUtil.addSite(sites, siteDetail, siteTags.PINNED))
+        } else {
+          state = state.set('sites', siteUtil.removeSite(sites, siteDetail, siteTags.PINNED))
+        }
+        if (syncEnabled()) {
+          state = syncUtil.updateSiteCache(state, siteDetail)
+        }
       }
       break
-
-    case appConstants.APP_MAYBE_CREATE_TAB_REQUESTED:
     case appConstants.APP_CREATE_TAB_REQUESTED: {
-      action = makeImmutable(action)
-      const createProperties = action.get('createProperties')
+      const createProperties = immutableAction.get('createProperties')
       if (createProperties.get('pinned')) {
         state = state.set('sites', siteUtil.addSite(state.get('sites'),
           siteUtil.getDetailFromCreateProperties(createProperties), siteTags.PINNED))

app/browser/reducers/tabsReducer.js

@@ -13,11 +13,9 @@ const windowAction = require('../../../js/actions/windowActions.js')
 const {makeImmutable} = require('../../common/state/immutableUtil')
 const {getFlashResourceId} = require('../../../js/flash')
 const {l10nErrorText} = require('../../common/lib/httpUtil')
-const windows = require('../windows')
 const Immutable = require('immutable')
 const dragTypes = require('../../../js/constants/dragTypes')
 const {frameOptsFromFrame} = require('../../../js/state/frameStateUtil')
-const {BrowserWindow} = require('electron')
 
 const tabsReducer = (state, action, immutableAction) => {
   action = immutableAction || makeImmutable(action)
@@ -29,91 +27,132 @@ const tabsReducer = (state, action, immutableAction) => {
       state = tabState.maybeCreateTab(state, action)
       break
     case appConstants.APP_TAB_MOVED: {
-      const tabId = action.get('tabId')
-      const frameOpts = action.get('frameOpts')
-      const browserOpts = action.get('browserOpts') || new Immutable.Map()
-      const windowId = action.get('windowId') || -1
-      state = tabs.moveTo(state, tabId, frameOpts, browserOpts, windowId)
+      setImmediate(() => {
+        const tabId = action.get('tabId')
+        const frameOpts = action.get('frameOpts')
+        const browserOpts = action.get('browserOpts') || new Immutable.Map()
+        const windowId = action.get('windowId') || -1
+        tabs.moveTo(state, tabId, frameOpts, browserOpts, windowId)
+      })
       break
     }
     case appConstants.APP_CREATE_TAB_REQUESTED:
-      tabs.createTab(action)
-      break
-    case appConstants.APP_MAYBE_CREATE_TAB_REQUESTED:
-      state = tabs.maybeCreateTab(state, action)
+      if (!action.getIn(['createProperties', 'windowId'])) {
+        const senderWindowId = action.getIn(['senderWindowId'])
+        if (senderWindowId) {
+          action = action.setIn(['createProperties', 'windowId'], senderWindowId)
+        }
+      }
+
+      setImmediate(() => {
+        if (action.get('activateIfOpen')) {
+          tabs.maybeCreateTab(state, action, action.get('createProperties'))
+        } else {
+          tabs.create(action.get('createProperties'), null, action.get('isRestore'))
+        }
+      })
       break
     case appConstants.APP_TAB_UPDATED:
       state = tabState.maybeCreateTab(state, action)
       break
-    case appConstants.APP_ACTIVE_WEB_CONTENTS_CLOSED: {
-      const tabValue = tabState.getActiveTabValue(state, BrowserWindow.getActiveWindow().id)
-      if (tabValue) {
-        const tabId = tabValue.get('tabId')
-        if (tabs.isDevToolsFocused(tabId)) {
-          state = tabs.toggleDevTools(state, tabId)
-        } else {
-          state = tabs.closeTab(state, tabId, false)
+    case appConstants.APP_TAB_CLOSE_REQUESTED:
+      {
+        const tabId = tabState.resolveTabId(state, action.get('tabId'))
+        if (tabId === tabState.TAB_ID_NONE) {
+          break
         }
+
+        setImmediate(() => {
+          if (tabId) {
+            if (tabs.isDevToolsFocused(tabId)) {
+              tabs.toggleDevTools(tabId)
+            } else {
+              tabs.closeTab(tabId, action.get('forceClosePinned'))
+            }
+          }
+        })
       }
       break
-    }
-    case appConstants.APP_TAB_CLOSED: {
-      const tabId = action.getIn(['tabValue', 'tabId'])
-      const forceClose = action.get('forceClose')
-      if (tabId) {
-        state = tabs.closeTab(state, tabId, forceClose)
+    case appConstants.APP_TAB_CLOSED:
+      {
+        const tabId = action.get('tabId')
+        if (tabId === tabState.TAB_ID_NONE) {
+          break
+        }
+        state = tabState.removeTabByTabId(state, tabId)
       }
       break
-    }
     case appConstants.APP_ALLOW_FLASH_ONCE:
     case appConstants.APP_ALLOW_FLASH_ALWAYS:
-      {
+      setImmediate(() => {
         const webContents = getWebContents(action.get('tabId'))
         if (webContents && !webContents.isDestroyed() && webContents.getURL() === action.get('url')) {
           webContents.authorizePlugin(getFlashResourceId())
         }
-        break
-      }
+      })
+      break
     case appConstants.APP_TAB_CLONED:
-      state = tabs.clone(state, action)
+      setImmediate(() => {
+        tabs.clone(action)
+      })
       break
     case appConstants.APP_TAB_PINNED:
-      state = tabs.pin(state, action)
+      setImmediate(() => {
+        tabs.pin(state, action.get('tabId'), action.get('pinned'))
+      })
       break
     case windowConstants.WINDOW_SET_AUDIO_MUTED:
-      state = tabs.setAudioMuted(state, action)
+      setImmediate(() => {
+        tabs.setAudioMuted(action)
+      })
       break
     case windowConstants.WINDOW_SET_ALL_AUDIO_MUTED:
       action.get('frameList').forEach((frameProp) => {
-        state = tabs.setAudioMuted(state, frameProp)
+        setImmediate(() => {
+          tabs.setAudioMuted(frameProp)
+        })
       })
       break
-    case windowConstants.WINDOW_SET_ACTIVE_FRAME:
-      tabs.setActive(action.getIn(['frameProps', 'tabId']))
+    case appConstants.APP_TAB_ACTIVATE_REQUESTED:
+      setImmediate(() => {
+        tabs.setActive(action.get('tabId'))
+      })
       break
     case appConstants.APP_TAB_TOGGLE_DEV_TOOLS:
-      state = tabs.toggleDevTools(state, action.get('tabId'))
+      setImmediate(() => {
+        tabs.toggleDevTools(action.get('tabId'))
+      })
       break
     case appConstants.APP_LOAD_URL_REQUESTED:
-      state = tabs.loadURL(state, action)
+      setImmediate(() => {
+        tabs.loadURL(action)
+      })
       break
     case appConstants.APP_LOAD_URL_IN_ACTIVE_TAB_REQUESTED:
-      state = tabs.loadURLInActiveTab(state, action.get('windowId'), action.get('url'))
+      setImmediate(() => {
+        tabs.loadURLInActiveTab(state, action.get('windowId'), action.get('url'))
+      })
       break
     case appConstants.APP_ON_GO_BACK:
-      state = tabs.goBack(state, action)
+      setImmediate(() => {
+        tabs.goBack(action.get('tabId'))
+      })
       break
     case appConstants.APP_ON_GO_FORWARD:
-      state = tabs.goForward(state, action)
+      setImmediate(() => {
+        tabs.goForward(action.get('tabId'))
+      })
       break
     case appConstants.APP_ON_GO_TO_INDEX:
-      state = tabs.goToIndex(state, action)
+      setImmediate(() => {
+        tabs.goToIndex(action.get('tabId'), action.get('index'))
+      })
       break
     case appConstants.APP_ON_GO_BACK_LONG:
       {
         const history = tabs.getHistoryEntries(state, action)
         const tabValue = tabState.getByTabId(state, action.get('tabId'))
-        const windowId = windows.getActiveWindowId()
+        const windowId = tabValue.get('windowId')
 
         if (history !== null) {
           windowAction.onLongBackHistory(
@@ -131,7 +170,7 @@ const tabsReducer = (state, action, immutableAction) => {
       {
         const history = tabs.getHistoryEntries(state, action)
         const tabValue = tabState.getByTabId(state, action.get('tabId'))
-        const windowId = windows.getActiveWindowId()
+        const windowId = tabValue.get('windowId')
 
         if (history !== null) {
           windowAction.onLongForwardHistory(

app/browser/reducers/windowsReducer.js

@@ -19,11 +19,12 @@ const windowsReducer = (state, action, immutableAction) => {
     case appConstants.APP_WINDOW_READY:
       windows.windowReady(action.get('windowId'))
       break
-    case appConstants.APP_TAB_PINNED:
-      setImmediate(() => {
-        // check after the state has been updated
-        windows.pinnedTabsChanged()
-      })
+    case appConstants.APP_TAB_UPDATED:
+      if (immutableAction.getIn(['changeInfo', 'pinned']) != null) {
+        setImmediate(() => {
+          windows.pinnedTabsChanged()
+        })
+      }
       break
     case appConstants.APP_CLOSE_WINDOW:
       state = windows.closeWindow(state, action)

app/browser/tabMessageBox.js

@@ -90,7 +90,7 @@ const tabMessageBox = {
 
   onTabClosed: (state, action) => {
     action = makeImmutable(action)
-    const tabId = action.getIn(['tabValue', 'tabId'])
+    const tabId = action.get('tabId')
     if (tabId) {
       // remove callback; call w/ defaults
       const cb = messageBoxCallbacks[tabId]