Fix #135 and Cleanup Timers

Cleanup timers to make clear what is going on. Simplify based on the fact we are inserting into the _timers in sorted order.  Remove multiple sources of truth.  Simplify stale timer handling (should we move this check in other spots like run)?

Remove duplicate test of hanging issue, the other one with the spin is more reliable.

Author: krisselden

.jshintrc

@@ -1,29 +1,11 @@
 {
   "predef": [
-    "define",
-    "console",
-    "require",
-    "requireModule",
-    "equal",
-    "notEqual",
-    "notStrictEqual",
-    "test",
-    "asyncTest",
-    "testBoth",
-    "testWithDefault",
-    "raises",
-    "throws",
-    "deepEqual",
-    "start",
-    "stop",
-    "ok",
-    "strictEqual",
-    "module",
-    "expect",
-    "process"
+    "setTimeout", "clearTimeout"
   ],
+  "esnext": true,
   "node" : false,
-  "browser" : true,
+  "browser" : false,
+
   "boss" : true,
   "curly": false,
   "debug": false,
@@ -46,5 +28,6 @@
   "strict": false,
   "white": false,
   "eqnull": true,
-  "esnext": true
+  "trailing": true,
+  "unused": "vars"
 }

lib/backburner.js

@@ -25,13 +25,23 @@ export default function Backburner(queueNames, options) {
   this.instanceStack = [];
   this._debouncees = [];
   this._throttlers = [];
-  this._timers = [];
   this._eventCallbacks = {
     end: [],
     begin: []
   };
+
+  this._timerTimeoutId = undefined;
+  this._timers = [];
+
+  var _this = this;
+  this._boundRunExpiredTimers = function () {
+    _this._runExpiredTimers();
+  };
 }
 
+// ms of delay before we conclude a timeout was lost
+var TIMEOUT_STALLED_THRESHOLD = 1000;
+
 Backburner.prototype = {
   begin: function() {
     var options = this.options;
@@ -359,12 +369,27 @@ Backburner.prototype = {
       }
     }
 
+    return this._setTimeout(fn, executeAt);
+  },
+
+  _setTimeout(fn, executeAt) {
+    if (this._timers.length === 0) {
+      this._timers.push(executeAt, fn);
+      this._installTimerTimeout();
+      return fn;
+    }
+
+    this._reinstallStalledTimerTimeout();
+
     // find position to insert
     var i = searchTimer(executeAt, this._timers);
 
     this._timers.splice(i, 0, executeAt, fn);
 
-    updateLaterTimer(this, executeAt, wait);
+    // we should be the new earliest timer if i == 0
+    if (i === 0) {
+      this._reinstallTimerTimeout();
+    }
 
     return fn;
   },
@@ -464,20 +489,17 @@ Backburner.prototype = {
   },
 
   cancelTimers: function() {
-    var clearItems = function(item) {
+    function clearItems(item) {
       clearTimeout(item[2]);
-    };
+    }
 
     each(this._throttlers, clearItems);
     this._throttlers = [];
 
     each(this._debouncees, clearItems);
     this._debouncees = [];
 
-    if (this._laterTimer) {
-      clearTimeout(this._laterTimer);
-      this._laterTimer = null;
-    }
+    this._clearTimerTimeout();
     this._timers = [];
 
     if (this._autorun) {
@@ -490,7 +512,7 @@ Backburner.prototype = {
     return !!this._timers.length || !!this._debouncees.length || !!this._throttlers.length || this._autorun;
   },
 
-  cancel: function(timer) {
+  cancel: function (timer) {
     var timerType = typeof timer;
 
     if (timer && timerType === 'object' && timer.queue && timer.method) { // we're cancelling a deferOnce
@@ -500,13 +522,7 @@ Backburner.prototype = {
         if (this._timers[i + 1] === timer) {
           this._timers.splice(i, 2); // remove the two elements
           if (i === 0) {
-            if (this._laterTimer) { // Active timer? Then clear timer and reset for future timer
-              clearTimeout(this._laterTimer);
-              this._laterTimer = null;
-            }
-            if (this._timers.length > 0) { // Update to next available timer when available
-              updateLaterTimer(this, this._timers[0], this._timers[0] - now());
-            }
+            this._reinstallTimerTimeout();
           }
           return true;
         }
@@ -538,6 +554,67 @@ Backburner.prototype = {
     }
 
     return false;
+  },
+
+  _runExpiredTimers: function () {
+    this._timerTimeoutId = undefined;
+    this.run(this, this._scheduleExpiredTimers);
+  },
+
+  _scheduleExpiredTimers: function () {
+    var n = now();
+    var timers = this._timers;
+    var i = 0;
+    var l = timers.length;
+    for (; i < l; i += 2) {
+      var executeAt = timers[i];
+      var fn = timers[i+1];
+      if (executeAt <= n) {
+        this.schedule(this.options.defaultQueue, null, fn);
+      } else {
+        break;
+      }
+    }
+    timers.splice(0, i);
+    this._installTimerTimeout();
+  },
+
+  _reinstallStalledTimerTimeout: function () {
+    if (!this._timerTimeoutId) {
+      return;
+    }
+    // if we have a timer we should always have a this._timerTimeoutId
+    var minExpiresAt = this._timers[0];
+    var delay = now() - minExpiresAt;
+    // threshold of a second before we assume that the currently
+    // installed timeout will not run, so we don't constantly reinstall
+    // timeouts that are delayed but good still
+    if (delay < TIMEOUT_STALLED_THRESHOLD) {
+      return;
+    }
+  },
+
+  _reinstallTimerTimeout: function () {
+    this._clearTimerTimeout();
+    this._installTimerTimeout();
+  },
+
+  _clearTimerTimeout: function () {
+    if (!this._timerTimeoutId) {
+      return;
+    }
+    clearTimeout(this._timerTimeoutId);
+    this._timerTimeoutId = undefined;
+  },
+
+  _installTimerTimeout: function () {
+    if (!this._timers.length) {
+      return;
+    }
+    var minExpiresAt = this._timers[0];
+    var n = now();
+    var wait = Math.max(0, minExpiresAt - n);
+    this._timerTimeoutId = setTimeout(this._boundRunExpiredTimers, wait);
   }
 };
 
@@ -565,51 +642,6 @@ function createAutorun(backburner) {
   });
 }
 
-function updateLaterTimer(backburner, executeAt, wait) {
-  var n = now();
-  if (!backburner._laterTimer || executeAt < backburner._laterTimerExpiresAt || backburner._laterTimerExpiresAt < n) {
-
-    if (backburner._laterTimer) {
-      // Clear when:
-      // - Already expired
-      // - New timer is earlier
-      clearTimeout(backburner._laterTimer);
-
-      if (backburner._laterTimerExpiresAt < n) { // If timer was never triggered
-        // Calculate the left-over wait-time
-        wait = Math.max(0, executeAt - n);
-      }
-    }
-
-    backburner._laterTimer = platform.setTimeout(function() {
-      backburner._laterTimer = null;
-      backburner._laterTimerExpiresAt = null;
-      executeTimers(backburner);
-    }, wait);
-
-    backburner._laterTimerExpiresAt = n + wait;
-  }
-}
-
-function executeTimers(backburner) {
-  var n = now();
-  var fns, i, l;
-
-  backburner.run(function() {
-    i = searchTimer(n, backburner._timers);
-
-    fns = backburner._timers.splice(0, i);
-
-    for (i = 1, l = fns.length; i < l; i += 2) {
-      backburner.schedule(backburner.options.defaultQueue, null, fns[i]);
-    }
-  });
-
-  if (backburner._timers.length) {
-    updateLaterTimer(backburner, backburner._timers[0], backburner._timers[0] - n);
-  }
-}
-
 function findDebouncee(target, method, debouncees) {
   return findItem(target, method, debouncees);
 }

lib/backburner.umd

@@ -1,3 +1,4 @@
+/* globals module */
 import Backburner from './backburner';
 import platform from './backburner/platform';
 

lib/backburner/deferred-action-queues.js

@@ -45,10 +45,9 @@ DeferredActionQueues.prototype = {
   flush: function() {
     var queues = this.queues;
     var queueNames = this.queueNames;
-    var queueName, queue, queueItems, priorQueueNameIndex;
+    var queueName, queue;
     var queueNameIndex = 0;
     var numberOfQueues = queueNames.length;
-    var options = this.options;
 
     while (queueNameIndex < numberOfQueues) {
       queueName = queueNames[queueNameIndex];

lib/backburner/queue.js

@@ -80,7 +80,6 @@ Queue.prototype = {
   },
 
   pushUnique: function(target, method, args, stack) {
-    var queue = this._queue, currentTarget, currentMethod, i, l;
     var KEY = this.globalOptions.GUID_KEY;
 
     if (target && KEY) {

tests/set-timeout-test.js

@@ -260,42 +260,6 @@ test('setTimeout doesn\'t trigger twice with earlier setTimeout', function() {
   }, 100);
 });
 
-test('setTimeout doesn\'t hang when timeout is unfulfilled', function() { // See issue #86
-  expect(3);
-
-  var bb = new Backburner(['one']);
-  var called1 = 0;
-  var called2 = 0;
-  var calls = 0;
-  var oldRun = bb.run;
-
-  // Count run() calls and relay them to original function
-  bb.run = function () {
-    calls++;
-    oldRun.apply(bb, arguments);
-  };
-
-  bb.setTimeout(function() {
-    called1++;
-  }, 0);
-
-  // Manually pass time and clear timeout
-  clearTimeout(bb._laterTimer);
-  bb._laterTimerExpiresAt = +(new Date()) - 5000;
-
-  bb.setTimeout(function() {
-    called2++;
-  }, 10);
-
-  stop();
-  setTimeout(function () {
-    start();
-    equal(called1, 1, 'timeout 1 was called once');
-    equal(called2, 1, 'timeout 2 was called once');
-    equal(calls, 1, 'run() was called once'); // both at once
-  }, 50);
-});
-
 test('setTimeout with two Backburner instances', function() {
   expect(8);