Merge pull request #8863 from brave/issue-8853

Improves tab switching and tab related perf
brave · May 16, 2017 · 1d5ba3f · 1d5ba3f
2 parents 8fc5c56 + 1388f21
commit 1d5ba3f
Show file tree

Hide file tree

Showing 33 changed files with 815 additions and 649 deletions.
diff --git a/.npmrc b/.npmrc
@@ -1,7 +1,7 @@
 runtime = electron
 target_arch = x64
-brave_electron_version = 3.0.100
+brave_electron_version = 3.0.102
 chromedriver_version = 2.27
-target = v3.0.1
+target = v3.0.102
 disturl=https://brave-laptop-binaries.s3.amazonaws.com/atom-shell/dist/
 build_from_source = true
diff --git a/app/browser/menu.js b/app/browser/menu.js
@@ -261,7 +261,7 @@ const createViewSubmenu = () => {
       accelerator: isDarwin ? 'Cmd+Alt+I' : 'Ctrl+Shift+I',
       click: function (item) {
         const win = BrowserWindow.getActiveWindow()
-        const activeTab = tabState.getActiveTabValue(appStore.getState(), win.id)
+        const activeTab = tabState.getActiveTab(appStore.getState(), win.id)
         appActions.toggleDevTools(activeTab.get('tabId'))
       }
     },

diff --git a/app/browser/reducers/tabsReducer.js b/app/browser/reducers/tabsReducer.js
@@ -127,6 +127,11 @@ const tabsReducer = (state, action, immutableAction) => {
         tabs.setActive(action.get('tabId'))
       })
       break
+    case appConstants.APP_TAB_INDEX_CHANGED:
+      setImmediate(() => {
+        tabs.setTabIndex(action.get('tabId'), action.get('index'))
+      })
+      break
     case appConstants.APP_TAB_TOGGLE_DEV_TOOLS:
       setImmediate(() => {
         tabs.toggleDevTools(action.get('tabId'))

diff --git a/app/browser/share.js b/app/browser/share.js
@@ -29,7 +29,7 @@ const validateShareType = (shareType) =>
  * @param {string} shareType - The template to use, see the property key names in templateUrls above.
  */
 const simpleShareActiveTab = (state, windowId, shareType) => {
-  const tabValue = tabState.getActiveTabValue(state, windowId)
+  const tabValue = tabState.getActiveTab(state, windowId)
   const encodedTitle = encodeURIComponent(tabValue.get('title') || '')
   const encodedUrl = encodeURIComponent(tabValue.get('url'))
 

diff --git a/app/browser/tabs.js b/app/browser/tabs.js
@@ -3,6 +3,7 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 const appActions = require('../../js/actions/appActions')
+const windowActions = require('../../js/actions/windowActions')
 const config = require('../../js/constants/config')
 const Immutable = require('immutable')
 const tabState = require('../common/state/tabState')
@@ -363,6 +364,12 @@ const api = {
         appActions.updatePassword(username, origin, tabId)
       })
 
+      tab.on('did-get-response-details', (evt, status, newURL, originalURL, httpResponseCode, requestMethod, referrer, headers, resourceType) => {
+        if (resourceType === 'mainFrame') {
+          windowActions.gotResponseDetails(tabId, {status, newURL, originalURL, httpResponseCode, requestMethod, referrer, headers, resourceType})
+        }
+      })
+
       updateWebContents(tabId, tab)
 
       let tabValue = getTabValue(tabId)
@@ -413,12 +420,17 @@ const api = {
   },
 
   setActive: (tabId) => {
-    setImmediate(() => {
-      let tab = getWebContents(tabId)
-      if (tab && !tab.isDestroyed()) {
-        tab.setActive(true)
-      }
-    })
+    let tab = getWebContents(tabId)
+    if (tab && !tab.isDestroyed()) {
+      tab.setActive(true)
+    }
+  },
+
+  setTabIndex: (tabId, index) => {
+    let tab = getWebContents(tabId)
+    if (tab && !tab.isDestroyed()) {
+      tab.setTabIndex(index)
+    }
   },
 
   loadURL: (action) => {
@@ -443,7 +455,7 @@ const api = {
   },
 
   loadURLInActiveTab: (state, windowId, url) => {
-    const tabValue = tabState.getActiveTabValue(state, windowId)
+    const tabValue = tabState.getActiveTab(state, windowId)
     if (tabValue) {
       api.loadURLInTab(state, tabValue.get('tabId'), url)
     }
@@ -611,7 +623,7 @@ const api = {
         if (windowId == null || windowId === -1) {
           appActions.newWindow(makeImmutable(frameOpts), browserOpts)
         } else {
-          appActions.newWebContentsAdded(windowId, frameOpts)
+          appActions.newWebContentsAdded(windowId, frameOpts, tabValue)
         }
       })
     }

diff --git a/app/common/state/tabState.js b/app/common/state/tabState.js
@@ -3,6 +3,7 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 const { makeImmutable, isMap, isList } = require('./immutableUtil')
+const Immutable = require('immutable')
 const assert = require('assert')
 const frameState = require('./frameState')
 const windowState = require('./windowState')
@@ -55,6 +56,36 @@ const matchTab = function (queryInfo, tab) {
   return !Object.keys(queryInfo).map((queryKey) => (tab.get(queryKey) === queryInfo[queryKey])).includes(false)
 }
 
+const deleteTabsInternalIndex = (state, tabValue) => {
+  const tabId = validateId('tabId', tabValue.get('tabId'))
+  if (tabId === tabState.TAB_ID_NONE) {
+    return state
+  }
+  state = state.deleteIn(['tabsInternal', tabId])
+  return state.deleteIn(['tabsInternal', tabId.toString()])
+}
+
+const updateTabsInternalIndex = (state, fromIndex) => {
+  let tabsInternal = state.get('tabsInternal') || Immutable.Map()
+  state.get('tabs').slice(fromIndex).forEach((tab, idx) => {
+    if (tab.get('tabId') !== tabState.TAB_ID_NONE) {
+      tabsInternal = tabsInternal.setIn(['index', tab.get('tabId')], idx + fromIndex)
+    }
+  })
+  return state.set('tabsInternal', tabsInternal)
+}
+
+const getTabInternalIndexByTabId = (state, tabId) => {
+  tabId = validateId('tabId', tabId)
+  state = validateState(state)
+
+  let index = state.getIn(['tabsInternal', 'index', tabId])
+  if (index == null) {
+    index = state.getIn(['tabsInternal', 'index', tabId.toString()])
+  }
+  return index == null ? -1 : index
+}
+
 const tabState = {
   queryTab: (state, queryInfo) => {
     state = validateState(state)
@@ -68,32 +99,12 @@ const tabState = {
     validateId('tabId', tabId)
   },
 
-  getTabIndex: (state, tabValue) => {
-    state = validateState(state)
-    tabValue = validateTabValue(tabValue)
-    let tabId = validateId('tabId', tabValue.get('tabId'))
-    return tabState.getTabIndexByTabId(state, tabId)
-  },
-
-  getTabIndexByTabId: (state, tabId) => {
-    tabId = validateId('tabId', tabId)
-    state = validateState(state)
-
-    return state.get('tabs').findIndex((tab) => tab.get('tabId') === tabId)
-  },
-
-  getActiveTabValue: (state, windowId) => {
-    windowId = validateId('windowId', windowId)
-    state = validateState(state)
-    return state.get('tabs').find((tab) => tab.get('windowId') === windowId && tab.get('active'))
-  },
-
   removeTabByTabId: (state, tabId) => {
     tabId = validateId('tabId', tabId)
     state = validateState(state)
 
-    let index = tabState.getTabIndexByTabId(state, tabId)
-    if (index === -1) {
+    let index = getTabInternalIndexByTabId(state, tabId)
+    if (index === tabState.TAB_ID_NONE) {
       return state
     }
     return tabState.removeTabByIndex(state, index)
@@ -103,7 +114,13 @@ const tabState = {
     index = parseInt(index)
     assert.ok(index >= 0, 'index must be positive')
     state = validateState(state)
-    return state.set('tabs', state.get('tabs').delete(index))
+    const tabValue = state.getIn(['tabs', index])
+    if (!tabValue) {
+      return state
+    }
+    state = deleteTabsInternalIndex(state, tabValue)
+    state = state.set('tabs', state.get('tabs').delete(index))
+    return updateTabsInternalIndex(state, index)
   },
 
   removeTab: (state, tabValue) => {
@@ -119,7 +136,8 @@ const tabState = {
     state = validateState(state)
     let tabValue = validateTabValue(action.get('tabValue'))
     assert.ok(!tabState.getTab(state, tabValue), 'Tab already exists')
-    return state.set('tabs', state.get('tabs').push(tabValue))
+    state = state.set('tabs', state.get('tabs').push(tabValue))
+    return updateTabsInternalIndex(state, state.get('tabs').size - 1)
   },
 
   maybeCreateTab: (state, action) => {
@@ -171,7 +189,7 @@ const tabState = {
     tabId = tabState.resolveTabId(state, tabId)
     state = validateState(state)
 
-    const index = state.get('tabs').findIndex((tab) => tab.get('tabId') === tabId)
+    const index = getTabInternalIndexByTabId(state, tabId)
     if (index === tabState.TAB_ID_NONE) {
       return null
     }
@@ -217,8 +235,8 @@ const tabState = {
   updateTabValue: (state, tabValue, replace = false) => {
     tabValue = validateTabValue(tabValue)
     const tabs = state.get('tabs')
-    const index = tabState.getTabIndex(state, tabValue)
-    if (index === -1) {
+    const index = getTabInternalIndexByTabId(state, tabValue.get('tabId'))
+    if (index === tabState.TAB_ID_NONE) {
       return state
     }
 
@@ -269,43 +287,63 @@ const tabState = {
     return tabState.updateTabValue(state, tabValue)
   },
 
-  getTabPropertyByTabId: (state, tabId, property) => {
+  getTabPropertyByTabId: (state, tabId, property, defaultValue = null) => {
     state = validateState(state)
     tabId = validateId('tabId', tabId)
+    if (tabId === tabState.TAB_ID_NONE) {
+      return defaultValue
+    }
     const tab = tabState.getByTabId(state, tabId)
     assert.ok(tab, `Could not find tab for ${tabId}`)
-    return tab.get(property)
+    const val = tab.get(property)
+    return val == null ? defaultValue : val
   },
 
   windowId: (state, tabId) => {
-    return tabState.getTabPropertyByTabId(state, tabId, 'windowId') || windowState.WINDOW_ID_NONE
+    return tabState.getTabPropertyByTabId(state, tabId, 'windowId', windowState.WINDOW_ID_NONE)
   },
 
   canGoForward: (state, tabId) => {
-    return tabState.getTabPropertyByTabId(state, tabId, 'canGoForward') || false
+    try {
+      return tabState.getTabPropertyByTabId(state, tabId, 'canGoForward', false)
+    } catch (e) {
+      return false
+    }
   },
 
   canGoBack: (state, tabId) => {
-    return tabState.getTabPropertyByTabId(state, tabId, 'canGoBack') || false
+    try {
+      return tabState.getTabPropertyByTabId(state, tabId, 'canGoBack', false)
+    } catch (e) {
+      return false
+    }
   },
 
   isShowingMessageBox: (state, tabId) => {
-    if (tabId === tabState.TAB_ID_NONE) {
+    try {
+      return tabState.getTabPropertyByTabId(state, tabId, 'messageBoxDetail', false)
+    } catch (e) {
       return false
     }
-    return tabState.getTabPropertyByTabId(state, tabId, 'messageBoxDetail') || false
+  },
+
+  isPinned: (state, tabId) => {
+    return tabState.getTabPropertyByTabId(state, tabId, 'pinned', false)
   },
 
   getTitle: (state, tabId) => {
-    return tabState.getTabPropertyByTabId(state, tabId, 'title') || ''
+    return tabState.getTabPropertyByTabId(state, tabId, 'title', '')
   },
 
   isActive: (state, tabId) => {
-    return tabState.getTabPropertyByTabId(state, tabId, 'active') || false
+    return tabState.getTabPropertyByTabId(state, tabId, 'active', false)
   },
 
   // TOOD(bridiver) - make everything work with TAB_ID_ACTIVE
   resolveTabId: (state, tabId) => {
+    if (tabId == null) {
+      tabId = tabState.TAB_ID_NONE
+    }
     if (tabId === tabState.TAB_ID_ACTIVE) {
       tabId = tabState.getActiveTabId(state)
     }
@@ -385,6 +423,7 @@ const tabState = {
 
     state = tabState.removeTabField(state, 'messageBoxDetail')
     state = tabState.removeTabField(state, 'frame')
+    state = state.delete('tabsInternal')
     return state.delete('tabs')
   }
 }

diff --git a/app/renderer/components/navigation/urlBar.js b/app/renderer/components/navigation/urlBar.js
@@ -317,19 +317,19 @@ class UrlBar extends React.Component {
   }
 
   select () {
-    if (this.urlInput) {
-      setImmediate(() => {
+    setImmediate(() => {
+      if (this.urlInput) {
         this.urlInput.select()
-      })
-    }
+      }
+    })
   }
 
   focus () {
-    if (this.urlInput) {
-      setImmediate(() => {
+    setImmediate(() => {
+      if (this.urlInput) {
         this.urlInput.focus()
-      })
-    }
+      }
+    })
   }
 
   onFocus (e) {

diff --git a/app/renderer/components/reduxComponent.js b/app/renderer/components/reduxComponent.js
@@ -32,6 +32,7 @@ class ReduxComponent extends ImmutableComponent {
   }
 
   componentDidMount () {
+    this.dontCheck = false
     appStore.addChangeListener(this.checkForUpdates)
     windowStore.addChangeListener(this.checkForUpdates)
   }

diff --git a/app/renderer/lib/tabUtil.js b/app/renderer/lib/tabUtil.js
@@ -103,13 +103,7 @@ module.exports.getTabIconColor = (props) => {
  * @param frameProps Any frame belonging to the page
  */
 module.exports.updateTabPageIndex = (state, frameProps) => {
-  // No need to update tab page index if we are given a pinned frame
-  if (!frameProps || frameProps.get('pinnedLocation')) {
-    return state
-  }
-
-  const index = frameStateUtil.getFrameTabPageIndex(state.get('frames')
-      .filter((frame) => !frame.get('pinnedLocation')), frameProps, getSetting(settings.TABS_PER_PAGE))
+  const index = frameStateUtil.getFrameTabPageIndex(state, frameProps, getSetting(settings.TABS_PER_PAGE))
 
   if (index === -1) {
     return state