Bug 1636365 - Split out persistence from `Tracker` into `LegacyTracker`. r=markh,rfkelly

The tracker base class currently does two things: bump the score in
response to observer notifications, and store a list of changed IDs.
The bookmarks, form autofill, and now bridged trackers need to hack
around this to opt out of persistence, since they handle change
tracking in the storage layer.

This commit keeps the score logic in `Tracker`, but moves all the
persistence code into an intermediate `LegacyTracker` class, and
changes all engines that need persistence to inherit from it.

`ignoreAll` is more interesting. We want new-style stores to emit
observer notifications with change sources, so that the tracker knows
to ignore changes made by Sync. Ignoring all observer notifications
during a sync is a blunter version of this. But, not every new store
supports change sources, so we reimplement `ignoreAll` manually for
ones that don't.

Differential Revision: https://phabricator.services.mozilla.com/D74374

browser/extensions/formautofill/FormAutofillSync.jsm

@@ -220,16 +220,6 @@ FormAutofillTracker.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() {
-    return false;
-  },
-
-  // Define an empty setter so that the engine doesn't throw a `TypeError`
-  // setting a read-only property.
-  set ignoreAll(value) {},
-
   onStart() {
     Services.obs.addObserver(this, "formautofill-storage-changed");
   },
@@ -237,31 +227,6 @@ FormAutofillTracker.prototype = {
   onStop() {
     Services.obs.removeObserver(this, "formautofill-storage-changed");
   },
-
-  // We never want to persist changed IDs, as the changes are already stored
-  // in FormAutofillStorage
-  persistChangedIDs: false,
-
-  // Ensure we aren't accidentally using the base persistence.
-  get changedIDs() {
-    throw new Error("changedIDs isn't meaningful for this engine");
-  },
-
-  set changedIDs(obj) {
-    throw new Error("changedIDs isn't meaningful for this engine");
-  },
-
-  addChangedID(id, when) {
-    throw new Error("Don't add IDs to the autofill tracker");
-  },
-
-  removeChangedID(id) {
-    throw new Error("Don't remove IDs from the autofill tracker");
-  },
-
-  // This method is called at various times, so we override with a no-op
-  // instead of throwing.
-  clearChangedIDs() {},
 };
 
 // This uses the same conventions as BookmarkChangeset in

services/sync/modules-testing/rotaryengine.js

@@ -11,7 +11,7 @@ var EXPORTED_SYMBOLS = [
   "RotaryTracker",
 ];
 
-const { Store, SyncEngine, Tracker } = ChromeUtils.import(
+const { Store, SyncEngine, LegacyTracker } = ChromeUtils.import(
   "resource://services-sync/engines.js"
 );
 const { CryptoWrapper } = ChromeUtils.import(
@@ -93,11 +93,10 @@ RotaryStore.prototype = {
 };
 
 function RotaryTracker(name, engine) {
-  Tracker.call(this, name, engine);
+  LegacyTracker.call(this, name, engine);
 }
 RotaryTracker.prototype = {
-  __proto__: Tracker.prototype,
-  persistChangedIDs: false,
+  __proto__: LegacyTracker.prototype,
 };
 
 function RotaryEngine(service) {

services/sync/modules/engines.js

@@ -6,6 +6,7 @@ var EXPORTED_SYMBOLS = [
   "EngineManager",
   "SyncEngine",
   "Tracker",
+  "LegacyTracker",
   "Store",
   "Changeset",
 ];
@@ -46,16 +47,15 @@ function ensureDirectory(path) {
   return OS.File.makeDir(basename, { from: OS.Constants.Path.profileDir });
 }
 
-/*
+/**
  * Trackers are associated with a single engine and deal with
  * listening for changes to their particular data type.
  *
- * There are two things they keep track of:
- * 1) A score, indicating how urgently the engine wants to sync
- * 2) A list of IDs for all the changed items that need to be synced
- * and updating their 'score', indicating how urgently they
- * want to sync.
- *
+ * The base `Tracker` only supports listening for changes, and bumping the score
+ * to indicate how urgently the engine wants to sync. It does not persist any
+ * data. Engines that track changes directly in the storage layer (like
+ * bookmarks, bridged engines, addresses, and credit cards) or only upload a
+ * single record (tabs and preferences) should subclass `Tracker`.
  */
 function Tracker(name, engine) {
   if (!engine) {
@@ -63,24 +63,28 @@ function Tracker(name, engine) {
   }
 
   name = name || "Unnamed";
-  this.name = this.file = name.toLowerCase();
+  this.name = name.toLowerCase();
   this.engine = engine;
 
   this._log = Log.repository.getLogger(`Sync.Engine.${name}.Tracker`);
 
   this._score = 0;
-  this._ignored = [];
-  this._storage = new JSONFile({
-    path: Utils.jsonFilePath("changes/" + this.file),
-    dataPostProcessor: json => this._dataPostProcessor(json),
-    beforeSave: () => this._beforeSave(),
-  });
-  this.ignoreAll = false;
 
   this.asyncObserver = Async.asyncObserver(this, this._log);
 }
 
 Tracker.prototype = {
+  // New-style trackers use change sources to filter out changes made by Sync in
+  // observer notifications, so we don't want to let the engine ignore all
+  // changes during a sync.
+  get ignoreAll() {
+    return false;
+  },
+
+  // Define an empty setter so that the engine doesn't throw a `TypeError`
+  // setting a read-only property.
+  set ignoreAll(value) {},
+
   /*
    * Score can be called as often as desired to decide which engines to sync
    *
@@ -95,16 +99,6 @@ Tracker.prototype = {
     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() {
-    return ensureDirectory(this._storage.path);
-  },
-
   set score(value) {
     this._score = value;
     Observers.notify("weave:engine:score:updated", this.name);
@@ -115,7 +109,131 @@ Tracker.prototype = {
     this._score = 0;
   },
 
-  persistChangedIDs: true,
+  // Unsupported, and throws a more descriptive error to ensure callers aren't
+  // accidentally using persistence.
+  async getChangedIDs() {
+    throw new TypeError("This tracker doesn't store changed IDs");
+  },
+
+  // Also unsupported.
+  async addChangedID(id, when) {
+    throw new TypeError("Can't add changed ID to this tracker");
+  },
+
+  // Ditto.
+  async removeChangedID(...ids) {
+    throw new TypeError("Can't remove changed IDs from this tracker");
+  },
+
+  // This method is called at various times, so we override with a no-op
+  // instead of throwing.
+  clearChangedIDs() {},
+
+  _now() {
+    return Date.now() / 1000;
+  },
+
+  _isTracking: false,
+
+  start() {
+    if (!this.engineIsEnabled()) {
+      return;
+    }
+    this._log.trace("start().");
+    if (!this._isTracking) {
+      this.onStart();
+      this._isTracking = true;
+    }
+  },
+
+  async stop() {
+    this._log.trace("stop().");
+    if (this._isTracking) {
+      await this.asyncObserver.promiseObserversComplete();
+      this.onStop();
+      this._isTracking = false;
+    }
+  },
+
+  // Override these in your subclasses.
+  onStart() {},
+  onStop() {},
+  async observe(subject, topic, data) {},
+
+  engineIsEnabled() {
+    if (!this.engine) {
+      // Can't tell -- we must be running in a test!
+      return true;
+    }
+    return this.engine.enabled;
+  },
+
+  /**
+   * Starts or stops listening for changes depending on the associated engine's
+   * enabled state.
+   *
+   * @param {Boolean} engineEnabled Whether the engine was enabled.
+   */
+  async onEngineEnabledChanged(engineEnabled) {
+    if (engineEnabled == this._isTracking) {
+      return;
+    }
+
+    if (engineEnabled) {
+      this.start();
+    } else {
+      await this.stop();
+      this.clearChangedIDs();
+    }
+  },
+
+  async finalize() {
+    await this.stop();
+  },
+};
+
+/*
+ * A tracker that persists a list of IDs for all changed items that need to be
+ * synced. This is 🚨 _extremely deprecated_ 🚨 and only kept around for current
+ * engines. ⚠️ Please **don't use it** for new engines! ⚠️
+ *
+ * Why is this kind of external change tracking deprecated? Because it causes
+ * consistency issues due to missed notifications, interrupted syncs, and the
+ * tracker's view of what changed diverging from the data store's.
+ */
+function LegacyTracker(name, engine) {
+  Tracker.call(this, name, engine);
+
+  this._ignored = [];
+  this.file = this.name;
+  this._storage = new JSONFile({
+    path: Utils.jsonFilePath("changes/" + this.file),
+    dataPostProcessor: json => this._dataPostProcessor(json),
+    beforeSave: () => this._beforeSave(),
+  });
+  this._ignoreAll = false;
+}
+
+LegacyTracker.prototype = {
+  __proto__: Tracker.prototype,
+
+  get ignoreAll() {
+    return this._ignoreAll;
+  },
+
+  set ignoreAll(value) {
+    this._ignoreAll = value;
+  },
+
+  // 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() {
+    return ensureDirectory(this._storage.path);
+  },
 
   async getChangedIDs() {
     await this._storage.load();
@@ -123,10 +241,6 @@ Tracker.prototype = {
   },
 
   _saveChangedIDs() {
-    if (!this.persistChangedIDs) {
-      this._log.debug("Not saving changedIDs.");
-      return;
-    }
     this._storage.saveSoon();
   },
 
@@ -196,72 +310,20 @@ Tracker.prototype = {
         delete changedIDs[id];
       }
     }
-    await this._saveChangedIDs();
+    this._saveChangedIDs();
     return true;
   },
 
-  async clearChangedIDs() {
+  clearChangedIDs() {
     this._log.trace("Clearing changed ID list");
     this._storage.data = {};
-    await this._saveChangedIDs();
-  },
-
-  _now() {
-    return Date.now() / 1000;
-  },
-
-  _isTracking: false,
-
-  start() {
-    if (!this.engineIsEnabled()) {
-      return;
-    }
-    this._log.trace("start().");
-    if (!this._isTracking) {
-      this.onStart();
-      this._isTracking = true;
-    }
-  },
-
-  async stop() {
-    this._log.trace("stop().");
-    if (this._isTracking) {
-      await this.asyncObserver.promiseObserversComplete();
-      this.onStop();
-      this._isTracking = false;
-    }
-  },
-
-  // Override these in your subclasses.
-  onStart() {},
-  onStop() {},
-  async observe(subject, topic, data) {},
-
-  engineIsEnabled() {
-    if (!this.engine) {
-      // Can't tell -- we must be running in a test!
-      return true;
-    }
-    return this.engine.enabled;
-  },
-
-  async onEngineEnabledChanged(engineEnabled) {
-    if (engineEnabled == this._isTracking) {
-      return;
-    }
-
-    if (engineEnabled) {
-      this.start();
-    } else {
-      await this.stop();
-      await this.clearChangedIDs();
-    }
+    this._saveChangedIDs();
   },
 
   async finalize() {
     // Persist all pending tracked changes to disk, and wait for the final write
     // to finish.
-    await this.stop();
+    await super.finalize();
     this._saveChangedIDs();
     await this._storage.finalize();
   },
@@ -720,10 +782,7 @@ function SyncEngine(name, service) {
     this,
     "_enabled",
     `services.sync.engine.${this.prefName}`,
-    false,
-    (data, previous, latest) =>
-      // We do not await on the promise onEngineEnabledChanged returns.
-      this._tracker.onEngineEnabledChanged(latest)
+    false
   );
   XPCOMUtils.defineLazyPreferenceGetter(
     this,
@@ -768,6 +827,8 @@ function SyncEngine(name, service) {
   // Additionally, we use this as the set of items to upload for bookmark
   // repair reponse, which has similar constraints.
   this._needWeakUpload = new Map();
+
+  this.asyncObserver = Async.asyncObserver(this, this._log);
 }
 
 // Enumeration to define approaches to handling bad records.
@@ -840,6 +901,7 @@ SyncEngine.prototype = {
   async initialize() {
     await this._toFetchStorage.load();
     await this._previousFailedStorage.load();
+    Svc.Prefs.observe(`engine.${this.prefName}`, this.asyncObserver);
     this._log.debug("SyncEngine initialized", this.name);
   },
 
@@ -898,6 +960,18 @@ SyncEngine.prototype = {
     return this._tracker.stop();
   },
 
+  // Listens for engine enabled state changes, and updates the tracker's state.
+  // This is an async observer because the tracker waits on all its async
+  // observers to finish when it's stopped.
+  async observe(subject, topic, data) {
+    if (
+      topic == "nsPref:changed" &&
+      data == `services.sync.engine.${this.prefName}`
+    ) {
+      await this._tracker.onEngineEnabledChanged(this._enabled);
+    }
+  },
+
   async sync() {
     if (!this.enabled) {
       return false;
@@ -1117,7 +1191,7 @@ SyncEngine.prototype = {
     this._modified.replace(initialChanges);
     // Clear the tracker now. If the sync fails we'll add the ones we failed
     // to upload back.
-    await this._tracker.clearChangedIDs();
+    this._tracker.clearChangedIDs();
     this._tracker.resetScore();
 
     this._log.info(
@@ -2113,7 +2187,7 @@ SyncEngine.prototype = {
     this._tracker.ignoreAll = true;
     await this._store.wipe();
     this._tracker.ignoreAll = false;
-    await this._tracker.clearChangedIDs();
+    this._tracker.clearChangedIDs();
   },
 
   /**
@@ -2126,6 +2200,8 @@ SyncEngine.prototype = {
   },
 
   async finalize() {
+    Svc.Prefs.ignore(`engine.${this.prefName}`, this.asyncObserver);
+    await this.asyncObserver.promiseObserversComplete();
     await this._tracker.finalize();
     await this._toFetchStorage.finalize();
     await this._previousFailedStorage.finalize();

services/sync/modules/engines/addons.js

@@ -46,7 +46,7 @@ const { AddonUtils } = ChromeUtils.import(
 const { AddonsReconciler } = ChromeUtils.import(
   "resource://services-sync/addonsreconciler.js"
 );
-const { Store, SyncEngine, Tracker } = ChromeUtils.import(
+const { Store, SyncEngine, LegacyTracker } = ChromeUtils.import(
   "resource://services-sync/engines.js"
 );
 const { CryptoWrapper } = ChromeUtils.import(
@@ -733,10 +733,10 @@ AddonsStore.prototype = {
  * It hooks up to the reconciler and receives notifications directly from it.
  */
 function AddonsTracker(name, engine) {
-  Tracker.call(this, name, engine);
+  LegacyTracker.call(this, name, engine);
 }
 AddonsTracker.prototype = {
-  __proto__: Tracker.prototype,
+  __proto__: LegacyTracker.prototype,
 
   get reconciler() {
     return this.engine._reconciler;

services/sync/modules/engines/bookmarks.js

@@ -1388,20 +1388,6 @@ function BookmarksTracker(name, engine) {
 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() {
-    return false;
-  },
-
-  // Define an empty setter so that the engine doesn't throw a `TypeError`
-  // setting a read-only property.
-  set ignoreAll(value) {},
-
-  // We never want to persist changed IDs, as the changes are already stored
-  // in Places.
-  persistChangedIDs: false,
-
   onStart() {
     PlacesUtils.bookmarks.addObserver(this, true);
     this._placesListener = new PlacesWeakCallbackWrapper(
@@ -1427,27 +1413,10 @@ BookmarksTracker.prototype = {
     Svc.Obs.remove("bookmarks-restore-failed", this);
   },
 
-  // Ensure we aren't accidentally using the base persistence.
-  addChangedID(id, when) {
-    throw new Error("Don't add IDs to the bookmarks tracker");
-  },
-
-  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() {},
-
   async getChangedIDs() {
     return PlacesSyncUtils.bookmarks.pullChanges();
   },
 
-  set changedIDs(obj) {
-    throw new Error("Don't set initial changed bookmark IDs");
-  },
-
   observe(subject, topic, data) {
     switch (topic) {
       case "bookmarks-restore-begin":
@@ -1477,7 +1446,6 @@ BookmarksTracker.prototype = {
 
   QueryInterface: ChromeUtils.generateQI([
     Ci.nsINavBookmarkObserver,
-    Ci.nsINavBookmarkObserver_MOZILLA_1_9_1_ADDITIONS,
     Ci.nsISupportsWeakReference,
   ]),
 

services/sync/modules/engines/clients.js

@@ -31,7 +31,7 @@ const {
   SINGLE_USER_THRESHOLD,
   SYNC_API_VERSION,
 } = ChromeUtils.import("resource://services-sync/constants.js");
-const { Store, SyncEngine, Tracker } = ChromeUtils.import(
+const { Store, SyncEngine, LegacyTracker } = ChromeUtils.import(
   "resource://services-sync/engines.js"
 );
 const { CryptoWrapper } = ChromeUtils.import(
@@ -1221,10 +1221,10 @@ ClientStore.prototype = {
 };
 
 function ClientsTracker(name, engine) {
-  Tracker.call(this, name, engine);
+  LegacyTracker.call(this, name, engine);
 }
 ClientsTracker.prototype = {
-  __proto__: Tracker.prototype,
+  __proto__: LegacyTracker.prototype,
 
   _enabled: false,
 

services/sync/modules/engines/extension-storage.js

@@ -104,10 +104,19 @@ ExtensionStorageEngine.prototype = {
 
 function ExtensionStorageTracker(name, engine) {
   Tracker.call(this, name, engine);
+  this._ignoreAll = false;
 }
 ExtensionStorageTracker.prototype = {
   __proto__: Tracker.prototype,
 
+  get ignoreAll() {
+    return this._ignoreAll;
+  },
+
+  set ignoreAll(value) {
+    this._ignoreAll = value;
+  },
+
   onStart() {
     Svc.Obs.add("ext.storage.sync-changed", this.asyncObserver);
   },
@@ -129,12 +138,4 @@ ExtensionStorageTracker.prototype = {
     // 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.
-  ignoreID() {},
-  unignoreID() {},
-  addChangedID() {},
-  removeChangedID() {},
-  clearChangedIDs() {},
 };

services/sync/modules/engines/forms.js

@@ -4,7 +4,7 @@
 
 var EXPORTED_SYMBOLS = ["FormEngine", "FormRec", "FormValidator"];
 
-const { Store, SyncEngine, Tracker } = ChromeUtils.import(
+const { Store, SyncEngine, LegacyTracker } = ChromeUtils.import(
   "resource://services-sync/engines.js"
 );
 const { CryptoWrapper } = ChromeUtils.import(
@@ -216,10 +216,10 @@ FormStore.prototype = {
 };
 
 function FormTracker(name, engine) {
-  Tracker.call(this, name, engine);
+  LegacyTracker.call(this, name, engine);
 }
 FormTracker.prototype = {
-  __proto__: Tracker.prototype,
+  __proto__: LegacyTracker.prototype,
 
   QueryInterface: ChromeUtils.generateQI([
     Ci.nsIObserver,

services/sync/modules/engines/history.js

@@ -17,7 +17,7 @@ const {
   SCORE_INCREMENT_SMALL,
   SCORE_INCREMENT_XLARGE,
 } = ChromeUtils.import("resource://services-sync/constants.js");
-const { Store, SyncEngine, Tracker } = ChromeUtils.import(
+const { Store, SyncEngine, LegacyTracker } = ChromeUtils.import(
   "resource://services-sync/engines.js"
 );
 const { CryptoWrapper } = ChromeUtils.import(
@@ -517,10 +517,10 @@ HistoryStore.prototype = {
 };
 
 function HistoryTracker(name, engine) {
-  Tracker.call(this, name, engine);
+  LegacyTracker.call(this, name, engine);
 }
 HistoryTracker.prototype = {
-  __proto__: Tracker.prototype,
+  __proto__: LegacyTracker.prototype,
 
   onStart() {
     this._log.info("Adding Places observer.");

services/sync/modules/engines/passwords.js

@@ -14,7 +14,7 @@ const { SCORE_INCREMENT_XLARGE } = ChromeUtils.import(
 const { CollectionValidator } = ChromeUtils.import(
   "resource://services-sync/collection_validator.js"
 );
-const { Store, SyncEngine, Tracker } = ChromeUtils.import(
+const { Store, SyncEngine, LegacyTracker } = ChromeUtils.import(
   "resource://services-sync/engines.js"
 );
 const { Svc, Utils } = ChromeUtils.import("resource://services-sync/util.js");
@@ -363,10 +363,10 @@ PasswordStore.prototype = {
 };
 
 function PasswordTracker(name, engine) {
-  Tracker.call(this, name, engine);
+  LegacyTracker.call(this, name, engine);
 }
 PasswordTracker.prototype = {
-  __proto__: Tracker.prototype,
+  __proto__: LegacyTracker.prototype,
 
   onStart() {
     Svc.Obs.add("passwordmgr-storage-changed", this.asyncObserver);

services/sync/modules/engines/prefs.js

@@ -359,15 +359,28 @@ PrefStore.prototype = {
   async wipe() {
     this._log.trace("Ignoring wipe request");
   },
+
+  async trackRemainingChanges() {
+    this._tracker.modified = true;
+  },
 };
 
 function PrefTracker(name, engine) {
   Tracker.call(this, name, engine);
+  this._ignoreAll = false;
   Svc.Obs.add("profile-before-change", this.asyncObserver);
 }
 PrefTracker.prototype = {
   __proto__: Tracker.prototype,
 
+  get ignoreAll() {
+    return this._ignoreAll;
+  },
+
+  set ignoreAll(value) {
+    this._ignoreAll = value;
+  },
+
   get modified() {
     return Svc.Prefs.get("engine.prefs.modified", false);
   },

services/sync/modules/engines/tabs.js

@@ -276,6 +276,10 @@ TabStore.prototype = {
   async update(record) {
     this._log.trace("Ignoring tab updates as local ones win");
   },
+
+  async trackRemainingChanges() {
+    this._tracker.modified = true;
+  },
 };
 
 function TabTracker(name, engine) {

services/sync/tests/unit/head_errorhandler_common.js

@@ -16,6 +16,7 @@ var {
   Store,
   SyncEngine,
   Tracker,
+  LegacyTracker,
 } = ChromeUtils.import("resource://services-sync/engines.js");
 var {
   ABORT_SYNC_COMMAND,

services/sync/tests/unit/test_addons_tracker.js

@@ -62,9 +62,6 @@ add_task(async function setup() {
   store = engine._store;
   tracker = engine._tracker;
 
-  // Don't write out by default.
-  tracker.persistChangedIDs = false;
-
   await cleanup();
 });
 

services/sync/tests/unit/test_bookmark_tracker.js

@@ -20,7 +20,6 @@ add_task(async function setup() {
   engine = Service.engineManager.get("bookmarks");
   store = engine._store;
   tracker = engine._tracker;
-  tracker.persistChangedIDs = false;
 });
 
 // Test helpers.

services/sync/tests/unit/test_engine.js

@@ -23,11 +23,10 @@ SteamStore.prototype = {
 };
 
 function SteamTracker(name, engine) {
-  Tracker.call(this, name || "Steam", engine);
+  LegacyTracker.call(this, name || "Steam", engine);
 }
 SteamTracker.prototype = {
-  __proto__: Tracker.prototype,
-  persistChangedIDs: false,
+  __proto__: LegacyTracker.prototype,
 };
 
 function SteamEngine(name, service) {
@@ -80,6 +79,7 @@ async function cleanup(engine) {
 add_task(async function test_members() {
   _("Engine object members");
   let engine = new SteamEngine("Steam", Service);
+  await engine.initialize();
   Assert.equal(engine.Name, "Steam");
   Assert.equal(engine.prefName, "steam");
   Assert.ok(engine._store instanceof SteamStore);
@@ -89,6 +89,7 @@ add_task(async function test_members() {
 add_task(async function test_score() {
   _("Engine.score corresponds to tracker.score and is readonly");
   let engine = new SteamEngine("Steam", Service);
+  await engine.initialize();
   Assert.equal(engine.score, 0);
   engine._tracker.score += 5;
   Assert.equal(engine.score, 5);
@@ -106,6 +107,7 @@ add_task(async function test_score() {
 add_task(async function test_resetClient() {
   _("Engine.resetClient calls _resetClient");
   let engine = new SteamEngine("Steam", Service);
+  await engine.initialize();
   Assert.ok(!engine.wasReset);
 
   await engine.resetClient();
@@ -119,6 +121,7 @@ add_task(async function test_resetClient() {
 add_task(async function test_invalidChangedIDs() {
   _("Test that invalid changed IDs on disk don't end up live.");
   let engine = new SteamEngine("Steam", Service);
+  await engine.initialize();
   let tracker = engine._tracker;
 
   await tracker._beforeSave();
@@ -145,6 +148,7 @@ add_task(async function test_invalidChangedIDs() {
 add_task(async function test_wipeClient() {
   _("Engine.wipeClient calls resetClient, wipes store, clears changed IDs");
   let engine = new SteamEngine("Steam", Service);
+  await engine.initialize();
   Assert.ok(!engine.wasReset);
   Assert.ok(!engine._store.wasWiped);
   Assert.ok(await engine._tracker.addChangedID("a-changed-id"));
@@ -167,6 +171,7 @@ add_task(async function test_wipeClient() {
 add_task(async function test_enabled() {
   _("Engine.enabled corresponds to preference");
   let engine = new SteamEngine("Steam", Service);
+  await engine.initialize();
   try {
     Assert.ok(!engine.enabled);
     Svc.Prefs.set("engine.steam", true);
@@ -181,6 +186,7 @@ add_task(async function test_enabled() {
 
 add_task(async function test_sync() {
   let engine = new SteamEngine("Steam", Service);
+  await engine.initialize();
   try {
     _("Engine.sync doesn't call _sync if it's not enabled");
     Assert.ok(!engine.enabled);
@@ -204,6 +210,7 @@ add_task(async function test_sync() {
 add_task(async function test_disabled_no_track() {
   _("When an engine is disabled, its tracker is not tracking.");
   let engine = new SteamEngine("Steam", Service);
+  await engine.initialize();
   let tracker = engine._tracker;
   Assert.equal(engine, tracker.engine);
 

services/sync/tests/unit/test_forms_tracker.js

@@ -11,8 +11,6 @@ add_task(async function run_test() {
   let engine = new FormEngine(Service);
   await engine.initialize();
   let tracker = engine._tracker;
-  // Don't do asynchronous writes.
-  tracker.persistChangedIDs = false;
 
   let changes = await tracker.getChangedIDs();
   do_check_empty(changes);

services/sync/tests/unit/test_history_tracker.js

@@ -17,9 +17,6 @@ add_task(async function setup() {
   await Service.engineManager.register(HistoryEngine);
   engine = Service.engineManager.get("history");
   tracker = engine._tracker;
-
-  // Don't write out by default.
-  tracker.persistChangedIDs = false;
 });
 
 async function verifyTrackerEmpty() {
@@ -78,7 +75,6 @@ add_task(async function test_not_tracking() {
 add_task(async function test_start_tracking() {
   _("Add hook for save completion.");
   let savePromise = new Promise((resolve, reject) => {
-    tracker.persistChangedIDs = true;
     let save = tracker._storage._save;
     tracker._storage._save = async function() {
       try {
@@ -87,8 +83,6 @@ add_task(async function test_start_tracking() {
       } catch (ex) {
         reject(ex);
       } finally {
-        // Turn this back off.
-        tracker.persistChangedIDs = false;
         tracker._storage._save = save;
       }
     };

services/sync/tests/unit/test_password_tracker.js

@@ -15,9 +15,6 @@ add_task(async function setup() {
   engine = Service.engineManager.get("passwords");
   store = engine._store;
   tracker = engine._tracker;
-
-  // Don't do asynchronous writes.
-  tracker.persistChangedIDs = false;
 });
 
 add_task(async function test_tracking() {

services/sync/tests/unit/test_prefs_tracker.js

@@ -10,9 +10,6 @@ add_task(async function run_test() {
   let engine = Service.engineManager.get("prefs");
   let tracker = engine._tracker;
 
-  // Don't write out by default.
-  tracker.persistChangedIDs = false;
-
   let prefs = new Preferences();
 
   try {

services/sync/tests/unit/test_tab_tracker.js

@@ -50,7 +50,6 @@ add_task(async function run_test() {
 
   _("We assume that tabs have changed at startup.");
   let tracker = engine._tracker;
-  tracker.persistChangedIDs = false;
 
   Assert.ok(tracker.modified);
   Assert.ok(

services/sync/tests/unit/test_telemetry.js

@@ -24,12 +24,11 @@ SteamStore.prototype = {
 };
 
 function SteamTracker(name, engine) {
-  Tracker.call(this, name || "Steam", engine);
+  LegacyTracker.call(this, name || "Steam", engine);
 }
 
 SteamTracker.prototype = {
-  __proto__: Tracker.prototype,
-  persistChangedIDs: false,
+  __proto__: LegacyTracker.prototype,
 };
 
 function SteamEngine(service) {

services/sync/tests/unit/test_tracker_addChanged.js

@@ -4,8 +4,7 @@
 const { Service } = ChromeUtils.import("resource://services-sync/service.js");
 
 add_task(async function test_tracker_basics() {
-  let tracker = new Tracker("Tracker", Service);
-  tracker.persistChangedIDs = false;
+  let tracker = new LegacyTracker("Tracker", Service);
 
   let id = "the_id!";
 
@@ -35,11 +34,9 @@ add_task(async function test_tracker_basics() {
 });
 
 add_task(async function test_tracker_persistence() {
-  let tracker = new Tracker("Tracker", Service);
+  let tracker = new LegacyTracker("Tracker", Service);
   let id = "abcdef";
 
-  tracker.persistChangedIDs = true;
-
   let promiseSave = new Promise((resolve, reject) => {
     let save = tracker._storage._save;
     tracker._storage._save = function() {
@@ -57,5 +54,4 @@ add_task(async function test_tracker_persistence() {
 
   let json = await Utils.jsonLoad("changes/tracker", tracker);
   Assert.equal(5, json[id]);
-  tracker.persistChangedIDs = false;
 });