Better vomnibar favicons (fix for domains and security issues)

background_scripts/completion.coffee

@@ -19,34 +19,32 @@ 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, @favIconDomain) ->
     @title ||= ""
 
   computeRelevancy: -> @relevancy = @computeRelevancyFunction(this)
 
   generateHtml: ->
     return @html if @html
-    favIconUrl = @tabFavIconUrl or "#{@getUrlRoot(@url)}/favicon.ico"
+    favIconDomain = @favIconDomain or Suggestion.parseDomain @url
+    favIconUrl = @favIconUrl or ""
     relevancyHtml = if @showRelevancy then "<span class='relevancy'>#{@computeRelevancy()}</span>" else ""
     # NOTE(philc): We're using these vimium-specific class names so we don't collide with the page's CSS.
     @html =
       """
       <div class="vimiumReset vomnibarTopHalf">
          <span class="vimiumReset vomnibarSource">#{@type}</span>
          <span class="vimiumReset vomnibarTitle">#{@highlightTerms(Utils.escapeHtml(@title))}</span>
-       </div>
-       <div class="vimiumReset vomnibarBottomHalf vomnibarIcon"
-            style="background-image: url(#{favIconUrl});">
-        <span class="vimiumReset vomnibarUrl">#{@shortenUrl(@highlightTerms(Utils.escapeHtml(@url)))}</span>
-        #{relevancyHtml}
+      </div>
+      <div class="vimiumReset vomnibarBottomHalf">
+         <img class="vimiumReset vomnibarIcon" domain="#{favIconDomain}" favIconUrl="#{favIconUrl}" src=""/><nobr>
+         <span class="vimiumReset vomnibarUrl">#{@shortenUrl(@highlightTerms(Utils.escapeHtml(@url)))}</span>
+         #{relevancyHtml}
       </div>
       """
 
-  # 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
+  # Static method.
+  @parseDomain: (url) -> url.split("/")[2] || ""
 
   shortenUrl: (url) -> @stripTrailingSlash(url).replace(/^https?:\/\//, "")
 
@@ -216,7 +214,10 @@ 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, undefined,
+      topDomain)
+    onComplete([suggestion])
+
 
   # Returns a list of domains of the form: [ [domain, relevancy], ... ]
   sortDomainsByRelevancy: (queryTerms, domainCandidates) ->
@@ -238,7 +239,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,12 +251,10 @@ 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]
 
-  parseDomain: (url) -> url.split("/")[2] || ""
-
   # Suggestions from the Domain completer have the maximum relevancy. They should be shown first in the list.
   computeRelevancy: -> 1
 
@@ -269,7 +268,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)
 

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])
+    # 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.
-  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,25 @@ 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 = ->
+      console.log request.url
+      sendResponse { data: reader.result, type: xhr.response.type }
+  xhr.send()
+  true # sendResponse will be called asynchronously.
+
 # Port handler mapping
 portHandlers =
   keyDown: handleKeyDown,
@@ -645,6 +679,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'))
 

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 {

content_scripts/vimium_frontend.coffee

@@ -398,10 +398,12 @@ 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()
+      if (isEditable(event.srcElement))
+        event.srcElement.blur()
       exitInsertMode()
       DomUtils.suppressEvent event
       handledKeydownEvents.push event

content_scripts/vomnibar.coffee

@@ -151,11 +151,60 @@ 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"
+  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"
+
+  # 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
+          delete callbacks[url]
+
+  guessFavicon: (favicon, guessers) ->
+    if 0 < guessers.length
+      url = guessers[0](favicon)
+      tryNextGuess = => @guessFavicon favicon, guessers[1..]
+      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
+        tryNextGuess()
+
   populateUiWithCompletions: (completions) ->
     # update completion list with the new data
     @completionList.innerHTML = completions.map((completion) -> "<li>#{completion.html}</li>").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]
     @updateSelection()
 
   update: (updateSynchronously, callback) ->

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()
 
@@ -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", ->