diff options
| -rw-r--r-- | background_scripts/completion.coffee | 42 | ||||
| -rw-r--r-- | tests/unit_tests/completion_test.coffee | 80 |
2 files changed, 111 insertions, 11 deletions
diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee index 25f76c1e..28bb1afa 100644 --- a/background_scripts/completion.coffee +++ b/background_scripts/completion.coffee @@ -169,7 +169,11 @@ class HistoryCompleter # The domain completer is designed to match a single-word query which looks like it is a domain. This supports # the user experience where they quickly type a partial domain, hit tab -> enter, and expect to arrive there. class DomainCompleter - domains: null # A map of domain -> history + # A map of domain -> { entry: <historyEntry>, referenceCount: <count> } + # - `entry` is the most recently accessed page in the History within this domain. + # - `referenceCount` is a count of the number of History entries within this domain. + # If `referenceCount` goes to zero, the domain entry can and should be deleted. + domains: null filter: (queryTerms, onComplete) -> return onComplete([]) if queryTerms.length > 1 @@ -190,7 +194,7 @@ class DomainCompleter sortDomainsByRelevancy: (queryTerms, domainCandidates) -> results = [] for domain in domainCandidates - recencyScore = RankingUtils.recencyScore(@domains[domain].lastVisitTime || 0) + recencyScore = RankingUtils.recencyScore(@domains[domain].entry.lastVisitTime || 0) wordRelevancy = RankingUtils.wordRelevancy(queryTerms, domain, null) score = (wordRelevancy + Math.max(recencyScore, wordRelevancy)) / 2 results.push([domain, score]) @@ -200,18 +204,27 @@ class DomainCompleter populateDomains: (onComplete) -> HistoryCache.use (history) => @domains = {} - history.forEach (entry) => - # We want each key in our domains hash to point to the most recent History entry for that domain. - domain = @parseDomain(entry.url) - if domain - previousEntry = @domains[domain] - @domains[domain] = entry if !previousEntry || (previousEntry.lastVisitTime < entry.lastVisitTime) + history.forEach (entry) => @onPageVisited entry chrome.history.onVisited.addListener(@onPageVisited.bind(this)) + chrome.history.onVisitRemoved.addListener(@onVisitRemoved.bind(this)) onComplete() onPageVisited: (newPage) -> domain = @parseDomain(newPage.url) - @domains[domain] = newPage if domain + if domain + slot = @domains[domain] ||= { entry: newPage, referenceCount: 0 } + # We want each entry in our domains hash to point to the most recent History entry for that domain. + slot.entry = newPage if slot.entry.lastVisitTime < newPage.lastVisitTime + slot.referenceCount += 1 + + onVisitRemoved: (toRemove) -> + if toRemove.allHistory + @domains = {} + else + toRemove.urls.forEach (url) => + domain = @parseDomain(url) + if domain and @domains[domain] and ( @domains[domain].referenceCount -= 1 ) == 0 + delete @domains[domain] parseDomain: (url) -> url.split("/")[2] || "" @@ -365,6 +378,7 @@ HistoryCache = history.sort @compareHistoryByUrl @history = history chrome.history.onVisited.addListener(@onPageVisited.bind(this)) + chrome.history.onVisitRemoved.addListener(@onVisitRemoved.bind(this)) callback(@history) for callback in @callbacks @callbacks = null @@ -383,6 +397,16 @@ HistoryCache = else @history.splice(i, 0, newPage) + # When a page is removed from the chrome history, remove it from the vimium history too. + onVisitRemoved: (toRemove) -> + if toRemove.allHistory + @history = [] + else + toRemove.urls.forEach (url) => + i = HistoryCache.binarySearch({url:url}, @history, @compareHistoryByUrl) + if i < @history.length and @history[i].url == url + @history.splice(i, 1) + # Returns the matching index or the closest matching index if the element is not found. That means you # must check the element at the returned index to know whether the element was actually found. # This method is used for quickly searching through our history cache. diff --git a/tests/unit_tests/completion_test.coffee b/tests/unit_tests/completion_test.coffee index 3de1d716..08ce46c2 100644 --- a/tests/unit_tests/completion_test.coffee +++ b/tests/unit_tests/completion_test.coffee @@ -41,6 +41,9 @@ context "HistoryCache", should "return length - 1 if it should be at the end of the list", -> assert.equal 0, HistoryCache.binarySearch(3, [3, 5, 8], @compare) + should "return one passed end of array (so: array.length) if greater than last element in array", -> + assert.equal 3, HistoryCache.binarySearch(10, [3, 5, 8], @compare) + should "found return the position if it's between two elements", -> assert.equal 1, HistoryCache.binarySearch(4, [3, 5, 8], @compare) assert.equal 2, HistoryCache.binarySearch(7, [3, 5, 8], @compare) @@ -51,9 +54,11 @@ context "HistoryCache", @history2 = { url: "a.com", lastVisitTime: 10 } history = [@history1, @history2] @onVisitedListener = null + @onVisitRemovedListener = null global.chrome.history = search: (options, callback) -> callback(history) onVisited: { addListener: (@onVisitedListener) => } + onVisitRemoved: { addListener: (@onVisitRemovedListener) => } HistoryCache.reset() should "store visits sorted by url ascending", -> @@ -75,6 +80,30 @@ context "HistoryCache", HistoryCache.use (@results) => assert.arrayEqual [newSite, @history1], @results + should "(not) remove page from the history, when page is not in history (it should be a no-op)", -> + HistoryCache.use (@results) => + assert.arrayEqual [@history2, @history1], @results + toRemove = { urls: [ "x.com" ], allHistory: false } + @onVisitRemovedListener(toRemove) + HistoryCache.use (@results) => + assert.arrayEqual [@history2, @history1], @results + + should "remove pages from the history", -> + HistoryCache.use (@results) => + assert.arrayEqual [@history2, @history1], @results + toRemove = { urls: [ "a.com" ], allHistory: false } + @onVisitRemovedListener(toRemove) + HistoryCache.use (@results) => + assert.arrayEqual [@history1], @results + + should "remove all pages from the history", -> + HistoryCache.use (@results) => + assert.arrayEqual [@history2, @history1], @results + toRemove = { allHistory: true } + @onVisitRemovedListener(toRemove) + HistoryCache.use (@results) => + assert.arrayEqual [], @results + context "history completer", setup -> @history1 = { title: "history1", url: "history1.com", lastVisitTime: hours(1) } @@ -83,6 +112,7 @@ context "history completer", global.chrome.history = search: (options, callback) => callback([@history1, @history2]) onVisited: { addListener: -> } + onVisitRemoved: { addListener: -> } @completer = new HistoryCompleter() @@ -102,7 +132,9 @@ context "domain completer", @history2 = { title: "history2", url: "http://history2.com", lastVisitTime: hours(1) } stub(HistoryCache, "use", (onComplete) => onComplete([@history1, @history2])) - global.chrome.history = { onVisited: { addListener: -> } } + global.chrome.history = + onVisited: { addListener: -> } + onVisitRemoved: { addListener: -> } stub(Date, "now", returns(hours(24))) @completer = new DomainCompleter() @@ -112,7 +144,7 @@ context "domain completer", assert.arrayEqual ["history1.com"], results.map (result) -> result.url should "pick domains which are more recent", -> - # This domains are the same except for their last visited time. + # These domains are the same except for their last visited time. assert.equal "history1.com", filterCompleter(@completer, ["story"])[0].url @history2.lastVisitTime = hours(3) assert.equal "history2.com", filterCompleter(@completer, ["story"])[0].url @@ -120,6 +152,50 @@ context "domain completer", should "returns no results when there's more than one query term, because clearly it's not a domain", -> assert.arrayEqual [], filterCompleter(@completer, ["his", "tory"]) +context "domain completer (removing entries)", + setup -> + @history1 = { title: "history1", url: "http://history1.com", lastVisitTime: hours(2) } + @history2 = { title: "history2", url: "http://history2.com", lastVisitTime: hours(1) } + @history3 = { title: "history2something", url: "http://history2.com/something", lastVisitTime: hours(0) } + + stub(HistoryCache, "use", (onComplete) => onComplete([@history1, @history2, @history3])) + @onVisitedListener = null + @onVisitRemovedListener = null + global.chrome.history = + onVisited: { addListener: (@onVisitedListener) => } + onVisitRemoved: { addListener: (@onVisitRemovedListener) => } + stub(Date, "now", returns(hours(24))) + + @completer = new DomainCompleter() + # Force installation of listeners. + filterCompleter(@completer, ["story"]) + + should "remove 1 entry for domain with reference count of 1", -> + @onVisitRemovedListener { allHistory: false, urls: [@history1.url] } + assert.equal "history2.com", filterCompleter(@completer, ["story"])[0].url + assert.equal 0, filterCompleter(@completer, ["story1"]).length + + should "remove 2 entries for domain with reference count of 2", -> + @onVisitRemovedListener { allHistory: false, urls: [@history2.url] } + assert.equal "history2.com", filterCompleter(@completer, ["story2"])[0].url + @onVisitRemovedListener { allHistory: false, urls: [@history3.url] } + assert.equal 0, filterCompleter(@completer, ["story2"]).length + assert.equal "history1.com", filterCompleter(@completer, ["story"])[0].url + + should "remove 3 (all) matching domain entries", -> + @onVisitRemovedListener { allHistory: false, urls: [@history2.url] } + @onVisitRemovedListener { allHistory: false, urls: [@history1.url] } + @onVisitRemovedListener { allHistory: false, urls: [@history3.url] } + assert.equal 0, filterCompleter(@completer, ["story"]).length + + should "remove 3 (all) matching domain entries, and do it all at once", -> + @onVisitRemovedListener { allHistory: false, urls: [ @history2.url, @history1.url, @history3.url ] } + assert.equal 0, filterCompleter(@completer, ["story"]).length + + should "remove *all* domain entries", -> + @onVisitRemovedListener { allHistory: true } + assert.equal 0, filterCompleter(@completer, ["story"]).length + context "tab completer", setup -> @tabs = [ |