Bug 1295521 - Remove `BookmarkSpecialIds` from Sync. r=markh,tcsc draft
authorKit Cambridge <kcambridge@mozilla.com>
Fri, 16 Sep 2016 14:08:10 -0700
changeset 423752 40f23512b976595a094bd4491db434958ae381a1
parent 423751 f13b90a122d0bcb375b2c0d4d7ae64b9fb43ce4c
child 423753 0d657035d02c8bd261f1124072d6a248c94c7e09
push id31975
push userbmo:kcambridge@mozilla.com
push dateTue, 11 Oct 2016 15:53:22 +0000
reviewersmarkh, tcsc
bugs1295521
milestone52.0a1
Bug 1295521 - Remove `BookmarkSpecialIds` from Sync. r=markh,tcsc MozReview-Commit-ID: IUVOu0jFXRv
services/sync/modules/bookmark_utils.js
services/sync/modules/bookmark_validator.js
services/sync/modules/collection_validator.js
services/sync/modules/engines/bookmarks.js
services/sync/moz.build
services/sync/tests/unit/test_bookmark_engine.js
services/sync/tests/unit/test_bookmark_tracker.js
deleted file mode 100644
--- a/services/sync/modules/bookmark_utils.js
+++ /dev/null
@@ -1,85 +0,0 @@
-/* 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/. */
-
-"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,
-    "mobile": PlacesUtils.bookmarks.mobileGuid,
-  };
-});
-
-let BookmarkSpecialIds = {
-
-  // Special IDs.
-  guids: ["menu", "places", "tags", "toolbar", "unfiled", "mobile"],
-
-  // Accessors for IDs.
-  isSpecialGUID: function isSpecialGUID(g) {
-    return this.guids.indexOf(g) != -1;
-  },
-
-  specialIdForGUID: function specialIdForGUID(guid) {
-    return this[guid];
-  },
-
-  specialGUIDForId: function specialGUIDForId(id) {
-    for (let guid of this.guids)
-      if (this.specialIdForGUID(guid) == 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;
-  },
-  get toolbar() {
-    return PlacesUtils.toolbarFolderId;
-  },
-  get unfiled() {
-    return PlacesUtils.unfiledBookmarksFolderId;
-  },
-  get mobile() {
-    return PlacesUtils.mobileFolderId;
-  },
-};
--- a/services/sync/modules/bookmark_validator.js
+++ b/services/sync/modules/bookmark_validator.js
@@ -2,19 +2,18 @@
  * 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/. */
 
 "use strict";
 
 const Cu = Components.utils;
 
 Cu.import("resource://gre/modules/PlacesUtils.jsm");
+Cu.import("resource://gre/modules/PlacesSyncUtils.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
-Cu.import("resource://services-sync/util.js");
-Cu.import("resource://services-sync/bookmark_utils.js");
 
 this.EXPORTED_SYMBOLS = ["BookmarkValidator", "BookmarkProblemData"];
 
 /**
  * Result of bookmark validation. Contains the following fields which describe
  * server-side problems unless otherwise specified.
  *
  * - missingIDs (number): # of objects with missing ids
@@ -192,26 +191,26 @@ class BookmarkValidator {
   }
 
   createClientRecordsFromTree(clientTree) {
     // Iterate over the treeNode, converting it to something more similar to what
     // the server stores.
     let records = [];
     let recordsByGuid = new Map();
     function traverse(treeNode) {
-      let guid = BookmarkSpecialIds.specialGUIDForId(treeNode.id) || treeNode.guid;
+      let guid = PlacesSyncUtils.bookmarks.guidToSyncId(treeNode.guid);
       let itemType = 'item';
-      treeNode.ignored = PlacesUtils.annotations.itemHasAnnotation(treeNode.id, BookmarkAnnos.EXCLUDEBACKUP_ANNO);
+      treeNode.ignored = PlacesUtils.annotations.itemHasAnnotation(treeNode.id, PlacesUtils.EXCLUDE_FROM_BACKUP_ANNO);
       treeNode.id = guid;
       switch (treeNode.type) {
         case PlacesUtils.TYPE_X_MOZ_PLACE:
           let query = null;
           if (treeNode.annos && treeNode.uri.startsWith("place:")) {
             query = treeNode.annos.find(({name}) =>
-              name === BookmarkAnnos.SMART_BOOKMARKS_ANNO);
+              name === PlacesSyncUtils.bookmarks.SMART_BOOKMARKS_ANNO);
           }
           if (query && query.value) {
             itemType = 'query';
           } else {
             itemType = 'bookmark';
           }
           break;
         case PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER:
@@ -328,42 +327,22 @@ class BookmarkValidator {
           problemData.childrenOnNonFolder.push(record.id);
         }
         folders.push(record);
 
         if (new Set(record.children).size !== record.children.length) {
           problemData.duplicateChildren.push(record.id)
         }
 
-        // This whole next part is a huge hack.
         // The children array stores special guids as their local guid values,
         // e.g. 'menu________' instead of 'menu', but all other parts of the
         // serverside bookmark info stores it as the special value ('menu').
-        //
-        // Since doing a sql query for every entry would be extremely slow, and
-        // wouldn't even be necessarially accurate (since these values are only
-        // the local values for whichever client created the records) We just
-        // strip off the trailing _ and see if that results in a special id.
-        //
-        // To make things worse, this doesn't even work for root________, which has
-        // the special id 'places'.
         record.childGUIDs = record.children;
         record.children = record.children.map(childID => {
-          let match = childID.match(/_+$/);
-          if (!match) {
-            return childID;
-          }
-          let possibleSpecialID = childID.slice(0, match.index);
-          if (possibleSpecialID === 'root') {
-            possibleSpecialID = 'places';
-          }
-          if (BookmarkSpecialIds.isSpecialGUID(possibleSpecialID)) {
-            return possibleSpecialID;
-          }
-          return childID;
+          return PlacesSyncUtils.bookmarks.guidToSyncId(childID);
         });
       }
     }
 
     for (let deletedId of deletedItemIds) {
       let record = idToRecord.get(deletedId);
       if (record && !record.isDeleted) {
         deletedRecords.push(record);
--- a/services/sync/modules/collection_validator.js
+++ b/services/sync/modules/collection_validator.js
@@ -2,19 +2,16 @@
  * 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/. */
 
 "use strict";
 
 const Cu = Components.utils;
 
 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://services-common/async.js");
 Cu.import("resource://services-sync/main.js");
 
 this.EXPORTED_SYMBOLS = ["CollectionValidator", "CollectionProblemData"];
 
 class CollectionProblemData {
   constructor() {
     this.missingIDs = 0;
     this.duplicates = [];
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -13,52 +13,43 @@ 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");
 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");
 });
 
-const ANNOS_TO_TRACK = [BookmarkAnnos.DESCRIPTION_ANNO, BookmarkAnnos.SIDEBAR_ANNO,
+const 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 FOLDER_SORTINDEX = 1000000;
 const {
   SOURCE_SYNC,
   SOURCE_IMPORT,
   SOURCE_IMPORT_REPLACE,
 } = Ci.nsINavBookmarksService;
 
 const SQLITE_MAX_VARIABLE_NUMBER = 999;
 
-// 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",
-};
+const ORGANIZERQUERY_ANNO = "PlacesOrganizer/OrganizerQuery";
+const ALLBOOKMARKS_ANNO = "AllBookmarks";
+const MOBILE_ANNO = "MobileBookmarks";
 
 // The tracker ignores changes made by bookmark import and restore, and
 // changes made by Sync. We don't need to exclude `SOURCE_IMPORT`, but both
 // import and restore fire `bookmarks-restore-*` observer notifications, and
 // the tracker doesn't currently distinguish between the two.
 const IGNORED_SOURCES = [SOURCE_SYNC, SOURCE_IMPORT, SOURCE_IMPORT_REPLACE];
 
 this.PlacesItem = function PlacesItem(collection, id, type) {
@@ -217,41 +208,39 @@ BookmarksEngine.prototype = {
         if (tree.children) {
           for (let child of tree.children) {
             yield* walkBookmarksTree(child, tree);
           }
         }
       }
     }
 
-    function* walkBookmarksRoots(tree, rootGUIDs) {
-      for (let guid of rootGUIDs) {
-        let id = BookmarkSpecialIds.specialIdForGUID(guid, false);
-        let bookmarkRoot = id === null ? null :
-          tree.children.find(child => child.id === id);
+    function* walkBookmarksRoots(tree, rootIDs) {
+      for (let id of rootIDs) {
+        let bookmarkRoot = tree.children.find(child => child.id === id);
         if (bookmarkRoot === null) {
           continue;
         }
         yield* walkBookmarksTree(bookmarkRoot, tree);
       }
     }
 
-    let rootsToWalk = BookmarkSpecialIds.guids.filter(guid =>
-      guid !== 'places' && guid !== 'tags');
+    let rootsToWalk = getChangeRootIds();
 
     for (let [node, parent] of walkBookmarksRoots(tree, rootsToWalk)) {
       let {guid, id, type: placeType} = node;
-      guid = BookmarkSpecialIds.specialGUIDForId(id) || guid;
+      guid = PlacesSyncUtils.bookmarks.guidToSyncId(guid);
       let key;
       switch (placeType) {
         case PlacesUtils.TYPE_X_MOZ_PLACE:
           // Bookmark
           let query = null;
           if (node.annos && node.uri.startsWith("place:")) {
-            query = node.annos.find(({name}) => name === BookmarkAnnos.SMART_BOOKMARKS_ANNO);
+            query = node.annos.find(({name}) =>
+              name === PlacesSyncUtils.bookmarks.SMART_BOOKMARKS_ANNO);
           }
           if (query && query.value) {
             key = "q" + query.value;
           } else {
             key = "b" + node.uri + ":" + node.title;
           }
           break;
         case PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER:
@@ -471,31 +460,31 @@ BookmarksEngine.prototype = {
         ),
         syncedItems(id) AS (
           VALUES ${getChangeRootIds().map(id => `(${id})`).join(", ")}
           UNION ALL
           SELECT b.id
           FROM moz_bookmarks b
           JOIN syncedItems s ON b.parent = s.id
         )
-        SELECT b.guid, b.id
+        SELECT b.guid
         FROM modifiedGuids m
         JOIN moz_bookmarks b ON b.guid = m.guid
         LEFT JOIN syncedItems s ON b.id = s.id
         WHERE s.id IS NULL
       `;
 
       let statement = db.createAsyncStatement(query);
       try {
         for (let i = 0; i < chunkLength; i++) {
           statement.bindByIndex(i, modifiedGUIDs[startIndex + i]);
         }
-        let results = Async.querySpinningly(statement, ["id", "guid"]);
-        for (let { id, guid } of results) {
-          let syncID = BookmarkSpecialIds.specialGUIDForId(id) || guid;
+        let results = Async.querySpinningly(statement, ["guid"]);
+        for (let { guid } of results) {
+          let syncID = PlacesSyncUtils.bookmarks.guidToSyncId(guid);
           this._tracker.removeChangedID(syncID);
         }
       } finally {
         statement.finalize();
       }
     }
 
     return new BookmarksChangeset(this._tracker.changedIDs);
@@ -506,17 +495,17 @@ BookmarksEngine.prototype = {
   _getModifiedGUIDs() {
     let guids = [];
     for (let syncID in this._tracker.changedIDs) {
       if (this._tracker.changedIDs[syncID].deleted === true) {
         // The `===` check also filters out old persisted timestamps,
         // which won't have a `deleted` property.
         continue;
       }
-      let guid = BookmarkSpecialIds.syncIDToPlacesGUID(syncID);
+      let guid = PlacesSyncUtils.bookmarks.syncIdToGuid(syncID);
       guids.push(guid);
     }
     return guids;
   },
 
   // Called when _findDupe returns a dupe item and the engine has decided to
   // switch the existing item to the new incoming item.
   _switchItemToDupe(localDupeGUID, incomingItem) {
@@ -588,17 +577,17 @@ BookmarksStore.prototype = {
   __proto__: Store.prototype,
 
   itemExists: function BStore_itemExists(id) {
     return this.idForGUID(id) > 0;
   },
 
   applyIncoming: function BStore_applyIncoming(record) {
     this._log.debug("Applying record " + record.id);
-    let isSpecial = record.id in BookmarkSpecialIds;
+    let isSpecial = PlacesSyncUtils.bookmarks.ROOTS.includes(record.id);
 
     if (record.deleted) {
       if (isSpecial) {
         this._log.warn("Ignoring deletion for special record " + record.id);
         return;
       }
 
       // Don't bother with pre and post-processing for deletions.
@@ -639,174 +628,155 @@ BookmarksStore.prototype = {
   create: function BStore_create(record) {
     switch (record.type) {
     case "bookmark":
     case "query":
     case "microsummary": {
       let info = {
         kind: record.type,
         url: record.bmkUri,
-        parentGuid: BookmarkSpecialIds.syncIDToPlacesGUID(record.parentid),
+        parentSyncId: record.parentid,
         title: record.title,
-        guid: BookmarkSpecialIds.syncIDToPlacesGUID(record.id),
+        syncId: record.id,
         tags: record.tags,
         keyword: record.keyword,
+        loadInSidebar: record.loadInSidebar,
+        query: record.queryId,
+        folder: record.folderName,
+        description: record.description,
       };
-      if (record.loadInSidebar) {
-        info.loadInSidebar = record.loadInSidebar;
-      }
-      if (record.queryId) {
-        info.query = record.queryId;
-      }
-      if (record.folderName) {
-        info.folder = record.folderName;
-      }
-      if (record.description) {
-        info.description = record.description;
-      }
 
       let bmk = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.insert(info));
-      this._log.debug("created bookmark " + bmk.guid + " under " + bmk.parentGuid
+      this._log.debug("created bookmark " + bmk.syncId + " under " + bmk.parentSyncId
                       + " as " + bmk.title + " " + bmk.url.href);
 
     } break;
     case "folder": {
       let info = {
         kind: PlacesSyncUtils.bookmarks.KINDS.FOLDER,
-        parentGuid: BookmarkSpecialIds.syncIDToPlacesGUID(record.parentid),
-        guid: BookmarkSpecialIds.syncIDToPlacesGUID(record.id),
+        parentSyncId: record.parentid,
+        syncId: record.id,
         title: record.title,
+        description: record.description,
       };
-      if (record.description) {
-        info.description = record.description;
-      }
 
       let folder = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.insert(info));
-      this._log.debug("created folder " + folder.guid + " under " + folder.parentGuid
+      this._log.debug("created folder " + folder.syncId + " under " + folder.parentSyncId
                       + " as " + folder.title);
 
       // record.children will be dealt with in _orderChildren.
     } break;
     case "livemark":
       if (!record.feedUri) {
         this._log.debug("No feed URI: skipping livemark record " + record.id);
         return;
       }
       let info = {
         kind: PlacesSyncUtils.bookmarks.KINDS.LIVEMARK,
         title: record.title,
-        parentGuid: BookmarkSpecialIds.syncIDToPlacesGUID(record.parentid),
+        parentSyncId: record.parentid,
         feed: record.feedUri,
         site: record.siteUri,
-        guid: BookmarkSpecialIds.syncIDToPlacesGUID(record.id),
+        syncId: 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);
+        this._log.debug("Created livemark " + livemark.syncId + " under " +
+                        livemark.parentSyncId + " as " + livemark.title +
+                        ", " + livemark.site.href + ", " +
+                        livemark.feed.href);
       }
       break;
     case "separator": {
       let separator = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.insert({
         kind: PlacesSyncUtils.bookmarks.KINDS.SEPARATOR,
-        parentGuid: BookmarkSpecialIds.syncIDToPlacesGUID(record.parentid),
-        guid: BookmarkSpecialIds.syncIDToPlacesGUID(record.id),
+        parentSyncId: record.parentid,
+        syncId: record.id,
       }));
-      this._log.debug("created separator " + separator.guid + " under " + separator.parentGuid);
+      this._log.debug("created separator " + separator.syncId + " under " + separator.parentSyncId);
     } 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;
     }
   },
 
   remove: function BStore_remove(record) {
-    if (BookmarkSpecialIds.isSpecialGUID(record.id)) {
-      this._log.warn("Refusing to remove special folder " + record.id);
-      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}`);
+      let info = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.remove(record.id));
+      if (info) {
+        this._log.debug(`Removed item ${record.id} with type ${record.type}`);
+      }
     } catch (ex) {
       // Likely already removed.
       this._log.debug(`Error removing ${record.id}`, ex);
     }
   },
 
   update: function BStore_update(record) {
     let info = {
-      parentGuid: BookmarkSpecialIds.syncIDToPlacesGUID(record.parentid),
-      guid: BookmarkSpecialIds.syncIDToPlacesGUID(record.id),
+      parentSyncId: record.parentid,
+      syncId: record.id,
       kind: record.type,
+      title: record.title,
+      url: record.bmkUri,
+      tags: record.tags,
+      keyword: record.keyword,
+      description: record.description,
+      loadInSidebar: record.loadInSidebar,
+      query: record.queryId,
+      site: record.siteUri,
+      feed: record.feedUri,
     };
 
-    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];
-      }
-    }
-
     let bmk = Async.promiseSpinningly(PlacesSyncUtils.bookmarks.update(info));
-    this._log.debug("updated bookmark " + bmk.guid + " under " + bmk.parentGuid);
+    this._log.debug("updated bookmark " + bmk.syncId + " under " + bmk.parentSyncId);
   },
 
   _orderChildren: function _orderChildren() {
     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}`);
+      return PlacesSyncUtils.bookmarks.order(syncID, children).catch(ex => {
+        this._log.debug(`Could not order children for ${syncID}`, ex);
       });
     });
     Async.promiseSpinningly(Promise.all(promises));
   },
 
   changeItemID: function BStore_changeItemID(oldID, newID) {
     this._log.debug("Changing GUID " + oldID + " to " + newID);
 
-    Async.promiseSpinningly(PlacesSyncUtils.bookmarks.changeGuid(
-      BookmarkSpecialIds.syncIDToPlacesGUID(oldID),
-      BookmarkSpecialIds.syncIDToPlacesGUID(newID)
-    ));
+    Async.promiseSpinningly(PlacesSyncUtils.bookmarks.changeGuid(oldID, newID));
   },
 
   _getTags: function BStore__getTags(uri) {
     try {
       if (typeof(uri) == "string")
         uri = Utils.makeURI(uri);
     } catch(e) {
       this._log.warn("Could not parse URI \"" + uri + "\": " + e);
     }
     return PlacesUtils.tagging.getTagsForURI(uri, {});
   },
 
   _getDescription: function BStore__getDescription(id) {
     try {
-      return PlacesUtils.annotations.getItemAnnotation(id, BookmarkAnnos.DESCRIPTION_ANNO);
+      return PlacesUtils.annotations.getItemAnnotation(id,
+        PlacesSyncUtils.bookmarks.DESCRIPTION_ANNO);
     } catch (e) {
       return null;
     }
   },
 
   _isLoadInSidebar: function BStore__isLoadInSidebar(id) {
-    return PlacesUtils.annotations.itemHasAnnotation(id, BookmarkAnnos.SIDEBAR_ANNO);
+    return PlacesUtils.annotations.itemHasAnnotation(id,
+      PlacesSyncUtils.bookmarks.SIDEBAR_ANNO);
   },
 
   // 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);
@@ -830,19 +800,21 @@ BookmarksStore.prototype = {
             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);
+          let anno = PlacesUtils.annotations.getItemAnnotation(placeId,
+            PlacesSyncUtils.bookmarks.SMART_BOOKMARKS_ANNO);
           if (anno != null) {
-            this._log.trace("query anno: " + BookmarkAnnos.SMART_BOOKMARKS_ANNO +
+            this._log.trace("query anno: " +
+                            PlacesSyncUtils.bookmarks.SMART_BOOKMARKS_ANNO +
                             " = " + anno);
             record.queryId = anno;
           }
         }
         catch(ex) {}
       }
       else {
         record = new Bookmark(collection, id);
@@ -870,18 +842,17 @@ BookmarksStore.prototype = {
         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 = Async.promiseSpinningly(
-        PlacesSyncUtils.bookmarks.fetchChildGuids(
-          BookmarkSpecialIds.syncIDToPlacesGUID(id)));
+        PlacesSyncUtils.bookmarks.fetchChildSyncIds(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);
@@ -916,30 +887,23 @@ BookmarksStore.prototype = {
         "SELECT frecency " +
         "FROM moz_places " +
         "WHERE url_hash = hash(:url) AND url = :url " +
         "LIMIT 1");
   },
   _frecencyCols: ["frecency"],
 
   GUIDForId: function GUIDForId(id) {
-    let special = BookmarkSpecialIds.specialGUIDForId(id);
-    if (special)
-      return special;
-
-    return Async.promiseSpinningly(
-      PlacesUtils.promiseItemGuid(id));
+    let guid = Async.promiseSpinningly(PlacesUtils.promiseItemGuid(id));
+    return PlacesSyncUtils.bookmarks.guidToSyncId(guid);
   },
 
   idForGUID: function idForGUID(guid) {
     // guid might be a String object rather than a string.
-    guid = guid.toString();
-
-    if (BookmarkSpecialIds.isSpecialGUID(guid))
-      return BookmarkSpecialIds.specialIdForGUID(guid);
+    guid = PlacesSyncUtils.bookmarks.syncIdToGuid(guid.toString());
 
     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")
@@ -969,25 +933,25 @@ BookmarksStore.prototype = {
       WITH RECURSIVE
       changeRootContents(id) AS (
         VALUES ${getChangeRootIds().map(id => `(${id})`).join(", ")}
         UNION ALL
         SELECT b.id
         FROM moz_bookmarks b
         JOIN changeRootContents c ON b.parent = c.id
       )
-      SELECT id, guid
+      SELECT guid
       FROM changeRootContents
       JOIN moz_bookmarks USING (id)
     `;
 
     let statement = this._getStmt(query);
-    let results = Async.querySpinningly(statement, ["id", "guid"]);
-    for (let { id, guid } of results) {
-      let syncID = BookmarkSpecialIds.specialGUIDForId(id) || guid;
+    let results = Async.querySpinningly(statement, ["guid"]);
+    for (let { guid } of results) {
+      let syncID = PlacesSyncUtils.bookmarks.guidToSyncId(guid);
       items[syncID] = { modified: 0, deleted: false };
     }
 
     return items;
   },
 
   wipe: function BStore_wipe() {
     Async.promiseSpinningly(Task.spawn(function* () {
@@ -1095,19 +1059,19 @@ BookmarksTracker.prototype = {
    * @param itemId
    *        The Places item ID of the bookmark to upload.
    * @param guid
    *        The Places GUID of the bookmark to upload.
    * @param isTombstone
    *        Whether we're uploading a tombstone for a removed bookmark.
    */
   _add: function BMT__add(itemId, guid, isTombstone = false) {
-    guid = BookmarkSpecialIds.specialGUIDForId(itemId) || guid;
+    let syncID = PlacesSyncUtils.bookmarks.guidToSyncId(guid);
     let info = { modified: Date.now() / 1000, deleted: isTombstone };
-    if (this.addChangedID(guid, info)) {
+    if (this.addChangedID(syncID, info)) {
       this._upScore();
     }
   },
 
   /* Every add/remove/change will trigger a sync for MULTI_DEVICE (except in
      a batch operation, where we do it at the end of the batch) */
   _upScore: function BMT__upScore() {
     if (this._batchDepth == 0) {
@@ -1175,40 +1139,40 @@ BookmarksTracker.prototype = {
      */
     this._log.trace("onItemRemoved: " + itemId);
     this._add(itemId, guid, /* isTombstone */ true);
     this._add(parentId, parentGuid);
   },
 
   _ensureMobileQuery: function _ensureMobileQuery() {
     let find = val =>
-      PlacesUtils.annotations.getItemsWithAnnotation(BookmarkAnnos.ORGANIZERQUERY_ANNO, {}).filter(
-        id => PlacesUtils.annotations.getItemAnnotation(id, BookmarkAnnos.ORGANIZERQUERY_ANNO) == val
+      PlacesUtils.annotations.getItemsWithAnnotation(ORGANIZERQUERY_ANNO, {}).filter(
+        id => PlacesUtils.annotations.getItemAnnotation(id, ORGANIZERQUERY_ANNO) == val
       );
 
     // Don't continue if the Library isn't ready
-    let all = find(BookmarkAnnos.ALLBOOKMARKS_ANNO);
+    let all = find(ALLBOOKMARKS_ANNO);
     if (all.length == 0)
       return;
 
-    let mobile = find(BookmarkAnnos.MOBILE_ANNO);
-    let queryURI = Utils.makeURI("place:folder=" + BookmarkSpecialIds.mobile);
+    let mobile = find(MOBILE_ANNO);
+    let queryURI = Utils.makeURI("place:folder=" + PlacesUtils.mobileFolderId);
     let title = PlacesBundle.GetStringFromName("MobileBookmarksFolderTitle");
 
     // Don't add OR remove the mobile bookmarks if there's nothing.
-    if (PlacesUtils.bookmarks.getIdForItemAt(BookmarkSpecialIds.mobile, 0) == -1) {
+    if (PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.mobileFolderId, 0) == -1) {
       if (mobile.length != 0)
         PlacesUtils.bookmarks.removeItem(mobile[0], SOURCE_SYNC);
     }
     // Add the mobile bookmarks query if it doesn't exist
     else if (mobile.length == 0) {
       let query = PlacesUtils.bookmarks.insertBookmark(all[0], queryURI, -1, title, /* guid */ null, SOURCE_SYNC);
-      PlacesUtils.annotations.setItemAnnotation(query, BookmarkAnnos.ORGANIZERQUERY_ANNO, BookmarkAnnos.MOBILE_ANNO, 0,
+      PlacesUtils.annotations.setItemAnnotation(query, ORGANIZERQUERY_ANNO, MOBILE_ANNO, 0,
                                   PlacesUtils.annotations.EXPIRE_NEVER, SOURCE_SYNC);
-      PlacesUtils.annotations.setItemAnnotation(query, BookmarkAnnos.EXCLUDEBACKUP_ANNO, 1, 0,
+      PlacesUtils.annotations.setItemAnnotation(query, PlacesUtils.EXCLUDE_FROM_BACKUP_ANNO, 1, 0,
                                   PlacesUtils.annotations.EXPIRE_NEVER, SOURCE_SYNC);
     }
     // Make sure the existing title is correct
     else {
       let queryTitle = PlacesUtils.bookmarks.getItemTitle(mobile[0]);
       if (queryTitle != title) {
         PlacesUtils.bookmarks.setItemTitle(mobile[0], title, SOURCE_SYNC);
       }
@@ -1256,17 +1220,18 @@ BookmarksTracker.prototype = {
     this._log.trace("onItemMoved: " + itemId);
     this._add(oldParent, oldParentGuid);
     if (oldParent != newParent) {
       this._add(itemId, guid);
       this._add(newParent, newParentGuid);
     }
 
     // Remove any position annotations now that the user moved the item
-    PlacesUtils.annotations.removeItemAnnotation(itemId, BookmarkAnnos.PARENT_ANNO, SOURCE_SYNC);
+    PlacesUtils.annotations.removeItemAnnotation(itemId,
+      PlacesSyncUtils.bookmarks.SYNC_PARENT_ANNO, SOURCE_SYNC);
   },
 
   onBeginUpdateBatch: function () {
     ++this._batchDepth;
   },
   onEndUpdateBatch: function () {
     if (--this._batchDepth === 0 && this._batchSawScoreIncrement) {
       this.score += SCORE_INCREMENT_XLARGE;
--- a/services/sync/moz.build
+++ b/services/sync/moz.build
@@ -14,17 +14,16 @@ XPCSHELL_TESTS_MANIFESTS += ['tests/unit
 EXTRA_COMPONENTS += [
     'SyncComponents.manifest',
     'Weave.js',
 ]
 
 EXTRA_JS_MODULES['services-sync'] += [
     'modules/addonsreconciler.js',
     'modules/addonutils.js',
-    'modules/bookmark_utils.js',
     'modules/bookmark_validator.js',
     'modules/browserid_identity.js',
     'modules/collection_validator.js',
     'modules/engines.js',
     'modules/FxaMigrator.jsm',
     'modules/identity.js',
     'modules/jpakeclient.js',
     'modules/keys.js',
--- a/services/sync/tests/unit/test_bookmark_engine.js
+++ b/services/sync/tests/unit/test_bookmark_engine.js
@@ -2,17 +2,16 @@
    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://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);
@@ -490,38 +489,38 @@ add_task(function* test_bookmark_tag_but
   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.
 
   yield PlacesSyncUtils.bookmarks.insert({
     kind: PlacesSyncUtils.bookmarks.KINDS.BOOKMARK,
-    guid: Utils.makeGUID(),
-    parentGuid: PlacesUtils.bookmarks.toolbarGuid,
+    syncId: Utils.makeGUID(),
+    parentSyncId: "toolbar",
     url: "http://example.com",
     tags: ["foo"],
   });
   yield PlacesSyncUtils.bookmarks.insert({
     kind: PlacesSyncUtils.bookmarks.KINDS.BOOKMARK,
-    guid: Utils.makeGUID(),
-    parentGuid: PlacesUtils.bookmarks.toolbarGuid,
+    syncId: Utils.makeGUID(),
+    parentSyncId: "toolbar",
     url: "http://example.org",
     tags: null,
   });
   yield PlacesSyncUtils.bookmarks.insert({
     kind: PlacesSyncUtils.bookmarks.KINDS.BOOKMARK,
-    guid: Utils.makeGUID(),
+    syncId: Utils.makeGUID(),
     url: "about:fake",
-    parentGuid: PlacesUtils.bookmarks.toolbarGuid,
+    parentSyncId: "toolbar",
     tags: null,
   });
 
   let record = {
-    parentid:    PlacesUtils.bookmarks.toolbarGuid,
+    parentid:    "toolbar",
     id:          Utils.makeGUID(),
     description: "",
     tags:        ["foo"],
     title:       "Taggy tag",
     type:        "folder"
   };
 
   // Because update() walks the cleartext.
--- a/services/sync/tests/unit/test_bookmark_tracker.js
+++ b/services/sync/tests/unit/test_bookmark_tracker.js
@@ -1,14 +1,14 @@
 /* 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/Task.jsm");
-Cu.import("resource://services-sync/bookmark_utils.js");
 Cu.import("resource://services-sync/constants.js");
 Cu.import("resource://services-sync/engines/bookmarks.js");
 Cu.import("resource://services-sync/engines.js");
 Cu.import("resource://services-sync/service.js");
 Cu.import("resource://services-sync/util.js");
 
 Service.engineManager.register(BookmarksEngine);
 var engine = Service.engineManager.get("bookmarks");
@@ -343,17 +343,17 @@ add_task(function* test_onItemChanged_ch
       Utils.makeURI("http://getfirefox.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Get Firefox!");
     let fx_guid = engine._store.GUIDForId(fx_id);
     _(`Firefox GUID: ${fx_guid}`);
 
     _("Set a tracked annotation to make sure we only notify once");
     PlacesUtils.annotations.setItemAnnotation(
-      fx_id, BookmarkAnnos.DESCRIPTION_ANNO, "A test description", 0,
+      fx_id, PlacesSyncUtils.bookmarks.DESCRIPTION_ANNO, "A test description", 0,
       PlacesUtils.annotations.EXPIRE_NEVER);
 
     yield startTracking();
 
     _("Change the bookmark's URI");
     PlacesUtils.bookmarks.changeBookmarkURI(fx_id,
       Utils.makeURI("https://www.mozilla.org/firefox"));
     yield verifyTrackedItems([fx_guid]);
@@ -683,25 +683,25 @@ add_task(function* test_onItemAnnoChange
       folder, Utils.makeURI("http://getfirefox.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX, "Get Firefox!");
     let bGUID = engine._store.GUIDForId(b);
     _("New item is " + b);
     _("GUID: " + bGUID);
 
     yield startTracking();
     PlacesUtils.annotations.setItemAnnotation(
-      b, BookmarkAnnos.DESCRIPTION_ANNO, "A test description", 0,
+      b, PlacesSyncUtils.bookmarks.DESCRIPTION_ANNO, "A test description", 0,
       PlacesUtils.annotations.EXPIRE_NEVER);
     // bookmark should be tracked, folder should not.
     yield verifyTrackedItems([bGUID]);
     do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE);
     yield resetTracker();
 
     PlacesUtils.annotations.removeItemAnnotation(b,
-      BookmarkAnnos.DESCRIPTION_ANNO);
+      PlacesSyncUtils.bookmarks.DESCRIPTION_ANNO);
     yield verifyTrackedItems([bGUID]);
     do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE);
   } finally {
     _("Clean up.");
     yield cleanup();
   }
 });