Bug 1274108 - Migrate the bookmarks engine to `PlacesSyncUtils`. r=markh draft
authorKit Cambridge <kcambridge@mozilla.com>
Wed, 10 Aug 2016 12:55:02 -0700
changeset 399700 5f9cdef7c48dc0b79506f74257dbfc5d4484d1ff
parent 399699 cabb31bd5a3d3438c1d7e357568733a55843757f
child 399701 9438057253155da8a17fd79cfa286f89b7d8a9a1
push id25947
push userbmo:kcambridge@mozilla.com
push dateThu, 11 Aug 2016 21:53:22 +0000
reviewersmarkh
bugs1274108
milestone51.0a1
Bug 1274108 - Migrate the bookmarks engine to `PlacesSyncUtils`. r=markh MozReview-Commit-ID: Gs35dVNVhPx
services/sync/modules/bookmark_utils.js
services/sync/modules/engines/bookmarks.js
services/sync/tests/unit/test_bookmark_engine.js
services/sync/tests/unit/test_bookmark_livemarks.js
services/sync/tests/unit/test_bookmark_order.js
services/sync/tests/unit/test_bookmark_places_query_rewriting.js
--- a/services/sync/modules/bookmark_utils.js
+++ b/services/sync/modules/bookmark_utils.js
@@ -4,29 +4,50 @@
 
 "use strict";
 
 this.EXPORTED_SYMBOLS = ["BookmarkSpecialIds", "BookmarkAnnos"];
 
 const { utils: Cu, interfaces: Ci, classes: Cc } = Components;
 
 Cu.import("resource://gre/modules/PlacesUtils.jsm");
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+Cu.import("resource://services-common/async.js");
 
 let BookmarkAnnos = {
   ALLBOOKMARKS_ANNO:    "AllBookmarks",
   DESCRIPTION_ANNO:     "bookmarkProperties/description",
   SIDEBAR_ANNO:         "bookmarkProperties/loadInSidebar",
   MOBILEROOT_ANNO:      "mobile/bookmarksRoot",
   MOBILE_ANNO:          "MobileBookmarks",
   EXCLUDEBACKUP_ANNO:   "places/excludeFromBackup",
   SMART_BOOKMARKS_ANNO: "Places/SmartBookmark",
   PARENT_ANNO:          "sync/parent",
   ORGANIZERQUERY_ANNO:  "PlacesOrganizer/OrganizerQuery",
 };
 
+// Accessing `PlacesUtils.bookmarks` initializes the bookmarks service, which,
+// in turn, initializes the database and upgrades the schema as a side effect.
+// This causes Sync unit tests that exercise older database formats to fail,
+// so we define this map as a lazy getter.
+XPCOMUtils.defineLazyGetter(this, "SpecialGUIDToPlacesGUID", () => {
+  return {
+    "menu": PlacesUtils.bookmarks.menuGuid,
+    "places": PlacesUtils.bookmarks.rootGuid,
+    "tags": PlacesUtils.bookmarks.tagsGuid,
+    "toolbar": PlacesUtils.bookmarks.toolbarGuid,
+    "unfiled": PlacesUtils.bookmarks.unfiledGuid,
+
+    get mobile() {
+      let mobileRootId = BookmarkSpecialIds.findMobileRoot(true);
+      return Async.promiseSpinningly(PlacesUtils.promiseItemGuid(mobileRootId));
+    },
+  };
+});
+
 let BookmarkSpecialIds = {
 
   // Special IDs. Note that mobile can attempt to create a record on
   // dereference; special accessors are provided to prevent recursion within
   // observers.
   guids: ["menu", "places", "tags", "toolbar", "unfiled", "mobile"],
 
   // Create the special mobile folder to store mobile bookmarks.
@@ -68,16 +89,20 @@ let BookmarkSpecialIds = {
   // Don't bother creating mobile: if it doesn't exist, this ID can't be it!
   specialGUIDForId: function specialGUIDForId(id) {
     for (let guid of this.guids)
       if (this.specialIdForGUID(guid, false) == id)
         return guid;
     return null;
   },
 
+  syncIDToPlacesGUID(g) {
+    return g in SpecialGUIDToPlacesGUID ? SpecialGUIDToPlacesGUID[g] : g;
+  },
+
   get menu() {
     return PlacesUtils.bookmarksMenuFolderId;
   },
   get places() {
     return PlacesUtils.placesRootId;
   },
   get tags() {
     return PlacesUtils.tagsFolderId;
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -6,32 +6,46 @@ this.EXPORTED_SYMBOLS = ['BookmarksEngin
                          "BookmarkFolder", "BookmarkQuery",
                          "Livemark", "BookmarkSeparator"];
 
 var Cc = Components.classes;
 var Ci = Components.interfaces;
 var Cu = Components.utils;
 
 Cu.import("resource://gre/modules/PlacesUtils.jsm");
+Cu.import("resource://gre/modules/PlacesSyncUtils.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.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");
 Cu.import("resource://services-sync/bookmark_utils.js");
 Cu.import("resource://gre/modules/Task.jsm");
 Cu.import("resource://gre/modules/PlacesBackups.jsm");
 
 const ANNOS_TO_TRACK = [BookmarkAnnos.DESCRIPTION_ANNO, BookmarkAnnos.SIDEBAR_ANNO,
                         PlacesUtils.LMANNO_FEEDURI, PlacesUtils.LMANNO_SITEURI];
 
 const SERVICE_NOT_SUPPORTED = "Service not supported on this platform";
 const FOLDER_SORTINDEX = 1000000;
 
+// Maps Sync record property names to `PlacesSyncUtils` bookmark properties.
+const RECORD_PROPS_TO_BOOKMARK_PROPS = {
+  title: "title",
+  bmkUri: "url",
+  tags: "tags",
+  keyword: "keyword",
+  description: "description",
+  loadInSidebar: "loadInSidebar",
+  queryId: "query",
+  siteUri: "site",
+  feedUri: "feed",
+};
+
 this.PlacesItem = function PlacesItem(collection, id, type) {
   CryptoWrapper.call(this, collection, id);
   this.type = type || "item";
 }
 PlacesItem.prototype = {
   decrypt: function PlacesItem_decrypt(keyBundle) {
     // Do the normal CryptoWrapper decrypt, but change types before returning
     let clear = CryptoWrapper.prototype.decrypt.call(this, keyBundle);
@@ -134,38 +148,16 @@ BookmarksEngine.prototype = {
   _recordObj: PlacesItem,
   _storeObj: BookmarksStore,
   _trackerObj: BookmarksTracker,
   version: 2,
   _defaultSort: "index",
 
   syncPriority: 4,
 
-  _sync: function _sync() {
-    let engine = this;
-    let batchEx = null;
-
-    // Try running sync in batch mode
-    PlacesUtils.bookmarks.runInBatchMode({
-      runBatched: function wrappedSync() {
-        try {
-          SyncEngine.prototype._sync.call(engine);
-        }
-        catch(ex) {
-          batchEx = ex;
-        }
-      }
-    }, null);
-
-    // Expose the exception if something inside the batch failed
-    if (batchEx != null) {
-      throw batchEx;
-    }
-  },
-
   // A diagnostic helper to get the string value for a bookmark's URL given
   // its ID. Always returns a string - on error will return a string in the
   // form of "<description of error>" as this is purely for, eg, logging.
   // (This means hitting the DB directly and we don't bother using a cached
   // statement - we should rarely hit this.)
   _getStringUrlForId(id) {
     let url;
     try {
@@ -304,37 +296,37 @@ BookmarksEngine.prototype = {
 
     // Figure out if we have a map to use!
     // This will throw in some circumstances. That's fine.
     let guidMap = this._guidMap;
 
     // Give the GUID if we have the matching pair.
     this._log.trace("Finding mapping: " + item.parentName + ", " + key);
     let parent = guidMap[item.parentName];
-    
+
     if (!parent) {
       this._log.trace("No parent => no dupe.");
       return undefined;
     }
-      
+
     let dupe = parent[key];
-    
+
     if (dupe) {
       this._log.trace("Mapped dupe: " + dupe);
       return dupe;
     }
-    
+
     if (altKey) {
       dupe = parent[altKey];
       if (dupe) {
         this._log.trace("Mapped dupe using altKey " + altKey + ": " + dupe);
         return dupe;
       }
     }
-    
+
     this._log.trace("No dupe found for key " + key + "/" + altKey + ".");
     return undefined;
   },
 
   _syncStartup: function _syncStart() {
     SyncEngine.prototype._syncStartup.call(this);
 
     let cb = Async.makeSpinningCallback();
@@ -439,75 +431,17 @@ function BookmarksStore(name, engine) {
   }, this);
 }
 BookmarksStore.prototype = {
   __proto__: Store.prototype,
 
   itemExists: function BStore_itemExists(id) {
     return this.idForGUID(id, true) > 0;
   },
-  
-  /*
-   * If the record is a tag query, rewrite it to refer to the local tag ID.
-   * 
-   * Otherwise, just return.
-   */
-  preprocessTagQuery: function preprocessTagQuery(record) {
-    if (record.type != "query" ||
-        record.bmkUri == null ||
-        !record.folderName)
-      return;
-    
-    // Yes, this works without chopping off the "place:" prefix.
-    let uri           = record.bmkUri
-    let queriesRef    = {};
-    let queryCountRef = {};
-    let optionsRef    = {};
-    PlacesUtils.history.queryStringToQueries(uri, queriesRef, queryCountRef,
-                                             optionsRef);
-    
-    // We only process tag URIs.
-    if (optionsRef.value.resultType != optionsRef.value.RESULTS_AS_TAG_CONTENTS)
-      return;
-    
-    // Tag something to ensure that the tag exists.
-    let tag = record.folderName;
-    let dummyURI = Utils.makeURI("about:weave#BStore_preprocess");
-    PlacesUtils.tagging.tagURI(dummyURI, [tag]);
 
-    // Look for the id of the tag, which might just have been added.
-    let tags = this._getNode(PlacesUtils.tagsFolderId);
-    if (!(tags instanceof Ci.nsINavHistoryQueryResultNode)) {
-      this._log.debug("tags isn't an nsINavHistoryQueryResultNode; aborting.");
-      return;
-    }
-
-    tags.containerOpen = true;
-    try {
-      for (let i = 0; i < tags.childCount; i++) {
-        let child = tags.getChild(i);
-        if (child.title == tag) {
-          // Found the tag, so fix up the query to use the right id.
-          this._log.debug("Tag query folder: " + tag + " = " + child.itemId);
-          
-          this._log.trace("Replacing folders in: " + uri);
-          for (let q of queriesRef.value)
-            q.setFolders([child.itemId], 1);
-          
-          record.bmkUri = PlacesUtils.history.queriesToQueryString(
-            queriesRef.value, queryCountRef.value, optionsRef.value);
-          return;
-        }
-      }
-    }
-    finally {
-      tags.containerOpen = false;
-    }
-  },
-  
   applyIncoming: function BStore_applyIncoming(record) {
     this._log.debug("Applying record " + record.id);
     let isSpecial = record.id in BookmarkSpecialIds;
 
     if (record.deleted) {
       if (isSpecial) {
         this._log.warn("Ignoring deletion for special record " + record.id);
         return;
@@ -528,415 +462,175 @@ BookmarksStore.prototype = {
 
     // Skip malformed records. (Bug 806460.)
     if (record.type == "query" &&
         !record.bmkUri) {
       this._log.warn("Skipping malformed query bookmark: " + record.id);
       return;
     }
 
-    // Preprocess the record before doing the normal apply.
-    this.preprocessTagQuery(record);
-
     // Figure out the local id of the parent GUID if available
     let parentGUID = record.parentid;
     if (!parentGUID) {
       throw "Record " + record.id + " has invalid parentid: " + parentGUID;
     }
     this._log.debug("Local parent is " + parentGUID);
 
-    let parentId = this.idForGUID(parentGUID);
-    if (parentId > 0) {
-      // Save the parent id for modifying the bookmark later
-      record._parent = parentId;
-      record._orphan = false;
-      this._log.debug("Record " + record.id + " is not an orphan.");
-    } else {
-      this._log.trace("Record " + record.id +
-                      " is an orphan: could not find parent " + parentGUID);
-      record._orphan = true;
-    }
-
     // Do the normal processing of incoming records
     Store.prototype.applyIncoming.call(this, record);
 
-    // Do some post-processing if we have an item
-    let itemId = this.idForGUID(record.id);
-    if (itemId > 0) {
-      // Move any children that are looking for this folder as a parent
-      if (record.type == "folder") {
-        this._reparentOrphans(itemId);
-        // Reorder children later
-        if (record.children)
-          this._childrenToOrder[record.id] = record.children;
-      }
-
-      // Create an annotation to remember that it needs reparenting.
-      if (record._orphan) {
-        PlacesUtils.annotations.setItemAnnotation(
-          itemId, BookmarkAnnos.PARENT_ANNO, parentGUID, 0,
-          PlacesUtils.annotations.EXPIRE_NEVER);
-      }
-    }
-  },
-
-  /**
-   * Find all ids of items that have a given value for an annotation
-   */
-  _findAnnoItems: function BStore__findAnnoItems(anno, val) {
-    return PlacesUtils.annotations.getItemsWithAnnotation(anno, {})
-                      .filter(function(id) {
-      return PlacesUtils.annotations.getItemAnnotation(id, anno) == val;
-    });
-  },
-
-  /**
-   * For the provided parent item, attach its children to it
-   */
-  _reparentOrphans: function _reparentOrphans(parentId) {
-    // Find orphans and reunite with this folder parent
-    let parentGUID = this.GUIDForId(parentId);
-    let orphans = this._findAnnoItems(BookmarkAnnos.PARENT_ANNO, parentGUID);
-
-    this._log.debug("Reparenting orphans " + orphans + " to " + parentId);
-    orphans.forEach(function(orphan) {
-      // Move the orphan to the parent and drop the missing parent annotation
-      if (this._reparentItem(orphan, parentId)) {
-        PlacesUtils.annotations.removeItemAnnotation(orphan, BookmarkAnnos.PARENT_ANNO);
-      }
-    }, this);
-  },
-
-  _reparentItem: function _reparentItem(itemId, parentId) {
-    this._log.trace("Attempting to move item " + itemId + " to new parent " +
-                    parentId);
-    try {
-      if (parentId > 0) {
-        PlacesUtils.bookmarks.moveItem(itemId, parentId,
-                                       PlacesUtils.bookmarks.DEFAULT_INDEX);
-        return true;
-      }
-    } catch(ex) {
-      this._log.debug("Failed to reparent item", ex);
-    }
-    return false;
-  },
-
-  // Turn a record's nsINavBookmarksService constant and other attributes into
-  // a granular type for comparison.
-  _recordType: function _recordType(itemId) {
-    let bms  = PlacesUtils.bookmarks;
-    let type = bms.getItemType(itemId);
-
-    switch (type) {
-      case bms.TYPE_FOLDER:
-        if (PlacesUtils.annotations
-                       .itemHasAnnotation(itemId, PlacesUtils.LMANNO_FEEDURI)) {
-          return "livemark";
-        }
-        return "folder";
-
-      case bms.TYPE_BOOKMARK:
-        let bmkUri = bms.getBookmarkURI(itemId).spec;
-        if (bmkUri.indexOf("place:") == 0) {
-          return "query";
-        }
-        return "bookmark";
-
-      case bms.TYPE_SEPARATOR:
-        return "separator";
-
-      default:
-        return null;
+    if (record.type == "folder" && record.children) {
+      this._childrenToOrder[record.id] = record.children;
     }
   },
 
   create: function BStore_create(record) {
-    // Default to unfiled if we don't have the parent yet.
-    
-    // Valid parent IDs are all positive integers. Other values -- undefined,
-    // null, -1 -- all compare false for > 0, so this catches them all. We
-    // don't just use <= without the !, because undefined and null compare
-    // false for that, too!
-    if (!(record._parent > 0)) {
-      this._log.debug("Parent is " + record._parent + "; reparenting to unfiled.");
-      record._parent = BookmarkSpecialIds.unfiled;
-    }
-
     switch (record.type) {
     case "bookmark":
     case "query":
     case "microsummary": {
-      let uri = Utils.makeURI(record.bmkUri);
-      let newId = PlacesUtils.bookmarks.insertBookmark(
-        record._parent, uri, PlacesUtils.bookmarks.DEFAULT_INDEX, record.title,
-        record.id);
-      this._log.debug("created bookmark " + newId + " under " + record._parent
-                      + " as " + record.title + " " + record.bmkUri);
-
-      // Smart bookmark annotations are strings.
+      let info = {
+        kind: record.type,
+        url: record.bmkUri,
+        parentGuid: BookmarkSpecialIds.syncIDToPlacesGUID(record.parentid),
+        title: record.title,
+        guid: BookmarkSpecialIds.syncIDToPlacesGUID(record.id),
+        tags: record.tags,
+        keyword: record.keyword,
+      };
+      if (record.loadInSidebar) {
+        info.loadInSidebar = record.loadInSidebar;
+      }
       if (record.queryId) {
-        PlacesUtils.annotations.setItemAnnotation(
-          newId, BookmarkAnnos.SMART_BOOKMARKS_ANNO, record.queryId, 0,
-          PlacesUtils.annotations.EXPIRE_NEVER);
+        info.query = record.queryId;
+      }
+      if (record.folderName) {
+        info.folder = record.folderName;
+      }
+      if (record.description) {
+        info.description = record.description;
       }
 
-      if (Array.isArray(record.tags)) {
-        this._tagURI(uri, record.tags);
-      }
-      PlacesUtils.bookmarks.setKeywordForBookmark(newId, record.keyword);
-      if (record.description) {
-        PlacesUtils.annotations.setItemAnnotation(
-          newId, BookmarkAnnos.DESCRIPTION_ANNO, record.description, 0,
-          PlacesUtils.annotations.EXPIRE_NEVER);
-      }
-
-      if (record.loadInSidebar) {
-        PlacesUtils.annotations.setItemAnnotation(
-          newId, BookmarkAnnos.SIDEBAR_ANNO, true, 0,
-          PlacesUtils.annotations.EXPIRE_NEVER);
-      }
+      let bmk = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.insert(info));
+      this._log.debug("created bookmark " + bmk.guid + " under " + bmk.parentGuid
+                      + " as " + bmk.title + " " + bmk.url.href);
 
     } break;
     case "folder": {
-      let newId = PlacesUtils.bookmarks.createFolder(
-        record._parent, record.title, PlacesUtils.bookmarks.DEFAULT_INDEX,
-        record.id);
-      this._log.debug("created folder " + newId + " under " + record._parent
-                      + " as " + record.title);
+      let info = {
+        kind: PlacesSyncUtils.bookmarks.KINDS.FOLDER,
+        parentGuid: BookmarkSpecialIds.syncIDToPlacesGUID(record.parentid),
+        guid: BookmarkSpecialIds.syncIDToPlacesGUID(record.id),
+        title: record.title,
+      };
+      if (record.description) {
+        info.description = record.description;
+      }
 
-      if (record.description) {
-        PlacesUtils.annotations.setItemAnnotation(
-          newId, BookmarkAnnos.DESCRIPTION_ANNO, record.description, 0,
-          PlacesUtils.annotations.EXPIRE_NEVER);
-      }
+      let folder = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.insert(info));
+      this._log.debug("created folder " + folder.guid + " under " + folder.parentGuid
+                      + " as " + folder.title);
 
       // record.children will be dealt with in _orderChildren.
     } break;
     case "livemark":
-      let siteURI = null;
       if (!record.feedUri) {
         this._log.debug("No feed URI: skipping livemark record " + record.id);
         return;
       }
-      if (PlacesUtils.annotations
-                     .itemHasAnnotation(record._parent, PlacesUtils.LMANNO_FEEDURI)) {
-        this._log.debug("Invalid parent: skipping livemark record " + record.id);
-        return;
+      let info = {
+        kind: PlacesSyncUtils.bookmarks.KINDS.LIVEMARK,
+        title: record.title,
+        parentGuid: BookmarkSpecialIds.syncIDToPlacesGUID(record.parentid),
+        feed: record.feedUri,
+        site: record.siteUri,
+        guid: BookmarkSpecialIds.syncIDToPlacesGUID(record.id),
+      };
+      let livemark = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.insert(info));
+      if (livemark) {
+        this._log.debug("Created livemark " + livemark.id + " under " +
+                        livemark.parentId + " as " + livemark.title +
+                        ", " + livemark.siteURI.spec + ", " +
+                        livemark.feedURI.spec + ", GUID " +
+                        livemark.guid);
       }
-
-      if (record.siteUri != null)
-        siteURI = Utils.makeURI(record.siteUri);
-
-      // Until this engine can handle asynchronous error reporting, we need to
-      // detect errors on creation synchronously.
-      let spinningCb = Async.makeSpinningCallback();
-
-      let livemarkObj = {title: record.title,
-                         parentId: record._parent,
-                         index: PlacesUtils.bookmarks.DEFAULT_INDEX,
-                         feedURI: Utils.makeURI(record.feedUri),
-                         siteURI: siteURI,
-                         guid: record.id};
-      PlacesUtils.livemarks.addLivemark(livemarkObj).then(
-        aLivemark => { spinningCb(null, [Components.results.NS_OK, aLivemark]) },
-        ex => {
-          this._log.error("creating livemark failed: " + ex);
-          spinningCb(null, [Components.results.NS_ERROR_UNEXPECTED, null])
-        }
-      );
-
-      let [status, livemark] = spinningCb.wait();
-      if (!Components.isSuccessCode(status)) {
-        throw status;
-      }
-
-      this._log.debug("Created livemark " + livemark.id + " under " +
-                      livemark.parentId + " as " + livemark.title +
-                      ", " + livemark.siteURI.spec + ", " +
-                      livemark.feedURI.spec + ", GUID " +
-                      livemark.guid);
       break;
     case "separator": {
-      let newId = PlacesUtils.bookmarks.insertSeparator(
-        record._parent, PlacesUtils.bookmarks.DEFAULT_INDEX, record.id);
-      this._log.debug("created separator " + newId + " under " + record._parent);
+      let separator = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.insert({
+        kind: PlacesSyncUtils.bookmarks.KINDS.SEPARATOR,
+        parentGuid: BookmarkSpecialIds.syncIDToPlacesGUID(record.parentid),
+        guid: BookmarkSpecialIds.syncIDToPlacesGUID(record.id),
+      }));
+      this._log.debug("created separator " + separator.guid + " under " + separator.parentGuid);
     } break;
     case "item":
       this._log.debug(" -> got a generic places item.. do nothing?");
       return;
     default:
       this._log.error("_create: Unknown item type: " + record.type);
       return;
     }
-
-  },
-
-  // Factored out of `remove` to avoid redundant DB queries when the Places ID
-  // is already known.
-  removeById: function removeById(itemId, guid) {
-    let type = PlacesUtils.bookmarks.getItemType(itemId);
-
-    switch (type) {
-    case PlacesUtils.bookmarks.TYPE_BOOKMARK:
-      this._log.debug("  -> removing bookmark " + guid);
-      PlacesUtils.bookmarks.removeItem(itemId);
-      break;
-    case PlacesUtils.bookmarks.TYPE_FOLDER:
-      this._log.debug("  -> removing folder " + guid);
-      PlacesUtils.bookmarks.removeItem(itemId);
-      break;
-    case PlacesUtils.bookmarks.TYPE_SEPARATOR:
-      this._log.debug("  -> removing separator " + guid);
-      PlacesUtils.bookmarks.removeItem(itemId);
-      break;
-    default:
-      this._log.error("remove: Unknown item type: " + type);
-      break;
-    }
   },
 
   remove: function BStore_remove(record) {
     if (BookmarkSpecialIds.isSpecialGUID(record.id)) {
       this._log.warn("Refusing to remove special folder " + record.id);
       return;
     }
 
-    let itemId = this.idForGUID(record.id);
-    if (itemId <= 0) {
-      this._log.debug("Item " + record.id + " already removed");
-      return;
+    let guid = BookmarkSpecialIds.syncIDToPlacesGUID(record.id);
+    try {
+      let info = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.remove(guid));
+      this._log.debug(`Removed item ${guid} with type ${info.type}`);
+    } catch (ex) {
+      // Likely already removed.
+      this._log.debug(`Error removing ${record.id}`, ex);
     }
-    this.removeById(itemId, record.id);
-  },
-
-  _taggableTypes: ["bookmark", "microsummary", "query"],
-  isTaggable: function isTaggable(recordType) {
-    return this._taggableTypes.indexOf(recordType) != -1;
   },
 
   update: function BStore_update(record) {
-    let itemId = this.idForGUID(record.id);
-
-    if (itemId <= 0) {
-      this._log.debug("Skipping update for unknown item: " + record.id);
-      return;
-    }
+    let info = {
+      parentGuid: BookmarkSpecialIds.syncIDToPlacesGUID(record.parentid),
+      guid: BookmarkSpecialIds.syncIDToPlacesGUID(record.id),
+      kind: record.type,
+    };
 
-    // Two items are the same type if they have the same ItemType in Places,
-    // and also share some key characteristics (e.g., both being livemarks).
-    // We figure this out by examining the item to find the equivalent granular
-    // (string) type.
-    // If they're not the same type, we can't just update attributes. Delete
-    // then recreate the record instead.
-    let localItemType    = this._recordType(itemId);
-    let remoteRecordType = record.type;
-    this._log.trace("Local type: " + localItemType + ". " +
-                    "Remote type: " + remoteRecordType + ".");
-
-    if (localItemType != remoteRecordType) {
-      this._log.debug("Local record and remote record differ in type. " +
-                      "Deleting and recreating.");
-      this.removeById(itemId, record.id);
-      this.create(record);
-      return;
-    }
-
-    this._log.trace("Updating " + record.id + " (" + itemId + ")");
-
-    // Move the bookmark to a new parent or new position if necessary
-    if (record._parent > 0 &&
-        PlacesUtils.bookmarks.getFolderIdForItem(itemId) != record._parent) {
-      this._reparentItem(itemId, record._parent);
+    for (let prop of Object.keys(RECORD_PROPS_TO_BOOKMARK_PROPS)) {
+      let bmkProp = RECORD_PROPS_TO_BOOKMARK_PROPS[prop];
+      if (prop in record.cleartext) {
+        info[bmkProp] = record.cleartext[prop];
+      }
     }
 
-    for (let [key, val] of Object.entries(record.cleartext)) {
-      switch (key) {
-      case "title":
-        PlacesUtils.bookmarks.setItemTitle(itemId, val);
-        break;
-      case "bmkUri":
-        PlacesUtils.bookmarks.changeBookmarkURI(itemId, Utils.makeURI(val));
-        break;
-      case "tags":
-        if (Array.isArray(val)) {
-          if (this.isTaggable(remoteRecordType)) {
-            this._tagID(itemId, val);
-          } else {
-            this._log.debug("Remote record type is invalid for tags: " + remoteRecordType);
-          }
-        }
-        break;
-      case "keyword":
-        PlacesUtils.bookmarks.setKeywordForBookmark(itemId, val);
-        break;
-      case "description":
-        if (val) {
-          PlacesUtils.annotations.setItemAnnotation(
-            itemId, BookmarkAnnos.DESCRIPTION_ANNO, val, 0,
-            PlacesUtils.annotations.EXPIRE_NEVER);
-        } else {
-          PlacesUtils.annotations.removeItemAnnotation(itemId, BookmarkAnnos.DESCRIPTION_ANNO);
-        }
-        break;
-      case "loadInSidebar":
-        if (val) {
-          PlacesUtils.annotations.setItemAnnotation(
-            itemId, BookmarkAnnos.SIDEBAR_ANNO, true, 0,
-            PlacesUtils.annotations.EXPIRE_NEVER);
-        } else {
-          PlacesUtils.annotations.removeItemAnnotation(itemId, BookmarkAnnos.SIDEBAR_ANNO);
-        }
-        break;
-      case "queryId":
-        PlacesUtils.annotations.setItemAnnotation(
-          itemId, BookmarkAnnos.SMART_BOOKMARKS_ANNO, val, 0,
-          PlacesUtils.annotations.EXPIRE_NEVER);
-        break;
-      }
-    }
+    let bmk = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.update(info));
+    this._log.debug("updated bookmark " + bmk.guid + " under " + bmk.parentGuid);
   },
 
   _orderChildren: function _orderChildren() {
-    for (let [guid, children] of Object.entries(this._childrenToOrder)) {
-      // Reorder children according to the GUID list. Gracefully deal
-      // with missing items, e.g. locally deleted.
-      let delta = 0;
-      let parent = null;
-      for (let idx = 0; idx < children.length; idx++) {
-        let itemid = this.idForGUID(children[idx]);
-        if (itemid == -1) {
-          delta += 1;
-          this._log.trace("Could not locate record " + children[idx]);
-          continue;
-        }
-        try {
-          // This code path could be optimized by caching the parent earlier.
-          // Doing so should take in count any edge case due to reparenting
-          // or parent invalidations though.
-          if (!parent) {
-            parent = PlacesUtils.bookmarks.getFolderIdForItem(itemid);
-          }
-          PlacesUtils.bookmarks.moveItem(itemid, parent, idx - delta);
-        } catch (ex) {
-          this._log.debug("Could not move item " + children[idx] + ": " + ex);
-        }
+    let promises = Object.keys(this._childrenToOrder).map(syncID => {
+      let children = this._childrenToOrder[syncID];
+      if (!children.length) {
+        return Promise.resolve();
       }
-    }
+      let guid = BookmarkSpecialIds.syncIDToPlacesGUID(syncID);
+      let childGUIDs = children.map(syncID => BookmarkSpecialIds.syncIDToPlacesGUID(syncID));
+      return PlacesSyncUtils.bookmarks.order(guid, childGUIDs).catch(ex => {
+        this._log.debug(`Could not order children for ${guid}: ${ex}`);
+      });
+    });
+    Async.promiseSpinningly(Promise.all(promises));
   },
 
   changeItemID: function BStore_changeItemID(oldID, newID) {
     this._log.debug("Changing GUID " + oldID + " to " + newID);
 
-    // Make sure there's an item to change GUIDs
-    let itemId = this.idForGUID(oldID);
-    if (itemId <= 0)
-      return;
-
-    this._setGUID(itemId, newID);
+    Async.promiseSpinningly(PlacesSyncUtils.bookmarks.changeGuid(
+      BookmarkSpecialIds.syncIDToPlacesGUID(oldID),
+      BookmarkSpecialIds.syncIDToPlacesGUID(newID)
+    ));
   },
 
   _getNode: function BStore__getNode(folder) {
     let query = PlacesUtils.history.getNewQuery();
     query.setFolders([folder], 1);
     return PlacesUtils.history.executeQuery(
       query, PlacesUtils.history.getNewQueryOptions()).root;
   },
@@ -958,38 +652,16 @@ BookmarksStore.prototype = {
       return null;
     }
   },
 
   _isLoadInSidebar: function BStore__isLoadInSidebar(id) {
     return PlacesUtils.annotations.itemHasAnnotation(id, BookmarkAnnos.SIDEBAR_ANNO);
   },
 
-  get _childGUIDsStm() {
-    return this._getStmt(
-      "SELECT id AS item_id, guid " +
-      "FROM moz_bookmarks " +
-      "WHERE parent = :parent " +
-      "ORDER BY position");
-  },
-  _childGUIDsCols: ["item_id", "guid"],
-
-  _getChildGUIDsForId: function _getChildGUIDsForId(itemid) {
-    let stmt = this._childGUIDsStm;
-    stmt.params.parent = itemid;
-    let rows = Async.querySpinningly(stmt, this._childGUIDsCols);
-    return rows.map(function (row) {
-      if (row.guid) {
-        return row.guid;
-      }
-      // A GUID hasn't been assigned to this item yet, do this now.
-      return this.GUIDForId(row.item_id);
-    }, this);
-  },
-
   // Create a record starting from the weave id (places guid)
   createRecord: function createRecord(id, collection) {
     let placeId = this.idForGUID(id);
     let record;
     if (placeId <= 0) { // deleted item
       record = new PlacesItem(collection, id);
       record.deleted = true;
       return record;
@@ -1008,17 +680,17 @@ BookmarksStore.prototype = {
           // There might not be the tag yet when creating on a new client
           if (folder != null) {
             folder = folder[1];
             record.folderName = PlacesUtils.bookmarks.getItemTitle(folder);
             this._log.trace("query id: " + folder + " = " + record.folderName);
           }
         }
         catch(ex) {}
-        
+
         // Persist the Smart Bookmark anno, if found.
         try {
           let anno = PlacesUtils.annotations.getItemAnnotation(placeId, BookmarkAnnos.SMART_BOOKMARKS_ANNO);
           if (anno != null) {
             this._log.trace("query anno: " + BookmarkAnnos.SMART_BOOKMARKS_ANNO +
                             " = " + anno);
             record.queryId = anno;
           }
@@ -1050,17 +722,19 @@ BookmarksStore.prototype = {
       } else {
         record = new BookmarkFolder(collection, id);
       }
 
       if (parent > 0)
         record.parentName = PlacesUtils.bookmarks.getItemTitle(parent);
       record.title = PlacesUtils.bookmarks.getItemTitle(placeId);
       record.description = this._getDescription(placeId);
-      record.children = this._getChildGUIDsForId(placeId);
+      record.children = Async.promiseSpinningly(
+        PlacesSyncUtils.bookmarks.fetchChildGuids(
+          BookmarkSpecialIds.syncIDToPlacesGUID(id)));
       break;
 
     case PlacesUtils.bookmarks.TYPE_SEPARATOR:
       record = new BookmarkSeparator(collection, id);
       if (parent > 0)
         record.parentName = PlacesUtils.bookmarks.getItemTitle(parent);
       // Create a positioning identifier for the separator, used by _mapDupe
       record.pos = PlacesUtils.bookmarks.getItemIndex(placeId);
@@ -1094,90 +768,36 @@ BookmarksStore.prototype = {
     return this._getStmt(
         "SELECT frecency " +
         "FROM moz_places " +
         "WHERE url_hash = hash(:url) AND url = :url " +
         "LIMIT 1");
   },
   _frecencyCols: ["frecency"],
 
-  get _setGUIDStm() {
-    return this._getStmt(
-      "UPDATE moz_bookmarks " +
-      "SET guid = :guid " +
-      "WHERE id = :item_id");
-  },
-
-  // Some helper functions to handle GUIDs
-  _setGUID: function _setGUID(id, guid) {
-    if (!guid)
-      guid = Utils.makeGUID();
-
-    let stmt = this._setGUIDStm;
-    stmt.params.guid = guid;
-    stmt.params.item_id = id;
-    Async.querySpinningly(stmt);
-    PlacesUtils.invalidateCachedGuidFor(id);
-    return guid;
-  },
-
-  get _guidForIdStm() {
-    return this._getStmt(
-      "SELECT guid " +
-      "FROM moz_bookmarks " +
-      "WHERE id = :item_id");
-  },
-  _guidForIdCols: ["guid"],
-
   GUIDForId: function GUIDForId(id) {
     let special = BookmarkSpecialIds.specialGUIDForId(id);
     if (special)
       return special;
 
-    let stmt = this._guidForIdStm;
-    stmt.params.item_id = id;
-
-    // Use the existing GUID if it exists
-    let result = Async.querySpinningly(stmt, this._guidForIdCols)[0];
-    if (result && result.guid)
-      return result.guid;
-
-    // Give the uri a GUID if it doesn't have one
-    return this._setGUID(id);
+    return Async.promiseSpinningly(
+      PlacesSyncUtils.bookmarks.ensureGuidForId(id));
   },
 
-  get _idForGUIDStm() {
-    return this._getStmt(
-      "SELECT id AS item_id " +
-      "FROM moz_bookmarks " +
-      "WHERE guid = :guid");
-  },
-  _idForGUIDCols: ["item_id"],
-
   // noCreate is provided as an optional argument to prevent the creation of
   // non-existent special records, such as "mobile".
   idForGUID: function idForGUID(guid, noCreate) {
+    // guid might be a String object rather than a string.
+    guid = guid.toString();
+
     if (BookmarkSpecialIds.isSpecialGUID(guid))
       return BookmarkSpecialIds.specialIdForGUID(guid, !noCreate);
 
-    let stmt = this._idForGUIDStm;
-    // guid might be a String object rather than a string.
-    stmt.params.guid = guid.toString();
-
-    let results = Async.querySpinningly(stmt, this._idForGUIDCols);
-    this._log.trace("Number of rows matching GUID " + guid + ": "
-                    + results.length);
-    
-    // Here's the one we care about: the first.
-    let result = results[0];
-    
-    if (!result)
-      return -1;
-    
-    return result.item_id;
+    return Async.promiseSpinningly(PlacesUtils.promiseItemId(guid).catch(
+      ex => -1));
   },
 
   _calculateIndex: function _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
@@ -1202,17 +822,17 @@ BookmarksStore.prototype = {
     if (typeof(node) == "string") { // callers will give us the guid as the first arg
       let nodeID = this.idForGUID(guid, true);
       if (!nodeID) {
         this._log.debug("No node for GUID " + guid + "; returning no children.");
         return items;
       }
       node = this._getNode(nodeID);
     }
-    
+
     if (node.type == node.RESULT_TYPE_FOLDER) {
       node.QueryInterface(Ci.nsINavHistoryQueryResultNode);
       node.containerOpen = true;
       try {
         // Remember all the children GUIDs and recursively get more
         for (let i = 0; i < node.childCount; i++) {
           let child = node.getChild(i);
           items[this.GUIDForId(child.itemId)] = true;
@@ -1222,58 +842,16 @@ BookmarksStore.prototype = {
       finally {
         node.containerOpen = false;
       }
     }
 
     return items;
   },
 
-  /**
-   * Associates the URI of the item with the provided ID with the
-   * provided array of tags.
-   * If the provided ID does not identify an item with a URI,
-   * returns immediately.
-   */
-  _tagID: function _tagID(itemID, tags) {
-    if (!itemID || !tags) {
-      return;
-    }
-
-    try {
-      let u = PlacesUtils.bookmarks.getBookmarkURI(itemID);
-      this._tagURI(u, tags);
-    } catch (e) {
-      this._log.warn(`Got exception fetching URI for ${itemID} not tagging`, e);
-
-      // I guess it doesn't have a URI. Don't try to tag it.
-      return;
-    }
-  },
-
-  /**
-   * Associate the provided URI with the provided array of tags.
-   * If the provided URI is falsy, returns immediately.
-   */
-  _tagURI: function _tagURI(bookmarkURI, tags) {
-    if (!bookmarkURI || !tags) {
-      return;
-    }
-
-    // Filter out any null/undefined/empty tags.
-    tags = tags.filter(t => t);
-
-    // Temporarily tag a dummy URI to preserve tag ids when untagging.
-    let dummyURI = Utils.makeURI("about:weave#BStore_tagURI");
-    PlacesUtils.tagging.tagURI(dummyURI, tags);
-    PlacesUtils.tagging.untagURI(bookmarkURI, null);
-    PlacesUtils.tagging.tagURI(bookmarkURI, tags);
-    PlacesUtils.tagging.untagURI(dummyURI, null);
-  },
-
   getAllIDs: function BStore_getAllIDs() {
     let items = {"menu": true,
                  "toolbar": true,
                  "unfiled": true,
                 };
     // We also want "mobile" but only if a local mobile folder already exists
     // (otherwise we'll later end up creating it, which we want to avoid until
     // we actually need it.)
@@ -1287,22 +865,32 @@ BookmarksStore.prototype = {
     return items;
   },
 
   wipe: function BStore_wipe() {
     let cb = Async.makeSpinningCallback();
     Task.spawn(function* () {
       // Save a backup before clearing out all bookmarks.
       yield PlacesBackups.create(null, true);
-      for (let guid of BookmarkSpecialIds.guids)
-        if (guid != "places") {
-          let id = BookmarkSpecialIds.specialIdForGUID(guid);
-          if (id)
-            PlacesUtils.bookmarks.removeFolderChildren(id);
+      // Instead of exposing a "clear folder" method, we should instead have
+      // `PlacesSyncUtils` clear all roots. `eraseEverything` comes close,
+      // but doesn't clear the mobile root. The fix is to move mobile root
+      // and query handling into Places.
+      for (let syncID of BookmarkSpecialIds.guids) {
+        if (syncID == "places") {
+          continue;
         }
+        if (syncID == "mobile" && !BookmarkSpecialIds.findMobileRoot(false)) {
+          // `syncIDToPlacesGUID` will create the mobile root as a side effect,
+          // which we don't want it to do if we're clearing bookmarks.
+          continue;
+        }
+        let guid = BookmarkSpecialIds.syncIDToPlacesGUID(syncID);
+        yield PlacesSyncUtils.bookmarks.clear(guid);
+      }
       cb();
     });
     cb.wait();
   }
 };
 
 function BookmarksTracker(name, engine) {
   this._batchDepth = 0;
--- a/services/sync/tests/unit/test_bookmark_engine.js
+++ b/services/sync/tests/unit/test_bookmark_engine.js
@@ -1,27 +1,28 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 Cu.import("resource://gre/modules/PlacesUtils.jsm");
+Cu.import("resource://gre/modules/PlacesSyncUtils.jsm");
 Cu.import("resource://gre/modules/BookmarkJSONUtils.jsm");
-Cu.import("resource://services-common/async.js");
 Cu.import("resource://gre/modules/Log.jsm");
 Cu.import("resource://services-sync/engines.js");
 Cu.import("resource://services-sync/engines/bookmarks.js");
+Cu.import("resource://services-sync/bookmark_utils.js");
 Cu.import("resource://services-sync/service.js");
 Cu.import("resource://services-sync/util.js");
 Cu.import("resource://testing-common/services/sync/utils.js");
 Cu.import("resource://gre/modules/Promise.jsm");
 
 initTestLogging("Trace");
 
 Service.engineManager.register(BookmarksEngine);
 
-add_test(function bad_record_allIDs() {
+add_task(function* bad_record_allIDs() {
   let server = new SyncServer();
   server.start();
   let syncTesting = new SyncTestingInfrastructure(server.server);
 
   _("Ensure that bad Places queries don't cause an error in getAllIDs.");
   let engine = new BookmarksEngine(Service);
   let store = engine._store;
   let badRecordID = PlacesUtils.bookmarks.insertBookmark(
@@ -41,32 +42,34 @@ add_test(function bad_record_allIDs() {
   let all = store.getAllIDs();
 
   _("All IDs: " + JSON.stringify(all));
   do_check_true("menu" in all);
   do_check_true("toolbar" in all);
 
   _("Clean up.");
   PlacesUtils.bookmarks.removeItem(badRecordID);
-  server.stop(run_next_test);
+  yield new Promise(r => server.stop(r));
 });
 
-add_test(function test_ID_caching() {
+add_task(function* test_ID_caching() {
   let server = new SyncServer();
   server.start();
   let syncTesting = new SyncTestingInfrastructure(server.server);
 
   _("Ensure that Places IDs are not cached.");
   let engine = new BookmarksEngine(Service);
   let store = engine._store;
   _("All IDs: " + JSON.stringify(store.getAllIDs()));
 
   let mobileID = store.idForGUID("mobile");
   _("Change the GUID for that item, and drop the mobile anno.");
-  store._setGUID(mobileID, "abcdefghijkl");
+  let mobileRoot = BookmarkSpecialIds.specialIdForGUID("mobile", false);
+  let mobileGUID = yield PlacesUtils.promiseItemGuid(mobileRoot);
+  yield PlacesSyncUtils.bookmarks.changeGuid(mobileGUID, "abcdefghijkl");
   PlacesUtils.annotations.removeItemAnnotation(mobileID, "mobile/bookmarksRoot");
 
   let err;
   let newMobileID;
 
   // With noCreate, we don't find an entry.
   try {
     newMobileID = store.idForGUID("mobile", true);
@@ -83,17 +86,17 @@ add_test(function test_ID_caching() {
   _("New mobile ID: " + newMobileID);
   do_check_true(!!newMobileID);
   do_check_neq(newMobileID, mobileID);
 
   // And it's repeatable, even with creation enabled.
   do_check_eq(newMobileID, store.idForGUID("mobile", false));
 
   do_check_eq(store.GUIDForId(mobileID), "abcdefghijkl");
-  server.stop(run_next_test);
+  yield new Promise(r => server.stop(r));
 });
 
 function serverForFoo(engine) {
   return serverForUsers({"foo": "password"}, {
     meta: {global: {engines: {bookmarks: {version: engine.version,
                                           syncID: engine.syncID}}}},
     bookmarks: {}
   });
@@ -305,57 +308,59 @@ add_task(function* test_restorePromptsRe
 function FakeRecord(constructor, r) {
   constructor.call(this, "bookmarks", r.id);
   for (let x in r) {
     this[x] = r[x];
   }
 }
 
 // Bug 632287.
-add_test(function test_mismatched_types() {
+add_task(function* test_mismatched_types() {
   _("Ensure that handling a record that changes type causes deletion " +
     "then re-adding.");
 
   let oldRecord = {
     "id": "l1nZZXfB8nC7",
     "type":"folder",
     "parentName":"Bookmarks Toolbar",
     "title":"Innerst i Sneglehode",
     "description":null,
     "parentid": "toolbar"
   };
+  oldRecord.cleartext = oldRecord;
 
   let newRecord = {
     "id": "l1nZZXfB8nC7",
     "type":"livemark",
     "siteUri":"http://sneglehode.wordpress.com/",
     "feedUri":"http://sneglehode.wordpress.com/feed/",
     "parentName":"Bookmarks Toolbar",
     "title":"Innerst i Sneglehode",
     "description":null,
     "children":
       ["HCRq40Rnxhrd", "YeyWCV1RVsYw", "GCceVZMhvMbP", "sYi2hevdArlF",
        "vjbZlPlSyGY8", "UtjUhVyrpeG6", "rVq8WMG2wfZI", "Lx0tcy43ZKhZ",
        "oT74WwV8_j4P", "IztsItWVSo3-"],
     "parentid": "toolbar"
   };
+  newRecord.cleartext = newRecord;
 
   let engine = new BookmarksEngine(Service);
   let store  = engine._store;
   let server = serverForFoo(engine);
   new SyncTestingInfrastructure(server.server);
 
   _("GUID: " + store.GUIDForId(6, true));
 
   try {
     let bms = PlacesUtils.bookmarks;
     let oldR = new FakeRecord(BookmarkFolder, oldRecord);
     let newR = new FakeRecord(Livemark, newRecord);
-    oldR._parent = PlacesUtils.bookmarks.toolbarFolder;
-    newR._parent = PlacesUtils.bookmarks.toolbarFolder;
+    oldR.parentid = PlacesUtils.bookmarks.toolbarGuid;
+    newR.parentid = PlacesUtils.bookmarks.toolbarGuid;
 
     store.applyIncoming(oldR);
     _("Applied old. It's a folder.");
     let oldID = store.idForGUID(oldR.id);
     _("Old ID: " + oldID);
     do_check_eq(bms.getItemType(oldID), bms.TYPE_FOLDER);
     do_check_false(PlacesUtils.annotations
                               .itemHasAnnotation(oldID, PlacesUtils.LMANNO_FEEDURI));
@@ -368,21 +373,21 @@ add_test(function test_mismatched_types(
     do_check_eq(bms.getItemType(newID), bms.TYPE_FOLDER);
     do_check_true(PlacesUtils.annotations
                              .itemHasAnnotation(newID, PlacesUtils.LMANNO_FEEDURI));
 
   } finally {
     store.wipe();
     Svc.Prefs.resetBranch("");
     Service.recordManager.clearCache();
-    server.stop(run_next_test);
+    yield new Promise(r => server.stop(r));
   }
 });
 
-add_test(function test_bookmark_guidMap_fail() {
+add_task(function* test_bookmark_guidMap_fail() {
   _("Ensure that failures building the GUID map cause early death.");
 
   let engine = new BookmarksEngine(Service);
   let store = engine._store;
 
   let server = serverForFoo(engine);
   let coll   = server.user("foo").collection("bookmarks");
   new SyncTestingInfrastructure(server.server);
@@ -421,68 +426,68 @@ add_test(function test_bookmark_guidMap_
   try {
     engine._processIncoming();
   } catch (ex) {
     err = ex;
   }
   do_check_eq(err, "Nooo");
 
   PlacesUtils.promiseBookmarksTree = pbt;
-  server.stop(run_next_test);
+  yield new Promise(r => server.stop(r));
 });
 
-add_test(function test_bookmark_is_taggable() {
-  let engine = new BookmarksEngine(Service);
-  let store = engine._store;
-
-  do_check_true(store.isTaggable("bookmark"));
-  do_check_true(store.isTaggable("microsummary"));
-  do_check_true(store.isTaggable("query"));
-  do_check_false(store.isTaggable("folder"));
-  do_check_false(store.isTaggable("livemark"));
-  do_check_false(store.isTaggable(null));
-  do_check_false(store.isTaggable(undefined));
-  do_check_false(store.isTaggable(""));
-
-  run_next_test();
-});
-
-add_test(function test_bookmark_tag_but_no_uri() {
+add_task(function* test_bookmark_tag_but_no_uri() {
   _("Ensure that a bookmark record with tags, but no URI, doesn't throw an exception.");
 
   let engine = new BookmarksEngine(Service);
   let store = engine._store;
 
   // We're simply checking that no exception is thrown, so
   // no actual checks in this test.
 
-  store._tagURI(null, ["foo"]);
-  store._tagURI(null, null);
-  store._tagURI(Utils.makeURI("about:fake"), null);
+  yield PlacesSyncUtils.bookmarks.insert({
+    kind: PlacesSyncUtils.bookmarks.KINDS.BOOKMARK,
+    guid: Utils.makeGUID(),
+    parentGuid: PlacesUtils.bookmarks.toolbarGuid,
+    url: "http://example.com",
+    tags: ["foo"],
+  });
+  yield PlacesSyncUtils.bookmarks.insert({
+    kind: PlacesSyncUtils.bookmarks.KINDS.BOOKMARK,
+    guid: Utils.makeGUID(),
+    parentGuid: PlacesUtils.bookmarks.toolbarGuid,
+    url: "http://example.org",
+    tags: null,
+  });
+  yield PlacesSyncUtils.bookmarks.insert({
+    kind: PlacesSyncUtils.bookmarks.KINDS.BOOKMARK,
+    guid: Utils.makeGUID(),
+    url: "about:fake",
+    parentGuid: PlacesUtils.bookmarks.toolbarGuid,
+    tags: null,
+  });
 
   let record = {
-    _parent:     PlacesUtils.bookmarks.toolbarFolder,
+    parentid:    PlacesUtils.bookmarks.toolbarGuid,
     id:          Utils.makeGUID(),
     description: "",
     tags:        ["foo"],
     title:       "Taggy tag",
     type:        "folder"
   };
 
   // Because update() walks the cleartext.
   record.cleartext = record;
 
   store.create(record);
   record.tags = ["bar"];
   store.update(record);
-
-  run_next_test();
 });
 
-add_test(function test_misreconciled_root() {
+add_task(function* test_misreconciled_root() {
   _("Ensure that we don't reconcile an arbitrary record with a root.");
 
   let engine = new BookmarksEngine(Service);
   let store = engine._store;
   let server = serverForFoo(engine);
 
   // Log real hard for this test.
   store._log.trace = store._log.debug;
@@ -533,16 +538,16 @@ add_test(function test_misreconciled_roo
   // the real GUID, instead using a generated one. Sync does the translation.
   let toolbarAfter = store.createRecord("toolbar", "bookmarks");
   let parentGUIDAfter = toolbarAfter.parentid;
   let parentIDAfter = store.idForGUID(parentGUIDAfter);
   do_check_eq(store.GUIDForId(toolbarIDBefore), "toolbar");
   do_check_eq(parentGUIDBefore, parentGUIDAfter);
   do_check_eq(parentIDBefore, parentIDAfter);
 
-  server.stop(run_next_test);
+  yield new Promise(r => server.stop(r));
 });
 
 function run_test() {
   initTestLogging("Trace");
   generateNewKeys(Service.collectionKeys);
   run_next_test();
 }
--- a/services/sync/tests/unit/test_bookmark_livemarks.js
+++ b/services/sync/tests/unit/test_bookmark_livemarks.js
@@ -98,46 +98,37 @@ add_test(function test_livemark_descript
                                             PlacesUtils.annotations.EXPIRE_NEVER);
 
   run_next_test();
 });
 
 add_test(function test_livemark_invalid() {
   _("Livemarks considered invalid by nsLivemarkService are skipped.");
 
-  _("Parent is 0, which is invalid. Will be set to unfiled.");
-  let noParentRec = makeLivemark(record631361.payload, true);
-  noParentRec._parent = 0;
-  store.create(noParentRec);
-  let recID = store.idForGUID(noParentRec.id, true);
-  do_check_true(recID > 0);
-  do_check_eq(PlacesUtils.bookmarks.getFolderIdForItem(recID), PlacesUtils.bookmarks.unfiledBookmarksFolder);
-
   _("Parent is unknown. Will be set to unfiled.");
   let lateParentRec = makeLivemark(record631361.payload, true);
   let parentGUID = Utils.makeGUID();
   lateParentRec.parentid = parentGUID;
-  lateParentRec._parent = store.idForGUID(parentGUID);   // Usually done by applyIncoming.
-  do_check_eq(-1, lateParentRec._parent);
+  do_check_eq(-1, store.idForGUID(parentGUID));
 
   store.create(lateParentRec);
   recID = store.idForGUID(lateParentRec.id, true);
   do_check_true(recID > 0);
   do_check_eq(PlacesUtils.bookmarks.getFolderIdForItem(recID),
               PlacesUtils.bookmarks.unfiledBookmarksFolder);
 
   _("No feed URI, which is invalid. Will be skipped.");
   let noFeedURIRec = makeLivemark(record631361.payload, true);
   delete noFeedURIRec.cleartext.feedUri;
   store.create(noFeedURIRec);
   // No exception, but no creation occurs.
   do_check_eq(-1, store.idForGUID(noFeedURIRec.id, true));
 
   _("Parent is a Livemark. Will be skipped.");
   let lmParentRec = makeLivemark(record631361.payload, true);
-  lmParentRec._parent = recID;
+  lmParentRec.parentid = store.GUIDForId(recID);
   store.create(lmParentRec);
   // No exception, but no creation occurs.
   do_check_eq(-1, store.idForGUID(lmParentRec.id, true));
 
   // Clear event loop.
   Utils.nextTick(run_next_test);
 });
--- a/services/sync/tests/unit/test_bookmark_order.js
+++ b/services/sync/tests/unit/test_bookmark_order.js
@@ -12,22 +12,38 @@ function getBookmarks(folderId) {
 
   let pos = 0;
   while (true) {
     let itemId = PlacesUtils.bookmarks.getIdForItemAt(folderId, pos);
     _("Got itemId", itemId, "under", folderId, "at", pos);
     if (itemId == -1)
       break;
 
+    let isOrphan = PlacesUtils.annotations.itemHasAnnotation(itemId,
+      "sync/parent");
     switch (PlacesUtils.bookmarks.getItemType(itemId)) {
       case PlacesUtils.bookmarks.TYPE_BOOKMARK:
-        bookmarks.push(PlacesUtils.bookmarks.getItemTitle(itemId));
+        let title = PlacesUtils.bookmarks.getItemTitle(itemId);
+        if (isOrphan) {
+          let requestedParent = PlacesUtils.annotations.getItemAnnotation(
+            itemId, "sync/parent");
+          bookmarks.push({ title, requestedParent });
+        } else {
+          bookmarks.push(title);
+        }
         break;
       case PlacesUtils.bookmarks.TYPE_FOLDER:
-        bookmarks.push(getBookmarks(itemId));
+        let titles = getBookmarks(itemId);
+        if (isOrphan) {
+          let requestedParent = PlacesUtils.annotations.getItemAnnotation(
+            itemId, "sync/parent");
+          bookmarks.push({ titles, requestedParent });
+        } else {
+          bookmarks.push(titles);
+        }
         break;
       default:
         _("Unsupported item type..");
     }
 
     pos++;
   }
 
@@ -91,23 +107,24 @@ function run_test() {
   let f30 = folder(id30, "", [id31]);
   apply(f30);
   check([id10, id20, [id31]]);
 
   let id41 = "41_aaaaaaaaa";
   let id40 = "f40_aaaaaaaa";
   _("insert missing parent -> append to unfiled");
   apply(bookmark(id41, id40));
-  check([id10, id20, [id31], id41]);
+  check([id10, id20, [id31], { title: id41, requestedParent: id40 }]);
 
   let id42 = "42_aaaaaaaaa";
 
   _("insert another missing parent -> append");
   apply(bookmark(id42, id40));
-  check([id10, id20, [id31], id41, id42]);
+  check([id10, id20, [id31], { title: id41, requestedParent: id40 },
+    { title: id42, requestedParent: id40 }]);
 
   _("insert folder -> move children and followers");
   let f40 = folder(id40, "", [id41, id42]);
   apply(f40);
   check([id10, id20, [id31], [id41, id42]]);
 
   _("Moving 41 behind 42 -> update f40");
   f40.children = [id42, id41];
--- a/services/sync/tests/unit/test_bookmark_places_query_rewriting.js
+++ b/services/sync/tests/unit/test_bookmark_places_query_rewriting.js
@@ -5,47 +5,53 @@
 Cu.import("resource://gre/modules/PlacesUtils.jsm");
 Cu.import("resource://services-sync/engines/bookmarks.js");
 Cu.import("resource://services-sync/service.js");
 Cu.import("resource://services-sync/util.js");
 
 var engine = new BookmarksEngine(Service);
 var store = engine._store;
 
+function makeTagRecord(id, uri) {
+  let tagRecord = new BookmarkQuery("bookmarks", id);
+  tagRecord.queryId = "MagicTags";
+  tagRecord.parentName = "Bookmarks Toolbar";
+  tagRecord.bmkUri = uri;
+  tagRecord.title = "tagtag";
+  tagRecord.folderName = "bar";
+  tagRecord.parentid = PlacesUtils.bookmarks.toolbarGuid;
+  return tagRecord;
+}
+
 function run_test() {
   initTestLogging("Trace");
   Log.repository.getLogger("Sync.Engine.Bookmarks").level = Log.Level.Trace;
   Log.repository.getLogger("Sync.Store.Bookmarks").level = Log.Level.Trace;
 
-  let tagRecord = new BookmarkQuery("bookmarks", "abcdefabcdef");
   let uri = "place:folder=499&type=7&queryType=1";
-  tagRecord.queryId = "MagicTags";
-  tagRecord.parentName = "Bookmarks Toolbar";
-  tagRecord.bmkUri = uri;
-  tagRecord.title = "tagtag";
-  tagRecord.folderName = "bar";
+  let tagRecord = makeTagRecord("abcdefabcdef", uri);
 
   _("Type: " + tagRecord.type);
   _("Folder name: " + tagRecord.folderName);
-  store.preprocessTagQuery(tagRecord);
-
-  _("Verify that the URI has been rewritten.");
-  do_check_neq(tagRecord.bmkUri, uri);
+  store.applyIncoming(tagRecord);
 
   let tags = store._getNode(PlacesUtils.tagsFolderId);
   tags.containerOpen = true;
   let tagID;
   for (let i = 0; i < tags.childCount; ++i) {
     let child = tags.getChild(i);
     if (child.title == "bar")
       tagID = child.itemId;
   }
   tags.containerOpen = false;
 
   _("Tag ID: " + tagID);
-  do_check_eq(tagRecord.bmkUri, uri.replace("499", tagID));
+  let insertedRecord = store.createRecord("abcdefabcdef", "bookmarks");
+  do_check_eq(insertedRecord.bmkUri, uri.replace("499", tagID));
 
   _("... but not if the type is wrong.");
   let wrongTypeURI = "place:folder=499&type=2&queryType=1";
-  tagRecord.bmkUri = wrongTypeURI;
-  store.preprocessTagQuery(tagRecord);
-  do_check_eq(tagRecord.bmkUri, wrongTypeURI);
+  let wrongTypeRecord = makeTagRecord("fedcbafedcba", wrongTypeURI);
+  store.applyIncoming(wrongTypeRecord);
+
+  insertedRecord = store.createRecord("fedcbafedcba", "bookmarks");
+  do_check_eq(insertedRecord.bmkUri, wrongTypeURI);
 }