Bug 1305563 - Add a `BufferedBookmarksEngine` that can be toggled with a pref. r=markh,tcsc draft
authorKit Cambridge <kit@yakshaving.ninja>
Thu, 07 Sep 2017 23:48:57 -0700
changeset 679412 92f68b00d2fd8b87b4cfce13e6266d12bb49938a
parent 679411 be018073f9b1e0bd8abe139f5f73c1d46f8c9d0b
child 735597 f073eb8ce3b2e40bf21857504b0a829d8499ce6e
push id84218
push userbmo:kit@mozilla.com
push dateThu, 12 Oct 2017 16:56:40 +0000
reviewersmarkh, tcsc
bugs1305563
milestone58.0a1
Bug 1305563 - Add a `BufferedBookmarksEngine` that can be toggled with a pref. r=markh,tcsc This patch adds a new bookmarks engine, pref'd off, that writes to the buffer instead of Places. MozReview-Commit-ID: 7idBa03kOzm
services/sync/modules/engines.js
services/sync/modules/engines/bookmarks.js
services/sync/modules/service.js
services/sync/services-sync.js
services/sync/tests/unit/test_bookmark_engine.js
services/sync/tests/unit/test_bookmark_repair.js
services/sync/tests/unit/test_bookmark_repair_responder.js
services/sync/tests/unit/test_bookmark_smart_bookmarks.js
services/sync/tests/unit/test_bookmark_tracker.js
services/sync/tests/unit/test_telemetry.js
tools/lint/eslint/modules.json
--- a/services/sync/modules/engines.js
+++ b/services/sync/modules/engines.js
@@ -846,16 +846,24 @@ SyncEngine.prototype = {
   get cryptoKeysURL() {
     return this.storageURL + "crypto/keys";
   },
 
   get metaURL() {
     return this.storageURL + "meta/global";
   },
 
+  async getLastSync() {
+    return this.lastSync;
+  },
+
+  async setLastSync(lastSync) {
+    this.lastSync = lastSync;
+  },
+
   get syncID() {
     // Generate a random syncID if we don't have one
     let syncID = Svc.Prefs.get(this.name + ".syncID", "");
     return syncID == "" ? this.syncID = Utils.makeGUID() : syncID;
   },
   set syncID(value) {
     Svc.Prefs.set(this.name + ".syncID", value);
   },
@@ -1029,36 +1037,39 @@ SyncEngine.prototype = {
 
     // Save objects that need to be uploaded in this._modified. We also save
     // the timestamp of this fetch in this.lastSyncLocal. As we successfully
     // upload objects we remove them from this._modified. If an error occurs
     // or any objects fail to upload, they will remain in this._modified. At
     // the end of a sync, or after an error, we add all objects remaining in
     // this._modified to the tracker.
     this.lastSyncLocal = Date.now();
-    let initialChanges;
-    if (this.lastSync) {
-      initialChanges = await this.pullNewChanges();
-    } else {
-      this._log.debug("First sync, uploading all items");
-      initialChanges = await this.pullAllChanges();
-    }
+    let initialChanges = await this.pullChanges();
     this._modified.replace(initialChanges);
     // Clear the tracker now. If the sync fails we'll add the ones we failed
     // to upload back.
     this._tracker.clearChangedIDs();
     this._tracker.resetScore();
 
     this._log.info(this._modified.count() +
                    " outgoing items pre-reconciliation");
 
     // Keep track of what to delete at the end of sync
     this._delete = {};
   },
 
+  async pullChanges() {
+    let lastSync = await this.getLastSync();
+    if (lastSync) {
+      return this.pullNewChanges();
+    }
+    this._log.debug("First sync, uploading all items");
+    return this.pullAllChanges();
+  },
+
   /**
    * A tiny abstraction to make it easier to test incoming record
    * application.
    */
   itemSource() {
     return new Collection(this.engineURL, this._recordObj, this.service);
   },
 
@@ -1076,17 +1087,18 @@ SyncEngine.prototype = {
     if (!newitems) {
       newitems = this.itemSource();
     }
 
     if (this._defaultSort) {
       newitems.sort = this._defaultSort;
     }
 
-    newitems.newer = this.lastSync;
+    let lastSync = await this.getLastSync();
+    newitems.newer = lastSync;
     newitems.full  = true;
     newitems.limit = batchSize;
 
     // applied    => number of items that should be applied.
     // failed     => number of items that failed in this sync.
     // newFailed  => number of items that failed for the first time in this sync.
     // reconciled => number of items that were reconciled.
     let count = {applied: 0, failed: 0, newFailed: 0, reconciled: 0};
@@ -1237,17 +1249,17 @@ SyncEngine.prototype = {
       }
 
       if (applyBatch.length == self.applyIncomingBatchSize) {
         await doApplyBatch.call(self);
       }
     };
 
     // Only bother getting data from the server if there's new things
-    if (this.lastModified == null || this.lastModified > this.lastSync) {
+    if (this.lastModified == null || this.lastModified > lastSync) {
       let { response, records } = await newitems.getBatched();
       if (!response.success) {
         response.failureCode = ENGINE_DOWNLOAD_FAIL;
         throw response;
       }
 
       let maybeYield = Async.jankYielder();
       for (let record of records) {
@@ -1267,17 +1279,17 @@ SyncEngine.prototype = {
       // or commenting the entire block causes no tests to fail.)
       // See bug 1368951 comment 3 for some insightful analysis of why this
       // might not be doing what we expect anyway, so it may be the case that
       // this needs both fixing *and* tests.
       let guidColl = new Collection(this.engineURL, null, this.service);
 
       // Sort and limit so that we only get the last X records.
       guidColl.limit = this.downloadLimit;
-      guidColl.newer = this.lastSync;
+      guidColl.newer = lastSync;
 
       // index: Orders by the sortindex descending (highest weight first).
       guidColl.sort  = "index";
 
       let guids = await guidColl.get();
       if (!guids.success)
         throw guids;
 
@@ -1287,18 +1299,19 @@ SyncEngine.prototype = {
       if (extra.length > 0) {
         fetchBatch = Utils.arrayUnion(extra, fetchBatch);
         this.toFetch = Utils.arrayUnion(extra, this.toFetch);
       }
     }
 
     // Fast-foward the lastSync timestamp since we have stored the
     // remaining items in toFetch.
-    if (this.lastSync < this.lastModified) {
-      this.lastSync = this.lastModified;
+    if (lastSync < this.lastModified) {
+      lastSync = this.lastModified;
+      await this.setLastSync(lastSync);
     }
 
     // Process any backlog of GUIDs.
     // At this point we impose an upper limit on the number of items to fetch
     // in a single request, even for desktop, to avoid hitting URI limits.
     batchSize = this.guidFetchBatchSize;
 
     while (fetchBatch.length && !aborting) {
@@ -1332,18 +1345,19 @@ SyncEngine.prototype = {
         this._log.debug("Records that failed to apply: " + failed);
       }
       failed = [];
 
       if (aborting) {
         throw aborting;
       }
 
-      if (this.lastSync < this.lastModified) {
-        this.lastSync = this.lastModified;
+      if (lastSync < this.lastModified) {
+        lastSync = this.lastModified;
+        await this.setLastSync(lastSync);
       }
     }
 
     // Apply remaining items.
     await doApplyBatchAndPersistFailed.call(this);
 
     count.newFailed = this.previousFailed.reduce((count, engine) => {
       if (failedInPreviousSync.indexOf(engine) == -1) {
@@ -1614,16 +1628,17 @@ SyncEngine.prototype = {
     if (modifiedIDs.size) {
       this._log.trace("Preparing " + modifiedIDs.size +
                       " outgoing records");
 
       counts.sent = modifiedIDs.size;
 
       let failed = [];
       let successful = [];
+      let lastSync = await this.getLastSync();
       let handleResponse = async (resp, batchOngoing = false) => {
         // Note: We don't want to update this.lastSync, or this._modified until
         // the batch is complete, however we want to remember success/failure
         // indicators for when that happens.
         if (!resp.success) {
           this._log.debug("Uploading records failed: " + resp);
           resp.failureCode = resp.status == 412 ? ENGINE_BATCH_INTERRUPTED : ENGINE_UPLOAD_FAIL;
           throw resp;
@@ -1632,41 +1647,44 @@ SyncEngine.prototype = {
         // Update server timestamp from the upload.
         failed = failed.concat(Object.keys(resp.obj.failed));
         successful = successful.concat(resp.obj.success);
 
         if (batchOngoing) {
           // Nothing to do yet
           return;
         }
-        // Advance lastSync since we've finished the batch.
-        let modified = resp.headers["x-weave-timestamp"];
-        if (modified > this.lastSync) {
-          this.lastSync = modified;
-        }
+        let serverModifiedTime = resp.headers["x-weave-timestamp"];
+
         if (failed.length && this._log.level <= Log.Level.Debug) {
           this._log.debug("Records that will be uploaded again because "
                           + "the server couldn't store them: "
                           + failed.join(", "));
         }
 
         counts.failed += failed.length;
 
         for (let id of successful) {
           this._modified.delete(id);
         }
 
-        await this._onRecordsWritten(successful, failed);
+        await this._onRecordsWritten(successful, failed, serverModifiedTime);
+
+        // Advance lastSync since we've finished the batch.
+        if (serverModifiedTime > lastSync) {
+          lastSync = serverModifiedTime;
+          await this.setLastSync(lastSync);
+        }
 
         // clear for next batch
         failed.length = 0;
         successful.length = 0;
       };
 
-      let postQueue = up.newPostQueue(this._log, this.lastSync, handleResponse);
+      let postQueue = up.newPostQueue(this._log, lastSync, handleResponse);
 
       for (let id of modifiedIDs) {
         let out;
         let ok = false;
         try {
           let { forceTombstone = false } = this._needWeakUpload.get(id) || {};
           if (forceTombstone) {
             out = await this._createTombstone(id);
@@ -1707,17 +1725,17 @@ SyncEngine.prototype = {
     }
     this._needWeakUpload.clear();
 
     if (counts.sent || counts.failed) {
       Observers.notify("weave:engine:sync:uploaded", counts, this.name);
     }
   },
 
-  async _onRecordsWritten(succeeded, failed) {
+  async _onRecordsWritten(succeeded, failed, modified) {
     // Implement this method to take specific actions against successfully
     // uploaded records and failed records.
   },
 
   // Any cleanup necessary.
   // Save the current snapshot so as to calculate changes at next sync
   async _syncFinish() {
     this._log.trace("Finishing up sync");
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -1,59 +1,55 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 this.EXPORTED_SYMBOLS = ["BookmarksEngine", "PlacesItem", "Bookmark",
                          "BookmarkFolder", "BookmarkQuery",
-                         "Livemark", "BookmarkSeparator"];
+                         "Livemark", "BookmarkSeparator",
+                         "BufferedBookmarksEngine"];
 
 var Cc = Components.classes;
 var Ci = Components.interfaces;
 var Cu = Components.utils;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://services-common/async.js");
 Cu.import("resource://services-sync/constants.js");
 Cu.import("resource://services-sync/engines.js");
 Cu.import("resource://services-sync/record.js");
 Cu.import("resource://services-sync/util.js");
 
-XPCOMUtils.defineLazyModuleGetter(this, "BookmarkValidator",
-                                  "resource://services-sync/bookmark_validator.js");
-XPCOMUtils.defineLazyGetter(this, "PlacesBundle", () => {
-  let bundleService = Cc["@mozilla.org/intl/stringbundle;1"]
-                        .getService(Ci.nsIStringBundleService);
-  return bundleService.createBundle("chrome://places/locale/places.properties");
+XPCOMUtils.defineLazyModuleGetters(this, {
+  AsyncResource: "resource://services-sync/resource.js",
+  BookmarkBuffer: "resource://gre/modules/SyncBookmarkBuffer.jsm",
+  BookmarkValidator: "resource://services-sync/bookmark_validator.js",
+  OS: "resource://gre/modules/osfile.jsm",
+  PlacesBackups: "resource://gre/modules/PlacesBackups.jsm",
+  PlacesSyncUtils: "resource://gre/modules/PlacesSyncUtils.jsm",
+  PlacesUtils: "resource://gre/modules/PlacesUtils.jsm",
 });
-XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
-                                  "resource://gre/modules/PlacesUtils.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "PlacesSyncUtils",
-                                  "resource://gre/modules/PlacesSyncUtils.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "PlacesBackups",
-                                  "resource://gre/modules/PlacesBackups.jsm");
 
-const ANNOS_TO_TRACK = [PlacesSyncUtils.bookmarks.DESCRIPTION_ANNO,
-                        PlacesSyncUtils.bookmarks.SIDEBAR_ANNO,
-                        PlacesUtils.LMANNO_FEEDURI, PlacesUtils.LMANNO_SITEURI];
+XPCOMUtils.defineLazyGetter(this, "ANNOS_TO_TRACK", () => [
+  PlacesSyncUtils.bookmarks.DESCRIPTION_ANNO,
+  PlacesSyncUtils.bookmarks.SIDEBAR_ANNO, PlacesUtils.LMANNO_FEEDURI,
+  PlacesUtils.LMANNO_SITEURI,
+]);
 
-const SERVICE_NOT_SUPPORTED = "Service not supported on this platform";
+const BUFFER_SCHEMA_VERSION = 1;
+
 const FOLDER_SORTINDEX = 1000000;
 const {
   SOURCE_SYNC,
   SOURCE_IMPORT,
   SOURCE_IMPORT_REPLACE,
   SOURCE_SYNC_REPARENT_REMOVED_FOLDER_CHILDREN,
 } = Ci.nsINavBookmarksService;
 
-const ORGANIZERQUERY_ANNO = "PlacesOrganizer/OrganizerQuery";
-const ALLBOOKMARKS_ANNO = "AllBookmarks";
-const MOBILE_ANNO = "MobileBookmarks";
-
 // Roots that should be deleted from the server, instead of applied locally.
 // This matches `AndroidBrowserBookmarksRepositorySession::forbiddenGUID`,
 // but allows tags because we don't want to reparent tag folders or tag items
 // to "unfiled".
 const FORBIDDEN_INCOMING_IDS = ["pinned", "places", "readinglist"];
 
 // Items with these parents should be deleted from the server. We allow
 // children of the Places root, to avoid orphaning left pane queries and other
@@ -270,30 +266,134 @@ BookmarkSeparator.prototype = {
   fromSyncBookmark(item) {
     PlacesItem.prototype.fromSyncBookmark.call(this, item);
     this.pos = item.index;
   },
 };
 
 Utils.deferGetSet(BookmarkSeparator, "cleartext", "pos");
 
-this.BookmarksEngine = function BookmarksEngine(service) {
+/**
+ * The rest of this file implements two different bookmarks engines and stores.
+ * The `services.sync.engine.bookmarks.buffer` pref controls which one we use.
+ * `BaseBookmarksEngine` and `BaseBookmarksStore` define a handful of methods
+ * shared between the two implementations.
+ *
+ * `BookmarksEngine` and `BookmarksStore` pull locally changed IDs before
+ * syncing, examine every incoming record, use the default record-level
+ * reconciliation to resolve merge conflicts, and update records in Places
+ * using public APIs. This is similar to how the other sync engines work.
+ *
+ * Unfortunately, this general approach doesn't serve bookmark sync well.
+ * Bookmarks form a tree locally, but they're stored as opaque, encrypted, and
+ * unordered records on the server. The records are interdependent, with a
+ * set of constraints: each parent must know the IDs and order of its children,
+ * and a child can't appear in multiple parents.
+ *
+ * This has two important implications.
+ *
+ * First, some changes require us to upload multiple records. For example,
+ * moving a bookmark into a different folder uploads the bookmark, old folder,
+ * and new folder.
+ *
+ * Second, conflict resolution, like adding a bookmark to a folder on one
+ * device, and moving a different bookmark out of the same folder on a different
+ * device, must account for the tree structure. Otherwise, we risk uploading an
+ * incomplete tree, and confuse other devices that try to sync.
+ *
+ * Historically, the lack of durable change tracking and atomic uploads meant
+ * that we'd miss these changes entirely, or leave the server in an inconsistent
+ * state after a partial sync. Another device would then sync, download and
+ * apply the partial state directly to Places, and upload its changes. This
+ * could easily result in Sync scrambling bookmarks on both devices, and user
+ * intervention to manually undo the damage would make things worse.
+ *
+ * `BufferedBookmarksEngine` and `BufferedBookmarksStore` mitigate this by
+ * staging incoming bookmarks in a buffer, constructing trees from the local
+ * and remote bookmarks, and merging the two trees into a single consistent
+ * tree that accounts for every bookmark. For more information about merging,
+ * please see the explanation above `BookmarkBuffer`.
+ */
+function BaseBookmarksEngine(service) {
   SyncEngine.call(this, "Bookmarks", service);
 }
-BookmarksEngine.prototype = {
+
+BaseBookmarksEngine.prototype = {
   __proto__: SyncEngine.prototype,
   _recordObj: PlacesItem,
-  _storeObj: BookmarksStore,
   _trackerObj: BookmarksTracker,
   version: 2,
   _defaultSort: "index",
 
   syncPriority: 4,
   allowSkippedRecord: false,
 
+  async _syncFinish() {
+    await SyncEngine.prototype._syncFinish.call(this);
+    await PlacesSyncUtils.bookmarks.ensureMobileQuery();
+  },
+
+  async _createRecord(id) {
+    if (this._modified.isTombstone(id)) {
+      // If we already know a changed item is a tombstone, just create the
+      // record without dipping into Places.
+      return this._createTombstone(id);
+    }
+    let record = await SyncEngine.prototype._createRecord.call(this, id);
+    if (record.deleted) {
+      // Make sure deleted items are marked as tombstones. We do this here
+      // in addition to the `isTombstone` call above because it's possible
+      // a changed bookmark might be deleted during a sync (bug 1313967).
+      this._modified.setTombstone(record.id);
+    }
+    return record;
+  },
+
+  async pullAllChanges() {
+    return this.pullNewChanges();
+  },
+
+  async trackRemainingChanges() {
+    let changes = this._modified.changes;
+    await PlacesSyncUtils.bookmarks.pushChanges(changes);
+  },
+
+  _deleteId(id) {
+    this._noteDeletedId(id);
+  },
+
+  async _resetClient() {
+    await super._resetClient();
+    await PlacesSyncUtils.bookmarks.reset();
+  },
+
+  // Cleans up the Places root, reading list items (ignored in bug 762118,
+  // removed in bug 1155684), and pinned sites.
+  _shouldDeleteRemotely(incomingItem) {
+    return FORBIDDEN_INCOMING_IDS.includes(incomingItem.id) ||
+           FORBIDDEN_INCOMING_PARENT_IDS.includes(incomingItem.parentid);
+  },
+
+  getValidator() {
+    return new BookmarkValidator();
+  }
+};
+
+/**
+ * The original bookmarks engine. Uses an in-memory GUID map for deduping, and
+ * the default implementation for reconciling changes. Handles child ordering
+ * and deletions at the end of a sync.
+ */
+this.BookmarksEngine = function BookmarksEngine(service) {
+  BaseBookmarksEngine.apply(this, arguments);
+}
+BookmarksEngine.prototype = {
+  __proto__: BaseBookmarksEngine.prototype,
+  _storeObj: BookmarksStore,
+
   emptyChangeset() {
     return new BookmarksChangeset();
   },
 
   async _buildGUIDMap() {
     let guidMap = {};
     let tree = await PlacesUtils.promiseBookmarksTree("");
 
@@ -519,44 +619,31 @@ BookmarksEngine.prototype = {
     await PlacesSyncUtils.bookmarks.markChangesAsSyncing(changes);
   },
 
   async _orderChildren() {
     await this._store._orderChildren();
     this._store._childrenToOrder = {};
   },
 
-  async _syncFinish() {
-    await SyncEngine.prototype._syncFinish.call(this);
-    await PlacesSyncUtils.bookmarks.ensureMobileQuery();
-  },
-
   async _syncCleanup() {
     await SyncEngine.prototype._syncCleanup.call(this);
     delete this._guidMap;
   },
 
   async _createRecord(id) {
-    if (this._modified.isTombstone(id)) {
-      // If we already know a changed item is a tombstone, just create the
-      // record without dipping into Places.
-      return this._createTombstone(id);
+    let record = await super._createRecord(id);
+    if (record.deleted) {
+      return record;
     }
-    // Create the record as usual, but mark it as having dupes if necessary.
-    let record = await SyncEngine.prototype._createRecord.call(this, id);
+    // Mark the record as having dupes if necessary.
     let entry = await this._mapDupe(record);
     if (entry != null && entry.hasDupe) {
       record.hasDupe = true;
     }
-    if (record.deleted) {
-      // Make sure deleted items are marked as tombstones. We do this here
-      // in addition to the `isTombstone` call above because it's possible
-      // a changed bookmark might be deleted during a sync (bug 1313967).
-      this._modified.setTombstone(record.id);
-    }
     return record;
   },
 
   async _findDupe(item) {
     this._log.trace("Finding dupe for " + item.id +
                     " (already duped: " + item.hasDupe + ").");
 
     // Don't bother finding a dupe if the incoming item has duplicates.
@@ -566,54 +653,29 @@ BookmarksEngine.prototype = {
     }
     let mapped = await this._mapDupe(item);
     this._log.debug(item.id + " mapped to " + mapped);
     // We must return a string, not an object, and the entries in the GUIDMap
     // are created via "new String()" making them an object.
     return mapped ? mapped.toString() : mapped;
   },
 
-  async pullAllChanges() {
-    return this.pullNewChanges();
-  },
-
   async pullNewChanges() {
     return this._tracker.promiseChangedIDs();
   },
 
-  async trackRemainingChanges() {
-    let changes = this._modified.changes;
-    await PlacesSyncUtils.bookmarks.pushChanges(changes);
-  },
-
-  _deleteId(id) {
-    this._noteDeletedId(id);
-  },
-
-  async _resetClient() {
-    await SyncEngine.prototype._resetClient.call(this);
-    await PlacesSyncUtils.bookmarks.reset();
-  },
-
   // Called when _findDupe returns a dupe item and the engine has decided to
   // switch the existing item to the new incoming item.
   async _switchItemToDupe(localDupeGUID, incomingItem) {
     let newChanges = await PlacesSyncUtils.bookmarks.dedupe(localDupeGUID,
                                                             incomingItem.id,
                                                             incomingItem.parentid);
     this._modified.insert(newChanges);
   },
 
-  // Cleans up the Places root, reading list items (ignored in bug 762118,
-  // removed in bug 1155684), and pinned sites.
-  _shouldDeleteRemotely(incomingItem) {
-    return FORBIDDEN_INCOMING_IDS.includes(incomingItem.id) ||
-           FORBIDDEN_INCOMING_PARENT_IDS.includes(incomingItem.parentid);
-  },
-
   beforeRecordDiscard(localRecord, remoteRecord, remoteIsNewer) {
     if (localRecord.type != "folder" || remoteRecord.type != "folder") {
       return;
     }
     // Resolve child order conflicts by taking the chronologically newer list,
     // then appending any missing items from the older list. This preserves the
     // order of those missing items relative to each other, but not relative to
     // the items that appear in the newer list.
@@ -626,28 +688,230 @@ BookmarksEngine.prototype = {
 
     // Some of the children in `order` might have been deleted, or moved to
     // other folders. `PlacesSyncUtils.bookmarks.order` ignores them.
     let order = newRecord.children ?
                 [...newRecord.children, ...missingChildren] : missingChildren;
     this._log.debug("Recording children of " + localRecord.id, order);
     this._store._childrenToOrder[localRecord.id] = order;
   },
+};
 
-  getValidator() {
-    return new BookmarkValidator();
+/**
+ * The buffered bookmarks engine uses a different store that stages downloaded
+ * bookmarks in a separate database, instead of writing directly to Places. The
+ * buffer handles reconciliation, so we stub out `_reconcile`, and wait to pull
+ * changes until we're ready to upload.
+ */
+this.BufferedBookmarksEngine = function BufferedBookmarksEngine() {
+  BaseBookmarksEngine.apply(this, arguments);
+};
+
+BufferedBookmarksEngine.prototype = {
+  __proto__: BaseBookmarksEngine.prototype,
+  _storeObj: BufferedBookmarksStore,
+
+  async getLastSync() {
+    let buffer = await this._store.ensureOpenBuffer();
+    return buffer.getCollectionHighWaterMark();
+  },
+
+  async setLastSync(lastSync) {
+    let buffer = await this._store.ensureOpenBuffer();
+    await buffer.setCollectionLastModified(lastSync);
+    // Update the pref so that reverting to the original bookmarks engine
+    // doesn't download records we've already applied.
+    super.lastSync = lastSync;
+  },
+
+  get lastSync() {
+    throw new TypeError("Use getLastSync");
+  },
+
+  set lastSync(value) {
+    throw new TypeError("Use setLastSync");
+  },
+
+  emptyChangeset() {
+    return new BufferedBookmarksChangeset();
+  },
+
+  async _processIncoming(newitems) {
+    try {
+      await super._processIncoming(newitems);
+    } finally {
+      let buffer = await this._store.ensureOpenBuffer();
+      let recordsToUpload = await buffer.apply({
+        remoteTimeSeconds: AsyncResource.serverTime,
+      });
+      this._modified.replace(recordsToUpload);
+    }
+  },
+
+  async _reconcile(item) {
+    return true;
+  },
+
+  async _createRecord(id) {
+    if (this._needWeakUpload.has(id)) {
+      return this._store.createRecord(id, this.name);
+    }
+    let change = this._modified.changes[id];
+    if (!change) {
+      this._log.error("Creating record for item ${id} not in strong " +
+                      "changeset", { id });
+      throw new TypeError("Can't create record for unchanged item");
+    }
+    let record = change.record;
+    record.sortindex = await this._store._calculateIndex(record);
+    return record;
+  },
+
+  async pullChanges() {
+    return {};
+  },
+
+  /**
+   * Writes successfully uploaded records back to the buffer, so that the
+   * buffer matches the server. We update the buffer before updating Places,
+   * which has implications for interrupted syncs.
+   *
+   * 1. Sync interrupted during upload; server doesn't support atomic uploads.
+   *    We'll download and reapply everything that we uploaded before the
+   *    interruption. All locally changed items retain their change counters.
+   * 2. Sync interrupted during upload; atomic uploads enabled. The server
+   *    discards the batch. All changed local items retain their change
+   *    counters, so the next sync resumes cleanly.
+   * 3. Sync interrupted during upload; outgoing records can't fit in a single
+   *    batch. We'll download and reapply all records through the most recent
+   *    committed batch. This is a variation of (1).
+   * 4. Sync interrupted after we update the buffer, but before cleanup. The
+   *    buffer matches the server, but locally changed items retain their change
+   *    counters. Reuploading them on the next sync should be idempotent, though
+   *    unnecessary. If another client makes a conflicting remote change before
+   *    we sync again, we may incorrectly prefer the local state.
+   * 5. Sync completes successfully. We'll update the buffer, and reset the
+   *    change counters for all items.
+   */
+  async _onRecordsWritten(succeeded, failed, serverModifiedTime) {
+    let records = [];
+    for (let id of succeeded) {
+      let change = this._modified.changes[id];
+      if (!change) {
+        this._log.info("Uploaded record not in strong changeset", id);
+        continue;
+      }
+      if (!change.synced) {
+        this._log.info("Record in strong changeset not uploaded", id);
+        continue;
+      }
+      let record = change.record;
+      if (!record) {
+        this._log.error("Missing Sync record for ${id} in ${change}",
+                        { id, change });
+        throw new TypeError("Missing Sync record for change");
+      }
+      // TODO(kitcambridge): This is inefficient. Encryption nulls out the
+      // cleartext, but we need the decrypted record so that we can store it
+      // in the buffer. We can save the cleartext when we build the changeset
+      // in `_processIncoming`, but we also need to account for weakly uploaded
+      // records.
+      await record.decrypt(this.service.collectionKeys.keyForCollection(this.name));
+      record.modified = serverModifiedTime;
+      records.push(record);
+    }
+    let buffer = await this._store.ensureOpenBuffer();
+    await buffer.store(records, { needsMerge: false });
+  },
+
+  async _resetClient() {
+    await super._resetClient();
+    let buffer = await this._store.ensureOpenBuffer();
+    await buffer.reset();
+  },
+
+  async finalize() {
+    await super.finalize();
+    await this._store.finalize();
+  },
+};
+
+/**
+ * The only code shared between `BookmarksStore` and `BufferedBookmarksStore`
+ * is for creating Sync records from Places items. Everything else is
+ * different.
+ */
+function BaseBookmarksStore(name, engine) {
+  Store.call(this, name, engine);
+}
+
+BaseBookmarksStore.prototype = {
+  __proto__: Store.prototype,
+
+  // Create a record starting from the weave id (places guid)
+  async createRecord(id, collection) {
+    let item = await PlacesSyncUtils.bookmarks.fetch(id);
+    if (!item) { // deleted item
+      let record = new PlacesItem(collection, id);
+      record.deleted = true;
+      return record;
+    }
+
+    let recordObj = getTypeObject(item.kind);
+    if (!recordObj) {
+      this._log.warn("Unknown item type, cannot serialize: " + item.kind);
+      recordObj = PlacesItem;
+    }
+    let record = new recordObj(collection, id);
+    record.fromSyncBookmark(item);
+
+    record.sortindex = await this._calculateIndex(record);
+
+    return record;
+  },
+
+  async _calculateIndex(record) {
+    // Ensure folders have a very high sort index so they're not synced last.
+    if (record.type == "folder")
+      return FOLDER_SORTINDEX;
+
+    // For anything directly under the toolbar, give it a boost of more than an
+    // unvisited bookmark
+    let index = 0;
+    if (record.parentid == "toolbar")
+      index += 150;
+
+    // Add in the bookmark's frecency if we have something.
+    if (record.bmkUri != null) {
+      let frecency = await PlacesSyncUtils.history.fetchURLFrecency(record.bmkUri);
+      if (frecency != -1)
+        index += frecency;
+    }
+
+    return index;
+  },
+
+  async wipe() {
+    // Save a backup before clearing out all bookmarks.
+    await PlacesBackups.create(null, true);
+    await PlacesSyncUtils.bookmarks.wipe();
   }
 };
 
-function BookmarksStore(name, engine) {
-  Store.call(this, name, engine);
+/**
+ * The original store updates Places during the sync, using public methods.
+ * `BookmarksStore` implements all abstract `Store` methods, and behaves like
+ * the other stores.
+ */
+function BookmarksStore() {
+  BaseBookmarksStore.apply(this, arguments);
   this._itemsToDelete = new Set();
 }
 BookmarksStore.prototype = {
-  __proto__: Store.prototype,
+  __proto__: BaseBookmarksStore.prototype,
 
   async itemExists(id) {
     return (await this.idForGUID(id)) > 0;
   },
 
   async applyIncoming(record) {
     this._log.debug("Applying record " + record.id);
     let isSpecial = PlacesSyncUtils.bookmarks.ROOTS.includes(record.id);
@@ -774,93 +1038,126 @@ BookmarksStore.prototype = {
     this.clearPendingDeletions();
     return guidsToUpdate;
   },
 
   clearPendingDeletions() {
     this._itemsToDelete.clear();
   },
 
-  // Create a record starting from the weave id (places guid)
-  async createRecord(id, collection) {
-    let item = await PlacesSyncUtils.bookmarks.fetch(id);
-    if (!item) { // deleted item
-      let record = new PlacesItem(collection, id);
-      record.deleted = true;
-      return record;
-    }
-
-    let recordObj = getTypeObject(item.kind);
-    if (!recordObj) {
-      this._log.warn("Unknown item type, cannot serialize: " + item.kind);
-      recordObj = PlacesItem;
-    }
-    let record = new recordObj(collection, id);
-    record.fromSyncBookmark(item);
-
-    record.sortindex = await this._calculateIndex(record);
-
-    return record;
-  },
-
-
   async GUIDForId(id) {
     let guid = await PlacesUtils.promiseItemGuid(id);
     return PlacesSyncUtils.bookmarks.guidToSyncId(guid);
   },
 
   async idForGUID(guid) {
     // guid might be a String object rather than a string.
     guid = PlacesSyncUtils.bookmarks.syncIdToGuid(guid.toString());
 
     try {
       return await PlacesUtils.promiseItemId(guid);
     } catch (ex) {
       return -1;
     }
   },
 
-  async _calculateIndex(record) {
-    // Ensure folders have a very high sort index so they're not synced last.
-    if (record.type == "folder")
-      return FOLDER_SORTINDEX;
+  async wipe() {
+    this.clearPendingDeletions();
+    await super.wipe();
+  }
+};
+
+/**
+ * The buffered store delegates to the buffer for staging and applying
+ * records. Unlike `BookmarksStore`, `BufferedBookmarksStore` only
+ * implements `applyIncoming`, and `createRecord` via `BaseBookmarksStore`.
+ * These are the only two methods that `BufferedBookmarksEngine` calls during
+ * download and upload.
+ *
+ * The other `Store` methods intentionally remain abstract, so you can't use
+ * this store to create or update bookmarks in Places. All changes must go
+ * through the buffer, which takes care of merging and producing a valid tree.
+ */
+function BufferedBookmarksStore() {
+  BaseBookmarksStore.apply(this, arguments);
+}
+
+BufferedBookmarksStore.prototype = {
+  __proto__: BaseBookmarksStore.prototype,
+
+  ensureOpenBuffer() {
+    if (this.openBufferPromise) {
+      return this.openBufferPromise;
+    }
+    this.openBufferPromise = (async () => {
+      let bufferPath = OS.Path.join(OS.Constants.Path.profileDir, "weave",
+                                    "bookmarks.sqlite");
+      await OS.File.makeDir(OS.Path.dirname(bufferPath), {
+        from: OS.Constants.Path.profileDir,
+      });
 
-    // For anything directly under the toolbar, give it a boost of more than an
-    // unvisited bookmark
-    let index = 0;
-    if (record.parentid == "toolbar")
-      index += 150;
+      return BookmarkBuffer.open({
+        path: bufferPath,
+        deletedRecordFactory: id => {
+          let record = new PlacesItem(this.name, id);
+          record.deleted = true;
+          return record;
+        },
+        recordFactory: (kind, id) => {
+          switch (kind) {
+            case BookmarkBuffer.KIND.BOOKMARK:
+              return new Bookmark(this.name, id);
+
+            case BookmarkBuffer.KIND.QUERY:
+              return new BookmarkQuery(this.name, id);
+
+            case BookmarkBuffer.KIND.FOLDER:
+              return new BookmarkFolder(this.name, id);
 
-    // Add in the bookmark's frecency if we have something.
-    if (record.bmkUri != null) {
-      let frecency = await PlacesSyncUtils.history.fetchURLFrecency(record.bmkUri);
-      if (frecency != -1)
-        index += frecency;
-    }
+            case BookmarkBuffer.KIND.LIVEMARK:
+              return new Livemark(this.name, id);
 
-    return index;
+            case BookmarkBuffer.KIND.SEPARATOR:
+              return new BookmarkSeparator(this.name, id);
+          }
+          this._log.error("Creating record for item ${id} with kind ${kind}",
+                          { id, kind });
+          throw new TypeError("Can't create record for unknown item kind");
+        },
+        recordTelemetryEvent: (object, method, value, extra) => {
+          this.engine.service.recordTelemetryEvent(object, method, value,
+                                                   extra);
+        },
+      });
+    })();
+    return this.openBufferPromise;
   },
 
-  async wipe() {
-    this.clearPendingDeletions();
-    // Save a backup before clearing out all bookmarks.
-    await PlacesBackups.create(null, true);
-    await PlacesSyncUtils.bookmarks.wipe();
-  }
+  async applyIncoming(record) {
+    let buffer = await this.ensureOpenBuffer();
+    await buffer.store([record]);
+  },
+
+  async finalize() {
+    if (!this.openBufferPromise) {
+      return;
+    }
+    let buffer = await this.openBufferPromise;
+    await buffer.finalize();
+  },
 };
 
 // The bookmarks tracker is a special flower. Instead of listening for changes
 // via observer notifications, it queries Places for the set of items that have
 // changed since the last sync. Because it's a "pull-based" tracker, it ignores
 // 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);
 }
 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() {
@@ -909,70 +1206,21 @@ BookmarksTracker.prototype = {
   get changedIDs() {
     throw new Error("Use promiseChangedIDs");
   },
 
   set changedIDs(obj) {
     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.
-  async _migrateOldEntries() {
-    let existingIDs = await Utils.jsonLoad("changes/" + this.file, this);
-    if (existingIDs === null) {
-      // If the tracker file doesn't exist, we don't need to migrate, even if
-      // the engine is enabled. It's possible we're upgrading before the first
-      // sync. In the worst case, getting this wrong has the same effect as a
-      // restore: we'll reupload everything to the server.
-      this._log.debug("migrateOldEntries: Missing bookmarks tracker file; " +
-                      "skipping migration");
-      return null;
-    }
-
-    if (!this._needsMigration()) {
-      // We have a tracker file, but bookmark syncing is disabled, or this is
-      // the first sync. It's likely the tracker file is stale. Remove it and
-      // skip migration.
-      this._log.debug("migrateOldEntries: Bookmarks engine disabled or " +
-                      "first sync; skipping migration");
-      return Utils.jsonRemove("changes/" + this.file, this);
-    }
-
-    // At this point, we know the engine is enabled, we have a tracker file
-    // (though it may be empty), and we've synced before.
-    this._log.debug("migrateOldEntries: Migrating old tracker entries");
-    let entries = [];
-    for (let syncID in existingIDs) {
-      let change = existingIDs[syncID];
-      // Allow raw timestamps for backward-compatibility with changed IDs
-      // persisted before bug 1274496.
-      let timestamp = typeof change == "number" ? change : change.modified;
-      entries.push({
-        syncId: syncID,
-        modified: timestamp * 1000,
-      });
-    }
-    await PlacesSyncUtils.bookmarks.migrateOldTrackerEntries(entries);
-    return Utils.jsonRemove("changes/" + this.file, this);
-  },
-
-  _needsMigration() {
-    return this.engine && this.engineIsEnabled() && this.engine.lastSync > 0;
-  },
-
   observe: function observe(subject, topic, data) {
     Tracker.prototype.observe.call(this, subject, topic, data);
 
     switch (topic) {
       case "weave:engine:start-tracking":
-        if (!this._migratedOldEntries) {
-          this._migratedOldEntries = true;
-          Async.promiseSpinningly(this._migrateOldEntries());
-        }
         break;
       case "bookmarks-restore-begin":
         this._log.debug("Ignoring changes from importing bookmarks.");
         break;
       case "bookmarks-restore-success":
         this._log.debug("Tracking all items on successful import.");
 
         if (data == "json") {
@@ -1075,26 +1323,56 @@ BookmarksTracker.prototype = {
     if (--this._batchDepth === 0 && this._batchSawScoreIncrement) {
       this.score += SCORE_INCREMENT_XLARGE;
       this._batchSawScoreIncrement = false;
     }
   },
   onItemVisited() {}
 };
 
-class BookmarksChangeset extends Changeset {
+/**
+ * A changeset that stores extra metadata in a change record for each ID. The
+ * engine updates this metadata when uploading Sync records, and writes it back
+ * to Places in `BaseBookmarksEngine#trackRemainingChanges`.
+ *
+ * The `synced` property on a change record means its corresponding item has
+ * been uploaded, and we should pretend it doesn't exist in the changeset.
+ */
+class BufferedBookmarksChangeset extends Changeset {
+  // Only `_reconcile` calls `getModifiedTimestamp` and `has`, and the buffered
+  // engine does its own reconciliation.
+  getModifiedTimestamp(id) {
+    throw new Error("Don't use timestamps to resolve bookmark conflicts");
+  }
 
-  getStatus(id) {
-    let change = this.changes[id];
-    if (!change) {
-      return PlacesUtils.bookmarks.SYNC_STATUS.UNKNOWN;
-    }
-    return change.status;
+  has(id) {
+    throw new Error("Don't use the changeset to resolve bookmark conflicts");
   }
 
+  delete(id) {
+    let change = this.changes[id];
+    if (change) {
+      // Mark the change as synced without removing it from the set. We do this
+      // so that we can update Places in `trackRemainingChanges`.
+      change.synced = true;
+    }
+  }
+
+  ids() {
+    let results = new Set();
+    for (let id in this.changes) {
+      if (!this.changes[id].synced) {
+        results.add(id);
+      }
+    }
+    return [...results];
+  }
+}
+
+class BookmarksChangeset extends BufferedBookmarksChangeset {
   getModifiedTimestamp(id) {
     let change = this.changes[id];
     if (change) {
       // Pretend the change doesn't exist if we've already synced or
       // reconciled it.
       return change.synced ? Number.NaN : change.modified;
     }
     return Number.NaN;
@@ -1110,35 +1388,16 @@ class BookmarksChangeset extends Changes
 
   setTombstone(id) {
     let change = this.changes[id];
     if (change) {
       change.tombstone = true;
     }
   }
 
-  delete(id) {
-    let change = this.changes[id];
-    if (change) {
-      // Mark the change as synced without removing it from the set. We do this
-      // so that we can update Places in `trackRemainingChanges`.
-      change.synced = true;
-    }
-  }
-
-  ids() {
-    let results = new Set();
-    for (let id in this.changes) {
-      if (!this.changes[id].synced) {
-        results.add(id);
-      }
-    }
-    return [...results];
-  }
-
   isTombstone(id) {
     let change = this.changes[id];
     if (change) {
       return change.tombstone;
     }
     return false;
   }
 }
--- a/services/sync/modules/service.js
+++ b/services/sync/modules/service.js
@@ -35,17 +35,16 @@ Cu.import("resource://services-sync/stag
 Cu.import("resource://services-sync/stages/declined.js");
 Cu.import("resource://services-sync/status.js");
 Cu.import("resource://services-sync/telemetry.js");
 Cu.import("resource://services-sync/util.js");
 
 function getEngineModules() {
   let result = {
     Addons: {module: "addons.js", symbol: "AddonsEngine"},
-    Bookmarks: {module: "bookmarks.js", symbol: "BookmarksEngine"},
     Form: {module: "forms.js", symbol: "FormEngine"},
     History: {module: "history.js", symbol: "HistoryEngine"},
     Password: {module: "passwords.js", symbol: "PasswordEngine"},
     Prefs: {module: "prefs.js", symbol: "PrefsEngine"},
     Tab: {module: "tabs.js", symbol: "TabEngine"},
     ExtensionStorage: {module: "extension-storage.js", symbol: "ExtensionStorageEngine"},
   }
   if (Svc.Prefs.get("engine.addresses.available", false)) {
@@ -55,16 +54,27 @@ function getEngineModules() {
     };
   }
   if (Svc.Prefs.get("engine.creditcards.available", false)) {
     result.CreditCards = {
       module: "resource://formautofill/FormAutofillSync.jsm",
       symbol: "CreditCardsEngine",
     };
   }
+  if (Svc.Prefs.get("engine.bookmarks.buffer", false)) {
+    result.Bookmarks = {
+      module: "bookmarks.js",
+      symbol: "BufferedBookmarksEngine",
+    };
+  } else {
+    result.Bookmarks = {
+      module: "bookmarks.js",
+      symbol: "BookmarksEngine",
+    };
+  }
   return result;
 }
 
 const STORAGE_INFO_TYPES = [INFO_COLLECTIONS,
                             INFO_COLLECTION_USAGE,
                             INFO_COLLECTION_COUNTS,
                             INFO_QUOTA];
 
--- a/services/sync/services-sync.js
+++ b/services/sync/services-sync.js
@@ -20,16 +20,17 @@ pref("services.sync.errorhandler.network
 // Note that new engines are typically added with a default of disabled, so
 // when an existing sync user gets the Firefox upgrade that supports the engine
 // it starts as disabled until the user has explicitly opted in.
 // The sync "create account" process typically *will* offer these engines, so
 // they may be flipped to enabled at that time.
 pref("services.sync.engine.addons", true);
 pref("services.sync.engine.addresses", false);
 pref("services.sync.engine.bookmarks", true);
+pref("services.sync.engine.bookmarks.buffer", false);
 pref("services.sync.engine.creditcards", false);
 pref("services.sync.engine.history", true);
 pref("services.sync.engine.passwords", true);
 pref("services.sync.engine.prefs", true);
 pref("services.sync.engine.tabs", true);
 pref("services.sync.engine.tabs.filteredUrls", "^(about:.*|resource:.*|chrome:.*|wyciwyg:.*|file:.*|blob:.*)$");
 
 // The addresses and CC engines might not actually be available at all.
--- a/services/sync/tests/unit/test_bookmark_engine.js
+++ b/services/sync/tests/unit/test_bookmark_engine.js
@@ -8,19 +8,16 @@ Cu.import("resource://gre/modules/Log.js
 Cu.import("resource://services-common/utils.js");
 Cu.import("resource://services-sync/constants.js");
 Cu.import("resource://services-sync/engines.js");
 Cu.import("resource://services-sync/engines/bookmarks.js");
 Cu.import("resource://services-sync/service.js");
 Cu.import("resource://services-sync/util.js");
 Cu.import("resource://testing-common/services/sync/utils.js");
 
-
-initTestLogging("Trace");
-
 async function fetchAllSyncIds() {
   let db = await PlacesUtils.promiseDBConnection();
   let rows = await db.executeCached(`
     WITH RECURSIVE
     syncedItems(id, guid) AS (
       SELECT b.id, b.guid FROM moz_bookmarks b
       WHERE b.guid IN ('menu________', 'toolbar_____', 'unfiled_____',
                        'mobile______')
@@ -33,32 +30,47 @@ async function fetchAllSyncIds() {
   for (let row of rows) {
     let syncId = PlacesSyncUtils.bookmarks.guidToSyncId(
       row.getResultByName("guid"));
     syncIds.add(syncId);
   }
   return syncIds;
 }
 add_task(async function setup() {
-  initTestLogging("Trace");
-  await generateNewKeys(Service.collectionKeys);
-})
-
-add_task(async function setup() {
   await Service.engineManager.unregister("bookmarks");
 
   initTestLogging("Trace");
-  generateNewKeys(Service.collectionKeys);
+  await generateNewKeys(Service.collectionKeys);
 });
 
-add_task(async function test_delete_invalid_roots_from_server() {
+function add_bookmark_test(task) {
+  add_task(async function() {
+    _(`Running test ${task.name} with legacy bookmarks engine`);
+    let legacyEngine = new BookmarksEngine(Service);
+    await legacyEngine.initialize();
+    try {
+      await task(legacyEngine);
+    } finally {
+      await legacyEngine.finalize();
+    }
+
+    _(`Running test ${task.name} with buffered bookmarks engine`);
+    let bufferedEngine = new BufferedBookmarksEngine(Service);
+    await bufferedEngine.initialize();
+    try {
+      await task(bufferedEngine);
+    } finally {
+      await bufferedEngine.finalize();
+    }
+  });
+}
+
+add_bookmark_test(async function test_delete_invalid_roots_from_server(engine) {
   _("Ensure that we delete the Places and Reading List roots from the server.");
 
-  let engine  = new BookmarksEngine(Service);
-  await engine.initialize();
   let store   = engine._store;
   let server = await serverForFoo(engine);
   await SyncTestingInfrastructure(server);
 
   let collection = server.user("foo").collection("bookmarks");
 
   Svc.Obs.notify("weave:engine:start-tracking");
 
@@ -87,19 +99,23 @@ add_task(async function test_delete_inva
     newBmk.parentid = "toolbar";
     collection.insert(newBmk.id, encryptPayload(newBmk.cleartext));
 
     deepEqual(collection.keys().sort(), ["places", "readinglist", listBmk.id, newBmk.id].sort(),
       "Should store Places root, reading list items, and new bookmark on server");
 
     await sync_engine_and_validate_telem(engine, false);
 
-    ok(!(await store.itemExists("readinglist")), "Should not apply Reading List root");
-    ok(!(await store.itemExists(listBmk.id)), "Should not apply items in Reading List");
-    ok((await store.itemExists(newBmk.id)), "Should apply new bookmark");
+    await Assert.rejects(PlacesUtils.promiseItemId("readinglist"),
+      /no item found for the given GUID/, "Should not apply Reading List root");
+    await Assert.rejects(PlacesUtils.promiseItemId(listBmk.id),
+      /no item found for the given GUID/,
+      "Should not apply items in Reading List");
+    ok((await PlacesUtils.promiseItemId(newBmk.id)) > 0,
+      "Should apply new bookmark");
 
     deepEqual(collection.keys().sort(), ["menu", "mobile", "toolbar", "unfiled", newBmk.id].sort(),
       "Should remove Places root and reading list items from server; upload local roots");
   } finally {
     await store.wipe();
     Svc.Prefs.resetBranch("");
     Service.recordManager.clearCache();
     await promiseStopServer(server);
@@ -131,40 +147,49 @@ add_task(async function bad_record_allID
   do_check_true(all.has("toolbar"));
 
   _("Clean up.");
   PlacesUtils.bookmarks.removeItem(badRecordID);
   await PlacesSyncUtils.bookmarks.reset();
   await promiseStopServer(server);
 });
 
-add_task(async function test_processIncoming_error_orderChildren() {
+add_bookmark_test(async function test_processIncoming_error_orderChildren(engine) {
   _("Ensure that _orderChildren() is called even when _processIncoming() throws an error.");
 
-  let engine = new BookmarksEngine(Service);
-  await engine.initialize();
   let store  = engine._store;
   let server = await serverForFoo(engine);
   await SyncTestingInfrastructure(server);
 
   let collection = server.user("foo").collection("bookmarks");
 
   try {
 
     let folder1_id = PlacesUtils.bookmarks.createFolder(
       PlacesUtils.bookmarks.toolbarFolder, "Folder 1", 0);
-    let folder1_guid = await store.GUIDForId(folder1_id);
+    let folder1_guid = await PlacesUtils.promiseItemGuid(folder1_id);
 
     let fxuri = CommonUtils.makeURI("http://getfirefox.com/");
     let tburi = CommonUtils.makeURI("http://getthunderbird.com/");
 
     let bmk1_id = PlacesUtils.bookmarks.insertBookmark(
       folder1_id, fxuri, PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
+    let bmk1_guid = await PlacesUtils.promiseItemGuid(bmk1_id);
     let bmk2_id = PlacesUtils.bookmarks.insertBookmark(
       folder1_id, tburi, PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Thunderbird!");
+    let bmk2_guid = await PlacesUtils.promiseItemGuid(bmk2_id);
+
+    let toolbar_record = await store.createRecord("toolbar");
+    collection.insert("toolbar", encryptPayload(toolbar_record.cleartext));
+
+    let bmk1_record = await store.createRecord(bmk1_guid);
+    collection.insert(bmk1_guid, encryptPayload(bmk1_record.cleartext));
+
+    let bmk2_record = await store.createRecord(bmk2_guid);
+    collection.insert(bmk2_guid, encryptPayload(bmk2_record.cleartext));
 
     // Create a server record for folder1 where we flip the order of
     // the children.
     let folder1_record = await store.createRecord(folder1_guid);
     let folder1_payload = folder1_record.cleartext;
     folder1_payload.children.reverse();
     collection.insert(folder1_guid, encryptPayload(folder1_payload));
 
@@ -173,17 +198,17 @@ add_task(async function test_processInco
     const BOGUS_GUID = "zzzzzzzzzzzz";
     let bogus_record = collection.insert(BOGUS_GUID, "I'm a bogus record!");
     bogus_record.get = function get() {
       throw new Error("Sync this!");
     };
 
     // Make the 10 minutes old so it will only be synced in the toFetch phase.
     bogus_record.modified = Date.now() / 1000 - 60 * 10;
-    engine.lastSync = Date.now() / 1000 - 60;
+    await engine.setLastSync(Date.now() / 1000 - 60);
     engine.toFetch = [BOGUS_GUID];
 
     let error;
     try {
       await sync_engine_and_validate_telem(engine, true)
     } catch (ex) {
       error = ex;
     }
@@ -204,74 +229,72 @@ add_task(async function test_processInco
     await engine.resetClient();
     Svc.Prefs.resetBranch("");
     Service.recordManager.clearCache();
     await PlacesSyncUtils.bookmarks.reset();
     await promiseStopServer(server);
   }
 });
 
-add_task(async function test_restorePromptsReupload() {
-  await test_restoreOrImport(true);
+add_bookmark_test(async function test_restorePromptsReupload(engine) {
+  await test_restoreOrImport(engine, { replace: true });
 });
 
-add_task(async function test_importPromptsReupload() {
-  await test_restoreOrImport(false);
+add_bookmark_test(async function test_importPromptsReupload(engine) {
+  await test_restoreOrImport(engine, { replace: false });
 });
 
-// Test a JSON restore or HTML import. Use JSON if `aReplace` is `true`, or
+// Test a JSON restore or HTML import. Use JSON if `replace` is `true`, or
 // HTML otherwise.
-async function test_restoreOrImport(aReplace) {
-  let verb = aReplace ? "restore" : "import";
-  let verbing = aReplace ? "restoring" : "importing";
-  let bookmarkUtils = aReplace ? BookmarkJSONUtils : BookmarkHTMLUtils;
+async function test_restoreOrImport(engine, { replace }) {
+  let verb = replace ? "restore" : "import";
+  let verbing = replace ? "restoring" : "importing";
+  let bookmarkUtils = replace ? BookmarkJSONUtils : BookmarkHTMLUtils;
 
   _(`Ensure that ${verbing} from a backup will reupload all records.`);
 
-  let engine = new BookmarksEngine(Service);
-  await engine.initialize();
   let store  = engine._store;
   let server = await serverForFoo(engine);
   await SyncTestingInfrastructure(server);
 
   let collection = server.user("foo").collection("bookmarks");
 
   Svc.Obs.notify("weave:engine:start-tracking");   // We skip usual startup...
 
   try {
 
     let folder1_id = PlacesUtils.bookmarks.createFolder(
       PlacesUtils.bookmarks.toolbarFolder, "Folder 1", 0);
-    let folder1_guid = await store.GUIDForId(folder1_id);
+    let folder1_guid = await PlacesUtils.promiseItemGuid(folder1_id);
     _("Folder 1: " + folder1_id + ", " + folder1_guid);
 
     let fxuri = CommonUtils.makeURI("http://getfirefox.com/");
     let tburi = CommonUtils.makeURI("http://getthunderbird.com/");
 
     _("Create a single record.");
     let bmk1_id = PlacesUtils.bookmarks.insertBookmark(
       folder1_id, fxuri, PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
-    let bmk1_guid = await store.GUIDForId(bmk1_id);
+    let bmk1_guid = await PlacesUtils.promiseItemGuid(bmk1_id);
     _(`Get Firefox!: ${bmk1_id}, ${bmk1_guid}`);
 
     let dirSvc = Cc["@mozilla.org/file/directory_service;1"]
       .getService(Ci.nsIProperties);
 
     let backupFile = dirSvc.get("TmpD", Ci.nsIFile);
 
     _("Make a backup.");
     backupFile.append("t_b_e_" + Date.now() + ".json");
 
     _(`Backing up to file ${backupFile.path}`);
     await bookmarkUtils.exportToFile(backupFile.path);
 
     _("Create a different record and sync.");
     let bmk2_id = PlacesUtils.bookmarks.insertBookmark(
       folder1_id, tburi, PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Thunderbird!");
-    let bmk2_guid = await store.GUIDForId(bmk2_id);
+    let bmk2_guid = await PlacesUtils.promiseItemGuid(bmk2_id);
     _(`Get Thunderbird!: ${bmk2_id}, ${bmk2_guid}`);
 
     PlacesUtils.bookmarks.removeItem(bmk1_id);
 
     let error;
     try {
       await sync_engine_and_validate_telem(engine, false);
     } catch (ex) {
@@ -284,51 +307,52 @@ async function test_restoreOrImport(aRep
     // Of course, there's also the Bookmarks Toolbar and Bookmarks Menu...
     let wbos = collection.keys(function(id) {
       return ["menu", "toolbar", "mobile", "unfiled", folder1_guid].indexOf(id) == -1;
     });
     do_check_eq(wbos.length, 1);
     do_check_eq(wbos[0], bmk2_guid);
 
     _(`Now ${verb} from a backup.`);
-    await bookmarkUtils.importFromFile(backupFile, aReplace);
+    await bookmarkUtils.importFromFile(backupFile, replace);
 
     let bookmarksCollection = server.user("foo").collection("bookmarks");
-    if (aReplace) {
+    if (replace) {
       _("Verify that we wiped the server.");
       do_check_true(!bookmarksCollection);
     } else {
       _("Verify that we didn't wipe the server.");
       do_check_true(!!bookmarksCollection);
     }
 
     _("Ensure we have the bookmarks we expect locally.");
-    let guids = await fetchAllSyncIds();
-    _("GUIDs: " + JSON.stringify([...guids]));
+    let syncIds = await fetchAllSyncIds();
+    _("GUIDs: " + JSON.stringify([...syncIds]));
     let bookmarkGuids = new Map();
     let count = 0;
-    for (let guid of guids) {
+    for (let syncId of syncIds) {
       count++;
-      let id = await store.idForGUID(guid, true);
+      let guid = PlacesSyncUtils.bookmarks.syncIdToGuid(syncId);
+      let id = await PlacesUtils.promiseItemId(guid);
       // Only one bookmark, so _all_ should be Firefox!
       if (PlacesUtils.bookmarks.getItemType(id) == PlacesUtils.bookmarks.TYPE_BOOKMARK) {
         let uri = PlacesUtils.bookmarks.getBookmarkURI(id);
         _(`Found URI ${uri.spec} for GUID ${guid}`);
         bookmarkGuids.set(uri.spec, guid);
       }
     }
     do_check_true(bookmarkGuids.has(fxuri.spec));
-    if (!aReplace) {
+    if (!replace) {
       do_check_true(bookmarkGuids.has(tburi.spec));
     }
 
     _("Have the correct number of IDs locally, too.");
     let expectedResults = ["menu", "toolbar", "mobile", "unfiled", folder1_id,
                            bmk1_id];
-    if (!aReplace) {
+    if (!replace) {
       expectedResults.push("toolbar", folder1_id, bmk2_id);
     }
     do_check_eq(count, expectedResults.length);
 
     _("Sync again. This'll wipe bookmarks from the server.");
     try {
       await sync_engine_and_validate_telem(engine, false);
     } catch (ex) {
@@ -360,29 +384,29 @@ async function test_restoreOrImport(aRep
     };
     let expectedTB = {
       id: bookmarkGuids.get(tburi.spec),
       bmkUri: tburi.spec,
       title: "Get Thunderbird!"
     };
 
     let expectedBookmarks;
-    if (aReplace) {
+    if (replace) {
       expectedBookmarks = [expectedFX];
     } else {
       expectedBookmarks = [expectedTB, expectedFX];
     }
 
     doCheckWBOs(bookmarkWBOs, expectedBookmarks);
 
     _("Our old friend Folder 1 is still in play.");
     let expectedFolder1 = { title: "Folder 1" };
 
     let expectedFolders;
-    if (aReplace) {
+    if (replace) {
       expectedFolders = [expectedFolder1];
     } else {
       expectedFolders = [expectedFolder1, expectedFolder1];
     }
 
     doCheckWBOs(folderWBOs, expectedFolders);
 
   } finally {
@@ -453,35 +477,35 @@ add_task(async function test_mismatched_
   newRecord.cleartext = newRecord;
 
   let engine = new BookmarksEngine(Service);
   await engine.initialize();
   let store  = engine._store;
   let server = await serverForFoo(engine);
   await SyncTestingInfrastructure(server);
 
-  _("GUID: " + (await store.GUIDForId(6, true)));
+  _("GUID: " + (await PlacesUtils.promiseItemGuid(6)));
 
   try {
     let bms = PlacesUtils.bookmarks;
     let oldR = new FakeRecord(BookmarkFolder, oldRecord);
     let newR = new FakeRecord(Livemark, newRecord);
     oldR.parentid = PlacesUtils.bookmarks.toolbarGuid;
     newR.parentid = PlacesUtils.bookmarks.toolbarGuid;
 
     await store.applyIncoming(oldR);
     _("Applied old. It's a folder.");
-    let oldID = await store.idForGUID(oldR.id);
+    let oldID = await PlacesUtils.promiseItemId(oldR.id);
     _("Old ID: " + oldID);
     do_check_eq(bms.getItemType(oldID), bms.TYPE_FOLDER);
     do_check_false(PlacesUtils.annotations
                               .itemHasAnnotation(oldID, PlacesUtils.LMANNO_FEEDURI));
 
     await store.applyIncoming(newR);
-    let newID = await store.idForGUID(newR.id);
+    let newID = await PlacesUtils.promiseItemId(newR.id);
     _("New ID: " + newID);
 
     _("Applied new. It's a livemark.");
     do_check_eq(bms.getItemType(newID), bms.TYPE_FOLDER);
     do_check_true(PlacesUtils.annotations
                              .itemHasAnnotation(newID, PlacesUtils.LMANNO_FEEDURI));
 
   } finally {
@@ -503,22 +527,22 @@ add_task(async function test_bookmark_gu
 
   let server = await serverForFoo(engine);
   let coll   = server.user("foo").collection("bookmarks");
   await SyncTestingInfrastructure(server);
 
   // Add one item to the server.
   let itemID = PlacesUtils.bookmarks.createFolder(
     PlacesUtils.bookmarks.toolbarFolder, "Folder 1", 0);
-  let itemGUID = await store.GUIDForId(itemID);
+  let itemGUID = await PlacesUtils.promiseItemGuid(itemID);
   let itemRecord = await store.createRecord(itemGUID);
   let itemPayload = itemRecord.cleartext;
   coll.insert(itemGUID, encryptPayload(itemPayload));
 
-  engine.lastSync = 1;   // So we don't back up.
+  await engine.setLastSync(1);   // So we don't back up.
 
   // Make building the GUID map fail.
 
   let pbt = PlacesUtils.promiseBookmarksTree;
   PlacesUtils.promiseBookmarksTree = function() { return Promise.reject("Nooo"); };
 
   // Ensure that we throw when calling getGuidMap().
   await engine._syncStartup();
@@ -589,39 +613,37 @@ add_task(async function test_bookmark_ta
     type:        "folder"
   });
 
   await store.create(record);
   record.tags = ["bar"];
   await store.update(record);
 });
 
-add_task(async function test_misreconciled_root() {
+add_bookmark_test(async function test_misreconciled_root(engine) {
   _("Ensure that we don't reconcile an arbitrary record with a root.");
 
-  let engine = new BookmarksEngine(Service);
-  await engine.initialize();
   let store = engine._store;
   let server = await serverForFoo(engine);
   await SyncTestingInfrastructure(server);
 
   // Log real hard for this test.
   store._log.trace = store._log.debug;
   engine._log.trace = engine._log.debug;
 
   await engine._syncStartup();
 
   // Let's find out where the toolbar is right now.
   let toolbarBefore = await store.createRecord("toolbar", "bookmarks");
-  let toolbarIDBefore = await store.idForGUID("toolbar");
+  let toolbarIDBefore = PlacesUtils.toolbarFolderId;
   do_check_neq(-1, toolbarIDBefore);
 
-  let parentGUIDBefore = toolbarBefore.parentid;
-  let parentIDBefore = await store.idForGUID(parentGUIDBefore);
-  do_check_neq(-1, parentIDBefore);
+  let parentSyncIDBefore = toolbarBefore.parentid;
+  let parentGUIDBefore = PlacesSyncUtils.bookmarks.syncIdToGuid(parentSyncIDBefore);
+  let parentIDBefore = await PlacesUtils.promiseItemId(parentGUIDBefore);
   do_check_eq("string", typeof(parentGUIDBefore));
 
   _("Current parent: " + parentGUIDBefore + " (" + parentIDBefore + ").");
 
   let to_apply = {
     id: "zzzzzzzzzzzz",
     type: "folder",
     title: "Bookmarks Toolbar",
@@ -629,48 +651,47 @@ add_task(async function test_misreconcil
     parentName: "",
     parentid: "mobile",   // Why not?
     children: [],
   };
 
   let rec = new FakeRecord(BookmarkFolder, to_apply);
 
   _("Applying record.");
-  store.applyIncoming(rec);
+  store.applyIncomingBatch([rec]);
 
   // Ensure that afterwards, toolbar is still there.
   // As of 2012-12-05, this only passes because Places doesn't use "toolbar" as
   // the real GUID, instead using a generated one. Sync does the translation.
   let toolbarAfter = await store.createRecord("toolbar", "bookmarks");
-  let parentGUIDAfter = toolbarAfter.parentid;
-  let parentIDAfter = await store.idForGUID(parentGUIDAfter);
-  do_check_eq((await store.GUIDForId(toolbarIDBefore)), "toolbar");
+  let parentSyncIDAfter = toolbarAfter.parentid;
+  let parentGUIDAfter = PlacesSyncUtils.bookmarks.syncIdToGuid(parentSyncIDAfter);
+  let parentIDAfter = await PlacesUtils.promiseItemId(parentGUIDAfter);
+  do_check_eq((await PlacesUtils.promiseItemGuid(toolbarIDBefore)), PlacesUtils.bookmarks.toolbarGuid);
   do_check_eq(parentGUIDBefore, parentGUIDAfter);
   do_check_eq(parentIDBefore, parentIDAfter);
 
   await store.wipe();
   await engine.resetClient();
   await PlacesSyncUtils.bookmarks.reset();
   await promiseStopServer(server);
 });
 
-add_task(async function test_sync_dateAdded() {
+add_bookmark_test(async function test_sync_dateAdded(engine) {
   await Service.recordManager.clearCache();
   await PlacesSyncUtils.bookmarks.reset();
-  let engine = new BookmarksEngine(Service);
-  await engine.initialize();
   let store  = engine._store;
   let server = await serverForFoo(engine);
   await SyncTestingInfrastructure(server);
 
   let collection = server.user("foo").collection("bookmarks");
 
   // TODO: Avoid random orange (bug 1374599), this is only necessary
   // intermittently - reset the last sync date so that we'll get all bookmarks.
-  engine.lastSync = 1;
+  await engine.setLastSync(1);
 
   Svc.Obs.notify("weave:engine:start-tracking");   // We skip usual startup...
 
   // Just matters that it's in the past, not how far.
   let now = Date.now();
   let oneYearMS = 365 * 24 * 60 * 60 * 1000;
 
   try {
--- a/services/sync/tests/unit/test_bookmark_repair.js
+++ b/services/sync/tests/unit/test_bookmark_repair.js
@@ -1,59 +1,59 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 // Tests the bookmark repair requestor and responder end-to-end (ie, without
 // many mocks)
 Cu.import("resource://gre/modules/Log.jsm");
 Cu.import("resource://gre/modules/osfile.jsm");
+Cu.import("resource://gre/modules/PlacesSyncUtils.jsm");
 Cu.import("resource://services-sync/bookmark_repair.js");
 Cu.import("resource://services-sync/constants.js");
 Cu.import("resource://services-sync/doctor.js");
 Cu.import("resource://services-sync/service.js");
 Cu.import("resource://services-sync/engines/clients.js");
 Cu.import("resource://services-sync/engines/bookmarks.js");
 Cu.import("resource://testing-common/services/sync/utils.js");
 
-const LAST_BOOKMARK_SYNC_PREFS = [
-  "bookmarks.lastSync",
-  "bookmarks.lastSyncLocal",
-];
-
 const BOOKMARK_REPAIR_STATE_PREFS = [
   "client.GUID",
   "doctor.lastRepairAdvance",
-  ...LAST_BOOKMARK_SYNC_PREFS,
   ...Object.values(BookmarkRepairRequestor.PREF).map(name =>
     `repairs.bookmarks.${name}`
   ),
 ];
 
 let clientsEngine;
 let bookmarksEngine;
 var recordedEvents = [];
 
 add_task(async function setup() {
+  await Service.engineManager.unregister("bookmarks");
+  await Service.engineManager.register(BufferedBookmarksEngine);
+
   clientsEngine = Service.clientsEngine;
   bookmarksEngine = Service.engineManager.get("bookmarks");
 
   await generateNewKeys(Service.collectionKeys);
 
   Service.recordTelemetryEvent = (object, method, value, extra = undefined) => {
     recordedEvents.push({ object, method, value, extra });
   };
 
   initTestLogging("Trace");
   Log.repository.getLogger("Sync.Engine.Bookmarks").level = Log.Level.Trace;
   Log.repository.getLogger("Sync.Engine.Clients").level = Log.Level.Trace;
   Log.repository.getLogger("Sqlite").level = Log.Level.Info; // less noisy
 });
 
 function checkRecordedEvents(expected, message) {
-  deepEqual(recordedEvents, expected, message);
+  // Ignore event telemetry from the merger.
+  let repairEvents = recordedEvents.filter(event => event.object != "buffer");
+  deepEqual(repairEvents, expected, message);
   // and clear the list so future checks are easier to write.
   recordedEvents = [];
 }
 
 // Backs up and resets all preferences to their default values. Returns a
 // function that restores the preferences when called.
 function backupPrefs(names) {
   let state = new Map();
@@ -130,29 +130,58 @@ add_task(async function test_bookmark_re
     _(`Upload ${folderInfo.guid} and ${bookmarkInfo.guid} to server`);
     let validationPromise = promiseValidationDone([]);
     await Service.sync();
     equal(clientsEngine.stats.numClients, 2, "Clients collection should have 2 records");
     await validationPromise;
     checkRecordedEvents([], "Should not start repair after first sync");
 
     _("Back up last sync timestamps for remote client");
-    let restoreRemoteLastBookmarkSync = backupPrefs(LAST_BOOKMARK_SYNC_PREFS);
+    let buf = await bookmarksEngine._store.ensureOpenBuffer();
+    let metaRows = await buf.db.execute(`
+      SELECT type, value FROM collectionMeta`);
+    let metaInfos = [];
+    for (let row of metaRows) {
+      metaInfos.push({
+        type: row.getResultByName("type"),
+        value: row.getResultByName("value"),
+      });
+    }
 
     _(`Delete ${bookmarkInfo.guid} locally and on server`);
     // Now we will reach into the server and hard-delete the bookmark
     user.collection("bookmarks").remove(bookmarkInfo.guid);
     // And delete the bookmark, but cheat by telling places that Sync did
     // it, so we don't end up with a tombstone.
     await PlacesUtils.bookmarks.remove(bookmarkInfo.guid, {
       source: PlacesUtils.bookmarks.SOURCE_SYNC,
     });
-    deepEqual((await bookmarksEngine.pullNewChanges()), {},
+    deepEqual((await PlacesSyncUtils.bookmarks.pullChanges()), {},
       `Should not upload tombstone for ${bookmarkInfo.guid}`);
 
+    // Remove the bookmark from the buffer, too.
+    let itemRows = await buf.db.execute(`
+      SELECT guid, kind, title, urlId
+      FROM items
+      WHERE guid = :guid`,
+      { guid: bookmarkInfo.guid });
+    equal(itemRows.length, 1, `Bookmark ${
+      bookmarkInfo.guid} should exist in buffer`);
+    let bufInfos = [];
+    for (let row of itemRows) {
+      bufInfos.push({
+        guid: row.getResultByName("guid"),
+        kind: row.getResultByName("kind"),
+        title: row.getResultByName("title"),
+        urlId: row.getResultByName("urlId"),
+      });
+    }
+    await buf.db.execute(`DELETE FROM items WHERE guid = :guid`,
+                         { guid: bookmarkInfo.guid });
+
     // sync again - we should have a few problems...
     _("Sync again to trigger repair");
     validationPromise = promiseValidationDone([
       {"name": "missingChildren", "count": 1},
       {"name": "structuralDifferences", "count": 1},
     ]);
     await Service.sync();
     await validationPromise;
@@ -203,17 +232,24 @@ add_task(async function test_bookmark_re
     await remoteClientsEngine.initialize();
     remoteClientsEngine.localID = remoteID;
 
     _("Restore missing bookmark");
     // Pretend Sync wrote the bookmark, so that we upload it as part of the
     // repair instead of the sync.
     bookmarkInfo.source = PlacesUtils.bookmarks.SOURCE_SYNC;
     await PlacesUtils.bookmarks.insert(bookmarkInfo);
-    restoreRemoteLastBookmarkSync();
+    await buf.db.execute(`
+      INSERT INTO items(guid, urlId, kind, title)
+      VALUES(:guid, :urlId, :kind, :title)`,
+      bufInfos);
+    await buf.db.execute(`
+      REPLACE INTO collectionMeta(type, value)
+      VALUES(:type, :value)`,
+      metaInfos);
 
     _("Sync as remote client");
     await Service.sync();
     checkRecordedEvents([{
       object: "processcommand",
       method: "repairRequest",
       value: undefined,
       extra: {
@@ -348,19 +384,24 @@ add_task(async function test_repair_clie
     equal(clientsEngine.stats.numClients, 2)
     await validationPromise;
 
     // Delete the bookmark localy, but cheat by telling places that Sync did
     // it, so Sync still thinks we have it.
     await PlacesUtils.bookmarks.remove(bookmarkInfo.guid, {
       source: PlacesUtils.bookmarks.SOURCE_SYNC,
     });
-    // sanity check we aren't going to sync this removal.
-    do_check_empty((await bookmarksEngine.pullNewChanges()));
-    // sanity check that the bookmark is not there anymore
+    // Delete the bookmark from the buffer, too.
+    let buf = await bookmarksEngine._store.ensureOpenBuffer();
+    await buf.db.execute(`DELETE FROM items WHERE guid = :guid`,
+                         { guid: bookmarkInfo.guid });
+
+    // Ensure we won't upload a tombstone for the removed bookmark.
+    do_check_empty((await PlacesSyncUtils.bookmarks.pullChanges()));
+    // Ensure the bookmark no longer exists in Places.
     do_check_false(await PlacesUtils.bookmarks.fetch(bookmarkInfo.guid));
 
     // sync again - we should have a few problems...
     _("Syncing again.");
     validationPromise = promiseValidationDone([
       {"name": "clientMissing", "count": 1},
       {"name": "structuralDifferences", "count": 1},
     ]);
@@ -479,16 +520,17 @@ add_task(async function test_repair_serv
     await Service.sync();
     // should have 2 clients
     equal(clientsEngine.stats.numClients, 2)
     await validationPromise;
 
     // Now we will reach into the server and create a tombstone for that bookmark
     // but with a last-modified in the past - this way our sync isn't going to
     // pick up the record.
+    _(`Adding server tombstone for ${bookmarkInfo.guid}`);
     server.insertWBO("foo", "bookmarks", new ServerWBO(bookmarkInfo.guid, encryptPayload({
       id: bookmarkInfo.guid,
       deleted: true,
     }), (Date.now() - 60000) / 1000));
 
     // sync again - we should have a few problems...
     _("Syncing again.");
     validationPromise = promiseValidationDone([
--- a/services/sync/tests/unit/test_bookmark_repair_responder.js
+++ b/services/sync/tests/unit/test_bookmark_repair_responder.js
@@ -17,17 +17,19 @@ Log.repository.getLogger("Sync.Engine.Bo
 // Disable validation so that we don't try to automatically repair the server
 // when we sync.
 Svc.Prefs.set("engine.bookmarks.validation.enabled", false);
 
 // stub telemetry so we can easily check the right things are recorded.
 var recordedEvents = [];
 
 function checkRecordedEvents(expected) {
-  deepEqual(recordedEvents, expected);
+  // Ignore event telemetry from the merger.
+  let repairEvents = recordedEvents.filter(event => event.object != "buffer");
+  deepEqual(repairEvents, expected);
   // and clear the list so future checks are easier to write.
   recordedEvents = [];
 }
 
 function getServerBookmarks(server) {
   return server.user("foo").collection("bookmarks");
 }
 
--- a/services/sync/tests/unit/test_bookmark_smart_bookmarks.js
+++ b/services/sync/tests/unit/test_bookmark_smart_bookmarks.js
@@ -88,17 +88,17 @@ add_task(async function test_annotation_
   _("New item ID: " + mostVisitedID);
   do_check_true(!!mostVisitedID);
 
   let annoValue = PlacesUtils.annotations.getItemAnnotation(mostVisitedID,
                                               SMART_BOOKMARKS_ANNO);
   _("Anno: " + annoValue);
   do_check_eq("MostVisited", annoValue);
 
-  let guid = await store.GUIDForId(mostVisitedID);
+  let guid = await PlacesUtils.promiseItemGuid(mostVisitedID);
   _("GUID: " + guid);
   do_check_true(!!guid);
 
   _("Create record object and verify that it's sane.");
   let record = await store.createRecord(guid);
   do_check_true(record instanceof Bookmark);
   do_check_true(record instanceof BookmarkQuery);
 
@@ -147,17 +147,17 @@ add_task(async function test_annotation_
     _("Sync. Verify that the downloaded record carries the annotation.");
     await sync_engine_and_validate_telem(engine, false);
 
     _("Verify that the Places DB now has an annotated bookmark.");
     _("Our count has increased again.");
     do_check_eq(smartBookmarkCount(), startCount + 1);
 
     _("Find by GUID and verify that it's annotated.");
-    let newID = await store.idForGUID(serverGUID);
+    let newID = await PlacesUtils.promiseItemId(serverGUID);
     let newAnnoValue = PlacesUtils.annotations.getItemAnnotation(
       newID, SMART_BOOKMARKS_ANNO);
     do_check_eq(newAnnoValue, "MostVisited");
     do_check_eq(PlacesUtils.bookmarks.getBookmarkURI(newID).spec, url);
 
     _("Test updating.");
     let newRecord = await store.createRecord(serverGUID);
     do_check_eq(newRecord.queryId, newAnnoValue);
--- a/services/sync/tests/unit/test_bookmark_tracker.js
+++ b/services/sync/tests/unit/test_bookmark_tracker.js
@@ -441,40 +441,40 @@ add_task(async function test_onItemAdded
 
   try {
     await startTracking();
 
     _("Insert a folder using the sync API");
     let syncFolderID = PlacesUtils.bookmarks.createFolder(
       PlacesUtils.bookmarks.bookmarksMenuFolder, "Sync Folder",
       PlacesUtils.bookmarks.DEFAULT_INDEX);
-    let syncFolderGUID = await engine._store.GUIDForId(syncFolderID);
+    let syncFolderGUID = await PlacesUtils.promiseItemGuid(syncFolderID);
     await verifyTrackedItems(["menu", syncFolderGUID]);
     do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE);
 
     await resetTracker();
     await startTracking();
 
     _("Insert a bookmark using the sync API");
     let syncBmkID = PlacesUtils.bookmarks.insertBookmark(syncFolderID,
       CommonUtils.makeURI("https://example.org/sync"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Sync Bookmark");
-    let syncBmkGUID = await engine._store.GUIDForId(syncBmkID);
+    let syncBmkGUID = await PlacesUtils.promiseItemGuid(syncBmkID);
     await verifyTrackedItems([syncFolderGUID, syncBmkGUID]);
     do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE);
 
     await resetTracker();
     await startTracking();
 
     _("Insert a separator using the sync API");
     let syncSepID = PlacesUtils.bookmarks.insertSeparator(
       PlacesUtils.bookmarks.bookmarksMenuFolder,
       PlacesUtils.bookmarks.getItemIndex(syncFolderID));
-    let syncSepGUID = await engine._store.GUIDForId(syncSepID);
+    let syncSepGUID = await PlacesUtils.promiseItemGuid(syncSepID);
     await verifyTrackedItems(["menu", syncSepGUID]);
     do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE);
   } finally {
     _("Clean up.");
     await cleanup();
   }
 });
 
@@ -565,17 +565,17 @@ add_task(async function test_onItemChang
     await stopTracking();
 
     _("Insert a bookmark");
     let fx_id = PlacesUtils.bookmarks.insertBookmark(
       PlacesUtils.bookmarks.bookmarksMenuFolder,
       CommonUtils.makeURI("http://getfirefox.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Get Firefox!");
-    let fx_guid = await engine._store.GUIDForId(fx_id);
+    let fx_guid = await PlacesUtils.promiseItemGuid(fx_id);
     _(`Firefox GUID: ${fx_guid}`);
 
     await startTracking();
 
     _("Reset the bookmark's added date");
     // Convert to microseconds for PRTime.
     let dateAdded = (Date.now() - DAY_IN_MS) * 1000;
     PlacesUtils.bookmarks.setItemDateAdded(fx_id, dateAdded);
@@ -601,17 +601,17 @@ add_task(async function test_onItemChang
     await stopTracking();
 
     _("Insert a bookmark");
     let fx_id = PlacesUtils.bookmarks.insertBookmark(
       PlacesUtils.bookmarks.bookmarksMenuFolder,
       CommonUtils.makeURI("http://getfirefox.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Get Firefox!");
-    let fx_guid = await engine._store.GUIDForId(fx_id);
+    let fx_guid = await PlacesUtils.promiseItemGuid(fx_id);
     _(`Firefox GUID: ${fx_guid}`);
 
     _("Set a tracked annotation to make sure we only notify once");
     PlacesUtils.annotations.setItemAnnotation(
       fx_id, PlacesSyncUtils.bookmarks.DESCRIPTION_ANNO, "A test description", 0,
       PlacesUtils.annotations.EXPIRE_NEVER);
 
     await startTracking();
@@ -632,26 +632,26 @@ add_task(async function test_onItemTagge
 
   try {
     await stopTracking();
 
     _("Create a folder");
     let folder = PlacesUtils.bookmarks.createFolder(
       PlacesUtils.bookmarks.bookmarksMenuFolder, "Parent",
       PlacesUtils.bookmarks.DEFAULT_INDEX);
-    let folderGUID = await engine._store.GUIDForId(folder);
+    let folderGUID = await PlacesUtils.promiseItemGuid(folder);
     _("Folder ID: " + folder);
     _("Folder GUID: " + folderGUID);
 
     _("Track changes to tags");
     let uri = CommonUtils.makeURI("http://getfirefox.com");
     let b = PlacesUtils.bookmarks.insertBookmark(
       folder, uri,
       PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
-    let bGUID = await engine._store.GUIDForId(b);
+    let bGUID = await PlacesUtils.promiseItemGuid(b);
     _("New item is " + b);
     _("GUID: " + bGUID);
 
     await startTracking();
 
     _("Tag the item");
     PlacesUtils.tagging.tagURI(uri, ["foo"]);
 
@@ -670,22 +670,22 @@ add_task(async function test_onItemUntag
   try {
     await stopTracking();
 
     _("Insert tagged bookmarks");
     let uri = CommonUtils.makeURI("http://getfirefox.com");
     let fx1ID = PlacesUtils.bookmarks.insertBookmark(
       PlacesUtils.bookmarks.bookmarksMenuFolder, uri,
       PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
-    let fx1GUID = await engine._store.GUIDForId(fx1ID);
+    let fx1GUID = await PlacesUtils.promiseItemGuid(fx1ID);
     // Different parent and title; same URL.
     let fx2ID = PlacesUtils.bookmarks.insertBookmark(
       PlacesUtils.bookmarks.toolbarFolder, uri,
       PlacesUtils.bookmarks.DEFAULT_INDEX, "Download Firefox");
-    let fx2GUID = await engine._store.GUIDForId(fx2ID);
+    let fx2GUID = await PlacesUtils.promiseItemGuid(fx2ID);
     PlacesUtils.tagging.tagURI(uri, ["foo"]);
 
     await startTracking();
 
     _("Remove the tag");
     PlacesUtils.tagging.untagURI(uri, ["foo"]);
 
     await verifyTrackedItems([fx1GUID, fx2GUID]);
@@ -805,17 +805,17 @@ add_task(async function test_onItemKeywo
     let folder = PlacesUtils.bookmarks.createFolder(
       PlacesUtils.bookmarks.bookmarksMenuFolder, "Parent",
       PlacesUtils.bookmarks.DEFAULT_INDEX);
     _("Track changes to keywords");
     let uri = CommonUtils.makeURI("http://getfirefox.com");
     let b = PlacesUtils.bookmarks.insertBookmark(
       folder, uri,
       PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
-    let bGUID = await engine._store.GUIDForId(b);
+    let bGUID = await PlacesUtils.promiseItemGuid(b);
     _("New item is " + b);
     _("GUID: " + bGUID);
 
     await startTracking();
 
     _("Give the item a keyword");
     PlacesUtils.bookmarks.setKeywordForBookmark(b, "the_keyword");
 
@@ -910,17 +910,17 @@ add_task(async function test_onItemPostD
     await stopTracking();
 
     _("Insert a bookmark");
     let fx_id = PlacesUtils.bookmarks.insertBookmark(
       PlacesUtils.bookmarks.bookmarksMenuFolder,
       CommonUtils.makeURI("http://getfirefox.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Get Firefox!");
-    let fx_guid = await engine._store.GUIDForId(fx_id);
+    let fx_guid = await PlacesUtils.promiseItemGuid(fx_id);
     _(`Firefox GUID: ${fx_guid}`);
 
     await startTracking();
 
     // PlacesUtils.setPostDataForBookmark is deprecated, but still used by
     // PlacesTransactions.NewBookmark.
     _("Post data for the bookmark should be ignored");
     await PlacesUtils.setPostDataForBookmark(fx_id, "postData");
@@ -939,17 +939,17 @@ add_task(async function test_onItemAnnoC
     await stopTracking();
     let folder = PlacesUtils.bookmarks.createFolder(
       PlacesUtils.bookmarks.bookmarksMenuFolder, "Parent",
       PlacesUtils.bookmarks.DEFAULT_INDEX);
     _("Track changes to annos.");
     let b = PlacesUtils.bookmarks.insertBookmark(
       folder, CommonUtils.makeURI("http://getfirefox.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
-    let bGUID = await engine._store.GUIDForId(b);
+    let bGUID = await PlacesUtils.promiseItemGuid(b);
     _("New item is " + b);
     _("GUID: " + bGUID);
 
     await startTracking();
     PlacesUtils.annotations.setItemAnnotation(
       b, PlacesSyncUtils.bookmarks.DESCRIPTION_ANNO, "A test description", 0,
       PlacesUtils.annotations.EXPIRE_NEVER);
     // bookmark should be tracked, folder should not.
@@ -973,34 +973,34 @@ add_task(async function test_onItemAdded
   try {
     await startTracking();
 
     _("Create a new root");
     let rootID = PlacesUtils.bookmarks.createFolder(
       PlacesUtils.bookmarks.placesRoot,
       "New root",
       PlacesUtils.bookmarks.DEFAULT_INDEX);
-    let rootGUID = await engine._store.GUIDForId(rootID);
+    let rootGUID = await PlacesUtils.promiseItemGuid(rootID);
     _(`New root GUID: ${rootGUID}`);
 
     _("Insert a bookmark underneath the new root");
     let untrackedBmkID = PlacesUtils.bookmarks.insertBookmark(
       rootID,
       CommonUtils.makeURI("http://getthunderbird.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Get Thunderbird!");
-    let untrackedBmkGUID = await engine._store.GUIDForId(untrackedBmkID);
+    let untrackedBmkGUID = await PlacesUtils.promiseItemGuid(untrackedBmkID);
     _(`New untracked bookmark GUID: ${untrackedBmkGUID}`);
 
     _("Insert a bookmark underneath the Places root");
     let rootBmkID = PlacesUtils.bookmarks.insertBookmark(
       PlacesUtils.bookmarks.placesRoot,
       CommonUtils.makeURI("http://getfirefox.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
-    let rootBmkGUID = await engine._store.GUIDForId(rootBmkID);
+    let rootBmkGUID = await PlacesUtils.promiseItemGuid(rootBmkID);
     _(`New Places root bookmark GUID: ${rootBmkGUID}`);
 
     _("New root and bookmark should be ignored");
     await verifyTrackedItems([]);
     do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE * 3);
   } finally {
     _("Clean up.");
     await cleanup();
@@ -1013,17 +1013,17 @@ add_task(async function test_onItemDelet
   try {
     await stopTracking();
 
     _("Insert a bookmark underneath the Places root");
     let rootBmkID = PlacesUtils.bookmarks.insertBookmark(
       PlacesUtils.bookmarks.placesRoot,
       CommonUtils.makeURI("http://getfirefox.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
-    let rootBmkGUID = await engine._store.GUIDForId(rootBmkID);
+    let rootBmkGUID = await PlacesUtils.promiseItemGuid(rootBmkID);
     _(`New Places root bookmark GUID: ${rootBmkGUID}`);
 
     await startTracking();
 
     PlacesUtils.bookmarks.removeItem(rootBmkID);
 
     await verifyTrackedItems([]);
     // We'll still increment the counter for the removed item.
@@ -1166,24 +1166,24 @@ add_task(async function test_onItemMoved
   _("Items moved via the synchronous API should be tracked");
 
   try {
     let fx_id = PlacesUtils.bookmarks.insertBookmark(
       PlacesUtils.bookmarks.bookmarksMenuFolder,
       CommonUtils.makeURI("http://getfirefox.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Get Firefox!");
-    let fx_guid = await engine._store.GUIDForId(fx_id);
+    let fx_guid = await PlacesUtils.promiseItemGuid(fx_id);
     _("Firefox GUID: " + fx_guid);
     let tb_id = PlacesUtils.bookmarks.insertBookmark(
       PlacesUtils.bookmarks.bookmarksMenuFolder,
       CommonUtils.makeURI("http://getthunderbird.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Get Thunderbird!");
-    let tb_guid = await engine._store.GUIDForId(tb_id);
+    let tb_guid = await PlacesUtils.promiseItemGuid(tb_id);
     _("Thunderbird GUID: " + tb_guid);
 
     await startTracking();
 
     // Moving within the folder will just track the folder.
     PlacesUtils.bookmarks.moveItem(
       tb_id, PlacesUtils.bookmarks.bookmarksMenuFolder, 0);
     await verifyTrackedItems(["menu"]);
@@ -1301,42 +1301,42 @@ add_task(async function test_onItemMoved
 
   try {
     await stopTracking();
 
     let folder_id = PlacesUtils.bookmarks.createFolder(
       PlacesUtils.bookmarks.bookmarksMenuFolder,
       "Test folder",
       PlacesUtils.bookmarks.DEFAULT_INDEX);
-    let folder_guid = await engine._store.GUIDForId(folder_id);
+    let folder_guid = await PlacesUtils.promiseItemGuid(folder_id);
     _(`Folder GUID: ${folder_guid}`);
 
     let tb_id = PlacesUtils.bookmarks.insertBookmark(
       folder_id,
       CommonUtils.makeURI("http://getthunderbird.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Thunderbird");
-    let tb_guid = await engine._store.GUIDForId(tb_id);
+    let tb_guid = await PlacesUtils.promiseItemGuid(tb_id);
     _(`Thunderbird GUID: ${tb_guid}`);
 
     let fx_id = PlacesUtils.bookmarks.insertBookmark(
       folder_id,
       CommonUtils.makeURI("http://getfirefox.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Firefox");
-    let fx_guid = await engine._store.GUIDForId(fx_id);
+    let fx_guid = await PlacesUtils.promiseItemGuid(fx_id);
     _(`Firefox GUID: ${fx_guid}`);
 
     let moz_id = PlacesUtils.bookmarks.insertBookmark(
       PlacesUtils.bookmarks.bookmarksMenuFolder,
       CommonUtils.makeURI("https://mozilla.org"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Mozilla"
     );
-    let moz_guid = await engine._store.GUIDForId(moz_id);
+    let moz_guid = await PlacesUtils.promiseItemGuid(moz_id);
     _(`Mozilla GUID: ${moz_guid}`);
 
     await startTracking();
 
     // PlacesSortFolderByNameTransaction exercises
     // PlacesUtils.bookmarks.setItemIndex.
     let txn = new PlacesSortFolderByNameTransaction(folder_id);
 
@@ -1364,31 +1364,31 @@ add_task(async function test_onItemDelet
   try {
     await stopTracking();
 
     _("Create a folder with two children");
     let folder_id = PlacesUtils.bookmarks.createFolder(
       PlacesUtils.bookmarks.bookmarksMenuFolder,
       "Test folder",
       PlacesUtils.bookmarks.DEFAULT_INDEX);
-    let folder_guid = await engine._store.GUIDForId(folder_id);
+    let folder_guid = await PlacesUtils.promiseItemGuid(folder_id);
     _(`Folder GUID: ${folder_guid}`);
     let fx_id = PlacesUtils.bookmarks.insertBookmark(
       folder_id,
       CommonUtils.makeURI("http://getfirefox.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Get Firefox!");
-    let fx_guid = await engine._store.GUIDForId(fx_id);
+    let fx_guid = await PlacesUtils.promiseItemGuid(fx_id);
     _(`Firefox GUID: ${fx_guid}`);
     let tb_id = PlacesUtils.bookmarks.insertBookmark(
       folder_id,
       CommonUtils.makeURI("http://getthunderbird.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Get Thunderbird!");
-    let tb_guid = await engine._store.GUIDForId(tb_id);
+    let tb_guid = await PlacesUtils.promiseItemGuid(tb_id);
     _(`Thunderbird GUID: ${tb_guid}`);
 
     await startTracking();
 
     let txn = PlacesUtils.bookmarks.getRemoveFolderTransaction(folder_id);
     // We haven't executed the transaction yet.
     await verifyTrackerEmpty();
 
@@ -1422,24 +1422,24 @@ add_task(async function test_treeMoved()
   _("Moving an entire tree of bookmarks should track the parents");
 
   try {
     // Create a couple of parent folders.
     let folder1_id = PlacesUtils.bookmarks.createFolder(
       PlacesUtils.bookmarks.bookmarksMenuFolder,
       "First test folder",
       PlacesUtils.bookmarks.DEFAULT_INDEX);
-    let folder1_guid = await engine._store.GUIDForId(folder1_id);
+    let folder1_guid = await PlacesUtils.promiseItemGuid(folder1_id);
 
     // A second folder in the first.
     let folder2_id = PlacesUtils.bookmarks.createFolder(
       folder1_id,
       "Second test folder",
       PlacesUtils.bookmarks.DEFAULT_INDEX);
-    let folder2_guid = await engine._store.GUIDForId(folder2_id);
+    let folder2_guid = await PlacesUtils.promiseItemGuid(folder2_id);
 
     // Create a couple of bookmarks in the second folder.
     PlacesUtils.bookmarks.insertBookmark(
       folder2_id,
       CommonUtils.makeURI("http://getfirefox.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Get Firefox!");
     PlacesUtils.bookmarks.insertBookmark(
@@ -1471,17 +1471,17 @@ add_task(async function test_onItemDelet
       CommonUtils.makeURI("http://getfirefox.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Get Firefox!");
     let tb_id = PlacesUtils.bookmarks.insertBookmark(
       PlacesUtils.bookmarks.bookmarksMenuFolder,
       CommonUtils.makeURI("http://getthunderbird.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Get Thunderbird!");
-    let tb_guid = await engine._store.GUIDForId(tb_id);
+    let tb_guid = await PlacesUtils.promiseItemGuid(tb_id);
 
     await startTracking();
 
     // Delete the last item - the item and parent should be tracked.
     PlacesUtils.bookmarks.removeItem(tb_id);
 
     await verifyTrackedItems(["menu", tb_guid]);
     do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE);
@@ -1611,34 +1611,34 @@ add_task(async function test_onItemDelet
   _("Removing a folder's children should track the folder and its children");
 
   try {
     let fx_id = PlacesUtils.bookmarks.insertBookmark(
       PlacesUtils.mobileFolderId,
       CommonUtils.makeURI("http://getfirefox.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Get Firefox!");
-    let fx_guid = await engine._store.GUIDForId(fx_id);
+    let fx_guid = await PlacesUtils.promiseItemGuid(fx_id);
     _(`Firefox GUID: ${fx_guid}`);
 
     let tb_id = PlacesUtils.bookmarks.insertBookmark(
       PlacesUtils.mobileFolderId,
       CommonUtils.makeURI("http://getthunderbird.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Get Thunderbird!");
-    let tb_guid = await engine._store.GUIDForId(tb_id);
+    let tb_guid = await PlacesUtils.promiseItemGuid(tb_id);
     _(`Thunderbird GUID: ${tb_guid}`);
 
     let moz_id = PlacesUtils.bookmarks.insertBookmark(
       PlacesUtils.bookmarks.bookmarksMenuFolder,
       CommonUtils.makeURI("https://mozilla.org"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Mozilla"
     );
-    let moz_guid = await engine._store.GUIDForId(moz_id);
+    let moz_guid = await PlacesUtils.promiseItemGuid(moz_id);
     _(`Mozilla GUID: ${moz_guid}`);
 
     await startTracking();
 
     _(`Mobile root ID: ${PlacesUtils.mobileFolderId}`);
     PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.mobileFolderId);
 
     await verifyTrackedItems(["mobile", fx_guid, tb_guid]);
@@ -1653,210 +1653,43 @@ add_task(async function test_onItemDelet
   _("Deleting a tree of bookmarks should track all items");
 
   try {
     // Create a couple of parent folders.
     let folder1_id = PlacesUtils.bookmarks.createFolder(
       PlacesUtils.bookmarks.bookmarksMenuFolder,
       "First test folder",
       PlacesUtils.bookmarks.DEFAULT_INDEX);
-    let folder1_guid = await engine._store.GUIDForId(folder1_id);
+    let folder1_guid = await PlacesUtils.promiseItemGuid(folder1_id);
 
     // A second folder in the first.
     let folder2_id = PlacesUtils.bookmarks.createFolder(
       folder1_id,
       "Second test folder",
       PlacesUtils.bookmarks.DEFAULT_INDEX);
-    let folder2_guid = await engine._store.GUIDForId(folder2_id);
+    let folder2_guid = await PlacesUtils.promiseItemGuid(folder2_id);
 
     // Create a couple of bookmarks in the second folder.
     let fx_id = PlacesUtils.bookmarks.insertBookmark(
       folder2_id,
       CommonUtils.makeURI("http://getfirefox.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Get Firefox!");
-    let fx_guid = await engine._store.GUIDForId(fx_id);
+    let fx_guid = await PlacesUtils.promiseItemGuid(fx_id);
     let tb_id = PlacesUtils.bookmarks.insertBookmark(
       folder2_id,
       CommonUtils.makeURI("http://getthunderbird.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Get Thunderbird!");
-    let tb_guid = await engine._store.GUIDForId(tb_id);
+    let tb_guid = await PlacesUtils.promiseItemGuid(tb_id);
 
     await startTracking();
 
     // Delete folder2 - everything we created should be tracked.
     PlacesUtils.bookmarks.removeItem(folder2_id);
 
     await verifyTrackedItems([fx_guid, tb_guid, folder1_guid, folder2_guid]);
     do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE * 3);
   } finally {
     _("Clean up.");
     await cleanup();
   }
 });
-
-add_task(async function test_skip_migration() {
-  await insertBookmarksToMigrate();
-
-  let originalTombstones = await PlacesTestUtils.fetchSyncTombstones();
-  let originalFields = await PlacesTestUtils.fetchBookmarkSyncFields(
-    "0gtWTOgYcoJD", "0dbpnMdxKxfg", "r5ouWdPB3l28", "YK5Bdq5MIqL6");
-
-  let filePath = OS.Path.join(OS.Constants.Path.profileDir, "weave", "changes",
-    "bookmarks.json");
-
-  _("No tracker file");
-  {
-    await Utils.jsonRemove("changes/bookmarks", tracker);
-    ok(!(await OS.File.exists(filePath)), "Tracker file should not exist");
-
-    await tracker._migrateOldEntries();
-
-    let fields = await PlacesTestUtils.fetchBookmarkSyncFields(
-      "0gtWTOgYcoJD", "0dbpnMdxKxfg", "r5ouWdPB3l28", "YK5Bdq5MIqL6");
-    deepEqual(fields, originalFields,
-      "Sync fields should not change if tracker file is missing");
-    let tombstones = await PlacesTestUtils.fetchSyncTombstones();
-    deepEqual(tombstones, originalTombstones,
-      "Tombstones should not change if tracker file is missing");
-  }
-
-  _("Existing tracker file; engine disabled");
-  {
-    await Utils.jsonSave("changes/bookmarks", tracker, {});
-    ok(await OS.File.exists(filePath),
-      "Tracker file should exist before disabled engine migration");
-
-    engine.disabled = true;
-    await tracker._migrateOldEntries();
-    engine.disabled = false;
-
-    let fields = await PlacesTestUtils.fetchBookmarkSyncFields(
-      "0gtWTOgYcoJD", "0dbpnMdxKxfg", "r5ouWdPB3l28", "YK5Bdq5MIqL6");
-    deepEqual(fields, originalFields,
-      "Sync fields should not change on disabled engine migration");
-    let tombstones = await PlacesTestUtils.fetchSyncTombstones();
-    deepEqual(tombstones, originalTombstones,
-      "Tombstones should not change if tracker file is missing");
-
-    ok(!(await OS.File.exists(filePath)),
-      "Tracker file should be deleted after disabled engine migration");
-  }
-
-  _("Existing tracker file; first sync");
-  {
-    await Utils.jsonSave("changes/bookmarks", tracker, {});
-    ok(await OS.File.exists(filePath),
-      "Tracker file should exist before first sync migration");
-
-    engine.lastSync = 0;
-    await tracker._migrateOldEntries();
-
-    let fields = await PlacesTestUtils.fetchBookmarkSyncFields(
-      "0gtWTOgYcoJD", "0dbpnMdxKxfg", "r5ouWdPB3l28", "YK5Bdq5MIqL6");
-    deepEqual(fields, originalFields,
-      "Sync fields should not change on first sync migration");
-    let tombstones = await PlacesTestUtils.fetchSyncTombstones();
-    deepEqual(tombstones, originalTombstones,
-      "Tombstones should not change if tracker file is missing");
-
-    ok(!(await OS.File.exists(filePath)),
-      "Tracker file should be deleted after first sync migration");
-  }
-
-  await cleanup();
-});
-
-add_task(async function test_migrate_empty_tracker() {
-  _("Migration with empty tracker file");
-  await insertBookmarksToMigrate();
-
-  await Utils.jsonSave("changes/bookmarks", tracker, {});
-
-  engine.lastSync = Date.now() / 1000;
-  await tracker._migrateOldEntries();
-
-  let fields = await PlacesTestUtils.fetchBookmarkSyncFields(
-    "0gtWTOgYcoJD", "0dbpnMdxKxfg", "r5ouWdPB3l28", "YK5Bdq5MIqL6");
-  for (let field of fields) {
-    equal(field.syncStatus, PlacesUtils.bookmarks.SYNC_STATUS.NORMAL,
-      `Sync status of migrated bookmark ${field.guid} should be NORMAL`);
-    strictEqual(field.syncChangeCounter, 0,
-      `Change counter of migrated bookmark ${field.guid} should be 0`);
-  }
-
-  let tombstones = await PlacesTestUtils.fetchSyncTombstones();
-  deepEqual(tombstones, [], "Migration should delete old tombstones");
-
-  let filePath = OS.Path.join(OS.Constants.Path.profileDir, "weave", "changes",
-    "bookmarks.json");
-  ok(!(await OS.File.exists(filePath)),
-    "Tracker file should be deleted after empty tracker migration");
-
-  await cleanup();
-});
-
-add_task(async function test_migrate_existing_tracker() {
-  _("Migration with existing tracker entries");
-  await insertBookmarksToMigrate();
-
-  let mozBmk = await PlacesUtils.bookmarks.fetch("0gtWTOgYcoJD");
-  let fxBmk = await PlacesUtils.bookmarks.fetch("0dbpnMdxKxfg");
-  let mozChangeTime = Math.floor(mozBmk.lastModified / 1000) - 60;
-  let fxChangeTime = Math.floor(fxBmk.lastModified / 1000) + 60;
-  await Utils.jsonSave("changes/bookmarks", tracker, {
-    "0gtWTOgYcoJD": mozChangeTime,
-    "0dbpnMdxKxfg": {
-      modified: fxChangeTime,
-      deleted: false,
-    },
-    "3kdIPWHs9hHC": {
-      modified: 1479494951,
-      deleted: true,
-    },
-    "l7DlMy2lL1jL": 1479496460,
-  });
-
-  engine.lastSync = Date.now() / 1000;
-  await tracker._migrateOldEntries();
-
-  let changedFields = await PlacesTestUtils.fetchBookmarkSyncFields(
-    "0gtWTOgYcoJD", "0dbpnMdxKxfg");
-  for (let field of changedFields) {
-    if (field.guid == "0gtWTOgYcoJD") {
-      ok(field.lastModified.getTime(), mozBmk.lastModified.getTime(),
-        `Modified time for ${field.guid} should not be reset to older change time`);
-    } else if (field.guid == "0dbpnMdxKxfg") {
-      equal(field.lastModified.getTime(), fxChangeTime * 1000,
-        `Modified time for ${field.guid} should be updated to newer change time`);
-    }
-    equal(field.syncStatus, PlacesUtils.bookmarks.SYNC_STATUS.NORMAL,
-      `Sync status of migrated bookmark ${field.guid} should be NORMAL`);
-    ok(field.syncChangeCounter > 0,
-      `Change counter of migrated bookmark ${field.guid} should be > 0`);
-  }
-
-  let unchangedFields = await PlacesTestUtils.fetchBookmarkSyncFields(
-    "r5ouWdPB3l28", "YK5Bdq5MIqL6");
-  for (let field of unchangedFields) {
-    equal(field.syncStatus, PlacesUtils.bookmarks.SYNC_STATUS.NORMAL,
-      `Sync status of unchanged bookmark ${field.guid} should be NORMAL`);
-    strictEqual(field.syncChangeCounter, 0,
-      `Change counter of unchanged bookmark ${field.guid} should be 0`);
-  }
-
-  let tombstones = await PlacesTestUtils.fetchSyncTombstones();
-  await deepEqual(tombstones, [{
-    guid: "3kdIPWHs9hHC",
-    dateRemoved: new Date(1479494951 * 1000),
-  }, {
-    guid: "l7DlMy2lL1jL",
-    dateRemoved: new Date(1479496460 * 1000),
-  }], "Should write tombstones for deleted tracked items");
-
-  let filePath = OS.Path.join(OS.Constants.Path.profileDir, "weave", "changes",
-    "bookmarks.json");
-  ok(!(await OS.File.exists(filePath)),
-    "Tracker file should be deleted after existing tracker migration");
-
-  await cleanup();
-});
--- a/services/sync/tests/unit/test_telemetry.js
+++ b/services/sync/tests/unit/test_telemetry.js
@@ -176,16 +176,17 @@ add_task(async function test_uploading()
     equal(ping.engines.length, 1);
     equal(ping.engines[0].name, "bookmarks");
     ok(!!ping.engines[0].outgoing);
     greater(ping.engines[0].outgoing[0].sent, 0)
     ok(!ping.engines[0].incoming);
 
     PlacesUtils.bookmarks.setItemTitle(bmk_id, "New Title");
 
+    await store.wipe();
     await engine.resetClient();
 
     ping = await sync_engine_and_validate_telem(engine, false);
     equal(ping.engines.length, 1);
     equal(ping.engines[0].name, "bookmarks");
     equal(ping.engines[0].outgoing.length, 1);
     ok(!!ping.engines[0].incoming);
 
--- a/tools/lint/eslint/modules.json
+++ b/tools/lint/eslint/modules.json
@@ -16,17 +16,17 @@
   "AsyncSpellCheckTestHelper.jsm": ["onSpellCheck"],
   "AutoMigrate.jsm": ["AutoMigrate"],
   "Battery.jsm": ["GetBattery", "Battery"],
   "blocklist-clients.js": ["AddonBlocklistClient", "GfxBlocklistClient", "OneCRLBlocklistClient", "PluginBlocklistClient"],
   "blocklist-updater.js": ["checkVersions", "addTestBlocklistClient"],
   "bogus_element_type.jsm": [],
   "bookmark_repair.js": ["BookmarkRepairRequestor", "BookmarkRepairResponder"],
   "bookmark_validator.js": ["BookmarkValidator", "BookmarkProblemData"],
-  "bookmarks.js": ["BookmarksEngine", "PlacesItem", "Bookmark", "BookmarkFolder", "BookmarkQuery", "Livemark", "BookmarkSeparator"],
+  "bookmarks.js": ["BookmarksEngine", "PlacesItem", "Bookmark", "BookmarkFolder", "BookmarkQuery", "Livemark", "BookmarkSeparator", "BufferedBookmarksEngine"],
   "bookmarks.jsm": ["PlacesItem", "Bookmark", "Separator", "Livemark", "BookmarkFolder", "DumpBookmarks"],
   "BootstrapMonitor.jsm": ["monitor"],
   "browser-loader.js": ["BrowserLoader"],
   "browserid_identity.js": ["BrowserIDManager", "AuthenticationError"],
   "CertUtils.jsm": ["BadCertHandler", "checkCert", "readCertPrefs", "validateCert"],
   "clients.js": ["ClientEngine", "ClientsRec"],
   "collection_repair.js": ["getRepairRequestor", "getAllRepairRequestors", "CollectionRepairRequestor", "getRepairResponder", "CollectionRepairResponder"],
   "collection_validator.js": ["CollectionValidator", "CollectionProblemData"],