Change URL suggestions to consider last access timestamp and access c…

…ount

      * Re-order results to show history items before bookmark items
      * Record number of times site is visited
      * Re-order history items based on number of times visited and last access timestamp
      * Update and create tests

    Fixes: #3049

    Auditors: @bbondy

    Test plan:

    A new attribute, count, is added to history items in the sites section of save-session-1. This attribute records the number of times a site has been accessed. The test plan is broken in two components, one to verify the count attribute is incremented on access, the second to verify that the count and last access time are used when sorting history suggestions.

    Scenario 1

    Part A

      1. Clear save-session-1
      2. Access a site
      3. Close browser
      4. Verify that the count attribute for the saved site in save-session-1 has a value of 1

    Part B

      1. Open browser
      2. Navigate to site from Part A
      3. Close browser
      4. Verify that the count attribute for the saved site in save-session-1 has an incremented value

    Scenario 2

      1. Clear save-session-1
      2. Open browser
      3. Visit gm.com and gm.ca
      4. Close browser
      5. Verify that the count attributes of the two new sites are identical (1)
      6. Open browser to a new tab
      7. Enter ‘gm’ into the url bar
      8. Verify that the most recently visited of the two gm sites is the first entry in the history auto-suggest list

app/renderer/lib/suggestion.js

@@ -0,0 +1,84 @@
+/* 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/. */
+
+const appConfig = require('../../../js/constants/appConfig')
+
+const sigmoid = (t) => {
+  return 1 / (1 + Math.pow(Math.E, -t))
+}
+
+const ONE_DAY = 1000 * 60 * 60 * 24
+
+/*
+ * Calculate the sorting priority for a history item based on number of
+ * accesses and time since last access
+ *
+ * @param {number} count - The number of times this site has been accessed
+ * @param {number} currentTime - Current epoch millisecnds
+ * @param {boolean} lastAccessedTime - Epoch milliseconds of last access
+ *
+ */
+module.exports.sortingPriority = (count, currentTime, lastAccessedTime, ageDecayConstant) => {
+  // number of days since last access (with fractional component)
+  const ageInDays = (currentTime - (lastAccessedTime || currentTime)) / ONE_DAY
+  // decay factor based on age
+  const ageFactor = 1 - ((sigmoid(ageInDays / ageDecayConstant) - 0.5) * 2)
+  // sorting priority
+  // console.log(count, ageInDays, ageFactor, count * ageFactor)
+  return count * ageFactor
+}
+
+/*
+ * Sort two history items by priority
+ *
+ * @param {ImmutableObject} s1 - first history item
+ * @param {ImmutableObject} s2 - second history item
+ *
+ * Return the relative order of two site entries taking into consideration
+ * the number of times the site has been accessed and the length of time
+ * since the last access.
+ *
+ * The base sort order is determined by the count attribute of the site
+ * entry. A modifier is then computed based on the length of time since
+ * the last access. A sigmoid function is used to weight more recent
+ * entries higher than entries in the past. This is not a linear function,
+ * entries in the far past with many counts will still be discounted
+ * heavily as the sigmoid modifier will cancel most of the count
+ * base parameter.
+ *
+ * Below is a sample comparison of two sites that have been accessed
+ * recently (but not at the identical time). Each site is accessed
+ * 9 times. The count is discounted by an aging factor calculated
+ * using the sigmoid decay function.
+ *
+ *   http://www.gm.ca/gm/
+ *
+ *   ageInDays 0.17171469907407408
+ *   ageFactor 0.9982828546969802
+ *   count     9
+ *   priority  0.9982828546969802
+ *
+ *   http://www.gm.com/index.html
+ *
+ *   ageInDays 0.17148791666666666
+ *   ageFactor 0.9982851225143763
+ *   count     9
+ *   priority  0.9982851225143763
+ *
+ */
+module.exports.sortByAccessCountWithAgeDecay = (s1, s2) => {
+  const s1Priority = module.exports.sortingPriority(
+    s1.get('count') || 0,
+    (new Date()).getTime(),
+    s1.get('lastAccessedTime') || (new Date()).getTime(),
+    appConfig.urlSuggestions.ageDecayConstant
+  )
+  const s2Priority = module.exports.sortingPriority(
+    s2.get('count') || 0,
+    (new Date()).getTime(),
+    s2.get('lastAccessedTime') || (new Date()).getTime(),
+    appConfig.urlSuggestions.ageDecayConstant
+  )
+  return s2Priority - s1Priority
+}

js/components/urlBarSuggestions.js

@@ -21,6 +21,7 @@ const eventUtil = require('../lib/eventUtil')
 const cx = require('../lib/classSet')
 const locale = require('../l10n')
 const windowStore = require('../stores/windowStore')
+const suggestion = require('../../app/renderer/lib/suggestion')
 
 class UrlBarSuggestions extends ImmutableComponent {
   constructor (props) {
@@ -149,8 +150,8 @@ class UrlBarSuggestions extends ImmutableComponent {
       }))
       index += suggestions.size
     }
-    addToItems(bookmarkSuggestions, 'bookmarksTitle', locale.translation('bookmarksSuggestionTitle'), 'fa-star-o')
     addToItems(historySuggestions, 'historyTitle', locale.translation('historySuggestionTitle'), 'fa-clock-o')
+    addToItems(bookmarkSuggestions, 'bookmarksTitle', locale.translation('bookmarksSuggestionTitle'), 'fa-star-o')
     addToItems(aboutPagesSuggestions, 'aboutPagesTitle', locale.translation('aboutPagesSuggestionTitle'), null)
     addToItems(tabSuggestions, 'tabsTitle', locale.translation('tabsSuggestionTitle'), 'fa-external-link')
     addToItems(searchSuggestions, 'searchTitle', locale.translation('searchSuggestionTitle'), 'fa-search')
@@ -254,18 +255,19 @@ class UrlBarSuggestions extends ImmutableComponent {
         if (pos1 - pos2 !== 0) {
           return pos1 - pos2
         } else {
-          // If there's a tie on the match location, use the shorter URL
-          return s1.get('location').length - s2.get('location').length
+          // If there's a tie on the match location, use the age
+          // decay modified access count
+          return suggestion.sortByAccessCountWithAgeDecay(s1, s2)
         }
       }
     }
 
-    // bookmarks
-    if (getSetting(settings.BOOKMARK_SUGGESTIONS)) {
+    // history
+    if (getSetting(settings.HISTORY_SUGGESTIONS)) {
       suggestions = suggestions.concat(mapListToElements({
         data: props.sites,
-        maxResults: config.urlBarSuggestions.maxBookmarkSites,
-        type: suggestionTypes.BOOKMARK,
+        maxResults: config.urlBarSuggestions.maxHistorySites,
+        type: suggestionTypes.HISTORY,
         clickHandler: navigateClickHandler((site) => {
           return site.get('location')
         }),
@@ -277,17 +279,17 @@ class UrlBarSuggestions extends ImmutableComponent {
           const location = site.get('location') || ''
           return (title.toLowerCase().includes(urlLocationLower) ||
             location.toLowerCase().includes(urlLocationLower)) &&
-            site.get('tags') && site.get('tags').includes(siteTags.BOOKMARK)
+            (!site.get('tags') || site.get('tags').size === 0)
         }
       }))
     }
 
-    // history
-    if (getSetting(settings.HISTORY_SUGGESTIONS)) {
+    // bookmarks
+    if (getSetting(settings.BOOKMARK_SUGGESTIONS)) {
       suggestions = suggestions.concat(mapListToElements({
         data: props.sites,
-        maxResults: config.urlBarSuggestions.maxHistorySites,
-        type: suggestionTypes.HISTORY,
+        maxResults: config.urlBarSuggestions.maxBookmarkSites,
+        type: suggestionTypes.BOOKMARK,
         clickHandler: navigateClickHandler((site) => {
           return site.get('location')
         }),
@@ -299,7 +301,7 @@ class UrlBarSuggestions extends ImmutableComponent {
           const location = site.get('location') || ''
           return (title.toLowerCase().includes(urlLocationLower) ||
             location.toLowerCase().includes(urlLocationLower)) &&
-            (!site.get('tags') || site.get('tags').size === 0)
+            site.get('tags') && site.get('tags').includes(siteTags.BOOKMARK)
         }
       }))
     }

js/constants/appConfig.js

@@ -85,6 +85,9 @@ module.exports = {
     baseUrl: `${updateHost}/1/releases`,
     winBaseUrl: `${winUpdateHost}/multi-channel/releases/CHANNEL/`
   },
+  urlSuggestions: {
+    ageDecayConstant: 50
+  },
   defaultSettings: {
     'adblock.customRules': '',
     'general.language': null, // null means to use the OS lang

js/state/siteUtil.js

@@ -124,6 +124,10 @@ const mergeSiteDetails = (oldSiteDetail, newSiteDetail, tag, folderId) => {
   if (newSiteDetail.get('themeColor') || oldSiteDetail && oldSiteDetail.get('themeColor')) {
     site = site.set('themeColor', newSiteDetail.get('themeColor') || oldSiteDetail.get('themeColor'))
   }
+  if (site.get('tags').size === 0) {
+    // Increment the visit count for history items
+    site = site.set('count', ((oldSiteDetail || site).get('count') || 0) + 1)
+  }
 
   return site
 }

test/unit/lib/urlSuggestionTest.js

@@ -0,0 +1,15 @@
+/* global describe, it */
+const suggestion = require('../../../app/renderer/lib/suggestion')
+const assert = require('assert')
+
+require('../braveUnit')
+
+const AGE_DECAY = 50
+
+describe('suggestion', function () {
+  it('sorts sites correctly', function () {
+    assert.ok(suggestion.sortingPriority(10, 100, 50, AGE_DECAY) > suggestion.sortingPriority(10, 100, 40, AGE_DECAY), 'newer sites with equal access counts sort earlier')
+    assert.ok(suggestion.sortingPriority(10, 100, 50, AGE_DECAY) < suggestion.sortingPriority(11, 100, 40, AGE_DECAY), 'Sites with higher access counts sort earlier (unless time delay overriden)')
+    assert.ok(suggestion.sortingPriority(10, 10000000000, 10000000000, AGE_DECAY) > suggestion.sortingPriority(11, 10000000000, 1000000000, AGE_DECAY), 'much newer sites without lower counts sort with higher priority')
+  })
+})

test/unit/state/siteUtilTest.js

@@ -27,6 +27,10 @@ describe('siteUtil', function () {
     parentFolderId: 0,
     tags: [siteTags.BOOKMARK_FOLDER]
   })
+  const siteMinFields = Immutable.fromJS({
+    location: testUrl1,
+    title: 'sample'
+  })
 
   describe('getSiteIndex', function () {
     it('returns -1 if sites is falsey', function () {
@@ -178,7 +182,17 @@ describe('siteUtil', function () {
       const expectedSites = Immutable.fromJS([bookmarkAllFields])
       assert.deepEqual(processedSites.getIn([0, 'tags']), expectedSites.getIn([0, 'tags']))
     })
-
+    describe('record count', function () {
+      var processedSites
+      it('create history record with count', function () {
+        processedSites = siteUtil.addSite(emptySites, siteMinFields)
+        assert.deepEqual(processedSites.getIn([0, 'count']), 1)
+      })
+      it('increments count for history item', function () {
+        processedSites = siteUtil.addSite(processedSites, siteMinFields)
+        assert.deepEqual(processedSites.getIn([0, 'count']), 2)
+      })
+    })
     describe('for new entries (oldSite is null)', function () {
       describe('when adding bookmark', function () {
         it('preserves existing siteDetail fields', function () {
@@ -192,7 +206,6 @@ describe('siteUtil', function () {
           assert.deepEqual(processedSites.getIn([0, 'tags']).toJS(), [siteTags.BOOKMARK])
         })
       })
-
       describe('when adding bookmark folder', function () {
         it('assigns a folderId', function () {
           const processedSites = siteUtil.addSite(emptySites, folderMinFields)