Change sites from List to Map

fix #4879 Auditors: @bbondy, @bsclifton, @cezaraugusto Test Plan: Performance: 1. Import bulk bookmarks (4000+) from other browsers (Don't merge into toolbar) 2. The process should finish instantly 3. Delete Import from XXX folder 4. The process should finish instantly Migration: 1. Make sure session-store-1 contains the sites data before 0.12.10 2. Lauch Brave and Close 3. sites of session-store-1 should change from [] tp {}
brave · Dec 5, 2016 · 0be2e3c · 0be2e3c
1 parent 9c8abce
commit 0be2e3c
Show file tree

Hide file tree

Showing 10 changed files with 516 additions and 513 deletions.
diff --git a/app/common/lib/historyUtil.js b/app/common/lib/historyUtil.js
@@ -17,7 +17,7 @@ const sortTimeDescending = (left, right) => {
 }
 
 module.exports.getHistory = (sites) => {
-  sites = makeImmutable(sites) || new Immutable.List()
+  sites = makeImmutable(sites) ? makeImmutable(sites).toList() : new Immutable.List()
   return sites.filter((site) => siteUtil.isHistoryEntry(site))
       .sort(sortTimeDescending)
       .slice(0, aboutHistoryMaxEntries)

diff --git a/app/common/state/aboutNewTabState.js b/app/common/state/aboutNewTabState.js
@@ -66,7 +66,7 @@ const removeDuplicateDomains = (list) => {
  */
 const getTopSites = (state) => {
   // remove folders; sort by visit count; enforce a max limit
-  const sites = (state.get('sites') || new Immutable.List())
+  const sites = (state.get('sites') ? state.get('sites').toList() : new Immutable.List())
     .filter((site) => !siteUtil.isFolder(site))
     .filter((site) => !isSourceAboutUrl(site.get('location')))
     .sort(sortCountDescending)

diff --git a/app/importer.js b/app/importer.js
@@ -23,6 +23,7 @@ const locale = require('./locale')
 var isMergeFavorites = false
 var isImportingBookmarks = false
 var hasBookmarks
+var importedSites
 
 exports.init = () => {
   importer.initialize()
@@ -171,6 +172,7 @@ importer.on('add-bookmarks', (e, bookmarks, topLevelFolder) => {
       sites.push(site)
     }
   }
+  importedSites = Immutable.fromJS(sites)
   appActions.addSite(Immutable.fromJS(sites))
 })
 
@@ -187,7 +189,7 @@ importer.on('add-favicons', (e, detail) => {
       }
     }
   })
-  let sites = AppStore.getState().get('sites')
+  let sites = importedSites
   sites = sites.map((site) => {
     if ((site.get('favicon') === undefined && site.get('location') !== undefined &&
       faviconMap[site.get('location')] !== undefined) ||

diff --git a/app/renderer/components/bookmarksToolbar.js b/app/renderer/components/bookmarksToolbar.js
@@ -319,27 +319,37 @@ class BookmarksToolbar extends ImmutableComponent {
 
     // Loop through until we fill up the entire bookmark toolbar width
     let i
-    for (i = 0; i < noParentItems.size; i++) {
+    let noParentItemsList = noParentItems.toList()
+    for (i = 0; i < noParentItemsList.size; i++) {
       let iconWidth = props.showFavicon ? iconSize : 0
       // font-awesome file icons are 3px smaller
-      if (props.showFavicon && !noParentItems.getIn([i, 'folderId']) && !noParentItems.getIn([i, 'favicon'])) {
+      if (props.showFavicon && !noParentItemsList.getIn([i, 'folderId']) && !noParentItemsList.getIn([i, 'favicon'])) {
         iconWidth -= 3
       }
-      const chevronWidth = props.showFavicon && noParentItems.getIn([i, 'folderId']) ? this.chevronWidth : 0
+      const chevronWidth = props.showFavicon && noParentItemsList.getIn([i, 'folderId']) ? this.chevronWidth : 0
       if (props.showFavicon && props.showOnlyFavicon) {
         widthAccountedFor += this.padding + iconWidth + chevronWidth
       } else {
-        const text = noParentItems.getIn([i, 'customTitle']) || noParentItems.getIn([i, 'title']) || noParentItems.getIn([i, 'location'])
+        const text = noParentItemsList.getIn([i, 'customTitle']) || noParentItemsList.getIn([i, 'title']) || noParentItemsList.getIn([i, 'location'])
         widthAccountedFor += Math.min(calculateTextWidth(text, `${this.fontSize} ${this.fontFamily}`) + this.padding + iconWidth + chevronWidth, this.maxWidth)
       }
       widthAccountedFor += margin
       if (widthAccountedFor >= props.windowWidth - overflowButtonWidth) {
         break
       }
     }
-    this.bookmarksForToolbar = noParentItems.take(i)
+    const bookmarkSort = (x, y) => {
+      if (x.get('order') < y.get('order')) {
+        return -1
+      } else if (x.get('order') > y.get('order')) {
+        return 1
+      } else {
+        return 0
+      }
+    }
+    this.bookmarksForToolbar = noParentItems.take(i).sort(bookmarkSort)
     // Show at most 100 items in the overflow menu
-    this.overflowBookmarkItems = noParentItems.skip(i).take(100)
+    this.overflowBookmarkItems = noParentItems.skip(i).take(100).sort(bookmarkSort)
   }
   componentWillMount () {
     this.updateBookmarkData(this.props)

diff --git a/app/sessionStore.js b/app/sessionStore.js
@@ -34,6 +34,18 @@ const getSetting = require('../js/settings').getSetting
 const promisify = require('../js/lib/promisify')
 const sessionStorageName = `session-store-${sessionStorageVersion}`
 
+const getTopSiteMap = () => {
+  if (Array.isArray(topSites) && topSites.length) {
+    let siteMap = {}
+    topSites.forEach((site) => {
+      let key = siteUtil.getSiteKey(Immutable.fromJS(site))
+      siteMap[key] = site
+    })
+    return siteMap
+  }
+  return {}
+}
+
 const getStoragePath = () => {
   return path.join(app.getPath('userData'), sessionStorageName)
 }
@@ -464,6 +476,28 @@ module.exports.loadAppState = () => {
         }
       }
       data = setVersionInformation(data)
+
+      // sites refactoring migration
+      if (Array.isArray(data.sites) && data.sites.length) {
+        let sites = {}
+        data.sites.forEach((site) => {
+          let key = siteUtil.getSiteKey(Immutable.fromJS(site))
+          sites[key] = site
+        })
+        data.sites = sites
+      }
+      if (data.about && data.about.newtab && data.about.newtab.sites) {
+        if (Array.isArray(data.about.newtab.sites) && data.about.newtab.sites.length) {
+          let sites = {}
+          data.about.newtab.sites.forEach((site) => {
+            if (site) {
+              let key = siteUtil.getSiteKey(Immutable.fromJS(site))
+              sites[key] = site
+            }
+          })
+          data.about.newtab.sites = sites
+        }
+      }
     } catch (e) {
       // TODO: Session state is corrupted, maybe we should backup this
       // corrupted value for people to report into support.
@@ -484,7 +518,7 @@ module.exports.loadAppState = () => {
 module.exports.defaultAppState = () => {
   return {
     firstRunTimestamp: new Date().getTime(),
-    sites: topSites,
+    sites: getTopSiteMap(),
     tabs: [],
     extensions: {},
     visits: [],
@@ -511,7 +545,7 @@ module.exports.defaultAppState = () => {
     about: {
       newtab: {
         gridLayoutSize: 'small',
-        sites: topSites,
+        sites: getTopSiteMap(),
         ignoredTopSites: [],
         pinnedTopSites: pinnedTopSites
       }

diff --git a/docs/state.md b/docs/state.md
@@ -34,19 +34,21 @@ AppStore
       }
     }
   },
-  sites: [{
-    location: string,
-    title: string,
-    customTitle: string, // User provided title for bookmark; overrides title
-    tags: [string], // empty, 'bookmark', 'bookmark-folder', 'pinned', or 'reader'
-    favicon: string, // URL of the favicon
-    themeColor: string, // css compatible color string
-    lastAccessedTime: number, // datetime.getTime()
-    creationTime: number, //creation time of bookmark
-    partitionNumber: number, // Optionally specifies a specific session
-    folderId: number, // Set for bookmark folders only
-    parentFolderId: number // Set for bookmarks and bookmark folders only
-  }],
+  sites: {
+    [siteKey]: { // folder: folderId; bookmark/history: location + partitionNumber + parentFolderId
+      location: string,
+      title: string,
+      customTitle: string, // User provided title for bookmark; overrides title
+      tags: [string], // empty, 'bookmark', 'bookmark-folder', 'pinned', or 'reader'
+      favicon: string, // URL of the favicon
+      themeColor: string, // css compatible color string
+      lastAccessedTime: number, // datetime.getTime()
+      creationTime: number, //creation time of bookmark
+      partitionNumber: number, // Optionally specifies a specific session
+      folderId: number, // Set for bookmark folders only
+      parentFolderId: number // Set for bookmarks and bookmark folders only
+    },
+  },
   downloads: [{
     [downloadId]: {
       startTime: number, // datetime.getTime()

diff --git a/js/contextMenus.js b/js/contextMenus.js
@@ -360,7 +360,8 @@ function showBookmarkFolderInit (allBookmarkItems, parentBookmarkFolder, activeF
 function bookmarkItemsInit (allBookmarkItems, items, activeFrame) {
   const btbMode = getSetting(settings.BOOKMARKS_TOOLBAR_MODE)
   const showFavicon = (btbMode === bookmarksToolbarMode.TEXT_AND_FAVICONS || btbMode === bookmarksToolbarMode.FAVICONS_ONLY)
-  const template = items.map((site) => {
+  const itemsList = items.toList()
+  const template = itemsList.map((site) => {
     const isFolder = siteUtil.isFolder(site)
     let faIcon
     if (showFavicon && !site.get('favicon')) {