Bug 662178 - Simplify timed callbacks. r=rnewman

browser/base/content/syncQuota.js

@@ -93,7 +93,7 @@ let gSyncQuota = {
     if (engines.length) {
       // The 'Weave' object will disappear once the window closes.
       let Service = Weave.Service;
-      Weave.Utils.delay(function() Service.sync(), 0);
+      Weave.Utils.nextTick(function() { Service.sync(); });
     }
     return true;
   },

services/sync/modules/async.js

@@ -255,7 +255,7 @@ let Async = {
         output(err);
         return;
       }
-      Utils.delay(function () { f(items[i], cb); });
+      Utils.nextTick(function () { f(items[i], cb); });
     }
     f(items[i], cb);
   },

services/sync/modules/engines.js

@@ -108,7 +108,7 @@ Tracker.prototype = {
   },
 
   saveChangedIDs: function T_saveChangedIDs() {
-    Utils.delay(function() {
+    Utils.namedTimer(function() {
       Utils.jsonSave("changes/" + this.file, this, this.changedIDs);
     }, 1000, this, "_lazySave");
   },
@@ -202,8 +202,7 @@ Store.prototype = {
 
   _sleep: function _sleep(delay) {
     let cb = Async.makeSyncCallback();
-    this._timer.initWithCallback({notify: cb}, delay,
-                                 Ci.nsITimer.TYPE_ONE_SHOT);
+    this._timer.initWithCallback(cb, delay, Ci.nsITimer.TYPE_ONE_SHOT);
     Async.waitForSyncCallback(cb);
   },
 
@@ -490,7 +489,7 @@ SyncEngine.prototype = {
       return;
     }
     this._toFetch = val;
-    Utils.delay(function () {
+    Utils.namedTimer(function () {
       Utils.jsonSave("toFetch/" + this.name, this, val);
     }, 0, this, "_toFetchDelay");
   },
@@ -512,7 +511,7 @@ SyncEngine.prototype = {
       return;
     }
     this._previousFailed = val;
-    Utils.delay(function () {
+    Utils.namedTimer(function () {
       Utils.jsonSave("failed/" + this.name, this, val);
     }, 0, this, "_previousFailedDelay");
   },

services/sync/modules/engines/forms.js

@@ -343,10 +343,10 @@ FormTracker.prototype = {
       }
 
       // Get the GUID on a delay so that it can be added to the DB first...
-      Utils.delay(function() {
+      Utils.nextTick(function() {
         this._log.trace("Logging form element: " + [name, el.value]);
         this.trackEntry(name, el.value);
-      }, 0, this);
+      }, this);
     }
   }
 };

services/sync/modules/engines/history.js

@@ -447,11 +447,11 @@ HistoryTracker.prototype = {
       return;
     this._log.trace("onVisit: " + uri.spec);
     let self = this;
-    Utils.delay(function() {
+    Utils.nextTick(function() {
       if (self.addChangedID(self._GUIDForUri(uri, true))) {
         self._upScore();
       }
-    }, 0);
+    });
   },
   onDeleteVisits: function onDeleteVisits() {
   },

services/sync/modules/jpakeclient.js

@@ -211,8 +211,8 @@ JPAKEClient.prototype = {
     if (error == JPAKE_ERROR_CHANNEL
         || error == JPAKE_ERROR_NETWORK
         || error == JPAKE_ERROR_NODATA) {
-      Utils.delay(function() { this.observer.onAbort(error); }, 0,
-                  this, "_timer_onAbort");
+      Utils.namedTimer(function() { this.observer.onAbort(error); }, 0,
+                       this, "_timer_onAbort");
     } else {
       this._reportFailure(error, function() { self.observer.onAbort(error); });
     }
@@ -278,8 +278,8 @@ JPAKEClient.prototype = {
 
       // Don't block on UI code.
       let pin = this._secret + this._channel;
-      Utils.delay(function() { this.observer.displayPIN(pin); }, 0,
-                  this, "_timer_displayPIN");
+      Utils.namedTimer(function() { this.observer.displayPIN(pin); }, 0,
+                       this, "_timer_displayPIN");
       callback();
     }));
   },
@@ -308,8 +308,8 @@ JPAKEClient.prototype = {
       // There's no point in returning early here since the next step will
       // always be a GET so let's pause for twice the poll interval.
       this._etag = response.headers["etag"];
-      Utils.delay(function () { callback(); }, this._pollInterval * 2, this,
-                  "_pollTimer");
+      Utils.namedTimer(function () { callback(); }, this._pollInterval * 2,
+                       this, "_pollTimer");
     }));
   },
 
@@ -341,8 +341,8 @@ JPAKEClient.prototype = {
           return;
         }
         this._pollTries += 1;
-        Utils.delay(function() { this._getStep(callback); },
-                    this._pollInterval, this, "_pollTimer");
+        Utils.namedTimer(function() { this._getStep(callback); },
+                         this._pollInterval, this, "_pollTimer");
         return;
       }
       this._pollTries = 0;
@@ -587,8 +587,8 @@ JPAKEClient.prototype = {
   _complete: function _complete() {
     this._log.debug("Exchange completed.");
     this._finished = true;
-    Utils.delay(function () { this.observer.onComplete(this._newData); },
-                0, this, "_timer_onComplete");
+    Utils.namedTimer(function () { this.observer.onComplete(this._newData); },
+                     0, this, "_timer_onComplete");
   }
 
 };

services/sync/modules/resource.js

@@ -599,7 +599,7 @@ ChannelListener.prototype = {
    * Create or push back the abort timer that kills this request
    */
   delayAbort: function delayAbort() {
-    Utils.delay(this.abortRequest, this._timeout, this, "abortTimer");
+    Utils.namedTimer(this.abortRequest, this._timeout, this, "abortTimer");
   },
 
   abortRequest: function abortRequest() {

services/sync/modules/service.js

@@ -438,10 +438,10 @@ WeaveSvc.prototype = {
     // Send an event now that Weave service is ready.  We don't do this
     // synchronously so that observers can import this module before
     // registering an observer.
-    Utils.delay(function() {
+    Utils.nextTick(function() {
       Status.ready = true;
       Svc.Obs.notify("weave:service:ready");
-    }, 0);
+    });
   },
 
   _checkSetup: function WeaveSvc__checkSetup() {
@@ -591,7 +591,7 @@ WeaveSvc.prototype = {
         this._log.trace("Idle time hit, trying to sync");
         Svc.Idle.removeIdleObserver(this, this._idleTime);
         this._idleTime = 0;
-        Utils.delay(function() this.sync(), 0, this);
+        Utils.nextTick(this.sync, this);
         break;
       case "nsPref:changed":
         if (this._ignorePrefObserver)
@@ -604,7 +604,8 @@ WeaveSvc.prototype = {
 
   _handleScoreUpdate: function WeaveSvc__handleScoreUpdate() {
     const SCORE_UPDATE_DELAY = 3000;
-    Utils.delay(this._calculateScore, SCORE_UPDATE_DELAY, this, "_scoreTimer");
+    Utils.namedTimer(this._calculateScore, SCORE_UPDATE_DELAY, this,
+                     "_scoreTimer");
   },
 
   _calculateScore: function WeaveSvc_calculateScoreAndDoStuff() {
@@ -1098,7 +1099,7 @@ WeaveSvc.prototype = {
       return;
 
     if (this._checkSetup() == STATUS_OK && Svc.Prefs.get("autoconnect")) {
-      Utils.delay(this._autoConnect, delay * 1000, this, "_autoTimer");
+      Utils.namedTimer(this._autoConnect, delay * 1000, this, "_autoTimer");
     }
   },
 
@@ -1135,7 +1136,7 @@ WeaveSvc.prototype = {
     let interval = this._calculateBackoff(++attempts, 60 * 1000);
     this._log.debug("Autoconnect failed: " + (reason || Status.login) +
       "; retry in " + Math.ceil(interval / 1000) + " sec.");
-    Utils.delay(function() this._autoConnect(), interval, this, "_autoTimer");
+    Utils.namedTimer(this._autoConnect, interval, this, "_autoTimer");
   },
 
   persistLogin: function persistLogin() {
@@ -1575,7 +1576,7 @@ WeaveSvc.prototype = {
     }
 
     this._log.trace("Next sync in " + Math.ceil(interval / 1000) + " sec.");
-    Utils.delay(function() this.syncOnIdle(), interval, this, "_syncTimer");
+    Utils.namedTimer(this.syncOnIdle, interval, this, "_syncTimer");
 
     // Save the next sync time in-case sync is disabled (logout/offline/etc.)
     this.nextSync = Date.now() + interval;
@@ -1651,7 +1652,7 @@ WeaveSvc.prototype = {
 
     this._log.trace("Setting up heartbeat, next ping in " +
                     Math.ceil(interval / 1000) + " sec.");
-    Utils.delay(function() this._doHeartbeat(), interval, this, "_heartbeatTimer");
+    Utils.namedTimer(this._doHeartbeat, interval, this, "_heartbeatTimer");
   },
 
   /**

services/sync/modules/util.js

@@ -933,17 +933,26 @@ let Utils = {
     });
   },
 
+  /**
+   * Execute a function on the next event loop tick.
+   */
+  nextTick: function nextTick(callback, thisObj) {
+    if (thisObj) {
+      callback = callback.bind(thisObj);
+    }
+    let timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
+    timer.initWithCallback(callback, 0, timer.TYPE_ONE_SHOT);
+  },
+
   /**
    * Return a timer that is scheduled to call the callback after waiting the
    * provided time or as soon as possible. The timer will be set as a property
    * of the provided object with the given timer name.
    */
-  delay: function delay(callback, wait, thisObj, name) {
-    // Default to running right away
-    wait = wait || 0;
-
-    // Use a dummy object if one wasn't provided
-    thisObj = thisObj || {};
+  namedTimer: function delay(callback, wait, thisObj, name) {
+    if (!thisObj || !name) {
+      throw "You must provide both an object and a property name for the timer!";
+    }
 
     // Delay an existing timer if it exists
     if (name in thisObj && thisObj[name] instanceof Ci.nsITimer) {

services/sync/tests/unit/test_async_helpers.js

@@ -192,7 +192,7 @@ add_test(function test_countedCallback() {
 
     // If we call the counted callback again (once this output function is
     // done, that is), then the component callback is not invoked.
-    Utils.delay(function () {
+    Utils.nextTick(function () {
       _("Don't expect component callback.");
       c1("not", "running", "now");
       do_check_eq(2, counter);
@@ -200,7 +200,7 @@ add_test(function test_countedCallback() {
       do_check_eq(2, output);
       do_check_eq("b", context);
       run_next_test();
-    }, 1, this);
+    });
   });
 
   c1(1, "foo", "a");

services/sync/tests/unit/test_async_querySpinningly.js

@@ -11,7 +11,7 @@ function run_test() {
 
   _("Make sure the call is async and allows other events to process");
   let isAsync = false;
-  Utils.delay(function() isAsync = true, 0);
+  Utils.nextTick(function() { isAsync = true; });
   do_check_false(isAsync);
 
   _("Empty out the formhistory table");

services/sync/tests/unit/test_bookmark_livemarks.js

@@ -136,5 +136,5 @@ add_test(function test_livemark_invalid() {
   do_check_eq(-1, store.idForGUID(lmParentRec.id, true));
   
   // Clear event loop.
-  Utils.delay(run_next_test, 0);
+  Utils.nextTick(run_next_test);
 });

services/sync/tests/unit/test_history_store.js

@@ -35,7 +35,7 @@ function onNextTitleChanged(callback) {
     onPageChanged: function onPageChanged() {},
     onTitleChanged: function onTitleChanged() {
       PlacesUtils.history.removeObserver(this);
-      Utils.delay(callback, 0, this);
+      Utils.nextTick(callback);
     },
     onVisit: function onVisit() {},
     onDeleteVisits: function onDeleteVisits() {},

services/sync/tests/unit/test_history_tracker.js

@@ -40,10 +40,10 @@ add_test(function test_empty() {
 add_test(function test_not_tracking(next) {
   _("Create history item. Won't show because we haven't started tracking yet");
   addVisit();
-  Utils.delay(function() {
+  Utils.nextTick(function() {
     do_check_eq([id for (id in tracker.changedIDs)].length, 0);
     run_next_test();
-  }, 0);
+  });
 });
 
 add_test(function test_start_tracking() {
@@ -85,20 +85,20 @@ add_test(function test_stop_tracking() {
   tracker.clearChangedIDs();
   Svc.Obs.notify("weave:engine:stop-tracking");
   addVisit();
-  Utils.delay(function() {
+  Utils.nextTick(function() {
     do_check_eq([id for (id in tracker.changedIDs)].length, 0);
     run_next_test();
-  }, 0);
+  });
 });
 
 add_test(function test_stop_tracking_twice() {
   _("Notifying twice won't do any harm.");
   Svc.Obs.notify("weave:engine:stop-tracking");
   addVisit();
-  Utils.delay(function() {
+  Utils.nextTick(function() {
     do_check_eq([id for (id in tracker.changedIDs)].length, 0);
     run_next_test();
-  }, 0);
+  });
 });
 
 add_test(function cleanup() {

services/sync/tests/unit/test_jpakeclient.js

@@ -157,8 +157,7 @@ add_test(function test_success_receiveNoPIN() {
     displayPIN: function displayPIN(pin) {
       _("Received PIN " + pin + ". Entering it in the other computer...");
       this.cid = pin.slice(JPAKE_LENGTH_SECRET);
-      Utils.delay(function() { snd.sendWithPIN(pin, DATA); }, 0,
-                  this, "_timer");
+      Utils.nextTick(function() { snd.sendWithPIN(pin, DATA); });
     },
     onAbort: function onAbort(error) {
       do_throw("Shouldn't have aborted! " + error);
@@ -223,8 +222,7 @@ add_test(function test_wrongPIN() {
       let new_pin = secret + this.cid;
       _("Received PIN " + pin + ", but I'm entering " + new_pin);
 
-      Utils.delay(function() { snd.sendWithPIN(new_pin, DATA); }, 0,
-                  this, "_timer");
+      Utils.nextTick(function() { snd.sendWithPIN(new_pin, DATA); });
     },
     onAbort: function onAbort(error) {
       do_check_eq(error, JPAKE_ERROR_NODATA);
@@ -258,8 +256,7 @@ add_test(function test_abort_receiver() {
     },
     displayPIN: function displayPIN(pin) {
       this.cid = pin.slice(JPAKE_LENGTH_SECRET);
-      Utils.delay(function() { rec.abort(); },
-                  0, this, "_timer");
+      Utils.nextTick(function() { rec.abort(); });
     }
   });
   rec.receiveNoPIN();
@@ -298,10 +295,9 @@ add_test(function test_abort_sender() {
     displayPIN: function displayPIN(pin) {
       _("Received PIN " + pin + ". Entering it in the other computer...");
       this.cid = pin.slice(JPAKE_LENGTH_SECRET);
-      Utils.delay(function() { snd.sendWithPIN(pin, DATA); }, 0,
-                  this, "_timer");
-      Utils.delay(function() { snd.abort(); },
-                  POLLINTERVAL, this, "_abortTimer");
+      Utils.nextTick(function() { snd.sendWithPIN(pin, DATA); });
+      Utils.namedTimer(function() { snd.abort(); },
+                       POLLINTERVAL, this, "_abortTimer");
     }
   });
   rec.receiveNoPIN();

services/sync/tests/unit/test_utils_namedTimer.js

@@ -0,0 +1,66 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+Cu.import("resource://services-sync/util.js");
+
+function run_test() {
+  run_next_test();
+}
+
+add_test(function test_required_args() {
+  try {
+    Utils.namedTimer(function callback() {
+      do_throw("Shouldn't fire.");
+    }, 0);
+    do_throw("Should have thrown!");
+  } catch(ex) {
+    run_next_test();
+  }
+});
+
+add_test(function test_simple() {
+  _("Test basic properties of Utils.namedTimer.");
+
+  const delay = 200;
+  let that = {};
+  let t0 = Date.now();
+  Utils.namedTimer(function callback(timer) {
+    do_check_eq(this, that);
+    do_check_eq(this._zetimer, null);
+    do_check_true(timer instanceof Ci.nsITimer);
+    do_check_true((Date.now() - t0) >= delay);
+    run_next_test();
+  }, delay, that, "_zetimer");
+});
+
+add_test(function test_delay() {
+  _("Test delaying a timer that hasn't fired yet.");
+  
+  const delay = 100;
+  let that = {};
+  let t0 = Date.now();
+  function callback(timer) {
+    // The 2nd delay counts.
+    do_check_true((Date.now() - t0) >= 2 * delay);
+    run_next_test();
+  }
+  Utils.namedTimer(callback, delay, that, "_zetimer");
+  Utils.namedTimer(callback, 2 * delay, that, "_zetimer");
+  run_next_test();
+});
+
+add_test(function test_clear() {
+  _("Test clearing a timer that hasn't fired yet.");
+
+  const delay = 0;
+  let that = {};
+  Utils.namedTimer(function callback(timer) {
+    do_throw("Shouldn't fire!");
+  }, delay, that, "_zetimer");
+
+  that._zetimer.clear();
+  do_check_eq(that._zetimer, null);
+  Utils.nextTick(run_next_test);
+
+  run_next_test();
+});

services/sync/tests/unit/xpcshell.ini

@@ -92,6 +92,7 @@ tail =
 [test_utils_lock.js]
 [test_utils_makeGUID.js]
 [test_utils_makeURI.js]
+[test_utils_namedTimer.js]
 [test_utils_notify.js]
 [test_utils_passphrase.js]
 [test_utils_pbkdf2.js]