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

Commit

Permalink
Change sites from List to Map
Browse files Browse the repository at this point in the history
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 {}
  • Loading branch information
darkdh committed Jan 15, 2017
1 parent 548e28a commit ba6cfde
Show file tree
Hide file tree
Showing 15 changed files with 529 additions and 525 deletions.
2 changes: 1 addition & 1 deletion app/common/lib/historyUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion app/common/state/aboutNewTabState.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion app/importer.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const locale = require('./locale')
var isMergeFavorites = false
var isImportingBookmarks = false
var hasBookmarks
var importedSites

exports.init = () => {
importer.initialize()
Expand Down Expand Up @@ -171,6 +172,7 @@ importer.on('add-bookmarks', (e, bookmarks, topLevelFolder) => {
sites.push(site)
}
}
importedSites = Immutable.fromJS(sites)
appActions.addSite(Immutable.fromJS(sites))
})

Expand All @@ -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) ||
Expand Down
13 changes: 7 additions & 6 deletions app/renderer/components/bookmarksToolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -319,27 +319,28 @@ 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)
this.bookmarksForToolbar = noParentItems.take(i).sort(siteUtil.siteSort)
// Show at most 100 items in the overflow menu
this.overflowBookmarkItems = noParentItems.skip(i).take(100)
this.overflowBookmarkItems = noParentItems.skip(i).take(100).sort(siteUtil.siteSort)
}
componentWillMount () {
this.updateBookmarkData(this.props)
Expand Down
28 changes: 27 additions & 1 deletion app/sessionStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,20 @@ 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 = {}
let order = 0
topSites.forEach((site) => {
let key = siteUtil.getSiteKey(Immutable.fromJS(site))
site.order = order++
siteMap[key] = site
})
return siteMap
}
return {}
}

const getStoragePath = () => {
return path.join(app.getPath('userData'), sessionStorageName)
}
Expand Down Expand Up @@ -474,6 +488,18 @@ module.exports.loadAppState = () => {
}
}
data = setVersionInformation(data)

// sites refactoring migration
if (Array.isArray(data.sites) && data.sites.length) {
let sites = {}
let order = 0
data.sites.forEach((site) => {
let key = siteUtil.getSiteKey(Immutable.fromJS(site))
site.order = order++
sites[key] = site
})
data.sites = sites
}
} catch (e) {
// TODO: Session state is corrupted, maybe we should backup this
// corrupted value for people to report into support.
Expand All @@ -496,7 +522,7 @@ module.exports.loadAppState = () => {
module.exports.defaultAppState = () => {
return {
firstRunTimestamp: new Date().getTime(),
sites: topSites,
sites: getTopSiteMap(),
tabs: [],
windows: [],
extensions: {},
Expand Down
28 changes: 15 additions & 13 deletions docs/state.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,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()
Expand Down
2 changes: 1 addition & 1 deletion js/components/frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class Frame extends ImmutableComponent {
this.webview.send(messages.BRAVERY_DEFAULTS_UPDATED, this.braveryDefaults)
} else if (location === 'about:bookmarks') {
this.webview.send(messages.BOOKMARKS_UPDATED, {
bookmarks: this.props.bookmarks.toJS(),
bookmarks: this.props.bookmarks.toList().sort(siteUtil.siteSort).toJS(),
bookmarkFolders: this.props.bookmarkFolders.toJS()
})
} else if (location === 'about:history') {
Expand Down
6 changes: 3 additions & 3 deletions js/components/navigationBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ class NavigationBar extends ImmutableComponent {
const editing = this.bookmarked
// show the AddEditBookmarkHanger control; saving/deleting takes place there
let siteDetail = siteUtil.getDetailFromFrame(this.activeFrame, siteTags.BOOKMARK)
const siteIndex = siteUtil.getSiteIndex(this.props.sites, siteDetail)
const key = siteUtil.getSiteKey(siteDetail)

if (siteIndex > 0) {
siteDetail = siteDetail.set('parentFolderId', this.props.sites.getIn([siteIndex]).get('parentFolderId'))
if (key !== null) {
siteDetail = siteDetail.set('parentFolderId', this.props.sites.getIn([key]).get('parentFolderId'))
}
windowActions.setBookmarkDetail(siteDetail, siteDetail, null, editing, true)
}
Expand Down
3 changes: 2 additions & 1 deletion js/contextMenus.js
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,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')) {
Expand Down
Loading

0 comments on commit ba6cfde

Please sign in to comment.