React perf: Store tabs separately

This is a better data fit so that TabsToolbar,Tabs, PinnedTabs, Tab do
not keep rendering for unrelated frame changes.

app/sessionStore.js

@@ -18,6 +18,7 @@ const app = electron.app
 const UpdateStatus = require('../js/constants/updateStatus')
 const settings = require('../js/constants/settings')
 const downloadStates = require('../js/constants/downloadStates')
+const {tabFromFrame} = require('../js/state/frameStateUtil')
 const sessionStorageVersion = 1
 const filtering = require('./filtering')
 
@@ -53,6 +54,11 @@ module.exports.saveAppState = (payload, isShutdown) => {
       payload.perWindowState.forEach((wndPayload) => {
         wndPayload.frames = wndPayload.frames.filter((frame) => !frame.isPrivate)
       })
+      // tabs will be auto-reset to what the frame is in cleanAppData but just in
+      // case clean fails we don't want to save private tabs.
+      payload.perWindowState.forEach((wndPayload) => {
+        wndPayload.tabs = wndPayload.tabs.filter((tab) => !tab.isPrivate)
+      })
     } else {
       delete payload.perWindowState
     }
@@ -63,6 +69,7 @@ module.exports.saveAppState = (payload, isShutdown) => {
     } catch (e) {
       payload.cleanedOnShutdown = false
     }
+    payload.lastAppVersion = app.getVersion()
 
     const epochTimestamp = (new Date()).getTime().toString()
     const tmpStoragePath = process.env.NODE_ENV !== 'test'
@@ -195,6 +202,8 @@ module.exports.cleanPerWindowData = (perWindowData) => {
       .filter((frame) => !frame.pinnedLocation)
     perWindowData.frames.forEach(cleanFrame)
   }
+  // Always recalculate tab data from frame data
+  perWindowData.tabs = perWindowData.frames.map(frame => tabFromFrame(frame))
 }
 
 /**
@@ -300,7 +309,7 @@ module.exports.loadAppState = () => {
       return
     }
     // Clean app data here if it wasn't cleared on shutdown
-    if (data.cleanedOnShutdown !== true) {
+    if (data.cleanedOnShutdown !== true || data.lastAppVersion !== app.getVersion()) {
       module.exports.cleanAppData(data, false)
     }
     data.cleanedOnShutdown = false

docs/state.md

@@ -176,6 +176,22 @@ WindowStore
 {
   activeFrameKey: number,
   previewFrameKey: number,
+  tabs: [{
+    themeColor: string, // css compatible color string
+    computedThemeColor: string, // css computed theme color from the favicon
+    icon: string, // favicon url
+    audioPlaybackActive: boolean, // frame is playing audio
+    audioMuted: boolean, // frame is muted
+    title: string, // page title
+    isPrivate: boolean, // private browsing tab
+    partitionNumber: number, // the session partition to use
+    pinnedLocation: string, // Indicates if a frame is pinned and its pin location
+    provisionalLocation: string,
+    icon: string, // favicon url
+    location: string, // The currently navigated location
+    loading: boolean,
+    frameKey: number
+  }],
   frames: [{
     audioMuted: boolean, // frame is muted
     audioPlaybackActive: boolean, // frame is playing audio
@@ -194,9 +210,9 @@ WindowStore
     partitionNumber: number, // the session partition to use
     loading: boolean,
     themeColor: string, // css compatible color string
+    computedThemeColor: string, // css computed theme color from the favicon
     isFullScreen: boolean, // true if the frame should be shown as full screen
     showFullScreenWarning: boolean, // true if a warning should be shown about full screen
-    computedThemeColor: string, // css computed theme color from the favicon
     startLoadTime: datetime,
     endtLoadTime: datetime,
     guestInstanceId: string, // not persisted
@@ -217,7 +233,7 @@ WindowStore
     fingerprintingProtection: {
       blocked: Array<string>
     },
-    provisionalLocation: string
+    provisionalLocation: string,
     security: {
       isSecure: boolean, // is using https
       loginRequiredDetail: {
@@ -341,5 +357,6 @@ WindowStore
   },
   flashInitialized: boolean, // Whether flash was initialized successfully. Cleared on shutdown.
   cleanedOnShutdown: boolean, // whether app data was successfully cleared on shutdown
+  lastAppVersion: string, // Version of the last file that was saved
 }
 ```

js/components/bookmarksToolbar.js

@@ -201,6 +201,9 @@ class BookmarksToolbar extends ImmutableComponent {
     this.clickBookmarkItem = this.clickBookmarkItem.bind(this)
     this.showBookmarkFolderMenu = this.showBookmarkFolderMenu.bind(this)
   }
+  get activeFrame () {
+    return windowStore.getFrame(this.props.activeFrameKey)
+  }
   onDrop (e) {
     e.preventDefault()
     const bookmark = dnd.prepareBookmarkDataFromCompatible(e.dataTransfer)
@@ -355,6 +358,7 @@ class BookmarksToolbar extends ImmutableComponent {
           <BookmarkToolbarButton
             ref={(node) => this.bookmarkRefs.push(node)}
             contextMenuDetail={this.props.contextMenuDetail}
+            activeFrameKey={this.props.activeFrameKey}
             draggingOverData={this.props.draggingOverData}
             openContextMenu={this.openContextMenu}
             clickBookmarkItem={this.clickBookmarkItem}

js/components/main.js

@@ -838,8 +838,8 @@ class Main extends ImmutableComponent {
           draggingOverData={this.props.windowState.getIn(['ui', 'dragging', 'draggingOver', 'dragType']) === dragTypes.TAB && this.props.windowState.getIn(['ui', 'dragging', 'draggingOver'])}
           previewTabs={getSetting(settings.SHOW_TAB_PREVIEWS)}
           tabsPerTabPage={tabsPerPage}
-          tabs={this.props.windowState.getIn(['ui', 'tabs'])}
-          frames={this.props.windowState.get('frames')}
+          tabPageIndex={this.props.windowState.getIn(['ui', 'tabs', 'tabPageIndex'])}
+          tabs={this.props.windowState.get('tabs')}
           sites={this.props.appState.get('sites')}
           key='tab-bar'
           activeFrameKey={activeFrame && activeFrame.get('key') || undefined}

js/components/pinnedTabs.js

@@ -7,6 +7,7 @@ const ReactDOM = require('react-dom')
 const ImmutableComponent = require('./immutableComponent')
 const Tab = require('./tab')
 const windowActions = require('../actions/windowActions')
+const windowStore = require('../stores/windowStore')
 const appActions = require('../actions/appActions')
 const siteTags = require('../constants/siteTags')
 const dragTypes = require('../constants/dragTypes')
@@ -34,10 +35,10 @@ class PinnedTabs extends ImmutableComponent {
     // will cause the onDragEnd to never run
     setTimeout(() => {
       const key = sourceDragData.get('key')
-      let droppedOnTab = dnd.closestFromXOffset(this.tabRefs.filter((tab) => tab && tab.props.frameProps.get('key') !== key), clientX).selectedRef
+      let droppedOnTab = dnd.closestFromXOffset(this.tabRefs.filter((node) => node && node.props.tab.get('frameKey') !== key), clientX).selectedRef
       if (droppedOnTab) {
         const isLeftSide = dnd.isLeftSide(ReactDOM.findDOMNode(droppedOnTab), clientX)
-        const droppedOnFrameProps = this.props.frames.find((frame) => frame.get('key') === droppedOnTab.props.frameProps.get('key'))
+        const droppedOnFrameProps = windowStore.getFrame(droppedOnTab.props.tab.get('frameKey'))
         windowActions.moveTab(sourceDragData, droppedOnFrameProps, isLeftSide)
         if (!sourceDragData.get('pinnedLocation')) {
           windowActions.setPinned(sourceDragData, true)
@@ -61,24 +62,15 @@ class PinnedTabs extends ImmutableComponent {
       onDragOver={this.onDragOver}
       onDrop={this.onDrop}>
        {
-          this.props.frames
-            .filter((frameProps) => frameProps.get('pinnedLocation'))
-            .map((frameProps) =>
-              <Tab activeDraggedTab={this.props.tabs.get('activeDraggedTab')}
-                ref={(node) => this.tabRefs.push(node)}
+          this.props.pinnedTabs
+            .map((tab) =>
+              <Tab ref={(node) => this.tabRefs.push(node)}
                 draggingOverData={this.props.draggingOverData}
-                themeColor={frameProps.get('themeColor') || frameProps.get('computedThemeColor')}
-                icon={frameProps.get('icon')}
-                audioPlaybackActive={frameProps.get('audioPlaybackActive')}
-                audioMuted={frameProps.get('audioMuted')}
-                title={frameProps.get('title')}
-                isPrivate={frameProps.get('isPrivate')}
-                partitionNumber={frameProps.get('partitionNumber')}
-                frameKey={frameProps.get('key')}
-                key={'tab-' + frameProps.get('key')}
+                tab={tab}
+                key={'tab-' + tab.get('frameKey')}
                 paintTabs={this.props.paintTabs}
                 previewTabs={this.props.previewTabs}
-                isActive={this.props.activeFrameKey === frameProps.get('key')}
+                isActive={this.props.activeFrameKey === tab.get('frameKey')}
                 partOfFullPageSet={this.props.partOfFullPageSet} />)
       }
     </div>

js/components/tab.js

@@ -18,15 +18,15 @@ const windowStore = require('../stores/windowStore')
 
 class Tab extends ImmutableComponent {
   get frame () {
-    return windowStore.getFrame(this.props.frameKey)
+    return windowStore.getFrame(this.props.tab.get('frameKey'))
   }
   get isPinned () {
-    return !!this.frame.get('pinnedLocation')
+    return !!this.props.tab.get('pinnedLocation')
   }
 
   get draggingOverData () {
     if (!this.props.draggingOverData ||
-        this.props.draggingOverData.get('dragOverKey') !== this.props.frameKey) {
+        this.props.draggingOverData.get('dragOverKey') !== this.props.tab.get('frameKey')) {
       return
     }
 
@@ -44,7 +44,7 @@ class Tab extends ImmutableComponent {
 
   get isDragging () {
     const sourceDragData = dnd.getInProcessDragData()
-    return sourceDragData && this.props.frameKey === sourceDragData.get('key')
+    return sourceDragData && this.props.tab.get('frameKey') === sourceDragData.get('key')
   }
 
   get isDraggingOverLeft () {
@@ -65,8 +65,8 @@ class Tab extends ImmutableComponent {
     // YouTube tries to change the title to add a play icon when
     // there is audio. Since we have our own audio indicator we get
     // rid of it.
-    return (this.frame.get('title') ||
-      this.frame.get('location')).replace('▶ ', '')
+    return (this.props.tab.get('title') ||
+      this.props.tab.get('location')).replace('▶ ', '')
   }
 
   onDragStart (e) {
@@ -78,7 +78,7 @@ class Tab extends ImmutableComponent {
   }
 
   onDragOver (e) {
-    dnd.onDragOver(dragTypes.TAB, this.tab.getBoundingClientRect(), this.props.frameKey, this.draggingOverData, e)
+    dnd.onDragOver(dragTypes.TAB, this.tabNode.getBoundingClientRect(), this.props.tab.get('frameKey'), this.draggingOverData, e)
   }
 
   setActiveFrame () {
@@ -97,9 +97,9 @@ class Tab extends ImmutableComponent {
 
   get loading () {
     return this.frame &&
-    this.frame.get('loading') &&
-    (!this.frame.get('provisionalLocation') ||
-    !this.frame.get('provisionalLocation').startsWith('chrome-extension://mnojpmjdmbbfmejpflffifhffcmidifd/'))
+    this.props.tab.get('loading') &&
+    (!this.props.tab.get('provisionalLocation') ||
+    !this.props.tab.get('provisionalLocation').startsWith('chrome-extension://mnojpmjdmbbfmejpflffifhffcmidifd/'))
   }
 
   onMouseLeave () {
@@ -136,7 +136,7 @@ class Tab extends ImmutableComponent {
       width: iconSize
     }
     const activeTabStyle = {}
-    const backgroundColor = this.props.paintTabs && this.props.themeColor
+    const backgroundColor = this.props.paintTabs && (this.props.tab.get('themeColor') || this.props.tab.get('computedThemeColor'))
     if (this.props.isActive && backgroundColor) {
       activeTabStyle.background = backgroundColor
       const textColor = getTextColorForBackground(backgroundColor)
@@ -146,7 +146,7 @@ class Tab extends ImmutableComponent {
       }
     }
 
-    const icon = this.props.icon
+    const icon = this.props.tab.get('icon')
     if (!this.loading && icon) {
       iconStyle = Object.assign(iconStyle, {
         backgroundImage: `url(${icon})`,
@@ -156,16 +156,16 @@ class Tab extends ImmutableComponent {
     }
 
     let playIcon = null
-    if (this.props.audioPlaybackActive || this.propsaudioMuted) {
+    if (this.props.tab.get('audioPlaybackActive') || this.props.tab.get('audioMuted')) {
       playIcon = <span className={cx({
         audioPlaybackActive: true,
         fa: true,
-        'fa-volume-up': this.props.audioPlaybackActive &&
-          !this.props.audioMuted,
-        'fa-volume-off': this.props.audioPlaybackActive &&
-          this.props.audioMuted
+        'fa-volume-up': this.props.tab.get('audioPlaybackActive') &&
+          !this.props.tab.get('audioMuted'),
+        'fa-volume-off': this.props.tab.get('audioPlaybackActive') &&
+          this.props.tab.get('audioMuted')
       })}
-        onClick={this.onMuteFrame.bind(this, !this.props.audioMuted)} />
+        onClick={this.onMuteFrame.bind(this, !this.props.tab.get('audioMuted'))} />
     }
 
     return <div
@@ -183,26 +183,26 @@ class Tab extends ImmutableComponent {
         tab: true,
         isPinned: this.isPinned,
         active: this.props.isActive,
-        private: this.props.isPrivate
+        private: this.props.tab.get('isPrivate')
       })}
-        data-frame-key={this.props.frameKey}
-        ref={(node) => { this.tab = node }}
+        data-frame-key={this.props.tab.get('frameKey')}
+        ref={(node) => { this.tabNode = node }}
         draggable
-        title={this.props.title}
+        title={this.props.tab.get('title')}
         onDragStart={this.onDragStart.bind(this)}
         onDragEnd={this.onDragEnd.bind(this)}
         onDragOver={this.onDragOver.bind(this)}
         onClick={this.onClickTab.bind(this)}
         onContextMenu={contextMenus.onTabContextMenu.bind(this, this.frame)}
         style={activeTabStyle}>
         {
-          this.props.isPrivate
+          this.props.tab.get('isPrivate')
           ? <div className='privateIcon fa fa-eye' />
           : null
         }
         {
-          this.props.partitionNumber
-          ? <div data-l10n-args={JSON.stringify({ partitionNumber: this.props.partitionNumber })}
+          this.props.tab.get('partitionNumber')
+          ? <div data-l10n-args={JSON.stringify({partitionNumber: this.props.tab.get('partitionNumber')})}
             data-l10n-id='sessionInfoTab'
             className='privateIcon fa fa-user' />
           : null

js/components/tabs.js

@@ -8,6 +8,7 @@ const ReactDOM = require('react-dom')
 const ImmutableComponent = require('./immutableComponent')
 
 const windowActions = require('../actions/windowActions')
+const windowStore = require('../stores/windowStore')
 const dragTypes = require('../constants/dragTypes')
 const cx = require('../lib/classSet')
 
@@ -26,23 +27,21 @@ class Tabs extends ImmutableComponent {
   }
 
   onPrevPage () {
-    if (this.props.tabs.get('tabPageIndex') === 0) {
+    if (this.props.tabPageIndex === 0) {
       return
     }
-    windowActions.setTabPageIndex(this.props.tabs.get('tabPageIndex') - 1)
+    windowActions.setTabPageIndex(this.props.tabPageIndex - 1)
   }
 
   onNextPage () {
-    if (this.props.tabs.get('tabPageIndex') + 1 === this.totalPages) {
+    if (this.props.tabPageIndex + 1 === this.totalPages) {
       return
     }
-    windowActions.setTabPageIndex(this.props.tabs.get('tabPageIndex') + 1)
+    windowActions.setTabPageIndex(this.props.tabPageIndex + 1)
   }
 
   get totalPages () {
-    return Math.ceil(this.props.frames
-        .filter((frame) => !frame.get('pinnedLocation'))
-        .size / this.props.tabsPerTabPage)
+    return Math.ceil(this.props.tabs.size / this.props.tabsPerTabPage)
   }
 
   onDrop (e) {
@@ -53,10 +52,10 @@ class Tabs extends ImmutableComponent {
       // will cause the onDragEnd to never run
       setTimeout(() => {
         const key = sourceDragData.get('key')
-        let droppedOnTab = dnd.closestFromXOffset(this.tabRefs.filter((tab) => tab && tab.props.frameProps.get('key') !== key), clientX).selectedRef
+        let droppedOnTab = dnd.closestFromXOffset(this.tabRefs.filter((node) => node && node.props.tab.get('frameKey') !== key), clientX).selectedRef
         if (droppedOnTab) {
           const isLeftSide = dnd.isLeftSide(ReactDOM.findDOMNode(droppedOnTab), clientX)
-          const droppedOnFrameProps = this.props.frames.find((frame) => frame.get('key') === droppedOnTab.props.frameProps.get('key'))
+          const droppedOnFrameProps = windowStore.getFrame(droppedOnTab.props.tab.get('frameKey'))
           windowActions.moveTab(sourceDragData, droppedOnFrameProps, isLeftSide)
           if (sourceDragData.get('pinnedLocation')) {
             windowActions.setPinned(sourceDragData, false)
@@ -105,27 +104,19 @@ class Tabs extends ImmutableComponent {
           }
         })()}
         {
-          this.props.currentFrames
-            .filter((frameProps) => !frameProps.get('pinnedLocation'))
-            .map((frameProps) =>
+          this.props.currentTabs
+            .map((tab) =>
               <Tab ref={(node) => this.tabRefs.push(node)}
                 draggingOverData={this.props.draggingOverData}
-                frameKey={frameProps.get('key')}
-                themeColor={frameProps.get('themeColor') || frameProps.get('computedThemeColor')}
-                icon={frameProps.get('icon')}
-                audioPlaybackActive={frameProps.get('audioPlaybackActive')}
-                audioMuted={frameProps.get('audioMuted')}
-                title={frameProps.get('title')}
-                isPrivate={frameProps.get('isPrivate')}
-                partitionNumber={frameProps.get('partitionNumber')}
-                key={'tab-' + frameProps.get('key')}
+                tab={tab}
+                key={'tab-' + tab.get('frameKey')}
                 paintTabs={this.props.paintTabs}
                 previewTabs={this.props.previewTabs}
-                isActive={this.props.activeFrameKey === frameProps.get('key')}
+                isActive={this.props.activeFrameKey === tab.get('frameKey')}
                 partOfFullPageSet={this.props.partOfFullPageSet} />)
         }
         {(() => {
-          if (this.props.currentFrames.size >= this.props.tabsPerTabPage && this.totalPages > this.props.tabPageIndex + 1) {
+          if (this.props.currentTabs.size >= this.props.tabsPerTabPage && this.totalPages > this.props.tabPageIndex + 1) {
             return <span
               className='nextTab fa fa-caret-right'
               onClick={this.onNextPage} />