Bug 1319175 - Switch to `JSONFile` for tracker persistence. r?markh draft
authorKit Cambridge <kit@yakshaving.ninja>
Tue, 20 Dec 2016 12:25:27 -0700
changeset 451575 6323e7468a01144f2e2c8118558a39ef72523b69
parent 451278 42c0235774ca34e097e858b6ef796c4d1bd4f122
child 540079 a4fe3abf5bd3e134bf8406cdb80b70e534ac15ba
push id39236
push userbmo:kit@mozilla.com
push dateTue, 20 Dec 2016 19:29:10 +0000
reviewersmarkh
bugs1319175
milestone53.0a1
Bug 1319175 - Switch to `JSONFile` for tracker persistence. r?markh MozReview-Commit-ID: 6nhGe9aHSk3
services/sync/modules/engines.js
services/sync/modules/engines/bookmarks.js
services/sync/modules/engines/extension-storage.js
services/sync/modules/engines/prefs.js
services/sync/modules/engines/tabs.js
services/sync/modules/util.js
services/sync/tests/unit/test_engine.js
services/sync/tests/unit/test_history_tracker.js
services/sync/tests/unit/test_tracker_addChanged.js
--- a/services/sync/modules/engines.js
+++ b/services/sync/modules/engines.js
@@ -9,17 +9,19 @@ this.EXPORTED_SYMBOLS = [
   "Tracker",
   "Store",
   "Changeset"
 ];
 
 var {classes: Cc, interfaces: Ci, results: Cr, utils: Cu} = Components;
 
 Cu.import("resource://services-common/async.js");
+Cu.import("resource://gre/modules/JSONFile.jsm");
 Cu.import("resource://gre/modules/Log.jsm");
+Cu.import("resource://gre/modules/osfile.jsm");
 Cu.import("resource://services-common/observers.js");
 Cu.import("resource://services-sync/constants.js");
 Cu.import("resource://services-sync/identity.js");
 Cu.import("resource://services-sync/record.js");
 Cu.import("resource://services-sync/resource.js");
 Cu.import("resource://services-sync/util.js");
 
 XPCOMUtils.defineLazyModuleGetter(this, "fxAccounts",
@@ -46,19 +48,24 @@ this.Tracker = function Tracker(name, en
   this.engine = engine;
 
   this._log = Log.repository.getLogger("Sync.Tracker." + name);
   let level = Svc.Prefs.get("log.logger.engine." + this.name, "Debug");
   this._log.level = Log.Level[level];
 
   this._score = 0;
   this._ignored = [];
+  this._storage = new JSONFile({
+    path: Utils.jsonFilePath("changes/" + this.file),
+    // We use arrow functions instead of `.bind(this)` so that tests can
+    // easily override these hooks.
+    dataPostProcessor: json => this._dataPostProcessor(json),
+    beforeSave: () => this._beforeSave(),
+  });
   this.ignoreAll = false;
-  this.changedIDs = {};
-  this.loadChangedIDs();
 
   Svc.Obs.add("weave:engine:start-tracking", this);
   Svc.Obs.add("weave:engine:stop-tracking", this);
 
   Svc.Prefs.observe("engine." + this.engine.prefName, this);
 };
 
 Tracker.prototype = {
@@ -71,55 +78,50 @@ Tracker.prototype = {
    * 100: Please sync me ASAP!
    *
    * Setting it to other values should (but doesn't currently) throw an exception
    */
   get score() {
     return this._score;
   },
 
+  // Default to an empty object if the file doesn't exist.
+  _dataPostProcessor(json) {
+    return typeof json == "object" && json || {};
+  },
+
+  // Ensure the Weave storage directory exists before writing the file.
+  _beforeSave() {
+    let basename = OS.Path.dirname(this._storage.path);
+    return OS.File.makeDir(basename, { from: OS.Constants.Path.profileDir });
+  },
+
+  get changedIDs() {
+    Async.promiseSpinningly(this._storage.load());
+    return this._storage.data;
+  },
+
   set score(value) {
     this._score = value;
     Observers.notify("weave:engine:score:updated", this.name);
   },
 
   // Should be called by service everytime a sync has been done for an engine
   resetScore: function () {
     this._score = 0;
   },
 
   persistChangedIDs: true,
 
-  /**
-   * Persist changedIDs to disk at a later date.
-   * Optionally pass a callback to be invoked when the write has occurred.
-   */
-  saveChangedIDs: function (cb) {
+  _saveChangedIDs() {
     if (!this.persistChangedIDs) {
       this._log.debug("Not saving changedIDs.");
       return;
     }
-    Utils.namedTimer(function () {
-      this._log.debug("Saving changed IDs to " + this.file);
-      Utils.jsonSave("changes/" + this.file, this, this.changedIDs, cb);
-    }, 1000, this, "_lazySave");
-  },
-
-  loadChangedIDs: function (cb) {
-    Utils.jsonLoad("changes/" + this.file, this, function(json) {
-      if (json && (typeof(json) == "object")) {
-        this.changedIDs = json;
-      } else if (json !== null) {
-        this._log.warn("Changed IDs file " + this.file + " contains non-object value.");
-        json = null;
-      }
-      if (cb) {
-        cb.call(this, json);
-      }
-    });
+    this._storage.saveSoon();
   },
 
   // ignore/unignore specific IDs.  Useful for ignoring items that are
   // being processed, or that shouldn't be synced.
   // But note: not persisted to disk
 
   ignoreID: function (id) {
     this.unignoreID(id);
@@ -130,17 +132,17 @@ Tracker.prototype = {
     let index = this._ignored.indexOf(id);
     if (index != -1)
       this._ignored.splice(index, 1);
   },
 
   _saveChangedID(id, when) {
     this._log.trace(`Adding changed ID: ${id}, ${JSON.stringify(when)}`);
     this.changedIDs[id] = when;
-    this.saveChangedIDs(this.onSavedChangedIDs);
+    this._saveChangedIDs();
   },
 
   addChangedID: function (id, when) {
     if (!id) {
       this._log.warn("Attempted to add undefined ID to tracker");
       return false;
     }
 
@@ -174,24 +176,24 @@ Tracker.prototype = {
         this._log.debug(`Not removing ignored ID ${id} from tracker`);
         continue;
       }
       if (this.changedIDs[id] != null) {
         this._log.trace("Removing changed ID " + id);
         delete this.changedIDs[id];
       }
     }
-    this.saveChangedIDs();
+    this._saveChangedIDs();
     return true;
   },
 
   clearChangedIDs: function () {
     this._log.trace("Clearing changed ID list");
-    this.changedIDs = {};
-    this.saveChangedIDs();
+    this._storage.data = {};
+    this._saveChangedIDs();
   },
 
   _now() {
     return Date.now() / 1000;
   },
 
   _isTracking: false,
 
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -856,18 +856,16 @@ BookmarksStore.prototype = {
 // all concepts of "add a changed ID." However, it still registers an observer
 // to bump the score, so that changed bookmarks are synced immediately.
 function BookmarksTracker(name, engine) {
   this._batchDepth = 0;
   this._batchSawScoreIncrement = false;
   this._migratedOldEntries = false;
   Tracker.call(this, name, engine);
 
-  delete this.changedIDs; // so our getter/setter takes effect.
-
   Svc.Obs.add("places-shutdown", this);
 }
 BookmarksTracker.prototype = {
   __proto__: Tracker.prototype,
 
   //`_ignore` checks the change source for each observer notification, so we
   // don't want to let the engine ignore all changes during a sync.
   get ignoreAll() {
@@ -904,41 +902,26 @@ BookmarksTracker.prototype = {
   removeChangedID(id) {
     throw new Error("Don't remove IDs from the bookmarks tracker");
   },
 
   // This method is called at various times, so we override with a no-op
   // instead of throwing.
   clearChangedIDs() {},
 
-  saveChangedIDs(cb) {
-    if (cb) {
-      cb();
-    }
-  },
-
-  loadChangedIDs(cb) {
-    if (cb) {
-      cb();
-    }
-  },
-
   promiseChangedIDs() {
     return PlacesSyncUtils.bookmarks.pullChanges();
   },
 
   get changedIDs() {
     throw new Error("Use promiseChangedIDs");
   },
 
   set changedIDs(obj) {
-    // let engine init set it to nothing.
-    if (Object.keys(obj).length != 0) {
-      throw new Error("Don't set initial changed bookmark IDs");
-    }
+    throw new Error("Don't set initial changed bookmark IDs");
   },
 
   // Migrates tracker entries from the old JSON-based tracker to Places. This
   // is called the first time we start tracking changes.
   _migrateOldEntries: Task.async(function* () {
     let existingIDs = yield Utils.jsonLoad("changes/" + this.file, this);
     if (existingIDs === null) {
       // If the tracker file doesn't exist, we don't need to migrate, even if
--- a/services/sync/modules/engines/extension-storage.js
+++ b/services/sync/modules/engines/extension-storage.js
@@ -89,20 +89,16 @@ ExtensionStorageTracker.prototype = {
 
     // Single adds, removes and changes are not so important on their
     // own, so let's just increment score a bit.
     this.score += SCORE_INCREMENT_MEDIUM;
   },
 
   // Override a bunch of methods which don't do anything for us.
   // This is a performance hack.
-  saveChangedIDs: function() {
-  },
-  loadChangedIDs: function() {
-  },
   ignoreID: function() {
   },
   unignoreID: function() {
   },
   addChangedID: function() {
   },
   removeChangedID: function() {
   },
--- a/services/sync/modules/engines/prefs.js
+++ b/services/sync/modules/engines/prefs.js
@@ -221,20 +221,16 @@ PrefTracker.prototype = {
 
   get modified() {
     return Svc.Prefs.get("engine.prefs.modified", false);
   },
   set modified(value) {
     Svc.Prefs.set("engine.prefs.modified", value);
   },
 
-  loadChangedIDs: function loadChangedIDs() {
-    // Don't read changed IDs from disk at start up.
-  },
-
   clearChangedIDs: function clearChangedIDs() {
     this.modified = false;
   },
 
  __prefs: null,
   get _prefs() {
     if (!this.__prefs) {
       this.__prefs = new Preferences();
--- a/services/sync/modules/engines/tabs.js
+++ b/services/sync/modules/engines/tabs.js
@@ -285,20 +285,16 @@ function TabTracker(name, engine) {
   this.onTab = Utils.bind2(this, this.onTab);
   this._unregisterListeners = Utils.bind2(this, this._unregisterListeners);
 }
 TabTracker.prototype = {
   __proto__: Tracker.prototype,
 
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver]),
 
-  loadChangedIDs: function () {
-    // Don't read changed IDs from disk at start up.
-  },
-
   clearChangedIDs: function () {
     this.modified = false;
   },
 
   _topics: ["pageshow", "TabOpen", "TabClose", "TabSelect"],
 
   _registerListenersForWindow: function (window) {
     this._log.trace("Registering tab listeners in window");
--- a/services/sync/modules/util.js
+++ b/services/sync/modules/util.js
@@ -327,31 +327,35 @@ this.Utils = {
    * Take a base64-encoded 128-bit AES key, returning it as five groups of five
    * uppercase alphanumeric characters, separated by hyphens.
    * A.K.A. base64-to-base32 encoding.
    */
   presentEncodedKeyAsSyncKey : function presentEncodedKeyAsSyncKey(encodedKey) {
     return Utils.encodeKeyBase32(atob(encodedKey));
   },
 
+  jsonFilePath(filePath) {
+    return OS.Path.normalize(OS.Path.join(OS.Constants.Path.profileDir, "weave", filePath + ".json"));
+  },
+
   /**
    * Load a JSON file from disk in the profile directory.
    *
    * @param filePath
    *        JSON file path load from profile. Loaded file will be
    *        <profile>/<filePath>.json. i.e. Do not specify the ".json"
    *        extension.
    * @param that
    *        Object to use for logging and "this" for callback.
    * @param callback
    *        Function to process json object as its first argument. If the file
    *        could not be loaded, the first argument will be undefined.
    */
   jsonLoad: Task.async(function*(filePath, that, callback) {
-    let path = OS.Path.join(OS.Constants.Path.profileDir, "weave", filePath + ".json");
+    let path = Utils.jsonFilePath(filePath);
 
     if (that._log) {
       that._log.trace("Loading json from disk: " + filePath);
     }
 
     let json;
 
     try {
--- a/services/sync/tests/unit/test_engine.js
+++ b/services/sync/tests/unit/test_engine.js
@@ -1,11 +1,12 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
+Cu.import("resource://gre/modules/osfile.jsm");
 Cu.import("resource://services-common/observers.js");
 Cu.import("resource://services-sync/engines.js");
 Cu.import("resource://services-sync/service.js");
 Cu.import("resource://services-sync/util.js");
 
 function SteamStore(engine) {
   Store.call(this, "Steam", engine);
   this.wasWiped = false;
@@ -58,79 +59,75 @@ var engineObserver = {
 };
 Observers.add("weave:engine:reset-client:start", engineObserver);
 Observers.add("weave:engine:reset-client:finish", engineObserver);
 Observers.add("weave:engine:wipe-client:start", engineObserver);
 Observers.add("weave:engine:wipe-client:finish", engineObserver);
 Observers.add("weave:engine:sync:start", engineObserver);
 Observers.add("weave:engine:sync:finish", engineObserver);
 
-function run_test() {
-  run_next_test();
-}
-
-add_test(function test_members() {
+add_task(async function test_members() {
   _("Engine object members");
   let engine = new SteamEngine("Steam", Service);
   do_check_eq(engine.Name, "Steam");
   do_check_eq(engine.prefName, "steam");
   do_check_true(engine._store instanceof SteamStore);
   do_check_true(engine._tracker instanceof SteamTracker);
-  run_next_test();
 });
 
-add_test(function test_score() {
+add_task(async function test_score() {
   _("Engine.score corresponds to tracker.score and is readonly");
   let engine = new SteamEngine("Steam", Service);
   do_check_eq(engine.score, 0);
   engine._tracker.score += 5;
   do_check_eq(engine.score, 5);
 
   try {
     engine.score = 10;
   } catch(ex) {
     // Setting an attribute that has a getter produces an error in
     // Firefox <= 3.6 and is ignored in later versions.  Either way,
     // the attribute's value won't change.
   }
   do_check_eq(engine.score, 5);
-  run_next_test();
 });
 
-add_test(function test_resetClient() {
+add_task(async function test_resetClient() {
   _("Engine.resetClient calls _resetClient");
   let engine = new SteamEngine("Steam", Service);
   do_check_false(engine.wasReset);
 
   engine.resetClient();
   do_check_true(engine.wasReset);
   do_check_eq(engineObserver.topics[0], "weave:engine:reset-client:start");
   do_check_eq(engineObserver.topics[1], "weave:engine:reset-client:finish");
 
   engine.wasReset = false;
   engineObserver.reset();
-  run_next_test();
 });
 
-add_test(function test_invalidChangedIDs() {
+add_task(async function test_invalidChangedIDs() {
   _("Test that invalid changed IDs on disk don't end up live.");
   let engine = new SteamEngine("Steam", Service);
   let tracker = engine._tracker;
-  tracker.changedIDs = 5;
-  tracker.saveChangedIDs(function onSaved() {
-      tracker.changedIDs = {placeholder: true};
-      tracker.loadChangedIDs(function onLoaded(json) {
-        do_check_null(json);
-        do_check_true(tracker.changedIDs.placeholder);
-        run_next_test();
-      });
-    });
+
+  await tracker._beforeSave();
+  await OS.File.writeAtomic(tracker._storage.path, new TextEncoder().encode("5"),
+                            { tmpPath: tracker._storage.path + ".tmp" });
+
+  ok(!tracker._storage.dataReady);
+  tracker.changedIDs.placeholder = true;
+  deepEqual(tracker.changedIDs, { placeholder: true },
+    "Accessing changed IDs should load changes from disk as a side effect");
+  ok(tracker._storage.dataReady);
+
+  do_check_true(tracker.changedIDs.placeholder);
 });
 
-add_test(function test_wipeClient() {
+add_task(async function test_wipeClient() {
   _("Engine.wipeClient calls resetClient, wipes store, clears changed IDs");
   let engine = new SteamEngine("Steam", Service);
   do_check_false(engine.wasReset);
   do_check_false(engine._store.wasWiped);
   do_check_true(engine._tracker.addChangedID("a-changed-id"));
   do_check_true("a-changed-id" in engine._tracker.changedIDs);
 
   engine.wipeClient();
@@ -140,61 +137,58 @@ add_test(function test_wipeClient() {
   do_check_eq(engineObserver.topics[0], "weave:engine:wipe-client:start");
   do_check_eq(engineObserver.topics[1], "weave:engine:reset-client:start");
   do_check_eq(engineObserver.topics[2], "weave:engine:reset-client:finish");
   do_check_eq(engineObserver.topics[3], "weave:engine:wipe-client:finish");
 
   engine.wasReset = false;
   engine._store.wasWiped = false;
   engineObserver.reset();
-  run_next_test();
 });
 
-add_test(function test_enabled() {
+add_task(async function test_enabled() {
   _("Engine.enabled corresponds to preference");
   let engine = new SteamEngine("Steam", Service);
   try {
     do_check_false(engine.enabled);
     Svc.Prefs.set("engine.steam", true);
     do_check_true(engine.enabled);
 
     engine.enabled = false;
     do_check_false(Svc.Prefs.get("engine.steam"));
-    run_next_test();
   } finally {
     Svc.Prefs.resetBranch("");
   }
 });
 
-add_test(function test_sync() {
+add_task(async function test_sync() {
   let engine = new SteamEngine("Steam", Service);
   try {
     _("Engine.sync doesn't call _sync if it's not enabled");
     do_check_false(engine.enabled);
     do_check_false(engine.wasSynced);
     engine.sync();
 
     do_check_false(engine.wasSynced);
 
     _("Engine.sync calls _sync if it's enabled");
     engine.enabled = true;
 
     engine.sync();
     do_check_true(engine.wasSynced);
     do_check_eq(engineObserver.topics[0], "weave:engine:sync:start");
     do_check_eq(engineObserver.topics[1], "weave:engine:sync:finish");
-    run_next_test();
   } finally {
     Svc.Prefs.resetBranch("");
     engine.wasSynced = false;
     engineObserver.reset();
   }
 });
 
-add_test(function test_disabled_no_track() {
+add_task(async function test_disabled_no_track() {
   _("When an engine is disabled, its tracker is not tracking.");
   let engine = new SteamEngine("Steam", Service);
   let tracker = engine._tracker;
   do_check_eq(engine, tracker.engine);
 
   do_check_false(engine.enabled);
   do_check_false(tracker._isTracking);
   do_check_empty(tracker.changedIDs);
@@ -209,11 +203,9 @@ add_test(function test_disabled_no_track
   do_check_true(tracker._isTracking);
   do_check_empty(tracker.changedIDs);
 
   tracker.addChangedID("abcdefghijkl");
   do_check_true(0 < tracker.changedIDs["abcdefghijkl"]);
   Svc.Prefs.set("engine." + engine.prefName, false);
   do_check_false(tracker._isTracking);
   do_check_empty(tracker.changedIDs);
-
-  run_next_test();
 });
--- a/services/sync/tests/unit/test_history_tracker.js
+++ b/services/sync/tests/unit/test_history_tracker.js
@@ -13,27 +13,60 @@ Cu.import("resource://services-sync/util
 Service.engineManager.clear();
 Service.engineManager.register(HistoryEngine);
 var engine = Service.engineManager.get("history");
 var tracker = engine._tracker;
 
 // Don't write out by default.
 tracker.persistChangedIDs = false;
 
+// Places notifies history observers asynchronously, so `addVisits` might return
+// before the tracker receives the notification. This helper registers an
+// observer that resolves once the expected notification fires.
+async function promiseVisit(expectedType, expectedURI) {
+  return new Promise(resolve => {
+    function done(type, uri) {
+      if (uri.equals(expectedURI) && type == expectedType) {
+        PlacesUtils.history.removeObserver(observer);
+        resolve();
+      }
+    }
+    let observer = {
+      onVisit(uri) {
+        done("added", uri);
+      },
+      onBeginUpdateBatch() {},
+      onEndUpdateBatch() {},
+      onTitleChanged() {},
+      onFrecencyChanged() {},
+      onManyFrecenciesChanged() {},
+      onDeleteURI(uri) {
+        done("removed", uri);
+      },
+      onClearHistory() {},
+      onPageChanged() {},
+      onDeleteVisits() {},
+    };
+    PlacesUtils.history.addObserver(observer, false);
+  });
+}
+
 async function addVisit(suffix, referrer = null, transition = PlacesUtils.history.TRANSITION_LINK) {
   let uriString = "http://getfirefox.com/" + suffix;
   let uri = Utils.makeURI(uriString);
   _("Adding visit for URI " + uriString);
 
+  let visitAddedPromise = promiseVisit("added", uri);
   await PlacesTestUtils.addVisits({
     uri,
     visitDate: Date.now() * 1000,
     transition,
     referrer,
   });
+  await visitAddedPromise;
 
   return uri;
 }
 
 function run_test() {
   initTestLogging("Trace");
   Log.repository.getLogger("Sync.Tracker.History").level = Log.Level.Trace;
   run_next_test();
@@ -95,23 +128,30 @@ add_task(async function test_not_trackin
   await addVisit("not_tracking");
   await verifyTrackerEmpty();
 
   await cleanup();
 });
 
 add_task(async function test_start_tracking() {
   _("Add hook for save completion.");
-  let savePromise = new Promise(resolve => {
+  let savePromise = new Promise((resolve, reject) => {
     tracker.persistChangedIDs = true;
-    tracker.onSavedChangedIDs = function () {
-      // Turn this back off.
-      tracker.persistChangedIDs = false;
-      delete tracker.onSavedChangedIDs;
-      resolve();
+    let save = tracker._storage._save;
+    tracker._storage._save = async function() {
+      try {
+        await save.call(this);
+        resolve();
+      } catch (ex) {
+        reject(ex);
+      } finally {
+        // Turn this back off.
+        tracker.persistChangedIDs = false;
+        tracker._storage._save = save;
+      }
     };
   });
 
   _("Tell the tracker to start tracking changes.");
   await startTracking();
   let scorePromise = promiseOneObserver("weave:engine:score:updated");
   await addVisit("start_tracking");
   await scorePromise;
@@ -151,19 +191,20 @@ add_task(async function test_track_delet
 
   // This isn't present because we weren't tracking when it was visited.
   await addVisit("track_delete");
   let uri = Utils.makeURI("http://getfirefox.com/track_delete");
   let guid = engine._store.GUIDForUri(uri);
   await verifyTrackerEmpty();
 
   await startTracking();
+  let visitRemovedPromise = promiseVisit("removed", uri);
   let scorePromise = promiseOneObserver("weave:engine:score:updated");
   PlacesUtils.history.removePage(uri);
-  await scorePromise;
+  await Promise.all([scorePromise, visitRemovedPromise]);
 
   await verifyTrackedItems([guid]);
   do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE);
 
   await cleanup();
 });
 
 add_task(async function test_dont_track_expiration() {
@@ -172,32 +213,33 @@ add_task(async function test_dont_track_
   let guidToExpire = engine._store.GUIDForUri(uriToExpire);
   let uriToRemove = await addVisit("to_remove");
   let guidToRemove = engine._store.GUIDForUri(uriToRemove);
 
   await resetTracker();
   await verifyTrackerEmpty();
 
   await startTracking();
+  let visitRemovedPromise = promiseVisit("removed", uriToRemove);
   let scorePromise = promiseOneObserver("weave:engine:score:updated");
 
   // Observe expiration.
   Services.obs.addObserver(function onExpiration(aSubject, aTopic, aData) {
     Services.obs.removeObserver(onExpiration, aTopic);
     // Remove the remaining page to update its score.
     PlacesUtils.history.removePage(uriToRemove);
   }, PlacesUtils.TOPIC_EXPIRATION_FINISHED, false);
 
   // Force expiration of 1 entry.
   Services.prefs.setIntPref("places.history.expiration.max_pages", 0);
   Cc["@mozilla.org/places/expiration;1"]
     .getService(Ci.nsIObserver)
     .observe(null, "places-debug-start-expiration", 1);
 
-  await scorePromise;
+  await Promise.all([scorePromise, visitRemovedPromise]);
   await verifyTrackedItems([guidToRemove]);
 
   await cleanup();
 });
 
 add_task(async function test_stop_tracking() {
   _("Let's stop tracking again.");
   await stopTracking();
--- a/services/sync/tests/unit/test_tracker_addChanged.js
+++ b/services/sync/tests/unit/test_tracker_addChanged.js
@@ -1,20 +1,16 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 Cu.import("resource://services-sync/engines.js");
 Cu.import("resource://services-sync/service.js");
 Cu.import("resource://services-sync/util.js");
 
-function run_test() {
-  run_next_test();
-}
-
-add_test(function test_tracker_basics() {
+add_task(async function test_tracker_basics() {
   let tracker = new Tracker("Tracker", Service);
   tracker.persistChangedIDs = false;
 
   let id = "the_id!";
 
   _("Make sure nothing exists yet..");
   do_check_eq(tracker.changedIDs[id], null);
 
@@ -28,32 +24,34 @@ add_test(function test_tracker_basics() 
 
   _("An older time will not replace the newer 10");
   tracker.addChangedID(id, 5);
   do_check_eq(tracker.changedIDs[id], 10);
 
   _("Adding without time defaults to current time");
   tracker.addChangedID(id);
   do_check_true(tracker.changedIDs[id] > 10);
-
-  run_next_test();
 });
 
-add_test(function test_tracker_persistence() {
+add_task(async function test_tracker_persistence() {
   let tracker = new Tracker("Tracker", Service);
   let id = "abcdef";
 
   tracker.persistChangedIDs = true;
-  tracker.onSavedChangedIDs = function () {
-    _("IDs saved.");
-    do_check_eq(5, tracker.changedIDs[id]);
 
-    // Verify the write by reading the file back.
-    Utils.jsonLoad("changes/tracker", this, function (json) {
-      do_check_eq(5, json[id]);
-      tracker.persistChangedIDs = false;
-      delete tracker.onSavedChangedIDs;
-      run_next_test();
-    });
-  };
+  let promiseSave = new Promise((resolve, reject) => {
+    let save = tracker._storage._save;
+    tracker._storage._save = function() {
+      save.call(tracker._storage).then(resolve, reject);
+    };
+  });
 
   tracker.addChangedID(id, 5);
+
+  await promiseSave;
+
+  _("IDs saved.");
+  do_check_eq(5, tracker.changedIDs[id]);
+
+  let json = await Utils.jsonLoad("changes/tracker", tracker);
+  do_check_eq(5, json[id]);
+  tracker.persistChangedIDs = false;
 });