Optimize getTopSites

app/browser/api/topSites.js

@@ -0,0 +1,151 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+'use strict'
+
+const Immutable = require('immutable')
+const appActions = require('../../../js/actions/appActions')
+const debounce = require('../../../js/lib/debounce')
+const siteUtil = require('../../../js/state/siteUtil')
+const {isSourceAboutUrl} = require('../../../js/lib/appUrlUtil')
+const aboutNewTabMaxEntries = 100
+let appStore
+
+let minCountOfTopSites
+let minAccessOfTopSites
+
+const compareSites = (site1, site2) => {
+  if (!site1 || !site2) return false
+  return site1.get('location') === site2.get('location') &&
+    site1.get('partitionNumber') === site2.get('partitionNumber')
+}
+
+const pinnedTopSites = (state) => {
+  return (state.getIn(['about', 'newtab', 'pinnedTopSites']) || Immutable.List()).setSize(18)
+}
+
+const ignoredTopSites = (state) => {
+  return state.getIn(['about', 'newtab', 'ignoredTopSites']) || Immutable.List()
+}
+
+const isPinned = (state, siteProps) => {
+  return pinnedTopSites(state).filter((site) => compareSites(site, siteProps)).size > 0
+}
+
+const isIgnored = (state, siteProps) => {
+  return ignoredTopSites(state).filter((site) => compareSites(site, siteProps)).size > 0
+}
+
+const sortCountDescending = (left, right) => {
+  const leftCount = left.get('count') || 0
+  const rightCount = right.get('count') || 0
+  if (leftCount < rightCount) {
+    return 1
+  }
+  if (leftCount > rightCount) {
+    return -1
+  }
+  if (left.get('lastAccessedTime') < right.get('lastAccessedTime')) {
+    return 1
+  }
+  if (left.get('lastAccessedTime') > right.get('lastAccessedTime')) {
+    return -1
+  }
+  return 0
+}
+
+const removeDuplicateDomains = (list) => {
+  const siteDomains = new Set()
+  return list.filter((site) => {
+    if (!site.get('location')) {
+      return false
+    }
+    try {
+      const hostname = require('../../common/urlParse')(site.get('location')).hostname
+      if (!siteDomains.has(hostname)) {
+        siteDomains.add(hostname)
+        return true
+      }
+    } catch (e) {
+      console.log('Error parsing hostname: ', e)
+    }
+    return false
+  })
+}
+
+const calculateTopSites = (clearCache) => {
+  if (clearCache) {
+    clearTopSiteCacheData()
+  }
+  startCalculatingTopSiteData()
+}
+
+/**
+ * TopSites are defined by users for the new tab page. Pinned sites are attached to their positions
+ * in the grid, and the non pinned indexes are populated with newly accessed sites
+ */
+const startCalculatingTopSiteData = debounce(() => {
+  if (!appStore) {
+    appStore = require('../../../js/stores/appStore')
+  }
+  const state = appStore.getState()
+  // remove folders; sort by visit count; enforce a max limit
+  const sites = (state.get('sites') ? state.get('sites').toList() : new Immutable.List())
+    .filter((site) => !siteUtil.isFolder(site) &&
+      !siteUtil.isImportedBookmark(site) &&
+      !isSourceAboutUrl(site.get('location')) &&
+      (minCountOfTopSites === undefined || (site.get('count') || 0) >= minCountOfTopSites) &&
+      (minAccessOfTopSites === undefined || (site.get('lastAccessedTime') || 0) >= minAccessOfTopSites))
+    .sort(sortCountDescending)
+    .slice(0, aboutNewTabMaxEntries)
+
+  for (let i = 0; i < sites.size; i++) {
+    const count = sites.getIn([i, 'count']) || 0
+    const access = sites.getIn([i, 'lastAccessedTime']) || 0
+    if (minCountOfTopSites === undefined || count < minCountOfTopSites) {
+      minCountOfTopSites = count
+    }
+    if (minAccessOfTopSites === undefined || access < minAccessOfTopSites) {
+      minAccessOfTopSites = access
+    }
+  }
+
+  // Filter out pinned and ignored sites
+  let unpinnedSites = sites.filter((site) => !(isPinned(state, site) || isIgnored(state, site)))
+  unpinnedSites = removeDuplicateDomains(unpinnedSites)
+
+  // Merge the pinned and unpinned lists together
+  // Pinned items have priority because the position is important
+  let gridSites = pinnedTopSites(state).map((pinnedSite) => {
+    // Fetch latest siteDetail objects from appState.sites using location/partition
+    if (pinnedSite) {
+      const matches = sites.filter((site) => compareSites(site, pinnedSite))
+      if (matches.size > 0) return matches.first()
+    }
+    // Default to unpinned items
+    const firstSite = unpinnedSites.first()
+    unpinnedSites = unpinnedSites.shift()
+    return firstSite
+  })
+
+  // Include up to [aboutNewTabMaxEntries] entries so that folks
+  // can ignore sites and have new items fill those empty spaces
+  if (unpinnedSites.size > 0) {
+    gridSites = gridSites.concat(unpinnedSites)
+  }
+
+  const finalData = gridSites.filter((site) => site != null)
+  appActions.topSiteDataAvailable(finalData)
+}, 5 * 1000)
+
+const clearTopSiteCacheData = () => {
+  minCountOfTopSites = undefined
+  minAccessOfTopSites = undefined
+}
+
+module.exports = {
+  calculateTopSites,
+  clearTopSiteCacheData,
+  aboutNewTabMaxEntries
+}

app/browser/reducers/topSitesReducer.js

@@ -0,0 +1,19 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+'use strict'
+
+const aboutNewTabState = require('../../common/state/aboutNewTabState')
+const appConstants = require('../../../js/constants/appConstants')
+
+const topSitesReducer = (state, action) => {
+  switch (action.actionType) {
+    case appConstants.APP_TOP_SITE_DATA_AVAILABLE:
+      state = aboutNewTabState.setSites(state, action.topSites)
+      break
+  }
+  return state
+}
+
+module.exports = topSitesReducer

app/common/state/aboutNewTabState.js

@@ -2,107 +2,14 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-const Immutable = require('immutable')
 const {makeImmutable} = require('./immutableUtil')
-const siteUtil = require('../../../js/state/siteUtil')
-const {isSourceAboutUrl} = require('../../../js/lib/appUrlUtil')
-const aboutNewTabMaxEntries = 100
 
-const compareSites = (site1, site2) => {
-  if (!site1 || !site2) return false
-  return site1.get('location') === site2.get('location') &&
-    site1.get('partitionNumber') === site2.get('partitionNumber')
-}
-const pinnedTopSites = (state) => {
-  return (state.getIn(['about', 'newtab', 'pinnedTopSites']) || Immutable.List()).setSize(18)
-}
-const ignoredTopSites = (state) => {
-  return state.getIn(['about', 'newtab', 'ignoredTopSites']) || Immutable.List()
-}
-const isPinned = (state, siteProps) => {
-  return pinnedTopSites(state).filter((site) => compareSites(site, siteProps)).size > 0
-}
-const isIgnored = (state, siteProps) => {
-  return ignoredTopSites(state).filter((site) => compareSites(site, siteProps)).size > 0
-}
-const sortCountDescending = (left, right) => {
-  const leftCount = left.get('count') || 0
-  const rightCount = right.get('count') || 0
-  if (leftCount < rightCount) {
-    return 1
-  }
-  if (leftCount > rightCount) {
-    return -1
-  }
-  if (left.get('lastAccessedTime') < right.get('lastAccessedTime')) {
-    return 1
-  }
-  if (left.get('lastAccessedTime') > right.get('lastAccessedTime')) {
-    return -1
-  }
-  return 0
-}
-const removeDuplicateDomains = (list) => {
-  const siteDomains = new Set()
-  return list.filter((site) => {
-    if (!site.get('location')) {
-      return false
-    }
-    try {
-      const hostname = require('../urlParse')(site.get('location')).hostname
-      if (!siteDomains.has(hostname)) {
-        siteDomains.add(hostname)
-        return true
-      }
-    } catch (e) {
-      console.log('Error parsing hostname: ', e)
-    }
-    return false
-  })
-}
 /**
  * topSites are defined by users. Pinned sites are attached to their positions
  * in the grid, and the non pinned indexes are populated with newly accessed sites
  */
-const getTopSites = (state) => {
-  // remove folders; sort by visit count; enforce a max limit
-  const sites = (state.get('sites') ? state.get('sites').toList() : new Immutable.List())
-    .filter((site) => !siteUtil.isFolder(site))
-    .filter((site) => !siteUtil.isImportedBookmark(site))
-    .filter((site) => !isSourceAboutUrl(site.get('location')))
-    .sort(sortCountDescending)
-    .slice(0, aboutNewTabMaxEntries)
-
-  // Filter out pinned and ignored sites
-  let unpinnedSites = sites.filter((site) => !(isPinned(state, site) || isIgnored(state, site)))
-  unpinnedSites = removeDuplicateDomains(unpinnedSites)
-
-  // Merge the pinned and unpinned lists together
-  // Pinned items have priority because the position is important
-  let gridSites = pinnedTopSites(state).map((pinnedSite) => {
-    // Fetch latest siteDetail objects from appState.sites using location/partition
-    if (pinnedSite) {
-      const matches = sites.filter((site) => compareSites(site, pinnedSite))
-      if (matches.size > 0) return matches.first()
-    }
-    // Default to unpinned items
-    const firstSite = unpinnedSites.first()
-    unpinnedSites = unpinnedSites.shift()
-    return firstSite
-  })
-
-  // Include up to [aboutNewTabMaxEntries] entries so that folks
-  // can ignore sites and have new items fill those empty spaces
-  if (unpinnedSites.size > 0) {
-    gridSites = gridSites.concat(unpinnedSites)
-  }
-
-  return gridSites.filter((site) => site != null)
-}
 
 const aboutNewTabState = {
-  maxSites: aboutNewTabMaxEntries,
-
   getSites: (state) => {
     return state.getIn(['about', 'newtab', 'sites'])
   },
@@ -117,11 +24,12 @@ const aboutNewTabState = {
     return state.setIn(['about', 'newtab', 'updatedStamp'], new Date().getTime())
   },
 
-  setSites: (state) => {
-    state = makeImmutable(state)
-
-    // return a filtered version of the sites array
-    state = state.setIn(['about', 'newtab', 'sites'], getTopSites(state))
+  setSites: (state, topSites) => {
+    if (!topSites) {
+      return state
+    }
+    topSites = makeImmutable(topSites)
+    state = state.setIn(['about', 'newtab', 'sites'], topSites)
     return state.setIn(['about', 'newtab', 'updatedStamp'], new Date().getTime())
   }
 }

app/renderer/components/frame/frame.js

@@ -496,7 +496,11 @@ class Frame extends React.Component {
       if (this.frame.isEmpty()) {
         return
       }
-      if (e.favicons && e.favicons.length > 0) {
+      if (e.favicons &&
+          e.favicons.length > 0 &&
+          // Favicon changes lead to recalculation of top site data so only fire
+          // this when needed.  Some sites update favicons very frequently.
+          e.favicons[0] !== this.frame.get('icon')) {
         imageUtil.getWorkingImageUrl(e.favicons[0], (imageFound) => {
           windowActions.setFavicon(this.frame, imageFound ? e.favicons[0] : null)
         })
@@ -599,7 +603,7 @@ class Frame extends React.Component {
       }
     }
 
-    const loadEnd = (savePage, url) => {
+    const loadEnd = (savePage, url, inPageNav) => {
       if (this.frame.isEmpty()) {
         return
       }
@@ -614,7 +618,7 @@ class Frame extends React.Component {
 
       const protocol = parsedUrl.protocol
       const isError = this.props.aboutDetailsErrorCode
-      if (!this.props.isPrivate && (protocol === 'http:' || protocol === 'https:') && !isError && savePage) {
+      if (!this.props.isPrivate && (protocol === 'http:' || protocol === 'https:') && !isError && savePage && !inPageNav) {
         // Register the site for recent history for navigation bar
         // calling with setTimeout is an ugly hack for a race condition
         // with setTitle. We either need to delay this call until the title is
@@ -732,18 +736,18 @@ class Frame extends React.Component {
     }, { passive: true })
     this.webview.addEventListener('did-fail-provisional-load', (e) => {
       if (e.isMainFrame) {
-        loadEnd(false, e.validatedURL)
+        loadEnd(false, e.validatedURL, false)
         loadFail(e, true, e.currentURL)
       }
     })
     this.webview.addEventListener('did-fail-load', (e) => {
       if (e.isMainFrame) {
-        loadEnd(false, e.validatedURL)
+        loadEnd(false, e.validatedURL, false)
         loadFail(e, false, e.validatedURL)
       }
     })
     this.webview.addEventListener('did-finish-load', (e) => {
-      loadEnd(true, e.validatedURL)
+      loadEnd(true, e.validatedURL, false)
       if (this.props.runInsecureContent) {
         appActions.removeSiteSetting(this.props.origin, 'runInsecureContent', this.props.isPrivate)
       }
@@ -754,7 +758,7 @@ class Frame extends React.Component {
       }
       if (e.isMainFrame) {
         windowActions.setNavigated(e.url, this.props.frameKey, true, this.props.tabId)
-        loadEnd(true, e.url)
+        loadEnd(true, e.url, true)
       }
     })
     this.webview.addEventListener('enter-html-full-screen', () => {

js/actions/appActions.js

@@ -124,6 +124,13 @@ const appActions = {
     })
   },
 
+  topSiteDataAvailable: function (topSites) {
+    dispatch({
+      actionType: appConstants.APP_TOP_SITE_DATA_AVAILABLE,
+      topSites
+    })
+  },
+
   /**
    * A request for a URL load
    * @param {number} tabId - the tab ID to load the URL inside of

js/constants/appConstants.js

@@ -1,6 +1,5 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this file,
- * You can obtain one at http://mozilla.org/MPL/2.0/. */
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 const mapValuesByKeys = require('../lib/functional').mapValuesByKeys
 
@@ -107,6 +106,7 @@ const appConstants = {
   APP_TAB_MESSAGE_BOX_UPDATED: _,
   APP_NAVIGATOR_HANDLER_REGISTERED: _,
   APP_NAVIGATOR_HANDLER_UNREGISTERED: _,
+  APP_TOP_SITE_DATA_AVAILABLE: _,
   APP_URL_BAR_TEXT_CHANGED: _,
   APP_URL_BAR_SUGGESTIONS_CHANGED: _,
   APP_SEARCH_SUGGESTION_RESULTS_AVAILABLE: _,