Optimize sending appState from browser to windows by filtering out la…

…rge often-changing tabs[].frame.x properties

These frame state objects are sent from the window to the browser, and whilst this could be looked at being removed as all the data should already be accessible on the tab state, they do not need to be sent back to the renderer, where they originated. The performance impact of this is measurable when there are a lot of frames being modified very quickly, such as app startup, tab removal, tab deletion or anything that should change many frame properties at once (such as index).

Also do not add on 'bookmarked' property since I cannot find an instance of it being used.
Remove double-setting of appStoreRenderer.state. Whilst this does have a check to make sure the argument is different each time it is called, it is not necessary to call it twice.

app/common/state/tabState.js

@@ -472,11 +472,7 @@ const tabState = {
     if (shouldDebugTabEvents) {
       console.log(`Tab [${tabId}] frame changed for tab`)
     }
-
-    const bookmarkUtil = require('../lib/bookmarkUtil')
-    const frameLocation = action.getIn(['frame', 'location'])
-    const frameBookmarked = bookmarkUtil.isLocationBookmarked(state, frameLocation)
-    const frameValue = action.get('frame').set('bookmarked', frameBookmarked)
+    const frameValue = action.get('frame')
     tabValue = tabValue.set('frame', makeImmutable(frameValue))
     return tabState.updateTabValue(state, tabValue)
   },

js/entry.js

@@ -61,8 +61,8 @@ if (process.env.NODE_ENV === 'test') {
 
 ipc.on(messages.APP_STATE_CHANGE, (e, action) => {
   appStoreRenderer.state = action.stateDiff
-    ? appStoreRenderer.state = patch(appStoreRenderer.state, Immutable.fromJS(action.stateDiff))
-    : appStoreRenderer.state = Immutable.fromJS(action.state)
+    ? patch(appStoreRenderer.state, Immutable.fromJS(action.stateDiff))
+    : Immutable.fromJS(action.state)
 })
 
 ipc.on(messages.CLEAR_CLOSED_FRAMES, (e, location) => {

js/stores/appStore.js

@@ -68,6 +68,14 @@ if (SHOULD_LOG_TIME) {
 }
 const timeLogger = new HrtimeLogger(TIME_LOG_PATH, TIME_LOG_THRESHOLD)
 
+function shouldIgnoreStateDiffForWindow (stateOp) {
+  const path = stateOp.get('path')
+  // remove tabs[].frame since it comes from the windowState anyway
+  // TODO: do we need to store this in the appState? It's expensive.
+  const shouldIgnore = (path.startsWith('/tabs/') && path.includes('/frame/'))
+  return shouldIgnore
+}
+
 class AppStore extends EventEmitter {
   constructor () {
     super()
@@ -83,6 +91,8 @@ class AppStore extends EventEmitter {
       let d
       try {
         d = diff(this.lastEmittedState, appState)
+          // remove paths the window does not care about
+          .filterNot(shouldIgnoreStateDiffForWindow)
       } catch (e) {
         console.error('Error getting a diff from latest state.')
         // one possible reason immutablediff can throw an error
@@ -101,13 +111,14 @@ class AppStore extends EventEmitter {
         throw error
       }
       if (d && !d.isEmpty()) {
+        const stateDiff = d.toJS()
         BrowserWindow.getAllWindows().forEach((wnd) => {
           if (wnd.webContents && !wnd.webContents.isDestroyed()) {
-            wnd.webContents.send(messages.APP_STATE_CHANGE, { stateDiff: d.toJS() })
+            wnd.webContents.send(messages.APP_STATE_CHANGE, { stateDiff })
           }
         })
         this.lastEmittedState = appState
-        this.emit(CHANGE_EVENT, d.toJS())
+        this.emit(CHANGE_EVENT, stateDiff)
       }
     } else {
       this.emit(CHANGE_EVENT, [])