Reduce redundant siteSort to sort once and used everywhere

fix #7240

Auditors: @bbondy, @bsclifton

Test Plan:
1. Import bunch of bookmarks
2. Open folder on bookmark toolbar shouldn't affect performance

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.toList().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 isDarwin = process.platform === 'darwin'
 const isLinux = process.platform === 'linux'
 
@@ -378,7 +378,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, syncActions.updateSite))
+      state = state.set('sites', state.get('sites').sort(siteUtil.siteSort))
       if (syncEnabled()) {
         state = syncUtil.updateSiteCache(state, action.destinationDetail)
       }

app/browser/tabs.js

@@ -13,7 +13,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')
@@ -132,8 +131,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)

app/browser/windows.js

@@ -11,7 +11,6 @@ const locale = require('../locale')
 const LocalShortcuts = require('../localShortcuts')
 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')
@@ -90,7 +89,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

@@ -76,6 +76,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')
 
@@ -298,7 +299,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,9 +150,9 @@ class BookmarksToolbar extends ImmutableComponent {
         break
       }
     }
-    this.bookmarksForToolbar = noParentItems.take(i).sort(siteUtil.siteSort)
+    this.bookmarksForToolbar = noParentItems.take(i)
     // Show at most 100 items in the overflow menu
-    this.overflowBookmarkItems = noParentItems.skip(i).take(100).sort(siteUtil.siteSort)
+    this.overflowBookmarkItems = noParentItems.skip(i).take(100)
   }
   componentWillMount () {
     this.updateBookmarkData(this.props)

js/stores/appStore.js

@@ -255,7 +255,10 @@ class AppStore extends EventEmitter {
 
   emitChanges (emitFullState) {
     if (lastEmittedState) {
-      const d = diff(lastEmittedState, appState)
+      // diff and patch will make map out of order so toList() is to maintain
+      // sites map order
+      const diffAppState = appState.set('sites', appState.get('sites').toList())
+      const d = diff(lastEmittedState, diffAppState)
       if (!d.isEmpty()) {
         BrowserWindow.getAllWindows().forEach((wnd) =>
           wnd.webContents.send(messages.APP_STATE_CHANGE, { stateDiff: d.toJS() }))
@@ -471,6 +474,7 @@ const handleAppAction = (action) => {
         }
         appState = syncUtil.updateSiteCache(appState, siteDetail)
       })
+      appState = appState.set('sites', appState.get('sites').sort(siteUtil.siteSort))
       appState = aboutNewTabState.setSites(appState)
       appState = aboutHistoryState.setHistory(appState)
       break