Reduce redundant siteSort to sort once and used everywhere

fix #7240

Auditors: @bbondy, @bsclifton

Test Plan:
1. Import bunch of bookmarks
2. Open bookmarks of menu shouldn't affect performance
3. Open folder on bookmark toolbar shouldn't affect performance
4. Open about:bookmarks shouldn't affect performance
5. Open/Switch/Close tabs shound't affect performance

Author: darkdh

app/browser/bookmarksExporter.js

@@ -52,7 +52,7 @@ function createBookmarkArray (sites, parentFolderId, first = true, depth = 1) {
 
   if (first) payload.push(`${indentFirst}<DL><p>`)
 
-  filteredBookmarks.toList().sort(siteUtil.siteSort).forEach((site) => {
+  filteredBookmarks.forEach((site) => {
     if (site.get('tags').includes(siteTags.BOOKMARK) && site.get('location')) {
       title = site.get('customTitle') || site.get('title') || site.get('location')
       payload.push(`${indentNext}<DT><A HREF="${site.get('location')}">${title}</A>`)

app/browser/menu.js

@@ -26,7 +26,7 @@ const menuUtil = require('../common/lib/menuUtil')
 const {getByTabId} = require('../common/state/tabState')
 const getSetting = require('../../js/settings').getSetting
 const locale = require('../locale')
-const {isSiteBookmarked, siteSort} = require('../../js/state/siteUtil')
+const {isSiteBookmarked} = require('../../js/state/siteUtil')
 const tabState = require('../../app/common/state/tabState')
 const isDarwin = process.platform === 'darwin'
 const isLinux = process.platform === 'linux'
@@ -381,7 +381,7 @@ const createBookmarksSubmenu = () => {
     CommonMenu.exportBookmarksMenuItem()
   ]
 
-  const bookmarks = menuUtil.createBookmarkTemplateItems(appStore.getState().get('sites').toList().sort(siteSort))
+  const bookmarks = menuUtil.createBookmarkTemplateItems(appStore.getState().get('sites'))
   if (bookmarks.length > 0) {
     submenu.push(CommonMenu.separatorMenuItem)
     submenu = submenu.concat(bookmarks)

app/browser/reducers/sitesReducer.js

@@ -37,13 +37,15 @@ const sitesReducer = (state, action, emitChanges) => {
         state = state.set('sites', siteUtil.moveSite(state.get('sites'),
           action.siteDetail, action.destinationDetail, false, false, true))
       }
+      state = state.set('sites', state.get('sites').sort(siteUtil.siteSort))
       if (syncEnabled()) {
         state = syncUtil.updateSiteCache(state, action.destinationDetail || action.siteDetail)
       }
       break
     case appConstants.APP_REMOVE_SITE:
       const removeSiteSyncCallback = action.skipSync ? undefined : syncActions.removeSite
       state = state.set('sites', siteUtil.removeSite(state.get('sites'), action.siteDetail, action.tag, true, removeSiteSyncCallback))
+      state = state.set('sites', state.get('sites').sort(siteUtil.siteSort))
       if (syncEnabled()) {
         state = syncUtil.updateSiteCache(state, action.siteDetail)
       }
@@ -52,6 +54,7 @@ const sitesReducer = (state, action, emitChanges) => {
       state = state.set('sites', siteUtil.moveSite(state.get('sites'),
         action.sourceDetail, action.destinationDetail, action.prepend,
         action.destinationIsParent, false))
+      state = state.set('sites', state.get('sites').sort(siteUtil.siteSort))
       if (syncEnabled()) {
         state = syncUtil.updateSiteCache(state, action.destinationDetail)
       }

app/browser/tabs.js

@@ -17,7 +17,6 @@ const settings = require('../../js/constants/settings')
 const {getBaseUrl, aboutUrls} = require('../../js/lib/appUrlUtil')
 const siteSettings = require('../../js/state/siteSettings')
 const messages = require('../../js/constants/messages')
-const siteUtil = require('../../js/state/siteUtil')
 const aboutHistoryState = require('../common/state/aboutHistoryState')
 const appStore = require('../../js/stores/appStore')
 const appConfig = require('../../js/constants/appConfig')
@@ -137,8 +136,8 @@ const updateAboutDetails = (tab, tabValue) => {
     allSiteSettings = allSiteSettings.mergeDeep(appState.get('temporarySiteSettings'))
   }
   const extensionsValue = appState.get('extensions')
-  const bookmarks = appState.get('sites').filter((site) => site.get('tags').includes(siteTags.BOOKMARK)).toList().sort(siteUtil.siteSort)
-  const bookmarkFolders = appState.get('sites').filter((site) => site.get('tags').includes(siteTags.BOOKMARK_FOLDER)).toList().sort(siteUtil.siteSort)
+  const bookmarks = appState.get('sites').filter((site) => site.get('tags').includes(siteTags.BOOKMARK))
+  const bookmarkFolders = appState.get('sites').filter((site) => site.get('tags').includes(siteTags.BOOKMARK_FOLDER))
   const sync = appState.get('sync')
   const braveryDefaults = siteSettings.braveryDefaults(appState, appConfig)
   const history = aboutHistoryState.getHistory(appState)
@@ -163,8 +162,8 @@ const updateAboutDetails = (tab, tabValue) => {
     tab.send(messages.EXTENSIONS_UPDATED, extensionsValue.toJS())
   } else if (location === 'about:bookmarks' && bookmarks) {
     tab.send(messages.BOOKMARKS_UPDATED, {
-      bookmarks: bookmarks.toJS(),
-      bookmarkFolders: bookmarkFolders.toJS()
+      bookmarks: bookmarks.toList().toJS(),
+      bookmarkFolders: bookmarkFolders.toList().toJS()
     })
   } else if (location === 'about:history' && history) {
     appActions.populateHistory()

app/browser/windows.js

@@ -12,7 +12,6 @@ const LocalShortcuts = require('../localShortcuts')
 const {getPinnedSiteProps} = require('../common/lib/windowsUtil')
 const {makeImmutable} = require('../common/state/immutableUtil')
 const {getPinnedTabsByWindowId} = require('../common/state/tabState')
-const {siteSort} = require('../../js/state/siteUtil')
 const messages = require('../../js/constants/messages')
 const settings = require('../../js/constants/settings')
 const siteTags = require('../../js/constants/siteTags')
@@ -84,7 +83,7 @@ const updatePinnedTabs = (win) => {
   const sitesToAdd = pinnedSites.filter((site) =>
     !win.__alreadyPinnedSites.find((pinned) => pinned.equals(site)))
 
-  sitesToAdd.sort(siteSort).forEach((site) => {
+  sitesToAdd.forEach((site) => {
     win.__alreadyPinnedSites = win.__alreadyPinnedSites.add(site)
     appActions.createTabRequested({
       url: site.get('location'),

app/index.js

@@ -77,6 +77,7 @@ const privacy = require('../js/state/privacy')
 const async = require('async')
 const settings = require('../js/constants/settings')
 const BookmarksExporter = require('./browser/bookmarksExporter')
+const siteUtil = require('../js/state/siteUtil')
 
 app.commandLine.appendSwitch('enable-features', 'BlockSmallPluginContent,PreferHtmlOverPlugins')
 
@@ -321,7 +322,10 @@ app.on('ready', () => {
     // For tests we always want to load default app state
     const loadedPerWindowState = initialState.perWindowState
     delete initialState.perWindowState
-    appActions.setState(Immutable.fromJS(initialState))
+    // Retore map order after load
+    let state = Immutable.fromJS(initialState)
+    state = state.set('sites', state.get('sites').sort(siteUtil.siteSort))
+    appActions.setState(state)
     setImmediate(() => perWindowStateLoaded(loadedPerWindowState))
   })
 

app/renderer/components/bookmarks/bookmarksToolbar.js

@@ -105,7 +105,7 @@ class BookmarksToolbar extends ImmutableComponent {
     contextMenus.onShowBookmarkFolderMenu(this.bookmarks, bookmark, this.activeFrame, e)
   }
   updateBookmarkData (props) {
-    this.bookmarks = siteUtil.getBookmarks(props.sites).toList().sort(siteUtil.siteSort)
+    this.bookmarks = siteUtil.getBookmarks(props.sites)
 
     const noParentItems = this.bookmarks
       .filter((bookmark) => !bookmark.get('parentFolderId'))
@@ -150,6 +150,7 @@ class BookmarksToolbar extends ImmutableComponent {
         break
       }
     }
+    // TODO(darkdh): Remove siteSort when we have shared appState
     this.bookmarksForToolbar = noParentItems.take(i).sort(siteUtil.siteSort)
     // Show at most 100 items in the overflow menu
     this.overflowBookmarkItems = noParentItems.skip(i).take(100).sort(siteUtil.siteSort)