From b27bd390d67e11bba09282970f99738499318714 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 29 Oct 2014 13:19:58 +0000 Subject: [PATCH 01/20] Repair insert mode. I broke it here: 77e1ded091062ca2e52264d222482dcd06290a9b --- content_scripts/vimium_frontend.coffee | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index d4347f26d..9e33b9103 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -402,9 +402,9 @@ onKeydown = (event) -> # Remove focus so the user can't just get himself back into insert mode by typing in the same input # box. event.srcElement.blur() - exitInsertMode() - DomUtils.suppressEvent event - handledKeydownEvents.push event + exitInsertMode() + DomUtils.suppressEvent event + handledKeydownEvents.push event else if (findMode) if (KeyboardUtils.isEscape(event)) From a277fa6332f3aa3c0aa5f2c541f539fb1569c6b9 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 29 Oct 2014 13:48:05 +0000 Subject: [PATCH 02/20] Revert 2c7bebb5f2c873850c2b2d82013cab4eb3d4913c On reflection, this seems too dangerous: - Somebody excluded flash for a reason, and without knowing that reason, it seems dangerous. - Imagine a flash game with key bindings. Perhaps it uses ? to show the key bindings, and ESC to hide them again. With 2c7bebb5f2c873850c2b2d82013cab4eb3d4913c, you can never hide the key bindings (I think). All in all, this just seems too risky. --- content_scripts/vimium_frontend.coffee | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index 9e33b9103..4f7becbae 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -398,13 +398,15 @@ onKeydown = (event) -> keyChar = "<" + keyChar + ">" if (isInsertMode() && KeyboardUtils.isEscape(event)) - if isEditable(event.srcElement) or isEmbed(event.srcElement) + # Note that we can't programmatically blur out of Flash embeds from Javascript. + if (!isEmbed(event.srcElement)) # Remove focus so the user can't just get himself back into insert mode by typing in the same input # box. - event.srcElement.blur() - exitInsertMode() - DomUtils.suppressEvent event - handledKeydownEvents.push event + if (isEditable(event.srcElement)) + event.srcElement.blur() + exitInsertMode() + DomUtils.suppressEvent event + handledKeydownEvents.push event else if (findMode) if (KeyboardUtils.isEscape(event)) From a720ad9e28969f80c6de0c3d3ffeccca6934e440 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 29 Oct 2014 16:55:53 +0000 Subject: [PATCH 03/20] Better vomnibar favicons. --- background_scripts/completion.coffee | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee index 23696185a..4ff8c8d80 100644 --- a/background_scripts/completion.coffee +++ b/background_scripts/completion.coffee @@ -26,7 +26,7 @@ class Suggestion generateHtml: -> return @html if @html - favIconUrl = @tabFavIconUrl or "#{@getUrlRoot(@url)}/favicon.ico" + favIconUrl = @favIconUrl or Suggestion.getFavIconURL @url relevancyHtml = if @showRelevancy then "#{@computeRelevancy()}" else "" # NOTE(philc): We're using these vimium-specific class names so we don't collide with the page's CSS. @html = @@ -42,11 +42,14 @@ class Suggestion """ - # Use neat trick to snatch a domain (http://stackoverflow.com/a/8498668). - getUrlRoot: (url) -> - a = document.createElement 'a' - a.href = url - a.protocol + "//" + a.hostname + # See: https://groups.google.com/a/chromium.org/forum/#!topic/chromium-extensions/DONzstBAJeo + # Google serves up a nice little "world" icon for things which aren't really domains, such as chrome + # and extension URLs. Static method. + @getFavIconURL: (url) -> + "https://www.google.com/profiles/c/favicons?domain=#{Suggestion.parseDomain(url)}" + + # Static method. + @parseDomain: (url) -> url.split("/")[2] || "" shortenUrl: (url) -> @stripTrailingSlash(url).replace(/^https?:\/\//, "") @@ -216,7 +219,9 @@ class DomainCompleter domains = @sortDomainsByRelevancy(queryTerms, domainCandidates) return onComplete([]) if domains.length == 0 topDomain = domains[0][0] - onComplete([new Suggestion(queryTerms, "domain", topDomain, null, @computeRelevancy)]) + suggestion = new Suggestion(queryTerms, "domain", topDomain, null, @computeRelevancy) + suggestion.favIconUrl = Suggestion.getFavIconURL "http://#{topDomain}" + onComplete([suggestion]) # Returns a list of domains of the form: [ [domain, relevancy], ... ] sortDomainsByRelevancy: (queryTerms, domainCandidates) -> @@ -238,7 +243,7 @@ class DomainCompleter onComplete() onPageVisited: (newPage) -> - domain = @parseDomain(newPage.url) + domain = Suggestion.parseDomain(newPage.url) 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. @@ -250,7 +255,7 @@ class DomainCompleter @domains = {} else toRemove.urls.forEach (url) => - domain = @parseDomain(url) + domain = Suggestion.parseDomain(url) if domain and @domains[domain] and ( @domains[domain].referenceCount -= 1 ) == 0 delete @domains[domain] @@ -269,7 +274,7 @@ class TabCompleter suggestions = results.map (tab) => suggestion = new Suggestion(queryTerms, "tab", tab.url, tab.title, @computeRelevancy) suggestion.tabId = tab.id - suggestion.tabFavIconUrl = tab.favIconUrl + suggestion.favIconUrl = tab.favIconUrl suggestion onComplete(suggestions) From ca6965dd81a758595a4303848a965633d4900702 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Thu, 30 Oct 2014 10:35:31 +0000 Subject: [PATCH 04/20] Revert to guessing favicon URLs. --- background_scripts/completion.coffee | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee index 4ff8c8d80..e5317a242 100644 --- a/background_scripts/completion.coffee +++ b/background_scripts/completion.coffee @@ -42,11 +42,10 @@ class Suggestion """ - # See: https://groups.google.com/a/chromium.org/forum/#!topic/chromium-extensions/DONzstBAJeo - # Google serves up a nice little "world" icon for things which aren't really domains, such as chrome - # and extension URLs. Static method. + # Guess the favicon URL. @getFavIconURL: (url) -> - "https://www.google.com/profiles/c/favicons?domain=#{Suggestion.parseDomain(url)}" + domain = Suggestion.parseDomain(url) + "http://#{domain}/favicon.ico" # Static method. @parseDomain: (url) -> url.split("/")[2] || "" From 92e51eaf2cf4d5f3ed50587144b633500ea6eb74 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Thu, 30 Oct 2014 10:47:39 +0000 Subject: [PATCH 05/20] Move favicons to Suggestion constructor. --- background_scripts/completion.coffee | 15 +++++++-------- tests/unit_tests/completion_test.coffee | 4 ++-- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee index e5317a242..731c5aebc 100644 --- a/background_scripts/completion.coffee +++ b/background_scripts/completion.coffee @@ -19,7 +19,7 @@ class Suggestion # - computeRelevancyFunction: a function which takes a Suggestion and returns a relevancy score # between [0, 1] # - extraRelevancyData: data (like the History item itself) which may be used by the relevancy function. - constructor: (@queryTerms, @type, @url, @title, @computeRelevancyFunction, @extraRelevancyData) -> + constructor: (@queryTerms, @type, @url, @title, @computeRelevancyFunction, @extraRelevancyData, @favIconUrl) -> @title ||= "" computeRelevancy: -> @relevancy = @computeRelevancyFunction(this) @@ -42,10 +42,9 @@ class Suggestion """ - # Guess the favicon URL. + # Guess the favicon URL; static method. @getFavIconURL: (url) -> - domain = Suggestion.parseDomain(url) - "http://#{domain}/favicon.ico" + "http://#{Suggestion.parseDomain(url)}/favicon.ico" # Static method. @parseDomain: (url) -> url.split("/")[2] || "" @@ -218,8 +217,8 @@ class DomainCompleter domains = @sortDomainsByRelevancy(queryTerms, domainCandidates) return onComplete([]) if domains.length == 0 topDomain = domains[0][0] - suggestion = new Suggestion(queryTerms, "domain", topDomain, null, @computeRelevancy) - suggestion.favIconUrl = Suggestion.getFavIconURL "http://#{topDomain}" + suggestion = new Suggestion(queryTerms, "domain", topDomain, null, @computeRelevancy, undefined, + Suggestion.getFavIconURL "http://#{topDomain}") onComplete([suggestion]) # Returns a list of domains of the form: [ [domain, relevancy], ... ] @@ -271,9 +270,9 @@ class TabCompleter chrome.tabs.query {}, (tabs) => results = tabs.filter (tab) -> RankingUtils.matches(queryTerms, tab.url, tab.title) suggestions = results.map (tab) => - suggestion = new Suggestion(queryTerms, "tab", tab.url, tab.title, @computeRelevancy) + suggestion = new Suggestion(queryTerms, "tab", tab.url, tab.title, @computeRelevancy, undefined, + tab.favIconUrl) suggestion.tabId = tab.id - suggestion.favIconUrl = tab.favIconUrl suggestion onComplete(suggestions) diff --git a/tests/unit_tests/completion_test.coffee b/tests/unit_tests/completion_test.coffee index 811436a9e..31d42302b 100644 --- a/tests/unit_tests/completion_test.coffee +++ b/tests/unit_tests/completion_test.coffee @@ -261,8 +261,8 @@ context "suggestions", assert.isTrue suggestion.generateHtml().indexOf(expected) >= 0 should "shorten urls", -> - suggestion = new Suggestion(["queryterm"], "tab", "http://ninjawords.com", "ninjawords", returns(1)) - assert.equal -1, suggestion.generateHtml().indexOf("http://ninjawords.com") + suggestion = new Suggestion(["queryterm"], "tab", "http://ninjawords.com/blah", "ninjawords", returns(1)) + assert.equal -1, suggestion.generateHtml().indexOf("http://ninjawords.com/blah") context "RankingUtils.wordRelevancy", should "score higher in shorter URLs", -> From 45e6767a5006b1a64b383941e6eb7e0a1222c9e4 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Fri, 31 Oct 2014 09:19:53 +0000 Subject: [PATCH 06/20] Handle favicon errors in vomnibar. --- background_scripts/completion.coffee | 18 +++++++++++------- content_scripts/vimium.css | 6 ++---- content_scripts/vomnibar.coffee | 12 ++++++++++++ 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee index 731c5aebc..a4520cf4a 100644 --- a/background_scripts/completion.coffee +++ b/background_scripts/completion.coffee @@ -34,17 +34,21 @@ class Suggestion
#{@type} #{@highlightTerms(Utils.escapeHtml(@title))} -
-
- #{@shortenUrl(@highlightTerms(Utils.escapeHtml(@url)))} - #{relevancyHtml} +
+
+ + #{@shortenUrl(@highlightTerms(Utils.escapeHtml(@url)))} + #{relevancyHtml}
""" # Guess the favicon URL; static method. @getFavIconURL: (url) -> - "http://#{Suggestion.parseDomain(url)}/favicon.ico" + # Omit the URL scheme, so that we get the scheme from the host page. If the host page is "https", we + # don't want to be generating "http" requests. + # TODO(smblott) For some sites (e.g. https://www.bbc.com/favicon.ico) the https request redirects to http, + # breaking the host page's security policy. + "//#{Suggestion.parseDomain(url)}/favicon.ico" # Static method. @parseDomain: (url) -> url.split("/")[2] || "" @@ -218,7 +222,7 @@ class DomainCompleter return onComplete([]) if domains.length == 0 topDomain = domains[0][0] suggestion = new Suggestion(queryTerms, "domain", topDomain, null, @computeRelevancy, undefined, - Suggestion.getFavIconURL "http://#{topDomain}") + Suggestion.getFavIconURL("http://#{topDomain}")) onComplete([suggestion]) # Returns a list of domains of the form: [ [domain, relevancy], ... ] diff --git a/content_scripts/vimium.css b/content_scripts/vimium.css index 24f229f38..77e1eb4d9 100644 --- a/content_scripts/vimium.css +++ b/content_scripts/vimium.css @@ -353,10 +353,8 @@ body.vimiumFindMode ::selection { } #vomnibar li .vomnibarIcon { - background-position-y: center; - background-size: 16px; - background-repeat: no-repeat; - padding-left: 20px; + height: 16px; + width: 16px; } #vomnibar li .vomnibarSource { diff --git a/content_scripts/vomnibar.coffee b/content_scripts/vomnibar.coffee index 22b9ed646..23f5f1b40 100644 --- a/content_scripts/vomnibar.coffee +++ b/content_scripts/vomnibar.coffee @@ -175,6 +175,17 @@ class VomnibarUI @updateTimer = null @refreshInterval) + activateFaviconObserver: (box) -> + observer = new MutationObserver (mutations) -> + for mutation in mutations + for element in mutation.addedNodes + for favicon in element.getElementsByClassName "vomnibarIcon" + do (favicon) -> + favicon.onerror = -> + favicon.onerror = null + favicon.src = "https://www.google.com/profiles/c/favicons?domain=" + observer.observe box, {childList: true, subtree:true} + initDom: -> @box = Utils.createElementFromHtml( """ @@ -186,6 +197,7 @@ class VomnibarUI """) @box.style.display = "none" + @activateFaviconObserver(@box) document.body.appendChild(@box) @input = document.querySelector("#vomnibar input") From 1dffdcc8fbd135d46aa88362d0246741166d337d Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 1 Nov 2014 11:41:22 +0000 Subject: [PATCH 07/20] Add Mutation Observer stub for vomnibar tests --- tests/dom_tests/chrome.coffee | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/dom_tests/chrome.coffee b/tests/dom_tests/chrome.coffee index 7f99e27f0..0df881a74 100644 --- a/tests/dom_tests/chrome.coffee +++ b/tests/dom_tests/chrome.coffee @@ -19,3 +19,11 @@ root.chrome = { getManifest: -> } } + +# Phantomjs does not seem to support MutationObserver; +# see https://github.com/ariya/phantomjs/issues/10715. +class MutationObserver + constructor: -> true + observe: -> true + +root.MutationObserver = MutationObserver From 2c2b5bb3249717a2f1f915aa782b427f3551ad04 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 1 Nov 2014 11:46:57 +0000 Subject: [PATCH 08/20] Pass domain to vomnibar for favicons. --- background_scripts/completion.coffee | 21 ++++--------- content_scripts/vomnibar.coffee | 41 +++++++++++++++++++++++-- tests/unit_tests/completion_test.coffee | 4 +-- 3 files changed, 46 insertions(+), 20 deletions(-) diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee index a4520cf4a..c78d06755 100644 --- a/background_scripts/completion.coffee +++ b/background_scripts/completion.coffee @@ -19,14 +19,14 @@ class Suggestion # - computeRelevancyFunction: a function which takes a Suggestion and returns a relevancy score # between [0, 1] # - extraRelevancyData: data (like the History item itself) which may be used by the relevancy function. - constructor: (@queryTerms, @type, @url, @title, @computeRelevancyFunction, @extraRelevancyData, @favIconUrl) -> + constructor: (@queryTerms, @type, @url, @title, @computeRelevancyFunction, @extraRelevancyData, @favIconDomain) -> @title ||= "" computeRelevancy: -> @relevancy = @computeRelevancyFunction(this) generateHtml: -> return @html if @html - favIconUrl = @favIconUrl or Suggestion.getFavIconURL @url + favIconDomain = @favIconDomain or Suggestion.parseDomain @url relevancyHtml = if @showRelevancy then "#{@computeRelevancy()}" else "" # NOTE(philc): We're using these vimium-specific class names so we don't collide with the page's CSS. @html = @@ -36,20 +36,12 @@ class Suggestion #{@highlightTerms(Utils.escapeHtml(@title))}
- + #{@shortenUrl(@highlightTerms(Utils.escapeHtml(@url)))} #{relevancyHtml}
""" - # Guess the favicon URL; static method. - @getFavIconURL: (url) -> - # Omit the URL scheme, so that we get the scheme from the host page. If the host page is "https", we - # don't want to be generating "http" requests. - # TODO(smblott) For some sites (e.g. https://www.bbc.com/favicon.ico) the https request redirects to http, - # breaking the host page's security policy. - "//#{Suggestion.parseDomain(url)}/favicon.ico" - # Static method. @parseDomain: (url) -> url.split("/")[2] || "" @@ -222,9 +214,10 @@ class DomainCompleter return onComplete([]) if domains.length == 0 topDomain = domains[0][0] suggestion = new Suggestion(queryTerms, "domain", topDomain, null, @computeRelevancy, undefined, - Suggestion.getFavIconURL("http://#{topDomain}")) + topDomain) onComplete([suggestion]) + # Returns a list of domains of the form: [ [domain, relevancy], ... ] sortDomainsByRelevancy: (queryTerms, domainCandidates) -> results = [] @@ -261,8 +254,6 @@ class DomainCompleter if domain and @domains[domain] and ( @domains[domain].referenceCount -= 1 ) == 0 delete @domains[domain] - parseDomain: (url) -> url.split("/")[2] || "" - # Suggestions from the Domain completer have the maximum relevancy. They should be shown first in the list. computeRelevancy: -> 1 @@ -275,7 +266,7 @@ class TabCompleter results = tabs.filter (tab) -> RankingUtils.matches(queryTerms, tab.url, tab.title) suggestions = results.map (tab) => suggestion = new Suggestion(queryTerms, "tab", tab.url, tab.title, @computeRelevancy, undefined, - tab.favIconUrl) + Suggestion.parseDomain(tab.favIconUrl)) suggestion.tabId = tab.id suggestion onComplete(suggestions) diff --git a/content_scripts/vomnibar.coffee b/content_scripts/vomnibar.coffee index 23f5f1b40..d915a0f09 100644 --- a/content_scripts/vomnibar.coffee +++ b/content_scripts/vomnibar.coffee @@ -175,15 +175,50 @@ class VomnibarUI @updateTimer = null @refreshInterval) + # Guess the URL of the favicon for each vomnibar entry; on error, try the next guess. The final guess, + # guessGoogle, yields a little globe. activateFaviconObserver: (box) -> + guessChromeUrl = (domain) -> "chrome://favicon/http://" + domain + guessHttpUrl = (domain) -> "http://" + domain + "/favicon.ico" + guessHttpsUrl = (domain) -> "https://" + domain + "/favicon.ico" + guessGoogleUrl = (domain) -> "https://www.google.com/profiles/c/favicons?domain=" + guessers = [guessHttpUrl, guessHttpsUrl, guessGoogleUrl] + + guessFavicon = (favicon, domain, guessers) -> + return if guessers.length == 0 + makeNextGuess = -> guessFavicon favicon, domain, guessers[1..] + url = guessers[0](domain) + xhr = new XMLHttpRequest() + xhr.open('GET', url , true) + xhr.responseType = 'blob' + xhr.timeout = 250 + xhr.onerror = makeNextGuess + xhr.ontimeout = makeNextGuess + xhr.onload = -> + if xhr.status == 200 and xhr.readyState == 4 + try + reader = new window.FileReader() + reader.readAsDataURL xhr.response + reader.onloadend = -> + base64data = reader.result + console.log base64data + favicon.src = base64data + catch + makeNextGuess() + else + makeNextGuess() + try + xhr.send() + catch + makeNextGuess() + observer = new MutationObserver (mutations) -> for mutation in mutations for element in mutation.addedNodes for favicon in element.getElementsByClassName "vomnibarIcon" do (favicon) -> - favicon.onerror = -> - favicon.onerror = null - favicon.src = "https://www.google.com/profiles/c/favicons?domain=" + domain = favicon.getAttribute "domain" + guessFavicon favicon, domain, guessers observer.observe box, {childList: true, subtree:true} initDom: -> diff --git a/tests/unit_tests/completion_test.coffee b/tests/unit_tests/completion_test.coffee index 31d42302b..898c9b920 100644 --- a/tests/unit_tests/completion_test.coffee +++ b/tests/unit_tests/completion_test.coffee @@ -221,8 +221,8 @@ context "domain completer (removing entries)", context "tab completer", setup -> @tabs = [ - { url: "tab1.com", title: "tab1", id: 1 } - { url: "tab2.com", title: "tab2", id: 2 }] + { url: "tab1.com", title: "tab1", id: 1, favIconUrl: "http://tab1.com/favicon.ico" } + { url: "tab2.com", title: "tab2", id: 2, favIconUrl: "http://tab2.com/favicon.ico" }] chrome.tabs = { query: (args, onComplete) => onComplete(@tabs) } @completer = new TabCompleter() From 052b21c98c3a6658323dc2fa79127d0d79ffb4f7 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 1 Nov 2014 13:25:49 +0000 Subject: [PATCH 09/20] Move favicon fetching to the background page. --- background_scripts/main.coffee | 39 +++++++++++++++++++- content_scripts/vomnibar.coffee | 65 +++++++++------------------------ tests/dom_tests/chrome.coffee | 8 ---- 3 files changed, 55 insertions(+), 57 deletions(-) diff --git a/background_scripts/main.coffee b/background_scripts/main.coffee index 898f46f16..28cad1682 100644 --- a/background_scripts/main.coffee +++ b/background_scripts/main.coffee @@ -56,11 +56,26 @@ chrome.runtime.onConnect.addListener((port, name) -> port.onMessage.addListener(portHandlers[port.name]) ) -chrome.runtime.onMessage.addListener((request, sender, sendResponse) -> +# With asynchronous responses to messages, we *must* call sendResponse (or we'll get a memory leak). +# We allow two minutes and one second for the response to be sent, after which time we send an error response. +sendResponseWithTimeout = (sendResponse) -> + timer = setTimeout ( -> + timer = null + sendResponse { error: "timed out"}), 121000 + (response) -> + if timer + sendResponse response + clearTimeout timer + timer = null + +chrome.runtime.onMessage.addListener (request, sender, sendResponse) -> + if (sendRequestAsyncHandlers[request.handler]) + sendRequestAsyncHandlers[request.handler](request, sender, sendResponseWithTimeout(sendResponse)) + return true # Ensure The sendResponse callback is not freed. if (sendRequestHandlers[request.handler]) sendResponse(sendRequestHandlers[request.handler](request, sender)) # Ensure the sendResponse callback is freed. - return false) + return false # # Used by the content scripts to get their full URL. This is needed for URLs like "view-source:http:# .." @@ -620,6 +635,23 @@ getCurrFrameIndex = (frames) -> return i if frames[i].id == focusedFrame frames.length + 1 +# Launch an asynchronous HTTP request and send the response as a base64-encoded data: URI. +fetchViaHttpAsBase64 = (request, sender, sendResponse) -> + sendError = -> sendResponse { error: "XMLHttpRequest error" } + xhr = new XMLHttpRequest() + xhr.open('GET', request.url, true) + xhr.responseType = 'blob' + xhr.timeout = 300 + xhr.ontimeout = xhr.onerror = sendError + xhr.onload = -> + return sendError() unless xhr.status == 200 and xhr.readyState == 4 + reader = new window.FileReader() + reader.readAsDataURL xhr.response + reader.onerror = sendError + reader.onloadend = -> + sendResponse { data: reader.result, type: xhr.response.type } + xhr.send() + # Port handler mapping portHandlers = keyDown: handleKeyDown, @@ -645,6 +677,9 @@ sendRequestHandlers = createMark: Marks.create.bind(Marks), gotoMark: Marks.goto.bind(Marks) +sendRequestAsyncHandlers = + fetchViaHttpAsBase64: fetchViaHttpAsBase64 + # Convenience function for development use. window.runTests = -> open(chrome.runtime.getURL('tests/dom_tests/dom_tests.html')) diff --git a/content_scripts/vomnibar.coffee b/content_scripts/vomnibar.coffee index d915a0f09..9079c03bc 100644 --- a/content_scripts/vomnibar.coffee +++ b/content_scripts/vomnibar.coffee @@ -151,11 +151,29 @@ class VomnibarUI @populateUiWithCompletions(completions) callback() if callback + guessChromeFaviconUrl: (domain) -> "chrome://favicon/http://" + domain + guessHttpFaviconUrl: (domain) -> "http://" + domain + "/favicon.ico" + guessHttpsFaviconUrl: (domain) -> "https://" + domain + "/favicon.ico" + guessGoogleFaviconUrl: (domain) -> "https://www.google.com/profiles/c/favicons?domain=" + + guessFavicon: (favicon, domain, guessers) -> + if 0 < guessers.length + chrome.runtime.sendMessage {handler: "fetchViaHttpAsBase64", url: guessers[0](domain)}, (response) => + if response.data and response.type and 0 == response.type.indexOf "image/" + favicon.src = response.data + else + @guessFavicon favicon, domain, guessers[1..] + populateUiWithCompletions: (completions) -> # update completion list with the new data @completionList.innerHTML = completions.map((completion) -> "
  • #{completion.html}
    • ").join("") @completionList.style.display = if completions.length > 0 then "block" else "none" @selection = Math.min(Math.max(@initialSelectionValue, @selection), @completions.length - 1) + # activate favicon guessers + for favicon in @completionList.getElementsByClassName "vomnibarIcon" + @guessFavicon favicon, favicon.getAttribute("domain"), + [@guessHttpFaviconUrl, @guessHttpsFaviconUrl, @guessGoogleFaviconUrl] + # update selection @updateSelection() update: (updateSynchronously, callback) -> @@ -175,52 +193,6 @@ class VomnibarUI @updateTimer = null @refreshInterval) - # Guess the URL of the favicon for each vomnibar entry; on error, try the next guess. The final guess, - # guessGoogle, yields a little globe. - activateFaviconObserver: (box) -> - guessChromeUrl = (domain) -> "chrome://favicon/http://" + domain - guessHttpUrl = (domain) -> "http://" + domain + "/favicon.ico" - guessHttpsUrl = (domain) -> "https://" + domain + "/favicon.ico" - guessGoogleUrl = (domain) -> "https://www.google.com/profiles/c/favicons?domain=" - guessers = [guessHttpUrl, guessHttpsUrl, guessGoogleUrl] - - guessFavicon = (favicon, domain, guessers) -> - return if guessers.length == 0 - makeNextGuess = -> guessFavicon favicon, domain, guessers[1..] - url = guessers[0](domain) - xhr = new XMLHttpRequest() - xhr.open('GET', url , true) - xhr.responseType = 'blob' - xhr.timeout = 250 - xhr.onerror = makeNextGuess - xhr.ontimeout = makeNextGuess - xhr.onload = -> - if xhr.status == 200 and xhr.readyState == 4 - try - reader = new window.FileReader() - reader.readAsDataURL xhr.response - reader.onloadend = -> - base64data = reader.result - console.log base64data - favicon.src = base64data - catch - makeNextGuess() - else - makeNextGuess() - try - xhr.send() - catch - makeNextGuess() - - observer = new MutationObserver (mutations) -> - for mutation in mutations - for element in mutation.addedNodes - for favicon in element.getElementsByClassName "vomnibarIcon" - do (favicon) -> - domain = favicon.getAttribute "domain" - guessFavicon favicon, domain, guessers - observer.observe box, {childList: true, subtree:true} - initDom: -> @box = Utils.createElementFromHtml( """ @@ -232,7 +204,6 @@ class VomnibarUI """) @box.style.display = "none" - @activateFaviconObserver(@box) document.body.appendChild(@box) @input = document.querySelector("#vomnibar input") diff --git a/tests/dom_tests/chrome.coffee b/tests/dom_tests/chrome.coffee index 0df881a74..7f99e27f0 100644 --- a/tests/dom_tests/chrome.coffee +++ b/tests/dom_tests/chrome.coffee @@ -19,11 +19,3 @@ root.chrome = { getManifest: -> } } - -# Phantomjs does not seem to support MutationObserver; -# see https://github.com/ariya/phantomjs/issues/10715. -class MutationObserver - constructor: -> true - observe: -> true - -root.MutationObserver = MutationObserver From 523c912b6aba27778a00dc200aa1038335f89a05 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 1 Nov 2014 13:25:49 +0000 Subject: [PATCH 10/20] Move favicon fetching to the background page. --- background_scripts/completion.coffee | 7 +-- background_scripts/main.coffee | 39 ++++++++++++++- content_scripts/vomnibar.coffee | 72 ++++++++++------------------ tests/dom_tests/chrome.coffee | 8 ---- 4 files changed, 66 insertions(+), 60 deletions(-) diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee index c78d06755..12f23dddf 100644 --- a/background_scripts/completion.coffee +++ b/background_scripts/completion.coffee @@ -27,6 +27,7 @@ class Suggestion generateHtml: -> return @html if @html favIconDomain = @favIconDomain or Suggestion.parseDomain @url + favIconUrl = @favIconUrl or "" relevancyHtml = if @showRelevancy then "#{@computeRelevancy()}" else "" # NOTE(philc): We're using these vimium-specific class names so we don't collide with the page's CSS. @html = @@ -36,7 +37,7 @@ class Suggestion #{@highlightTerms(Utils.escapeHtml(@title))}
    - + #{@shortenUrl(@highlightTerms(Utils.escapeHtml(@url)))} #{relevancyHtml}
    @@ -265,9 +266,9 @@ class TabCompleter chrome.tabs.query {}, (tabs) => results = tabs.filter (tab) -> RankingUtils.matches(queryTerms, tab.url, tab.title) suggestions = results.map (tab) => - suggestion = new Suggestion(queryTerms, "tab", tab.url, tab.title, @computeRelevancy, undefined, - Suggestion.parseDomain(tab.favIconUrl)) + suggestion = new Suggestion(queryTerms, "tab", tab.url, tab.title, @computeRelevancy) suggestion.tabId = tab.id + suggestion.favIconUrl = tab.favIconUrl suggestion onComplete(suggestions) diff --git a/background_scripts/main.coffee b/background_scripts/main.coffee index 898f46f16..28cad1682 100644 --- a/background_scripts/main.coffee +++ b/background_scripts/main.coffee @@ -56,11 +56,26 @@ chrome.runtime.onConnect.addListener((port, name) -> port.onMessage.addListener(portHandlers[port.name]) ) -chrome.runtime.onMessage.addListener((request, sender, sendResponse) -> +# With asynchronous responses to messages, we *must* call sendResponse (or we'll get a memory leak). +# We allow two minutes and one second for the response to be sent, after which time we send an error response. +sendResponseWithTimeout = (sendResponse) -> + timer = setTimeout ( -> + timer = null + sendResponse { error: "timed out"}), 121000 + (response) -> + if timer + sendResponse response + clearTimeout timer + timer = null + +chrome.runtime.onMessage.addListener (request, sender, sendResponse) -> + if (sendRequestAsyncHandlers[request.handler]) + sendRequestAsyncHandlers[request.handler](request, sender, sendResponseWithTimeout(sendResponse)) + return true # Ensure The sendResponse callback is not freed. if (sendRequestHandlers[request.handler]) sendResponse(sendRequestHandlers[request.handler](request, sender)) # Ensure the sendResponse callback is freed. - return false) + return false # # Used by the content scripts to get their full URL. This is needed for URLs like "view-source:http:# .." @@ -620,6 +635,23 @@ getCurrFrameIndex = (frames) -> return i if frames[i].id == focusedFrame frames.length + 1 +# Launch an asynchronous HTTP request and send the response as a base64-encoded data: URI. +fetchViaHttpAsBase64 = (request, sender, sendResponse) -> + sendError = -> sendResponse { error: "XMLHttpRequest error" } + xhr = new XMLHttpRequest() + xhr.open('GET', request.url, true) + xhr.responseType = 'blob' + xhr.timeout = 300 + xhr.ontimeout = xhr.onerror = sendError + xhr.onload = -> + return sendError() unless xhr.status == 200 and xhr.readyState == 4 + reader = new window.FileReader() + reader.readAsDataURL xhr.response + reader.onerror = sendError + reader.onloadend = -> + sendResponse { data: reader.result, type: xhr.response.type } + xhr.send() + # Port handler mapping portHandlers = keyDown: handleKeyDown, @@ -645,6 +677,9 @@ sendRequestHandlers = createMark: Marks.create.bind(Marks), gotoMark: Marks.goto.bind(Marks) +sendRequestAsyncHandlers = + fetchViaHttpAsBase64: fetchViaHttpAsBase64 + # Convenience function for development use. window.runTests = -> open(chrome.runtime.getURL('tests/dom_tests/dom_tests.html')) diff --git a/content_scripts/vomnibar.coffee b/content_scripts/vomnibar.coffee index d915a0f09..1c6a62022 100644 --- a/content_scripts/vomnibar.coffee +++ b/content_scripts/vomnibar.coffee @@ -151,11 +151,36 @@ class VomnibarUI @populateUiWithCompletions(completions) callback() if callback + useKnownFaviconUrl: (favicon) -> favicon.getAttribute "favIconUrl" + guessChromeFaviconUrl: (favicon) -> "chrome://favicon/http://" + favicon.getAttribute "domain" + guessHttpFaviconUrl: (favicon) -> "http://" + favicon.getAttribute("domain") + "/favicon.ico" + guessHttpsFaviconUrl: (favicon) -> "https://" + favicon.getAttribute("domain") + "/favicon.ico" + guessGoogleFaviconUrl: (favicon) -> "https://www.google.com/profiles/c/favicons?domain=" + + guessFavicon: (favicon, guessers) -> + if 0 < guessers.length + url = guessers[0](favicon) + return @guessFavicon favicon, guessers[1..] unless url + chrome.runtime.sendMessage {handler: "fetchViaHttpAsBase64", url: url}, (response) => + if response.data and response.type and 0 == response.type.indexOf "image/" + favicon.src = response.data + else + @guessFavicon favicon, guessers[1..] + populateUiWithCompletions: (completions) -> # update completion list with the new data @completionList.innerHTML = completions.map((completion) -> "
  • #{completion.html}
    • ").join("") @completionList.style.display = if completions.length > 0 then "block" else "none" @selection = Math.min(Math.max(@initialSelectionValue, @selection), @completions.length - 1) + # activate favicon guessers + for favicon in @completionList.getElementsByClassName "vomnibarIcon" + # @guessHttpsFaviconUrl is currently omitted because it's likely to fail after @guessHttpFaviconUrl has + # failed. @guessGoogleFaviconUrl is currently omitted because @guessChromeFaviconUrl is guaranteed to + # return *something*. The effectiveness of @guessChromeFaviconUrl depends upon the contents of chrome's + # cache. However, @guessGoogleFaviconUrl is more expensive -- particularly for failures (which cannot + # be cached). + @guessFavicon favicon, [@useKnownFaviconUrl, @guessHttpFaviconUrl, @guessChromeFaviconUrl] + # update selection @updateSelection() update: (updateSynchronously, callback) -> @@ -175,52 +200,6 @@ class VomnibarUI @updateTimer = null @refreshInterval) - # Guess the URL of the favicon for each vomnibar entry; on error, try the next guess. The final guess, - # guessGoogle, yields a little globe. - activateFaviconObserver: (box) -> - guessChromeUrl = (domain) -> "chrome://favicon/http://" + domain - guessHttpUrl = (domain) -> "http://" + domain + "/favicon.ico" - guessHttpsUrl = (domain) -> "https://" + domain + "/favicon.ico" - guessGoogleUrl = (domain) -> "https://www.google.com/profiles/c/favicons?domain=" - guessers = [guessHttpUrl, guessHttpsUrl, guessGoogleUrl] - - guessFavicon = (favicon, domain, guessers) -> - return if guessers.length == 0 - makeNextGuess = -> guessFavicon favicon, domain, guessers[1..] - url = guessers[0](domain) - xhr = new XMLHttpRequest() - xhr.open('GET', url , true) - xhr.responseType = 'blob' - xhr.timeout = 250 - xhr.onerror = makeNextGuess - xhr.ontimeout = makeNextGuess - xhr.onload = -> - if xhr.status == 200 and xhr.readyState == 4 - try - reader = new window.FileReader() - reader.readAsDataURL xhr.response - reader.onloadend = -> - base64data = reader.result - console.log base64data - favicon.src = base64data - catch - makeNextGuess() - else - makeNextGuess() - try - xhr.send() - catch - makeNextGuess() - - observer = new MutationObserver (mutations) -> - for mutation in mutations - for element in mutation.addedNodes - for favicon in element.getElementsByClassName "vomnibarIcon" - do (favicon) -> - domain = favicon.getAttribute "domain" - guessFavicon favicon, domain, guessers - observer.observe box, {childList: true, subtree:true} - initDom: -> @box = Utils.createElementFromHtml( """ @@ -232,7 +211,6 @@ class VomnibarUI """) @box.style.display = "none" - @activateFaviconObserver(@box) document.body.appendChild(@box) @input = document.querySelector("#vomnibar input") diff --git a/tests/dom_tests/chrome.coffee b/tests/dom_tests/chrome.coffee index 0df881a74..7f99e27f0 100644 --- a/tests/dom_tests/chrome.coffee +++ b/tests/dom_tests/chrome.coffee @@ -19,11 +19,3 @@ root.chrome = { getManifest: -> } } - -# Phantomjs does not seem to support MutationObserver; -# see https://github.com/ariya/phantomjs/issues/10715. -class MutationObserver - constructor: -> true - observe: -> true - -root.MutationObserver = MutationObserver From 593dff5836bdb9ea82a19e8a66cf94a05b6e394e Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 1 Nov 2014 15:12:52 +0000 Subject: [PATCH 11/20] Initialize favicon with chrome's default. --- content_scripts/vomnibar.coffee | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/content_scripts/vomnibar.coffee b/content_scripts/vomnibar.coffee index 1c6a62022..257c230bd 100644 --- a/content_scripts/vomnibar.coffee +++ b/content_scripts/vomnibar.coffee @@ -157,6 +157,10 @@ class VomnibarUI guessHttpsFaviconUrl: (favicon) -> "https://" + favicon.getAttribute("domain") + "/favicon.ico" guessGoogleFaviconUrl: (favicon) -> "https://www.google.com/profiles/c/favicons?domain=" + # Chrome and Google's default favicons; cached here for the benefit of their servers :-) + chromeCacheMissFavicon: "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAArklEQVR4XqWQQQqDQAxFf1zY21k3XsAeoHgYu7NasV5QqM5mUlACw5RMWvrh7T6Pn2TMjH/IEGSaJtYgIoRIQsFurKrqg2VZMI4jI04s8P7obJsTICmKM4bhIRJ9QSplWaLvB04s8ADiW4975/m5s64vdN2df1pQ15cQ6SkLojjnQqSnC4hgYAiOUAJbYCA9/YkW9hOJdOwFIOT5SQWg1AJG295MvFcETXOlbxHBG8Vy2fHIq9l6AAAAAElFTkSuQmCC" + googleCacheMissFavicon: "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAACXBIWXMAAAsSAAALEgHS3X78AAACiElEQVQ4EaVTzU8TURCf2tJuS7tQtlRb6UKBIkQwkRRSEzkQgyEc6lkOKgcOph78Y+CgjXjDs2i44FXY9AMTlQRUELZapVlouy3d7kKtb0Zr0MSLTvL2zb75eL838xtTvV6H/xELBptMJojeXLCXyobnyog4YhzXYvmCFi6qVSfaeRdXdrfaU1areV5KykmX06rcvzumjY/1ggkR3Jh+bNf1mr8v1D5bLuvR3qDgFbvbBJYIrE1mCIoCrKxsHuzK+Rzvsi29+6DEbTZz9unijEYI8ObBgXOzlcrx9OAlXyDYKUCzwwrDQx1wVDGg089Dt+gR3mxmhcUnaWeoxwMbm/vzDFzmDEKMMNhquRqduT1KwXiGt0vre6iSeAUHNDE0d26NBtAXY9BACQyjFusKuL2Ry+IPb/Y9ZglwuVscdHaknUChqLF/O4jn3V5dP4mhgRJgwSYm+gV0Oi3XrvYB30yvhGa7BS70eGFHPoTJyQHhMK+F0ZesRVVznvXw5Ixv7/C10moEo6OZXbWvlFAF9FVZDOqEABUMRIkMd8GnLwVWg9/RkJF9sA4oDfYQAuzzjqzwvnaRUFxn/X2ZlmGLXAE7AL52B4xHgqAUqrC1nSNuoJkQtLkdqReszz/9aRvq90NOKdOS1nch8TpL555WDp49f3uAMXhACRjD5j4ykuCtf5PP7Fm1b0DIsl/VHGezzP1KwOiZQobFF9YyjSRYQETRENSlVzI8iK9mWlzckpSSCQHVALmN9Az1euDho9Xo8vKGd2rqooA8yBcrwHgCqYR0kMkWci08t/R+W4ljDCanWTg9TJGwGNaNk3vYZ7VUdeKsYJGFNkfSzjXNrSX20s4/h6kB81/271ghG17l+rPTAAAAAElFTkSuQmCC" + guessFavicon: (favicon, guessers) -> if 0 < guessers.length url = guessers[0](favicon) @@ -179,6 +183,7 @@ class VomnibarUI # return *something*. The effectiveness of @guessChromeFaviconUrl depends upon the contents of chrome's # cache. However, @guessGoogleFaviconUrl is more expensive -- particularly for failures (which cannot # be cached). + favicon.src = @chromeCacheMissFavicon @guessFavicon favicon, [@useKnownFaviconUrl, @guessHttpFaviconUrl, @guessChromeFaviconUrl] # update selection @updateSelection() From ea1ba067b5aa92fbdb9188c32057c64535067d9a Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 1 Nov 2014 16:08:59 +0000 Subject: [PATCH 12/20] Do not use default favicons from chrome://favicon/. --- background_scripts/main.coffee | 2 +- content_scripts/vomnibar.coffee | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/background_scripts/main.coffee b/background_scripts/main.coffee index 28cad1682..fbf1fc297 100644 --- a/background_scripts/main.coffee +++ b/background_scripts/main.coffee @@ -641,7 +641,7 @@ fetchViaHttpAsBase64 = (request, sender, sendResponse) -> xhr = new XMLHttpRequest() xhr.open('GET', request.url, true) xhr.responseType = 'blob' - xhr.timeout = 300 + xhr.timeout = 1000 xhr.ontimeout = xhr.onerror = sendError xhr.onload = -> return sendError() unless xhr.status == 200 and xhr.readyState == 4 diff --git a/content_scripts/vomnibar.coffee b/content_scripts/vomnibar.coffee index 2d530c689..9dd41275b 100644 --- a/content_scripts/vomnibar.coffee +++ b/content_scripts/vomnibar.coffee @@ -151,6 +151,8 @@ class VomnibarUI @populateUiWithCompletions(completions) callback() if callback + # Various ways in which we met get or guess the favicon for a suggestion. Not all of these are currently + # used. useKnownFaviconUrl: (favicon) -> favicon.getAttribute "favIconUrl" guessChromeFaviconUrl: (favicon) -> "chrome://favicon/http://" + favicon.getAttribute "domain" guessHttpFaviconUrl: (favicon) -> "http://" + favicon.getAttribute("domain") + "/favicon.ico" @@ -164,12 +166,13 @@ class VomnibarUI guessFavicon: (favicon, guessers) -> if 0 < guessers.length url = guessers[0](favicon) - return @guessFavicon favicon, guessers[1..] unless url + tryNextGuess = => @guessFavicon favicon, guessers[1..] + return tryNextGuess() unless url chrome.runtime.sendMessage {handler: "fetchViaHttpAsBase64", url: url}, (response) => if response.data and response.type and 0 == response.type.indexOf "image/" - favicon.src = response.data - else - @guessFavicon favicon, guessers[1..] + if response.data != @chromeCacheMissFavicon + return favicon.src = response.data + tryNextGuess() populateUiWithCompletions: (completions) -> # update completion list with the new data @@ -178,14 +181,11 @@ class VomnibarUI @selection = Math.min(Math.max(@initialSelectionValue, @selection), @completions.length - 1) # activate favicon guessers for favicon in @completionList.getElementsByClassName "vomnibarIcon" - favicon.src = @chromeCacheMissFavicon - # @guessHttpsFaviconUrl is currently omitted because it's likely to fail after @guessHttpFaviconUrl has - # failed. @guessGoogleFaviconUrl is currently omitted because @guessChromeFaviconUrl is guaranteed to - # return *something*. The effectiveness of @guessChromeFaviconUrl depends upon the contents of chrome's - # cache. However, @guessGoogleFaviconUrl is more expensive -- particularly for failures (which cannot - # be cached). - @guessFavicon favicon, [@useKnownFaviconUrl, @guessHttpFaviconUrl, @guessChromeFaviconUrl] - # update selection + favicon.src = @googleCacheMissFavicon + # Strategy. Use a known favicon URL, if we have one. Then try chrome's favicon cache (but we only use + # this favicon if it's not chrome's default favicon). Then try guessing over HTTP. If all of that + # fails, we'll be left with Google's default favicon, which is a little globe (@googleCacheMissFavicon). + @guessFavicon favicon, [@useKnownFaviconUrl, @guessChromeFaviconUrl, @guessHttpFaviconUrl] @updateSelection() update: (updateSynchronously, callback) -> From ade3fb1100b66ebddc244ae170dfdbf8d19e4ff0 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 1 Nov 2014 16:56:39 +0000 Subject: [PATCH 13/20] Cache favicons to reduce the number of XMLHttpRequests. --- background_scripts/main.coffee | 12 +++++++----- content_scripts/vomnibar.coffee | 20 +++++++++++++++++++- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/background_scripts/main.coffee b/background_scripts/main.coffee index fbf1fc297..3295210d6 100644 --- a/background_scripts/main.coffee +++ b/background_scripts/main.coffee @@ -70,8 +70,8 @@ sendResponseWithTimeout = (sendResponse) -> chrome.runtime.onMessage.addListener (request, sender, sendResponse) -> if (sendRequestAsyncHandlers[request.handler]) - sendRequestAsyncHandlers[request.handler](request, sender, sendResponseWithTimeout(sendResponse)) - return true # Ensure The sendResponse callback is not freed. + # Handlers must return true if they will respond asynchronously, false otherwise. + return sendRequestAsyncHandlers[request.handler](request, sender, sendResponseWithTimeout(sendResponse)) if (sendRequestHandlers[request.handler]) sendResponse(sendRequestHandlers[request.handler](request, sender)) # Ensure the sendResponse callback is freed. @@ -639,9 +639,9 @@ getCurrFrameIndex = (frames) -> fetchViaHttpAsBase64 = (request, sender, sendResponse) -> sendError = -> sendResponse { error: "XMLHttpRequest error" } xhr = new XMLHttpRequest() - xhr.open('GET', request.url, true) - xhr.responseType = 'blob' - xhr.timeout = 1000 + xhr.open("GET", request.url, true) + xhr.responseType = "blob" + xhr.timeout = 300 xhr.ontimeout = xhr.onerror = sendError xhr.onload = -> return sendError() unless xhr.status == 200 and xhr.readyState == 4 @@ -649,8 +649,10 @@ fetchViaHttpAsBase64 = (request, sender, sendResponse) -> reader.readAsDataURL xhr.response reader.onerror = sendError reader.onloadend = -> + console.log request.url sendResponse { data: reader.result, type: xhr.response.type } xhr.send() + true # sendResponse will be called asynchronously. # Port handler mapping portHandlers = diff --git a/content_scripts/vomnibar.coffee b/content_scripts/vomnibar.coffee index 9dd41275b..f4de79ce4 100644 --- a/content_scripts/vomnibar.coffee +++ b/content_scripts/vomnibar.coffee @@ -163,12 +163,30 @@ class VomnibarUI chromeCacheMissFavicon: "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAArklEQVR4XqWQQQqDQAxFf1zY21k3XsAeoHgYu7NasV5QqM5mUlACw5RMWvrh7T6Pn2TMjH/IEGSaJtYgIoRIQsFurKrqg2VZMI4jI04s8P7obJsTICmKM4bhIRJ9QSplWaLvB04s8ADiW4975/m5s64vdN2df1pQ15cQ6SkLojjnQqSnC4hgYAiOUAJbYCA9/YkW9hOJdOwFIOT5SQWg1AJG295MvFcETXOlbxHBG8Vy2fHIq9l6AAAAAElFTkSuQmCC" googleCacheMissFavicon: "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAACXBIWXMAAAsSAAALEgHS3X78AAACiElEQVQ4EaVTzU8TURCf2tJuS7tQtlRb6UKBIkQwkRRSEzkQgyEc6lkOKgcOph78Y+CgjXjDs2i44FXY9AMTlQRUELZapVlouy3d7kKtb0Zr0MSLTvL2zb75eL838xtTvV6H/xELBptMJojeXLCXyobnyog4YhzXYvmCFi6qVSfaeRdXdrfaU1areV5KykmX06rcvzumjY/1ggkR3Jh+bNf1mr8v1D5bLuvR3qDgFbvbBJYIrE1mCIoCrKxsHuzK+Rzvsi29+6DEbTZz9unijEYI8ObBgXOzlcrx9OAlXyDYKUCzwwrDQx1wVDGg089Dt+gR3mxmhcUnaWeoxwMbm/vzDFzmDEKMMNhquRqduT1KwXiGt0vre6iSeAUHNDE0d26NBtAXY9BACQyjFusKuL2Ry+IPb/Y9ZglwuVscdHaknUChqLF/O4jn3V5dP4mhgRJgwSYm+gV0Oi3XrvYB30yvhGa7BS70eGFHPoTJyQHhMK+F0ZesRVVznvXw5Ixv7/C10moEo6OZXbWvlFAF9FVZDOqEABUMRIkMd8GnLwVWg9/RkJF9sA4oDfYQAuzzjqzwvnaRUFxn/X2ZlmGLXAE7AL52B4xHgqAUqrC1nSNuoJkQtLkdqReszz/9aRvq90NOKdOS1nch8TpL555WDp49f3uAMXhACRjD5j4ykuCtf5PP7Fm1b0DIsl/VHGezzP1KwOiZQobFF9YyjSRYQETRENSlVzI8iK9mWlzckpSSCQHVALmN9Az1euDho9Xo8vKGd2rqooA8yBcrwHgCqYR0kMkWci08t/R+W4ljDCanWTg9TJGwGNaNk3vYZ7VUdeKsYJGFNkfSzjXNrSX20s4/h6kB81/271ghG17l+rPTAAAAAElFTkSuQmCC" + # Note(smblott) In certain (unknown) circumstances, chrome serializes XMLHttpRequests to the same domain, + # and doesn't cache the results. The following is an asynchronous memo function which prevents us sending + # off multiple XMLHttpRequests for the same URL. + faviconCache: do -> + cache = {} + callbacks = {} + (url,callback) -> + if url of cache + callback cache[url] + else if url of callbacks + callbacks[url].push callback + else + callbacks[url] = [ callback ] + chrome.runtime.sendMessage {handler: "fetchViaHttpAsBase64", url: url}, (response) => + cache[url] = response + for callback in callbacks[url] + callback response + guessFavicon: (favicon, guessers) -> if 0 < guessers.length url = guessers[0](favicon) tryNextGuess = => @guessFavicon favicon, guessers[1..] return tryNextGuess() unless url - chrome.runtime.sendMessage {handler: "fetchViaHttpAsBase64", url: url}, (response) => + @faviconCache url, (response) => if response.data and response.type and 0 == response.type.indexOf "image/" if response.data != @chromeCacheMissFavicon return favicon.src = response.data From c197443086e42e8e5f4f5610e736156cec5016c8 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 1 Nov 2014 17:11:16 +0000 Subject: [PATCH 14/20] Delete favicon cache callbacks. --- content_scripts/vomnibar.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/content_scripts/vomnibar.coffee b/content_scripts/vomnibar.coffee index f4de79ce4..332c63100 100644 --- a/content_scripts/vomnibar.coffee +++ b/content_scripts/vomnibar.coffee @@ -180,6 +180,7 @@ class VomnibarUI cache[url] = response for callback in callbacks[url] callback response + delete callbacks[url] guessFavicon: (favicon, guessers) -> if 0 < guessers.length From 793aed85ed152467b1bec521d95d1d2a506d5edc Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sun, 2 Nov 2014 14:02:22 +0000 Subject: [PATCH 15/20] No need for bottom-half padding. --- content_scripts/vimium.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/content_scripts/vimium.css b/content_scripts/vimium.css index 77e1eb4d9..ad0715606 100644 --- a/content_scripts/vimium.css +++ b/content_scripts/vimium.css @@ -349,7 +349,7 @@ body.vimiumFindMode ::selection { #vomnibar li .vomnibarBottomHalf { font-size: 15px; margin-top: 3px; - padding: 2px 0; + padding: 0px 0; } #vomnibar li .vomnibarIcon { From 114ac578f2c98cfbcab3800c94a0e6d5879e3df6 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 5 Nov 2014 07:50:12 +0000 Subject: [PATCH 16/20] Favicon code tidy up. --- background_scripts/main.coffee | 29 +++++++++++++++-------------- content_scripts/vomnibar.coffee | 25 +++++++++++++------------ 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/background_scripts/main.coffee b/background_scripts/main.coffee index 3295210d6..89cdb9568 100644 --- a/background_scripts/main.coffee +++ b/background_scripts/main.coffee @@ -56,17 +56,19 @@ chrome.runtime.onConnect.addListener((port, name) -> port.onMessage.addListener(portHandlers[port.name]) ) -# With asynchronous responses to messages, we *must* call sendResponse (or we'll get a memory leak). -# We allow two minutes and one second for the response to be sent, after which time we send an error response. +# With asynchronous message handling, failure to call sendResponse will result in a memory leak. We allow two +# minutes and one second for the response to be sent, after which time we simply *assume* there is an an error +# and call sendResponse with an error message. sendResponseWithTimeout = (sendResponse) -> - timer = setTimeout ( -> + timer = null + doSendResponse = (response=null) -> timer = null - sendResponse { error: "timed out"}), 121000 + sendResponse (response || { error: "timeout", errorSource: "sendResponseWithTimeout" }) + timer = setTimeout doSendResponse, 121000 (response) -> if timer - sendResponse response clearTimeout timer - timer = null + doSendResponse response chrome.runtime.onMessage.addListener (request, sender, sendResponse) -> if (sendRequestAsyncHandlers[request.handler]) @@ -74,8 +76,7 @@ chrome.runtime.onMessage.addListener (request, sender, sendResponse) -> return sendRequestAsyncHandlers[request.handler](request, sender, sendResponseWithTimeout(sendResponse)) if (sendRequestHandlers[request.handler]) sendResponse(sendRequestHandlers[request.handler](request, sender)) - # Ensure the sendResponse callback is freed. - return false + false # sendResponse will not now be called, and can be freed. # # Used by the content scripts to get their full URL. This is needed for URLs like "view-source:http:# .." @@ -637,19 +638,19 @@ getCurrFrameIndex = (frames) -> # Launch an asynchronous HTTP request and send the response as a base64-encoded data: URI. fetchViaHttpAsBase64 = (request, sender, sendResponse) -> - sendError = -> sendResponse { error: "XMLHttpRequest error" } + sendError = (error) -> (-> sendResponse { error: error, errorSource: "fetchViaHttpAsBase64" }) xhr = new XMLHttpRequest() xhr.open("GET", request.url, true) xhr.responseType = "blob" - xhr.timeout = 300 - xhr.ontimeout = xhr.onerror = sendError + xhr.timeout = request.timeout || 5000 + xhr.ontimeout = sendError "xmlhttprequest-timeout" + xhr.onerror = sendError "xmlhttprequest-error" xhr.onload = -> - return sendError() unless xhr.status == 200 and xhr.readyState == 4 + return sendError("http-error")() unless xhr.status == 200 and xhr.readyState == 4 reader = new window.FileReader() reader.readAsDataURL xhr.response - reader.onerror = sendError + reader.onerror = sendError "read-error" reader.onloadend = -> - console.log request.url sendResponse { data: reader.result, type: xhr.response.type } xhr.send() true # sendResponse will be called asynchronously. diff --git a/content_scripts/vomnibar.coffee b/content_scripts/vomnibar.coffee index 332c63100..359442246 100644 --- a/content_scripts/vomnibar.coffee +++ b/content_scripts/vomnibar.coffee @@ -163,9 +163,10 @@ class VomnibarUI chromeCacheMissFavicon: "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAArklEQVR4XqWQQQqDQAxFf1zY21k3XsAeoHgYu7NasV5QqM5mUlACw5RMWvrh7T6Pn2TMjH/IEGSaJtYgIoRIQsFurKrqg2VZMI4jI04s8P7obJsTICmKM4bhIRJ9QSplWaLvB04s8ADiW4975/m5s64vdN2df1pQ15cQ6SkLojjnQqSnC4hgYAiOUAJbYCA9/YkW9hOJdOwFIOT5SQWg1AJG295MvFcETXOlbxHBG8Vy2fHIq9l6AAAAAElFTkSuQmCC" googleCacheMissFavicon: "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAACXBIWXMAAAsSAAALEgHS3X78AAACiElEQVQ4EaVTzU8TURCf2tJuS7tQtlRb6UKBIkQwkRRSEzkQgyEc6lkOKgcOph78Y+CgjXjDs2i44FXY9AMTlQRUELZapVlouy3d7kKtb0Zr0MSLTvL2zb75eL838xtTvV6H/xELBptMJojeXLCXyobnyog4YhzXYvmCFi6qVSfaeRdXdrfaU1areV5KykmX06rcvzumjY/1ggkR3Jh+bNf1mr8v1D5bLuvR3qDgFbvbBJYIrE1mCIoCrKxsHuzK+Rzvsi29+6DEbTZz9unijEYI8ObBgXOzlcrx9OAlXyDYKUCzwwrDQx1wVDGg089Dt+gR3mxmhcUnaWeoxwMbm/vzDFzmDEKMMNhquRqduT1KwXiGt0vre6iSeAUHNDE0d26NBtAXY9BACQyjFusKuL2Ry+IPb/Y9ZglwuVscdHaknUChqLF/O4jn3V5dP4mhgRJgwSYm+gV0Oi3XrvYB30yvhGa7BS70eGFHPoTJyQHhMK+F0ZesRVVznvXw5Ixv7/C10moEo6OZXbWvlFAF9FVZDOqEABUMRIkMd8GnLwVWg9/RkJF9sA4oDfYQAuzzjqzwvnaRUFxn/X2ZlmGLXAE7AL52B4xHgqAUqrC1nSNuoJkQtLkdqReszz/9aRvq90NOKdOS1nch8TpL555WDp49f3uAMXhACRjD5j4ykuCtf5PP7Fm1b0DIsl/VHGezzP1KwOiZQobFF9YyjSRYQETRENSlVzI8iK9mWlzckpSSCQHVALmN9Az1euDho9Xo8vKGd2rqooA8yBcrwHgCqYR0kMkWci08t/R+W4ljDCanWTg9TJGwGNaNk3vYZ7VUdeKsYJGFNkfSzjXNrSX20s4/h6kB81/271ghG17l+rPTAAAAAElFTkSuQmCC" - # Note(smblott) In certain (unknown) circumstances, chrome serializes XMLHttpRequests to the same domain, - # and doesn't cache the results. The following is an asynchronous memo function which prevents us sending - # off multiple XMLHttpRequests for the same URL. + # Note(smblott) In certain (unknown) circumstances, chrome serializes XMLHttpRequests to the same domain + # (which is normal), but doesn't cache the results. The following is an asynchronous memo function which + # caches favicons, sending off at most one request for each URL. Favicons are represented as base64-encoded + # data: URLs. faviconCache: do -> cache = {} callbacks = {} @@ -176,7 +177,8 @@ class VomnibarUI callbacks[url].push callback else callbacks[url] = [ callback ] - chrome.runtime.sendMessage {handler: "fetchViaHttpAsBase64", url: url}, (response) => + # We use a short timeout because the URLs we guess sometimes do not exist and hang. + chrome.runtime.sendMessage {handler: "fetchViaHttpAsBase64", url: url, timeout: 300}, (response) -> cache[url] = response for callback in callbacks[url] callback response @@ -184,8 +186,8 @@ class VomnibarUI guessFavicon: (favicon, guessers) -> if 0 < guessers.length - url = guessers[0](favicon) tryNextGuess = => @guessFavicon favicon, guessers[1..] + url = guessers[0] favicon return tryNextGuess() unless url @faviconCache url, (response) => if response.data and response.type and 0 == response.type.indexOf "image/" @@ -193,18 +195,17 @@ class VomnibarUI return favicon.src = response.data tryNextGuess() + guessFavicons: -> + for favicon in @completionList.getElementsByClassName "vomnibarIcon" + favicon.src = @googleCacheMissFavicon # Set a default favicon, Google's small globe. + @guessFavicon favicon, [@useKnownFaviconUrl, @guessChromeFaviconUrl, @guessHttpFaviconUrl] + populateUiWithCompletions: (completions) -> # update completion list with the new data @completionList.innerHTML = completions.map((completion) -> "
  • #{completion.html}
    • ").join("") @completionList.style.display = if completions.length > 0 then "block" else "none" @selection = Math.min(Math.max(@initialSelectionValue, @selection), @completions.length - 1) - # activate favicon guessers - for favicon in @completionList.getElementsByClassName "vomnibarIcon" - favicon.src = @googleCacheMissFavicon - # Strategy. Use a known favicon URL, if we have one. Then try chrome's favicon cache (but we only use - # this favicon if it's not chrome's default favicon). Then try guessing over HTTP. If all of that - # fails, we'll be left with Google's default favicon, which is a little globe (@googleCacheMissFavicon). - @guessFavicon favicon, [@useKnownFaviconUrl, @guessChromeFaviconUrl, @guessHttpFaviconUrl] + @guessFavicons() @updateSelection() update: (updateSynchronously, callback) -> From 6157b7dfa295b35c20431d37b974fe3344506d38 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 8 Nov 2014 08:57:28 +0000 Subject: [PATCH 17/20] Favicons; fetch chrome default dynamically. --- background_scripts/main.coffee | 21 ++++++++++++++++++++- content_scripts/vomnibar.coffee | 26 ++++++++++---------------- 2 files changed, 30 insertions(+), 17 deletions(-) diff --git a/background_scripts/main.coffee b/background_scripts/main.coffee index 89cdb9568..96656c34f 100644 --- a/background_scripts/main.coffee +++ b/background_scripts/main.coffee @@ -655,6 +655,25 @@ fetchViaHttpAsBase64 = (request, sender, sendResponse) -> xhr.send() true # sendResponse will be called asynchronously. +# Fetch a favicon and send it to the requester. However, if the favicon is chrome's default favicon, then +# instead send an error. +fetchFavicon = do -> + nonExistentFaviconUrl = "chrome://favicon/ThisShouldNotExist-SoWeGetTheChromeDefaultFavicon" + chromeDefaultFavicon = null + + fetcher = (request, sender, sendResponse) -> + fetchViaHttpAsBase64 request, sender, (response) -> + if response.data and response.type and 0 == response.type.indexOf "image/" + if response.data != chromeDefaultFavicon + return sendResponse response + sendResponse { error: "chrome-default-favicon", errorSource: "fetchFavicon" } + + # Fetch chrome's default favicon. + fetcher { url: nonExistentFaviconUrl }, null, (response) -> + chromeDefaultFavicon = response.data if response.data and not response.error + + return fetcher + # Port handler mapping portHandlers = keyDown: handleKeyDown, @@ -681,7 +700,7 @@ sendRequestHandlers = gotoMark: Marks.goto.bind(Marks) sendRequestAsyncHandlers = - fetchViaHttpAsBase64: fetchViaHttpAsBase64 + fetchFavicon: fetchFavicon # Convenience function for development use. window.runTests = -> open(chrome.runtime.getURL('tests/dom_tests/dom_tests.html')) diff --git a/content_scripts/vomnibar.coffee b/content_scripts/vomnibar.coffee index 359442246..11413c27e 100644 --- a/content_scripts/vomnibar.coffee +++ b/content_scripts/vomnibar.coffee @@ -151,22 +151,17 @@ class VomnibarUI @populateUiWithCompletions(completions) callback() if callback - # Various ways in which we met get or guess the favicon for a suggestion. Not all of these are currently - # used. + # Various ways in which we guess a favicon's URL. useKnownFaviconUrl: (favicon) -> favicon.getAttribute "favIconUrl" guessChromeFaviconUrl: (favicon) -> "chrome://favicon/http://" + favicon.getAttribute "domain" guessHttpFaviconUrl: (favicon) -> "http://" + favicon.getAttribute("domain") + "/favicon.ico" guessHttpsFaviconUrl: (favicon) -> "https://" + favicon.getAttribute("domain") + "/favicon.ico" - guessGoogleFaviconUrl: (favicon) -> "https://www.google.com/profiles/c/favicons?domain=" - # Chrome and Google's default favicons; cached here for the benefit of their servers :-) - chromeCacheMissFavicon: "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAArklEQVR4XqWQQQqDQAxFf1zY21k3XsAeoHgYu7NasV5QqM5mUlACw5RMWvrh7T6Pn2TMjH/IEGSaJtYgIoRIQsFurKrqg2VZMI4jI04s8P7obJsTICmKM4bhIRJ9QSplWaLvB04s8ADiW4975/m5s64vdN2df1pQ15cQ6SkLojjnQqSnC4hgYAiOUAJbYCA9/YkW9hOJdOwFIOT5SQWg1AJG295MvFcETXOlbxHBG8Vy2fHIq9l6AAAAAElFTkSuQmCC" + # Google's default favicon; cached here for the benefit of their servers :-) googleCacheMissFavicon: "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAACXBIWXMAAAsSAAALEgHS3X78AAACiElEQVQ4EaVTzU8TURCf2tJuS7tQtlRb6UKBIkQwkRRSEzkQgyEc6lkOKgcOph78Y+CgjXjDs2i44FXY9AMTlQRUELZapVlouy3d7kKtb0Zr0MSLTvL2zb75eL838xtTvV6H/xELBptMJojeXLCXyobnyog4YhzXYvmCFi6qVSfaeRdXdrfaU1areV5KykmX06rcvzumjY/1ggkR3Jh+bNf1mr8v1D5bLuvR3qDgFbvbBJYIrE1mCIoCrKxsHuzK+Rzvsi29+6DEbTZz9unijEYI8ObBgXOzlcrx9OAlXyDYKUCzwwrDQx1wVDGg089Dt+gR3mxmhcUnaWeoxwMbm/vzDFzmDEKMMNhquRqduT1KwXiGt0vre6iSeAUHNDE0d26NBtAXY9BACQyjFusKuL2Ry+IPb/Y9ZglwuVscdHaknUChqLF/O4jn3V5dP4mhgRJgwSYm+gV0Oi3XrvYB30yvhGa7BS70eGFHPoTJyQHhMK+F0ZesRVVznvXw5Ixv7/C10moEo6OZXbWvlFAF9FVZDOqEABUMRIkMd8GnLwVWg9/RkJF9sA4oDfYQAuzzjqzwvnaRUFxn/X2ZlmGLXAE7AL52B4xHgqAUqrC1nSNuoJkQtLkdqReszz/9aRvq90NOKdOS1nch8TpL555WDp49f3uAMXhACRjD5j4ykuCtf5PP7Fm1b0DIsl/VHGezzP1KwOiZQobFF9YyjSRYQETRENSlVzI8iK9mWlzckpSSCQHVALmN9Az1euDho9Xo8vKGd2rqooA8yBcrwHgCqYR0kMkWci08t/R+W4ljDCanWTg9TJGwGNaNk3vYZ7VUdeKsYJGFNkfSzjXNrSX20s4/h6kB81/271ghG17l+rPTAAAAAElFTkSuQmCC" - # Note(smblott) In certain (unknown) circumstances, chrome serializes XMLHttpRequests to the same domain - # (which is normal), but doesn't cache the results. The following is an asynchronous memo function which - # caches favicons, sending off at most one request for each URL. Favicons are represented as base64-encoded - # data: URLs. + # Note(smblott) Chrome serializes XMLHttpRequests to the same domain, and (sometimes?) doesn't cache the + # results. So we cache favicons here. faviconCache: do -> cache = {} callbacks = {} @@ -177,8 +172,8 @@ class VomnibarUI callbacks[url].push callback else callbacks[url] = [ callback ] - # We use a short timeout because the URLs we guess sometimes do not exist and hang. - chrome.runtime.sendMessage {handler: "fetchViaHttpAsBase64", url: url, timeout: 300}, (response) -> + # Use a short timeout, because the URLs we guess sometimes do not exist and hang. + chrome.runtime.sendMessage {handler: "fetchFavicon", url: url, timeout: 250}, (response) -> cache[url] = response for callback in callbacks[url] callback response @@ -189,16 +184,15 @@ class VomnibarUI tryNextGuess = => @guessFavicon favicon, guessers[1..] url = guessers[0] favicon return tryNextGuess() unless url - @faviconCache url, (response) => - if response.data and response.type and 0 == response.type.indexOf "image/" - if response.data != @chromeCacheMissFavicon - return favicon.src = response.data + @faviconCache url, (response) -> + if response.data and not response.error + return favicon.src = response.data tryNextGuess() guessFavicons: -> for favicon in @completionList.getElementsByClassName "vomnibarIcon" favicon.src = @googleCacheMissFavicon # Set a default favicon, Google's small globe. - @guessFavicon favicon, [@useKnownFaviconUrl, @guessChromeFaviconUrl, @guessHttpFaviconUrl] + @guessFavicon favicon, [@useKnownFaviconUrl, @guessChromeFaviconUrl, @guessHttpFaviconUrl, @guessHttpsFaviconUrl] populateUiWithCompletions: (completions) -> # update completion list with the new data From 658bc569d8646ad61cc64cf95c05e2908f41f86e Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 8 Nov 2014 11:03:45 +0000 Subject: [PATCH 18/20] Favicons; move caching to the background page. --- background_scripts/main.coffee | 58 ++++++++++++++++++----- content_scripts/vomnibar.coffee | 55 +++++++++------------ lib/utils.coffee | 22 +++++++++ tests/unit_tests/test_chrome_stubs.coffee | 5 ++ tests/unit_tests/utils_test.coffee | 34 +++++++++++++ 5 files changed, 130 insertions(+), 44 deletions(-) diff --git a/background_scripts/main.coffee b/background_scripts/main.coffee index 96656c34f..f36001c04 100644 --- a/background_scripts/main.coffee +++ b/background_scripts/main.coffee @@ -652,24 +652,58 @@ fetchViaHttpAsBase64 = (request, sender, sendResponse) -> reader.onerror = sendError "read-error" reader.onloadend = -> sendResponse { data: reader.result, type: xhr.response.type } - xhr.send() + # Some favicon URL guesses are so bad that they raise exceptions immediately. + try + xhr.send() + catch + sendError("invalid-request")() + return false # sendResponse will not be called asynchronously. true # sendResponse will be called asynchronously. -# Fetch a favicon and send it to the requester. However, if the favicon is chrome's default favicon, then -# instead send an error. +# Fetch a favicon and send it to the requester. However, if the response is chrome's default favicon, which +# we don't want to use, then instead send an error. +# +# Favicons are cached based on both the domain of the request and on the URL. Caching on the domain +# short-circuits the current and future guesses (if the domain is in the cache, then we don't need to try any +# URLs). Caching on the URL is primarily for cases where we fail to find *any* usable favicon. In this case, +# we fall back to @guessGoogleFaviconUrl (which doesn't depend upon the domain). So we get a cache hit (even +# for domains we've never seen before), and all such failures end up sharing a single response object, which +# itself is only fetched once. fetchFavicon = do -> nonExistentFaviconUrl = "chrome://favicon/ThisShouldNotExist-SoWeGetTheChromeDefaultFavicon" - chromeDefaultFavicon = null - - fetcher = (request, sender, sendResponse) -> - fetchViaHttpAsBase64 request, sender, (response) -> - if response.data and response.type and 0 == response.type.indexOf "image/" - if response.data != chromeDefaultFavicon - return sendResponse response - sendResponse { error: "chrome-default-favicon", errorSource: "fetchFavicon" } + defaultFaviconError = { error: "chrome-default-favicon", errorSource: "fetchFavicon" } + unfetchableError = { error: "unfetchable", errorSource: "fetchFavicon" } + cache = new SimpleCache 1000*60*60*24*7 # Rotate the cache every seven days. + callbacks = {} + chromeDefaultFavicon = "" + + fetcher = (request, sender=undefined, sendResponse=undefined) -> + if 0 == request.url.indexOf "chrome://theme/" # We can't fetch these. + sendResponse unfetchableError + return false # sendResponse will not be called asynchronously. + if response = cache.get request.url + cache.set request.domain, response + if response = (cache.get(request.domain) or cache.get(request.url)) + sendResponse response + false # sendResponse will not be called asynchronously. + else if callbacks[request.url] + callbacks[request.url].push sendResponse + true # sendResponse will be called asynchronously. + else + callbacks[request.url] = [ sendResponse ] + fetchViaHttpAsBase64 request, sender, (response) -> + responseOk = response.type and not response.type.indexOf "image/" + responseOk &&= response.data and response.data != chromeDefaultFavicon + if responseOk + cache.set request.url, response + cache.set request.domain, response + else + response = defaultFaviconError + callback(response) for callback in callbacks[request.url] when callback + delete callbacks[request.url] # Fetch chrome's default favicon. - fetcher { url: nonExistentFaviconUrl }, null, (response) -> + fetcher { url: nonExistentFaviconUrl, domain: nonExistentFaviconUrl }, null, (response) -> chromeDefaultFavicon = response.data if response.data and not response.error return fetcher diff --git a/content_scripts/vomnibar.coffee b/content_scripts/vomnibar.coffee index 11413c27e..11c4e117b 100644 --- a/content_scripts/vomnibar.coffee +++ b/content_scripts/vomnibar.coffee @@ -151,48 +151,39 @@ class VomnibarUI @populateUiWithCompletions(completions) callback() if callback - # Various ways in which we guess a favicon's URL. - useKnownFaviconUrl: (favicon) -> favicon.getAttribute "favIconUrl" - guessChromeFaviconUrl: (favicon) -> "chrome://favicon/http://" + favicon.getAttribute "domain" - guessHttpFaviconUrl: (favicon) -> "http://" + favicon.getAttribute("domain") + "/favicon.ico" - guessHttpsFaviconUrl: (favicon) -> "https://" + favicon.getAttribute("domain") + "/favicon.ico" - # Google's default favicon; cached here for the benefit of their servers :-) googleCacheMissFavicon: "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAACXBIWXMAAAsSAAALEgHS3X78AAACiElEQVQ4EaVTzU8TURCf2tJuS7tQtlRb6UKBIkQwkRRSEzkQgyEc6lkOKgcOph78Y+CgjXjDs2i44FXY9AMTlQRUELZapVlouy3d7kKtb0Zr0MSLTvL2zb75eL838xtTvV6H/xELBptMJojeXLCXyobnyog4YhzXYvmCFi6qVSfaeRdXdrfaU1areV5KykmX06rcvzumjY/1ggkR3Jh+bNf1mr8v1D5bLuvR3qDgFbvbBJYIrE1mCIoCrKxsHuzK+Rzvsi29+6DEbTZz9unijEYI8ObBgXOzlcrx9OAlXyDYKUCzwwrDQx1wVDGg089Dt+gR3mxmhcUnaWeoxwMbm/vzDFzmDEKMMNhquRqduT1KwXiGt0vre6iSeAUHNDE0d26NBtAXY9BACQyjFusKuL2Ry+IPb/Y9ZglwuVscdHaknUChqLF/O4jn3V5dP4mhgRJgwSYm+gV0Oi3XrvYB30yvhGa7BS70eGFHPoTJyQHhMK+F0ZesRVVznvXw5Ixv7/C10moEo6OZXbWvlFAF9FVZDOqEABUMRIkMd8GnLwVWg9/RkJF9sA4oDfYQAuzzjqzwvnaRUFxn/X2ZlmGLXAE7AL52B4xHgqAUqrC1nSNuoJkQtLkdqReszz/9aRvq90NOKdOS1nch8TpL555WDp49f3uAMXhACRjD5j4ykuCtf5PP7Fm1b0DIsl/VHGezzP1KwOiZQobFF9YyjSRYQETRENSlVzI8iK9mWlzckpSSCQHVALmN9Az1euDho9Xo8vKGd2rqooA8yBcrwHgCqYR0kMkWci08t/R+W4ljDCanWTg9TJGwGNaNk3vYZ7VUdeKsYJGFNkfSzjXNrSX20s4/h6kB81/271ghG17l+rPTAAAAAElFTkSuQmCC" - # Note(smblott) Chrome serializes XMLHttpRequests to the same domain, and (sometimes?) doesn't cache the - # results. So we cache favicons here. - faviconCache: do -> - cache = {} - callbacks = {} - (url,callback) -> - if url of cache - callback cache[url] - else if url of callbacks - callbacks[url].push callback - else - callbacks[url] = [ callback ] - # Use a short timeout, because the URLs we guess sometimes do not exist and hang. - chrome.runtime.sendMessage {handler: "fetchFavicon", url: url, timeout: 250}, (response) -> - cache[url] = response - for callback in callbacks[url] - callback response - delete callbacks[url] - - guessFavicon: (favicon, guessers) -> + skipNonDomains: (domain, url) -> + if 0 <= domain.indexOf(".") then url else "" + + # Various ways in which we guess a favicon's URL. + useKnownFaviconUrl: (domain, favicon) -> favicon.getAttribute "favIconUrl" + guessChromeFaviconUrl: (domain) => @skipNonDomains domain, "chrome://favicon/http://" + domain + guessHttpFaviconUrl: (domain) => @skipNonDomains domain, "http://" + domain + "/favicon.ico" + guessHttpsFaviconUrl: (domain) => @skipNonDomains domain, "https://" + domain + "/favicon.ico" + + # This generates (and *is*) the default favicon; see @googleCacheMissFavicon. So, a priori, we don't need + # this guesser. However, generating these guesses populates the background page's cache, which in turn + # eliminates many fruitless requests. + guessGoogleFaviconUrl: -> "https://www.google.com/profiles/c/favicons?domain=" + + guessFavicon: (favicon, guessers, domain=undefined) -> if 0 < guessers.length - tryNextGuess = => @guessFavicon favicon, guessers[1..] - url = guessers[0] favicon + domain ||= favicon.getAttribute("domain") + tryNextGuess = => @guessFavicon favicon, guessers[1..], domain + url = guessers[0] domain, favicon return tryNextGuess() unless url - @faviconCache url, (response) -> + chrome.runtime.sendMessage {handler: "fetchFavicon", url: url, domain: domain, timeout: 250}, (response) -> if response.data and not response.error - return favicon.src = response.data - tryNextGuess() + favicon.src = response.data + else + tryNextGuess() guessFavicons: -> for favicon in @completionList.getElementsByClassName "vomnibarIcon" favicon.src = @googleCacheMissFavicon # Set a default favicon, Google's small globe. - @guessFavicon favicon, [@useKnownFaviconUrl, @guessChromeFaviconUrl, @guessHttpFaviconUrl, @guessHttpsFaviconUrl] + @guessFavicon favicon, [@useKnownFaviconUrl, @guessChromeFaviconUrl, @guessHttpFaviconUrl, @guessHttpsFaviconUrl, @guessGoogleFaviconUrl] populateUiWithCompletions: (completions) -> # update completion list with the new data diff --git a/lib/utils.coffee b/lib/utils.coffee index a93831d73..64a5e3db4 100644 --- a/lib/utils.coffee +++ b/lib/utils.coffee @@ -150,5 +150,27 @@ globalRoot.extend = (hash1, hash2) -> hash1[key] = hash2[key] hash1 +# A simple synchronous cache. Entries which are used within each timeout period will remain cached. Those +# that aren't will be dropped. +class SimpleCache + constructor: (timeout=1000*60*60*24) -> + @current = {} + @previous = {} + setInterval((=> @rotate()), timeout) + + set: (key, value) -> + delete @previous[key] + @current[key] = value + + get: (key) -> + return @current[key] if @current[key] + return @set(key, @previous[key]) if @previous[key] + return null + + rotate: -> + @previous = @current + @current = {} + root = exports ? window root.Utils = Utils +root.SimpleCache = SimpleCache diff --git a/tests/unit_tests/test_chrome_stubs.coffee b/tests/unit_tests/test_chrome_stubs.coffee index 2abd26c98..2bbd93d8f 100644 --- a/tests/unit_tests/test_chrome_stubs.coffee +++ b/tests/unit_tests/test_chrome_stubs.coffee @@ -91,3 +91,8 @@ exports.chrome = callback() if callback # Now, generate (supposedly asynchronous) notification for listeners. global.chrome.storage.onChanged.callEmpty(key) + +class XMLHttpRequest + open: -> true + +exports.XMLHttpRequest = XMLHttpRequest diff --git a/tests/unit_tests/utils_test.coffee b/tests/unit_tests/utils_test.coffee index 76a023edd..a001f0256 100644 --- a/tests/unit_tests/utils_test.coffee +++ b/tests/unit_tests/utils_test.coffee @@ -60,3 +60,37 @@ context "compare versions", assert.equal -1, Utils.compareVersions("1.40.1", "1.40.2") assert.equal -1, Utils.compareVersions("1.40.1", "1.41") assert.equal 1, Utils.compareVersions("1.41", "1.40") + +context "SimpleCache", + setup -> + @cache = new SimpleCache() + @cache.set "a", 1 + @cache.set "b", 2 + + should "cache values", -> + assert.equal 1, @cache.get "a" + assert.equal 2, @cache.get "b" + + should "return value from set", -> + assert.equal @cache.set("z", 123), 123 + + should "keep cached values when rotated once", -> + assert.equal 1, @cache.get "a" + assert.equal 2, @cache.get "b" + @cache.rotate() + assert.equal 1, @cache.get "a" + assert.equal 2, @cache.get "b" + + should "keep cached values when rotated twice and entries are re-used", -> + @cache.rotate() + assert.equal 1, @cache.get "a" + assert.equal 2, @cache.get "b" + @cache.rotate() + assert.equal 1, @cache.get "a" + assert.equal 2, @cache.get "b" + + should "not keep cached values when rotated twice and entries not are re-used", -> + @cache.rotate() + @cache.rotate() + assert.isFalse @cache.get "a" + assert.isFalse @cache.get "b" From 3c7be6027d69727ee93844d1ae9538e0cf168321 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 8 Nov 2014 17:12:06 +0000 Subject: [PATCH 19/20] Favicons; no guessing URLs. --- background_scripts/completion.coffee | 11 +--- background_scripts/main.coffee | 83 +++++++++------------------- content_scripts/vomnibar.coffee | 47 +++++----------- lib/utils.coffee | 4 +- 4 files changed, 46 insertions(+), 99 deletions(-) diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee index 12f23dddf..deec932bb 100644 --- a/background_scripts/completion.coffee +++ b/background_scripts/completion.coffee @@ -19,15 +19,13 @@ class Suggestion # - computeRelevancyFunction: a function which takes a Suggestion and returns a relevancy score # between [0, 1] # - extraRelevancyData: data (like the History item itself) which may be used by the relevancy function. - constructor: (@queryTerms, @type, @url, @title, @computeRelevancyFunction, @extraRelevancyData, @favIconDomain) -> + constructor: (@queryTerms, @type, @url, @title, @computeRelevancyFunction, @extraRelevancyData) -> @title ||= "" computeRelevancy: -> @relevancy = @computeRelevancyFunction(this) generateHtml: -> return @html if @html - favIconDomain = @favIconDomain or Suggestion.parseDomain @url - favIconUrl = @favIconUrl or "" relevancyHtml = if @showRelevancy then "#{@computeRelevancy()}" else "" # NOTE(philc): We're using these vimium-specific class names so we don't collide with the page's CSS. @html = @@ -37,7 +35,7 @@ class Suggestion #{@highlightTerms(Utils.escapeHtml(@title))}
    - + #{@shortenUrl(@highlightTerms(Utils.escapeHtml(@url)))} #{relevancyHtml}
    @@ -214,9 +212,7 @@ class DomainCompleter domains = @sortDomainsByRelevancy(queryTerms, domainCandidates) return onComplete([]) if domains.length == 0 topDomain = domains[0][0] - suggestion = new Suggestion(queryTerms, "domain", topDomain, null, @computeRelevancy, undefined, - topDomain) - onComplete([suggestion]) + onComplete([new Suggestion(queryTerms, "domain", topDomain, null, @computeRelevancy)]) # Returns a list of domains of the form: [ [domain, relevancy], ... ] @@ -268,7 +264,6 @@ class TabCompleter suggestions = results.map (tab) => suggestion = new Suggestion(queryTerms, "tab", tab.url, tab.title, @computeRelevancy) suggestion.tabId = tab.id - suggestion.favIconUrl = tab.favIconUrl suggestion onComplete(suggestions) diff --git a/background_scripts/main.coffee b/background_scripts/main.coffee index f36001c04..22477cc6a 100644 --- a/background_scripts/main.coffee +++ b/background_scripts/main.coffee @@ -637,74 +637,45 @@ getCurrFrameIndex = (frames) -> frames.length + 1 # Launch an asynchronous HTTP request and send the response as a base64-encoded data: URI. -fetchViaHttpAsBase64 = (request, sender, sendResponse) -> - sendError = (error) -> (-> sendResponse { error: error, errorSource: "fetchViaHttpAsBase64" }) +fetchViaHttpAsBase64 = (request, sendResponse) -> xhr = new XMLHttpRequest() xhr.open("GET", request.url, true) xhr.responseType = "blob" xhr.timeout = request.timeout || 5000 - xhr.ontimeout = sendError "xmlhttprequest-timeout" - xhr.onerror = sendError "xmlhttprequest-error" + xhr.ontimeout = -> sendResponse {} + xhr.onerror = -> sendResponse {} xhr.onload = -> - return sendError("http-error")() unless xhr.status == 200 and xhr.readyState == 4 + return sendResponse({}) unless xhr.status == 200 and xhr.readyState == 4 reader = new window.FileReader() reader.readAsDataURL xhr.response - reader.onerror = sendError "read-error" + reader.onerror = -> sendResponse {} reader.onloadend = -> sendResponse { data: reader.result, type: xhr.response.type } - # Some favicon URL guesses are so bad that they raise exceptions immediately. - try - xhr.send() - catch - sendError("invalid-request")() - return false # sendResponse will not be called asynchronously. + xhr.send() true # sendResponse will be called asynchronously. -# Fetch a favicon and send it to the requester. However, if the response is chrome's default favicon, which -# we don't want to use, then instead send an error. -# -# Favicons are cached based on both the domain of the request and on the URL. Caching on the domain -# short-circuits the current and future guesses (if the domain is in the cache, then we don't need to try any -# URLs). Caching on the URL is primarily for cases where we fail to find *any* usable favicon. In this case, -# we fall back to @guessGoogleFaviconUrl (which doesn't depend upon the domain). So we get a cache hit (even -# for domains we've never seen before), and all such failures end up sharing a single response object, which -# itself is only fetched once. fetchFavicon = do -> - nonExistentFaviconUrl = "chrome://favicon/ThisShouldNotExist-SoWeGetTheChromeDefaultFavicon" - defaultFaviconError = { error: "chrome-default-favicon", errorSource: "fetchFavicon" } - unfetchableError = { error: "unfetchable", errorSource: "fetchFavicon" } - cache = new SimpleCache 1000*60*60*24*7 # Rotate the cache every seven days. - callbacks = {} - chromeDefaultFavicon = "" - - fetcher = (request, sender=undefined, sendResponse=undefined) -> - if 0 == request.url.indexOf "chrome://theme/" # We can't fetch these. - sendResponse unfetchableError - return false # sendResponse will not be called asynchronously. - if response = cache.get request.url - cache.set request.domain, response - if response = (cache.get(request.domain) or cache.get(request.url)) - sendResponse response - false # sendResponse will not be called asynchronously. - else if callbacks[request.url] - callbacks[request.url].push sendResponse - true # sendResponse will be called asynchronously. - else - callbacks[request.url] = [ sendResponse ] - fetchViaHttpAsBase64 request, sender, (response) -> - responseOk = response.type and not response.type.indexOf "image/" - responseOk &&= response.data and response.data != chromeDefaultFavicon - if responseOk - cache.set request.url, response - cache.set request.domain, response - else - response = defaultFaviconError - callback(response) for callback in callbacks[request.url] when callback - delete callbacks[request.url] - - # Fetch chrome's default favicon. - fetcher { url: nonExistentFaviconUrl, domain: nonExistentFaviconUrl }, null, (response) -> - chromeDefaultFavicon = response.data if response.data and not response.error + defaultFavicon = "" + cache = new SimpleCache 1000*60*60*24*8 # Eight days. + + fetcher = (request, _, sendResponse) -> + url = request.url + return sendResponse(response) if response = cache.get url + request.url = "chrome://favicon/#{url}" + fetchViaHttpAsBase64 request, (response) -> + return sendResponse({}) unless response.data + return sendResponse({}) unless response.type + return sendResponse({}) unless response.type.startsWith "image/" + if response.data == defaultFavicon + # Approach: only use the chrome default favicon for chrome URLs. + isChromeUrl = Utils.hasChromePrefix(url) + cache.set(url,response) if isChromeUrl + return sendResponse(if isChromeUrl then response else {}) + sendResponse cache.set url, response + + # Fetch the default chrome favicon. + fetcher {url: "chrome://favicon/does-not-exits---hopefully---kz85S6j"}, null, (response) -> + defaultFavicon = response.data if response.data return fetcher diff --git a/content_scripts/vomnibar.coffee b/content_scripts/vomnibar.coffee index 11c4e117b..981b30be3 100644 --- a/content_scripts/vomnibar.coffee +++ b/content_scripts/vomnibar.coffee @@ -151,39 +151,20 @@ class VomnibarUI @populateUiWithCompletions(completions) callback() if callback - # Google's default favicon; cached here for the benefit of their servers :-) - googleCacheMissFavicon: "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAACXBIWXMAAAsSAAALEgHS3X78AAACiElEQVQ4EaVTzU8TURCf2tJuS7tQtlRb6UKBIkQwkRRSEzkQgyEc6lkOKgcOph78Y+CgjXjDs2i44FXY9AMTlQRUELZapVlouy3d7kKtb0Zr0MSLTvL2zb75eL838xtTvV6H/xELBptMJojeXLCXyobnyog4YhzXYvmCFi6qVSfaeRdXdrfaU1areV5KykmX06rcvzumjY/1ggkR3Jh+bNf1mr8v1D5bLuvR3qDgFbvbBJYIrE1mCIoCrKxsHuzK+Rzvsi29+6DEbTZz9unijEYI8ObBgXOzlcrx9OAlXyDYKUCzwwrDQx1wVDGg089Dt+gR3mxmhcUnaWeoxwMbm/vzDFzmDEKMMNhquRqduT1KwXiGt0vre6iSeAUHNDE0d26NBtAXY9BACQyjFusKuL2Ry+IPb/Y9ZglwuVscdHaknUChqLF/O4jn3V5dP4mhgRJgwSYm+gV0Oi3XrvYB30yvhGa7BS70eGFHPoTJyQHhMK+F0ZesRVVznvXw5Ixv7/C10moEo6OZXbWvlFAF9FVZDOqEABUMRIkMd8GnLwVWg9/RkJF9sA4oDfYQAuzzjqzwvnaRUFxn/X2ZlmGLXAE7AL52B4xHgqAUqrC1nSNuoJkQtLkdqReszz/9aRvq90NOKdOS1nch8TpL555WDp49f3uAMXhACRjD5j4ykuCtf5PP7Fm1b0DIsl/VHGezzP1KwOiZQobFF9YyjSRYQETRENSlVzI8iK9mWlzckpSSCQHVALmN9Az1euDho9Xo8vKGd2rqooA8yBcrwHgCqYR0kMkWci08t/R+W4ljDCanWTg9TJGwGNaNk3vYZ7VUdeKsYJGFNkfSzjXNrSX20s4/h6kB81/271ghG17l+rPTAAAAAElFTkSuQmCC" - - skipNonDomains: (domain, url) -> - if 0 <= domain.indexOf(".") then url else "" - - # Various ways in which we guess a favicon's URL. - useKnownFaviconUrl: (domain, favicon) -> favicon.getAttribute "favIconUrl" - guessChromeFaviconUrl: (domain) => @skipNonDomains domain, "chrome://favicon/http://" + domain - guessHttpFaviconUrl: (domain) => @skipNonDomains domain, "http://" + domain + "/favicon.ico" - guessHttpsFaviconUrl: (domain) => @skipNonDomains domain, "https://" + domain + "/favicon.ico" - - # This generates (and *is*) the default favicon; see @googleCacheMissFavicon. So, a priori, we don't need - # this guesser. However, generating these guesses populates the background page's cache, which in turn - # eliminates many fruitless requests. - guessGoogleFaviconUrl: -> "https://www.google.com/profiles/c/favicons?domain=" - - guessFavicon: (favicon, guessers, domain=undefined) -> - if 0 < guessers.length - domain ||= favicon.getAttribute("domain") - tryNextGuess = => @guessFavicon favicon, guessers[1..], domain - url = guessers[0] domain, favicon - return tryNextGuess() unless url - chrome.runtime.sendMessage {handler: "fetchFavicon", url: url, domain: domain, timeout: 250}, (response) -> - if response.data and not response.error - favicon.src = response.data - else - tryNextGuess() - - guessFavicons: -> - for favicon in @completionList.getElementsByClassName "vomnibarIcon" - favicon.src = @googleCacheMissFavicon # Set a default favicon, Google's small globe. - @guessFavicon favicon, [@useKnownFaviconUrl, @guessChromeFaviconUrl, @guessHttpFaviconUrl, @guessHttpsFaviconUrl, @guessGoogleFaviconUrl] + guessFavicons: do -> + googleCacheMissFavicon = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAACXBIWXMAAAsSAAALEgHS3X78AAACiElEQVQ4EaVTzU8TURCf2tJuS7tQtlRb6UKBIkQwkRRSEzkQgyEc6lkOKgcOph78Y+CgjXjDs2i44FXY9AMTlQRUELZapVlouy3d7kKtb0Zr0MSLTvL2zb75eL838xtTvV6H/xELBptMJojeXLCXyobnyog4YhzXYvmCFi6qVSfaeRdXdrfaU1areV5KykmX06rcvzumjY/1ggkR3Jh+bNf1mr8v1D5bLuvR3qDgFbvbBJYIrE1mCIoCrKxsHuzK+Rzvsi29+6DEbTZz9unijEYI8ObBgXOzlcrx9OAlXyDYKUCzwwrDQx1wVDGg089Dt+gR3mxmhcUnaWeoxwMbm/vzDFzmDEKMMNhquRqduT1KwXiGt0vre6iSeAUHNDE0d26NBtAXY9BACQyjFusKuL2Ry+IPb/Y9ZglwuVscdHaknUChqLF/O4jn3V5dP4mhgRJgwSYm+gV0Oi3XrvYB30yvhGa7BS70eGFHPoTJyQHhMK+F0ZesRVVznvXw5Ixv7/C10moEo6OZXbWvlFAF9FVZDOqEABUMRIkMd8GnLwVWg9/RkJF9sA4oDfYQAuzzjqzwvnaRUFxn/X2ZlmGLXAE7AL52B4xHgqAUqrC1nSNuoJkQtLkdqReszz/9aRvq90NOKdOS1nch8TpL555WDp49f3uAMXhACRjD5j4ykuCtf5PP7Fm1b0DIsl/VHGezzP1KwOiZQobFF9YyjSRYQETRENSlVzI8iK9mWlzckpSSCQHVALmN9Az1euDho9Xo8vKGd2rqooA8yBcrwHgCqYR0kMkWci08t/R+W4ljDCanWTg9TJGwGNaNk3vYZ7VUdeKsYJGFNkfSzjXNrSX20s4/h6kB81/271ghG17l+rPTAAAAAElFTkSuQmCC" + cache = new SimpleCache(0) + -> + for favicon in @completionList.getElementsByClassName "vomnibarIcon" + do (favicon) -> + url = favicon.getAttribute("url").split("?",1)[0].split("#",1)[0] + if cachedFavicon = cache.get url + favicon.src = cachedFavicon + else + favicon.src = googleCacheMissFavicon + chrome.runtime.sendMessage {handler: "fetchFavicon", url: url}, (response) -> + if response.data + favicon.src = cache.set url, response.data populateUiWithCompletions: (completions) -> # update completion list with the new data diff --git a/lib/utils.coffee b/lib/utils.coffee index 64a5e3db4..fc0185e9d 100644 --- a/lib/utils.coffee +++ b/lib/utils.coffee @@ -26,7 +26,7 @@ Utils = -> id += 1 hasChromePrefix: (url) -> - chromePrefixes = [ "about", "view-source", "chrome-extension", "data" ] + chromePrefixes = [ "about", "view-source", "chrome-extension", "data", "chrome-devtools" ] for prefix in chromePrefixes return true if url.startsWith prefix false @@ -156,7 +156,7 @@ class SimpleCache constructor: (timeout=1000*60*60*24) -> @current = {} @previous = {} - setInterval((=> @rotate()), timeout) + setInterval((=> @rotate()), timeout) if timeout set: (key, value) -> delete @previous[key] From 04e8b6ba5db86d5e972949ba48aaafcde79435d8 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 8 Nov 2014 17:31:14 +0000 Subject: [PATCH 20/20] Favicons; fix test, and drop one test that is no longer expected to be true. --- tests/unit_tests/completion_test.coffee | 4 ---- tests/unit_tests/test_chrome_stubs.coffee | 1 + 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/unit_tests/completion_test.coffee b/tests/unit_tests/completion_test.coffee index 898c9b920..28a30e22f 100644 --- a/tests/unit_tests/completion_test.coffee +++ b/tests/unit_tests/completion_test.coffee @@ -260,10 +260,6 @@ context "suggestions", expected = "ninjawords" assert.isTrue suggestion.generateHtml().indexOf(expected) >= 0 - should "shorten urls", -> - suggestion = new Suggestion(["queryterm"], "tab", "http://ninjawords.com/blah", "ninjawords", returns(1)) - assert.equal -1, suggestion.generateHtml().indexOf("http://ninjawords.com/blah") - context "RankingUtils.wordRelevancy", should "score higher in shorter URLs", -> highScore = RankingUtils.wordRelevancy(["stack"], "http://stackoverflow.com/short", "a-title") diff --git a/tests/unit_tests/test_chrome_stubs.coffee b/tests/unit_tests/test_chrome_stubs.coffee index 2bbd93d8f..e329d4a93 100644 --- a/tests/unit_tests/test_chrome_stubs.coffee +++ b/tests/unit_tests/test_chrome_stubs.coffee @@ -94,5 +94,6 @@ exports.chrome = class XMLHttpRequest open: -> true + send: -> true exports.XMLHttpRequest = XMLHttpRequest