Search completion; rework SimpleCache.

3 files changed, 35 insertions, 32 deletions

diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee
index 94109b84..024ea54c 100644
--- a/background_scripts/completion.coffee
+++ b/background_scripts/completion.coffee
@@ -355,11 +355,14 @@ class TabCompleter
       tabRecency.recencyScore(suggestion.tabId)
 
 class SearchEngineCompleter
-  searchEngineConfig: null
+  searchEngines: null
+
+  cancel: ->
+    CompletionEngines.cancel()
 
   refresh: (port) ->
     # Load and parse the search-engine configuration.
-    @searchEngineConfig = new AsyncDataFetcher (callback) ->
+    @searchEngines = new AsyncDataFetcher (callback) ->
       engines = {}
       for line in Settings.get("searchEngines").split "\n"
         line = line.trim()
@@ -373,18 +376,18 @@ class SearchEngineCompleter
           searchUrl: tokens[1]
           description: description
 
-      # Deliver the resulting engines lookup table.
+      # Deliver the resulting engines AsyncDataFetcher lookup table.
       callback engines
 
       # Let the vomnibar know the custom search engine keywords.
       port.postMessage
-        handler: "customSearchEngineKeywords"
+        handler: "keywords"
         keywords: key for own key of engines
 
   filter: ({ queryTerms, query }, onComplete) ->
     return onComplete [] if queryTerms.length == 0
 
-    @searchEngineConfig.use (engines) =>
+    @searchEngines.use (engines) =>
       keyword = queryTerms[0]
 
       { custom, searchUrl, description, queryTerms } =
@@ -482,9 +485,6 @@ class SearchEngineCompleter
             count = Math.min 6, Math.max 3, MultiCompleter.maxResults - existingSuggestions.length
             onComplete suggestions[...count]
 
-  cancel: ->
-    CompletionEngines.cancel()
-
 # A completer which calls filter() on many completers, aggregates the results, ranks them, and returns the top
 # 10. Queries from the vomnibar frontend script come through a multi completer.
 class MultiCompleter
diff --git a/lib/utils.coffee b/lib/utils.coffee
index 51b16351..b0abfd8f 100644
--- a/lib/utils.coffee
+++ b/lib/utils.coffee
@@ -212,24 +212,45 @@ globalRoot.extend = (hash1, hash2) ->
 
 # A simple cache. Entries used within two expiry periods are retained, otherwise they are discarded.
 # At most 2 * @entries entries are retained.
+#
+# Note.  We need to be careful with @timer.  If all references to a cache are lost, then eventually its
+# contents must be garbage collected, which will not happen if there are active timers.
 class SimpleCache
   # expiry: expiry time in milliseconds (default, one hour)
   # entries: maximum number of entries in @cache (there may be this many entries in @previous, too)
   constructor: (@expiry = 60 * 60 * 1000, @entries = 1000) ->
     @cache = {}
-    @rotate() # Force start the rotation timer.
+    @previous = {}
+    @timer = null
 
   rotate: ->
     @previous = @cache
     @cache = {}
     # We reset the timer every time the cache is rotated (which could be because a previous timer expired, or
-    # because the number of @entries was exceeded.
+    # because the number of @entries was exceeded).  We only restart the timer if the cache is not empty.
     clearTimeout @timer if @timer?
-    @timer = Utils.setTimeout @expiry, => @rotate()
+    @timer = null
+    @checkTimer() if 0 < Object.keys(@previous).length
+
+  checkTimer: ->
+    unless @timer?
+      @timer = Utils.setTimeout @expiry, => @rotate()
 
   has: (key) ->
     (key of @cache) or key of @previous
 
+  # Set value, and return that value.  If value is null, then delete key.
+  set: (key, value = null) ->
+    @checkTimer()
+    if value?
+      @cache[key] = value
+      delete @previous[key]
+      @rotate() if @entries < Object.keys(@cache).length
+    else
+      delete @cache[key]
+      delete @previous[key]
+    value
+
   get: (key) ->
     if key of @cache
       @cache[key]
@@ -242,27 +263,9 @@ class SimpleCache
     @rotate()
     @rotate()
 
-  # Because of the timer, we can't just let these caches go out of scope and have the garbage collector
-  # harvest them.  Whenever they may fall out of use, we need to remove the timer. @rotate() can be used to
-  # restart the cache.
-  suspend: ->
-    clearTimeout @timer if @timer?
-    @timer = null
-
-  # Set value, and return that value.  If value is null, then delete key.
-  set: (key, value = null) ->
-    if value?
-      @cache[key] = value
-      delete @previous[key]
-      @rotate() if @entries < Object.keys(@cache).length
-    else
-      delete @cache[key]
-      delete @previous[key]
-    value
-
 # This is a simple class for the common case where we want to use some data value which may be immediately
-# available, or we may have to wait.  It implements the use-immediately-or-wait queue, and calls the function
-# to fetch the data asynchronously.
+# available, or for which we may have to wait.  It implements the use-immediately-or-wait queue, and calls the
+# function to fetch the data asynchronously.
 class AsyncDataFetcher
   constructor: (fetch) ->
     @data = null
diff --git a/pages/vomnibar.coffee b/pages/vomnibar.coffee
index b53028ca..b8ada233 100644
--- a/pages/vomnibar.coffee
+++ b/pages/vomnibar.coffee
@@ -352,7 +352,7 @@ class BackgroundCompleter
 
     @port.onMessage.addListener (msg) =>
       switch msg.handler
-        when "customSearchEngineKeywords"
+        when "keywords"
           @keywords = msg.keywords
           @lastUI.setKeywords @keywords
         when "completions"