From 3dc8a03d7271c659139c157bb658795dd21298f9 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sun, 11 Nov 2012 13:22:18 +0000 Subject: Remove history entries. When a chrome history entry is removed, remove that entry from our history too. Same when the entire history is removed. --- background_scripts/completion.coffee | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'background_scripts') diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee index 25f76c1e..8ab71290 100644 --- a/background_scripts/completion.coffee +++ b/background_scripts/completion.coffee @@ -365,6 +365,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 +384,18 @@ 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.map (url) => + i = HistoryCache.binarySearch({url:url}, @history, @compareHistoryByUrl) + # TODO (smblott) + # The `i < @history.length` condition below should not be necessary. It can be removed when `binarySearch` is fixed. + 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. -- cgit v1.2.3 From 03823f97a526ed5e5104b724d4f2a0b582505a6c Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sun, 11 Nov 2012 14:10:58 +0000 Subject: Extend removing entries to the domain completer. --- background_scripts/completion.coffee | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) (limited to 'background_scripts') diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee index 8ab71290..91f79338 100644 --- a/background_scripts/completion.coffee +++ b/background_scripts/completion.coffee @@ -169,7 +169,7 @@ 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 + domains: null # A map of domain -> { entry: , referenceCount: } filter: (queryTerms, onComplete) -> return onComplete([]) if queryTerms.length > 1 @@ -190,7 +190,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]) @@ -205,13 +205,37 @@ class DomainCompleter domain = @parseDomain(entry.url) if domain previousEntry = @domains[domain] - @domains[domain] = entry if !previousEntry || (previousEntry.lastVisitTime < entry.lastVisitTime) + if previousEntry + previousEntry.entry = entry if previousEntry.lastVisitTime < entry.lastVisitTime + previousEntry.referenceCount +=1 + else + @domains[domain] = { entry: entry, referenceCount: 1 } 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 + previousEntry = @domains[domain] + if previousEntry + previousEntry.entry = newPage + previousEntry.referenceCount += 1 + else + @domains[domain] = { entry: newPage, referenceCount: 1 } + + onVisitRemoved: (toRemove) -> + if toRemove.allHistory + @domains = {} + else + toRemove.urls.forEach (url) => + domain = @parseDomain(url) + if domain + previousEntry = @domains[domain] + if previousEntry + previousEntry.referenceCount -= 1 + if previousEntry.referenceCount == 0 + delete @domains[domain] parseDomain: (url) -> url.split("/")[2] || "" @@ -389,7 +413,7 @@ HistoryCache = if toRemove.allHistory @history = [] else - toRemove.urls.map (url) => + toRemove.urls.forEach (url) => i = HistoryCache.binarySearch({url:url}, @history, @compareHistoryByUrl) # TODO (smblott) # The `i < @history.length` condition below should not be necessary. It can be removed when `binarySearch` is fixed. -- cgit v1.2.3 From 2071873f5c9f496e4fde94d44cbf8b0e50bf4c21 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sun, 11 Nov 2012 15:59:27 +0000 Subject: Remove comments regarding non-existent bug. --- background_scripts/completion.coffee | 2 -- 1 file changed, 2 deletions(-) (limited to 'background_scripts') diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee index 91f79338..c12c6d80 100644 --- a/background_scripts/completion.coffee +++ b/background_scripts/completion.coffee @@ -415,8 +415,6 @@ HistoryCache = else toRemove.urls.forEach (url) => i = HistoryCache.binarySearch({url:url}, @history, @compareHistoryByUrl) - # TODO (smblott) - # The `i < @history.length` condition below should not be necessary. It can be removed when `binarySearch` is fixed. if i < @history.length and @history[i].url == url @history.splice(i, 1) -- cgit v1.2.3 From 0a8443d30dde0b7ba914a742182a1900baae2836 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sun, 11 Nov 2012 16:31:17 +0000 Subject: Refactor domain maintenance code. --- background_scripts/completion.coffee | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) (limited to 'background_scripts') diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee index c12c6d80..95c50baf 100644 --- a/background_scripts/completion.coffee +++ b/background_scripts/completion.coffee @@ -200,29 +200,19 @@ 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] - if previousEntry - previousEntry.entry = entry if previousEntry.lastVisitTime < entry.lastVisitTime - previousEntry.referenceCount +=1 - else - @domains[domain] = { entry: entry, referenceCount: 1 } + history.forEach (entry) => @onPageVisited entry chrome.history.onVisited.addListener(@onPageVisited.bind(this)) chrome.history.onVisitRemoved.addListener(@onVisitRemoved.bind(this)) onComplete() + # We want each key in our domains hash to point to the most recent History entry for that domain. onPageVisited: (newPage) -> domain = @parseDomain(newPage.url) if domain - previousEntry = @domains[domain] - if previousEntry - previousEntry.entry = newPage - previousEntry.referenceCount += 1 - else - @domains[domain] = { entry: newPage, referenceCount: 1 } + @domains[domain] ||= { entry: newPage, referenceCount: 0 } + slot = @domains[domain] + slot.entry = newPage if slot.entry.lastVisitTime < newPage.lastVisitTime + slot.referenceCount += 1 onVisitRemoved: (toRemove) -> if toRemove.allHistory -- cgit v1.2.3 From 9215d975207a3eb07e01cbc202fa2ee3402d332c Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sun, 11 Nov 2012 16:52:36 +0000 Subject: More simplification/refactoring. --- background_scripts/completion.coffee | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) (limited to 'background_scripts') diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee index 95c50baf..08c33430 100644 --- a/background_scripts/completion.coffee +++ b/background_scripts/completion.coffee @@ -205,12 +205,12 @@ class DomainCompleter chrome.history.onVisitRemoved.addListener(@onVisitRemoved.bind(this)) onComplete() - # We want each key in our domains hash to point to the most recent History entry for that domain. onPageVisited: (newPage) -> domain = @parseDomain(newPage.url) if domain @domains[domain] ||= { entry: newPage, referenceCount: 0 } slot = @domains[domain] + # 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 @@ -220,12 +220,8 @@ class DomainCompleter else toRemove.urls.forEach (url) => domain = @parseDomain(url) - if domain - previousEntry = @domains[domain] - if previousEntry - previousEntry.referenceCount -= 1 - if previousEntry.referenceCount == 0 - delete @domains[domain] + if domain and @domains[domain] and ( @domains[domain].referenceCount -= 1 ) == 0 + delete @domains[domain] parseDomain: (url) -> url.split("/")[2] || "" -- cgit v1.2.3 From c44f0217fbfc4df1ac6bffea4a07abe3930e634e Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sun, 11 Nov 2012 22:15:10 +0000 Subject: Changes in responce to philc's recommendations. See: https://github.com/philc/vimium/pull/715 --- background_scripts/completion.coffee | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'background_scripts') diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee index 08c33430..13e700d8 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 -> { entry: , referenceCount: } + # A map of domain -> { entry: , referenceCount: } + # - `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 -- cgit v1.2.3 From f0347388264b0fb437489f4f021ba8c0dbcca8e3 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Mon, 12 Nov 2012 06:43:11 +0000 Subject: Improve unit test structure and coverage. --- background_scripts/completion.coffee | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'background_scripts') diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee index 13e700d8..28bb1afa 100644 --- a/background_scripts/completion.coffee +++ b/background_scripts/completion.coffee @@ -212,8 +212,7 @@ class DomainCompleter onPageVisited: (newPage) -> domain = @parseDomain(newPage.url) if domain - @domains[domain] ||= { entry: newPage, referenceCount: 0 } - slot = @domains[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 -- cgit v1.2.3