From fdbb4b8f579765724f2fea0a740b92d7c20af466 Mon Sep 17 00:00:00 2001
From: petemill <miller.pete+commit@gmail.com>
Date: Thu, 10 May 2018 17:50:53 -0700
Subject: [PATCH] Optimize sending appState from browser to windows by
 filtering out large 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 |  6 +-----
 js/entry.js                  |  4 ++--
 js/stores/appStore.js        | 15 +++++++++++++--
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/app/common/state/tabState.js b/app/common/state/tabState.js
index c50ca1d39b1..346d2a53514 100644
--- a/app/common/state/tabState.js
+++ b/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)
   },
diff --git a/js/entry.js b/js/entry.js
index 1cbbedf67da..88378a6cade 100644
--- a/js/entry.js
+++ b/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) => {
diff --git a/js/stores/appStore.js b/js/stores/appStore.js
index ad16b29deb7..ac582ba24ea 100644
--- a/js/stores/appStore.js
+++ b/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, [])