Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Commit

Permalink
Merge pull request #10005 from brave/top-sites
Browse files Browse the repository at this point in the history
Optimize getTopSites
  • Loading branch information
bsclifton authored and bbondy committed Jul 17, 2017
1 parent f198f1a commit c58a310
Show file tree
Hide file tree
Showing 10 changed files with 456 additions and 203 deletions.
151 changes: 151 additions & 0 deletions app/browser/api/topSites.js
Original file line number Diff line number Diff line change
@@ -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
}
19 changes: 19 additions & 0 deletions app/browser/reducers/topSitesReducer.js
Original file line number Diff line number Diff line change
@@ -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
104 changes: 6 additions & 98 deletions app/common/state/aboutNewTabState.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'])
},
Expand All @@ -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())
}
}
Expand Down
18 changes: 11 additions & 7 deletions app/renderer/components/frame/frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,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)
})
Expand Down Expand Up @@ -636,7 +640,7 @@ class Frame extends React.Component {
}
}

const loadEnd = (savePage, url) => {
const loadEnd = (savePage, url, inPageNav) => {
if (this.frame.isEmpty()) {
return
}
Expand All @@ -651,7 +655,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
Expand Down Expand Up @@ -773,18 +777,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.runInsecureContent()) {
appActions.removeSiteSetting(this.origin, 'runInsecureContent', this.props.isPrivate)
}
Expand All @@ -795,7 +799,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', () => {
Expand Down
7 changes: 7 additions & 0 deletions js/actions/appActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions js/constants/appConstants.js
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -109,6 +108,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: _,
Expand Down
Loading

0 comments on commit c58a310

Please sign in to comment.