[wip] Cache bookmark-folder tree and use for system menu

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 {isLocationBookmarked, siteSort} = require('../../js/state/siteUtil')
+const {isLocationBookmarked} = 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())
   if (bookmarks.length > 0) {
     submenu.push(CommonMenu.separatorMenuItem)
     submenu = submenu.concat(bookmarks)

app/browser/reducers/sitesReducer.js

@@ -41,6 +41,7 @@ const sitesReducer = (state, action, immutableAction) => {
   switch (action.actionType) {
     case appConstants.APP_SET_STATE:
       state = siteCache.loadLocationSiteKeysCache(state)
+      state = siteCache.createSiteKeysByFolderCache(state)
       break
     case appConstants.APP_ON_CLEAR_BROWSING_DATA:
       if (immutableAction.getIn(['clearDataDetail', 'browserHistory'])) {

app/common/lib/menuUtil.js

@@ -3,9 +3,8 @@
 
 'use strict'
 
-const {makeImmutable} = require('../../common/state/immutableUtil')
+const {makeImmutable, isMap} = require('../../common/state/immutableUtil')
 const CommonMenu = require('../../common/commonMenu')
-const siteTags = require('../../../js/constants/siteTags')
 const eventUtil = require('../../../js/lib/eventUtil')
 const siteUtil = require('../../../js/state/siteUtil')
 const locale = require('../../locale')
@@ -71,45 +70,48 @@ module.exports.setTemplateItemChecked = (template, label, checked) => {
   return null
 }
 
-const createBookmarkTemplateItems = (bookmarks, parentFolderId) => {
-  const filteredBookmarks = parentFolderId
-    ? bookmarks.filter((bookmark) => bookmark.get('parentFolderId') === parentFolderId)
-    : bookmarks.filter((bookmark) => !bookmark.get('parentFolderId'))
-
-  const payload = []
-  filteredBookmarks.forEach((site) => {
-    if (site.get('tags').includes(siteTags.BOOKMARK) && site.get('location')) {
-      payload.push({
-        // TODO include label made from favicon. It needs to be of type NativeImage
-        // which can be made using a Buffer / DataURL / local image
-        // the image will likely need to be included in the site data
-        // there was potentially concern about the size of the app state
-        // and as such there may need to be another mechanism or cache
-        //
-        // see: https://github.com/brave/browser-laptop/issues/3050
-        label: site.get('customTitle') || site.get('title') || site.get('location'),
-        click: (item, focusedWindow, e) => {
-          if (eventUtil.isForSecondaryAction(e)) {
-            appActions.createTabRequested({
-              url: site.get('location'),
-              windowId: focusedWindow.id,
-              active: !!e.shiftKey
-            })
-          } else {
-            appActions.loadURLInActiveTabRequested(focusedWindow.id, site.get('location'))
+// TODO include label made from favicon. It needs to be of type NativeImage
+// which can be made using a Buffer / DataURL / local image
+// the image will likely need to be included in the site data
+// there was potentially concern about the size of the app state
+// and as such there may need to be another mechanism or cache
+//
+// see: https://github.com/brave/browser-laptop/issues/3050
+const createBookmarkTemplateItems = (state) => {
+  const sites = state.get('sites')
+  const createPayload = (groupedKeys) => {
+    const payload = []
+    groupedKeys.forEach((value, key) => {
+      const site = sites.get(key)
+      if (!site) {
+        return
+      }
+      const label = site.get('customTitle') || site.get('title') || site.get('location')
+      // Folder
+      if (isMap(value)) {
+        const submenu = createPayload(value)
+        payload.push({label, submenu})
+      } else {
+        payload.push({
+          label,
+          click: (item, focusedWindow, e) => {
+            if (eventUtil.isForSecondaryAction(e)) {
+              appActions.createTabRequested({
+                url: site.get('location'),
+                windowId: focusedWindow.id,
+                active: !!e.shiftKey
+              })
+            } else {
+              appActions.loadURLInActiveTabRequested(focusedWindow.id, site.get('location'))
+            }
           }
-        }
-      })
-    } else if (siteUtil.isFolder(site)) {
-      const folderId = site.get('folderId')
-      const submenuItems = bookmarks.filter((bookmark) => bookmark.get('parentFolderId') === folderId)
-      payload.push({
-        label: site.get('customTitle') || site.get('title'),
-        submenu: submenuItems.count() > 0 ? createBookmarkTemplateItems(bookmarks, folderId) : null
-      })
-    }
-  })
-  return payload
+        })
+      }
+    })
+    return payload
+  }
+  const groupedSiteKeys = siteUtil.getSiteKeysByFolder(state)
+  return createPayload(groupedSiteKeys)
 }
 
 /**
@@ -118,7 +120,7 @@ const createBookmarkTemplateItems = (bookmarks, parentFolderId) => {
  * @param sites The application state's Immutable sites list
  */
 module.exports.createBookmarkTemplateItems = (sites) => {
-  return createBookmarkTemplateItems(siteUtil.getBookmarks(sites))
+  return createBookmarkTemplateItems(sites)
 }
 
 /**

app/common/state/siteCache.js

@@ -3,6 +3,7 @@
 
 'use strict'
 const Immutable = require('immutable')
+const {isMap} = require('./immutableUtil')
 const siteUtil = require('../../../js/state/siteUtil')
 const UrlUtil = require('../../../js/lib/urlutil')
 
@@ -18,6 +19,65 @@ const createLocationSiteKeysCache = (state) => {
   return state
 }
 
+// See getSiteKeysByFolder() for explanation.
+const createSiteKeysByFolderCache = (state) => {
+  const sites = state.get('sites')
+  // Memoize nested folder paths.
+  // '1': ['2', '1']
+  // '3': ['2', '1', '3']
+  let groupedPathMemo = {}
+  const groupedPath = (siteKey) => {
+    if (typeof siteKey === 'number') {
+      siteKey = siteKey + ''
+    }
+    const memoResult = groupedPathMemo[siteKey]
+    if (memoResult) {
+      return memoResult
+    }
+
+    const site = sites.get(siteKey)
+    if (!site) {
+      return [siteKey]
+    }
+    const parentFolderId = site.get('parentFolderId')
+    if (parentFolderId && parentFolderId !== -1) {
+      const path = groupedPath(parentFolderId).concat([siteKey])
+      groupedPathMemo[siteKey] = path
+      return path
+    } else {
+      return [siteKey]
+    }
+  }
+
+  // For sorting
+  let keysOrders = {}
+  let result = new Immutable.Map()
+  sites.forEach((site, key) => {
+    if (typeof key === 'number') {
+      key = key + ''
+    }
+    const tags = site.get('tags')
+    if (!tags || tags.size === 0) {
+      return
+    }
+    if (siteUtil.isBookmark(site)) {
+      result = result.setIn(groupedPath(key), null)
+    } else if (siteUtil.isFolder(site)) {
+      result = result.setIn(groupedPath(key), new Immutable.Map())
+    } else {
+      return
+    }
+    keysOrders[key] = site.get('order')
+  })
+
+  const deepSort = (map) => {
+    map = map.map(value => isMap(value) ? deepSort(value) : value)
+    return map.sortBy((value, key) => keysOrders[key])
+  }
+  result = deepSort(result)
+  return state.set('siteKeysByFolderCache', result)
+}
+
 module.exports.loadLocationSiteKeysCache = (state) => {
   const cache = state.get('locationSiteKeysCache')
   if (cache) {
@@ -26,6 +86,8 @@ module.exports.loadLocationSiteKeysCache = (state) => {
   return createLocationSiteKeysCache(state)
 }
 
+module.exports.createSiteKeysByFolderCache = createSiteKeysByFolderCache
+
 /**
  * Given a location, get matching appState siteKeys based on cache.
  * Loads cache from appState if it hasn't been loaded yet.
@@ -38,6 +100,24 @@ module.exports.getLocationSiteKeys = (state, location) => {
   return state.getIn(['locationSiteKeysCache', normalLocation])
 }
 
+/**
+ * Group Immutable Map key paths by folder and order as seen in menus.
+ * OrderedMap {
+ *   '1': OrderedMap {
+ *     '3': OrderedMap { },
+ *     'https://archive.org|0|1': null
+ *   },
+ *   '2': OrderedMap { 'https://example.com|0|2 },
+ *   'https://wikipedia.org|0|0': null
+ * }
+ *
+ * @param state Application state
+ * @return {Immutable.Map} Site keys by folder
+ */
+module.exports.getSiteKeysByFolder = (state) => {
+  return state.get('siteKeysByFolderCache')
+}
+
 /**
  * Given a location, add appState siteKey to cached siteKeys list.
  * Returns new state.

js/state/siteUtil.js

@@ -325,10 +325,14 @@ module.exports.addSite = function (state, siteDetail, tag, originalSiteDetail, s
   state = state.set('sites', sites)
   const key = module.exports.getSiteKey(site)
   if (key === null) {
+    state = siteCache.createSiteKeysByFolderCache(state)
     return state
   }
   state = state.setIn(['sites', key], site)
   state = siteCache.addLocationSiteKey(state, location, key)
+  if ((location && location !== oldLocation) || (key && key !== oldKey)) {
+    state = siteCache.createSiteKeysByFolderCache(state)
+  }
   return state
 }
 
@@ -491,6 +495,7 @@ module.exports.moveSite = function (state, sourceKey, destinationKey, prepend,
   }
   const destinationSiteKey = module.exports.getSiteKey(sourceSite)
   state = siteCache.addLocationSiteKey(state, location, destinationSiteKey)
+  state = siteCache.createSiteKeysByFolderCache(state)
   return state.setIn(['sites', destinationSiteKey], sourceSite)
 }
 
@@ -778,6 +783,29 @@ module.exports.getFolders = function (sites, folderId, parentId, labelPrefix) {
   return folders
 }
 
+/**
+ * Group Immutable Map key paths by folder and sort by order.
+ * OrderedMap {
+ *   '1': OrderedMap {
+ *     '3': OrderedMap { },
+ *     'https://archive.org|0|1': null
+ *   },
+ *   '2': OrderedMap { 'https://example.com|0|2 },
+ *   'https://wikipedia.org|0|0': null
+ * }
+ *
+ * @param state Application state
+ * @return {Immutable.Map} Ordered site keys by folder
+ */
+module.exports.getSiteKeysByFolder = function (state) {
+  const cache = siteCache.getSiteKeysByFolder(state)
+  if (cache) {
+    return cache
+  } else {
+    return new Immutable.Map()
+  }
+}
+
 /**
  * Filters out non recent sites based on the app setting for history size.
  * @param sites The application state's Immutable sites list.