Bug 1302901 - Remove mobile root creation from Sync. r=markh draft
authorKit Cambridge <kcambridge@mozilla.com>
Thu, 15 Sep 2016 17:15:20 -0700
changeset 422560 11fa774388d91b07d9b18989ba328a89c8666e5f
parent 422559 6cff6aa8ec50b5adb4be16dc16d72934c806ffe7
child 533305 8e02bf6fa230307c728c50d57acf78034b957b8f
push id31740
push userbmo:kcambridge@mozilla.com
push dateFri, 07 Oct 2016 20:40:57 +0000
reviewersmarkh
bugs1302901
milestone52.0a1
Bug 1302901 - Remove mobile root creation from Sync. r=markh MozReview-Commit-ID: 6fphDT1Owni
services/sync/locales/en-US/sync.properties
services/sync/modules/bookmark_utils.js
services/sync/modules/engines/bookmarks.js
services/sync/tests/unit/test_bookmark_duping.js
services/sync/tests/unit/test_bookmark_engine.js
services/sync/tests/unit/test_bookmark_order.js
services/sync/tests/unit/test_bookmark_tracker.js
--- a/services/sync/locales/en-US/sync.properties
+++ b/services/sync/locales/en-US/sync.properties
@@ -7,18 +7,16 @@ client.name2 = %1$S’s %2$S on %3$S
 
 # %S is the date and time at which the last sync successfully completed
 lastSync2.label = Last sync: %S
 
 # signInToSync.description is the tooltip for the Sync buttons when Sync is
 # not configured.
 signInToSync.description = Sign In To Sync
 
-mobile.label = Mobile Bookmarks
-
 error.sync.title = Error While Syncing
 error.sync.description = Sync encountered an error while syncing: %1$S.  Sync will automatically retry this action.
 warning.sync.eol.label = Service Shutting Down
 # %1: the app name (Firefox)
 warning.sync.eol.description = Your Firefox Sync service is shutting down soon. Upgrade %1$S to keep syncing.
 error.sync.eol.label = Service Unavailable
 # %1: the app name (Firefox)
 error.sync.eol.description = Your Firefox Sync service is no longer available. You need to upgrade %1$S to keep syncing.
--- a/services/sync/modules/bookmark_utils.js
+++ b/services/sync/modules/bookmark_utils.js
@@ -30,74 +30,37 @@ let BookmarkAnnos = {
 // 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));
-    },
+    "mobile": PlacesUtils.bookmarks.mobileGuid,
   };
 });
 
 let BookmarkSpecialIds = {
 
-  // Special IDs. Note that mobile can attempt to create a record on
-  // dereference; special accessors are provided to prevent recursion within
-  // observers.
+  // Special IDs.
   guids: ["menu", "places", "tags", "toolbar", "unfiled", "mobile"],
 
-  // Create the special mobile folder to store mobile bookmarks.
-  createMobileRoot: function createMobileRoot() {
-    let root = PlacesUtils.placesRootId;
-    let mRoot = PlacesUtils.bookmarks.createFolder(root, "mobile", -1,
-      /* guid */ null, PlacesUtils.bookmarks.SOURCE_SYNC);
-    PlacesUtils.annotations.setItemAnnotation(
-      mRoot, BookmarkAnnos.MOBILEROOT_ANNO, 1, 0,
-      PlacesUtils.annotations.EXPIRE_NEVER, PlacesUtils.bookmarks.SOURCE_SYNC);
-    PlacesUtils.annotations.setItemAnnotation(
-      mRoot, BookmarkAnnos.EXCLUDEBACKUP_ANNO, 1, 0,
-      PlacesUtils.annotations.EXPIRE_NEVER, PlacesUtils.bookmarks.SOURCE_SYNC);
-    return mRoot;
-  },
-
-  findMobileRoot: function findMobileRoot(create) {
-    // Use the (one) mobile root if it already exists.
-    let root = PlacesUtils.annotations.getItemsWithAnnotation(
-      BookmarkAnnos.MOBILEROOT_ANNO, {});
-    if (root.length != 0)
-      return root[0];
-
-    if (create)
-      return this.createMobileRoot();
-
-    return null;
-  },
-
   // Accessors for IDs.
   isSpecialGUID: function isSpecialGUID(g) {
     return this.guids.indexOf(g) != -1;
   },
 
-  specialIdForGUID: function specialIdForGUID(guid, create) {
-    if (guid == "mobile") {
-      return this.findMobileRoot(create);
-    }
+  specialIdForGUID: function specialIdForGUID(guid) {
     return this[guid];
   },
 
-  // 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)
+      if (this.specialIdForGUID(guid) == id)
         return guid;
     return null;
   },
 
   syncIDToPlacesGUID(g) {
     return g in SpecialGUIDToPlacesGUID ? SpecialGUIDToPlacesGUID[g] : g;
   },
 
@@ -112,11 +75,11 @@ let BookmarkSpecialIds = {
   },
   get toolbar() {
     return PlacesUtils.toolbarFolderId;
   },
   get unfiled() {
     return PlacesUtils.unfiledBookmarksFolderId;
   },
   get mobile() {
-    return this.findMobileRoot(true);
+    return PlacesUtils.mobileFolderId;
   },
 };
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -18,16 +18,21 @@ Cu.import("resource://services-sync/cons
 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,
                         PlacesUtils.LMANNO_FEEDURI, PlacesUtils.LMANNO_SITEURI];
 
 const SERVICE_NOT_SUPPORTED = "Service not supported on this platform";
 const FOLDER_SORTINDEX = 1000000;
 const {
   SOURCE_SYNC,
@@ -578,17 +583,17 @@ function BookmarksStore(name, engine) {
     }
     this._stmts = {};
   }, this);
 }
 BookmarksStore.prototype = {
   __proto__: Store.prototype,
 
   itemExists: function BStore_itemExists(id) {
-    return this.idForGUID(id, true) > 0;
+    return this.idForGUID(id) > 0;
   },
 
   applyIncoming: function BStore_applyIncoming(record) {
     this._log.debug("Applying record " + record.id);
     let isSpecial = record.id in BookmarkSpecialIds;
 
     if (record.deleted) {
       if (isSpecial) {
@@ -919,24 +924,22 @@ BookmarksStore.prototype = {
     let special = BookmarkSpecialIds.specialGUIDForId(id);
     if (special)
       return special;
 
     return Async.promiseSpinningly(
       PlacesUtils.promiseItemGuid(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) {
+  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, !noCreate);
+      return BookmarkSpecialIds.specialIdForGUID(guid);
 
     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")
@@ -982,39 +985,23 @@ BookmarksStore.prototype = {
       let syncID = BookmarkSpecialIds.specialGUIDForId(id) || guid;
       items[syncID] = { modified: 0, deleted: false };
     }
 
     return items;
   },
 
   wipe: function BStore_wipe() {
-    let cb = Async.makeSpinningCallback();
-    Task.spawn(function* () {
+    Async.promiseSpinningly(Task.spawn(function* () {
       // Save a backup before clearing out all bookmarks.
       yield PlacesBackups.create(null, true);
-      // 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();
+      yield PlacesUtils.bookmarks.eraseEverything({
+        source: SOURCE_SYNC,
+      });
+    }));
   }
 };
 
 function BookmarksTracker(name, engine) {
   this._batchDepth = 0;
   this._batchSawScoreIncrement = false;
   Tracker.call(this, name, engine);
 
@@ -1199,34 +1186,43 @@ BookmarksTracker.prototype = {
 
     // Don't continue if the Library isn't ready
     let all = find(BookmarkAnnos.ALLBOOKMARKS_ANNO);
     if (all.length == 0)
       return;
 
     let mobile = find(BookmarkAnnos.MOBILE_ANNO);
     let queryURI = Utils.makeURI("place:folder=" + BookmarkSpecialIds.mobile);
-    let title = Str.sync.get("mobile.label");
+    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 (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.EXPIRE_NEVER, SOURCE_SYNC);
       PlacesUtils.annotations.setItemAnnotation(query, BookmarkAnnos.EXCLUDEBACKUP_ANNO, 1, 0,
                                   PlacesUtils.annotations.EXPIRE_NEVER, SOURCE_SYNC);
     }
     // Make sure the existing title is correct
-    else if (PlacesUtils.bookmarks.getItemTitle(mobile[0]) != title) {
-      PlacesUtils.bookmarks.setItemTitle(mobile[0], title, SOURCE_SYNC);
+    else {
+      let queryTitle = PlacesUtils.bookmarks.getItemTitle(mobile[0]);
+      if (queryTitle != title) {
+        PlacesUtils.bookmarks.setItemTitle(mobile[0], title, SOURCE_SYNC);
+      }
+      let rootTitle =
+        PlacesUtils.bookmarks.getItemTitle(BookmarkSpecialIds.mobile);
+      if (rootTitle != title) {
+        PlacesUtils.bookmarks.setItemTitle(BookmarkSpecialIds.mobile, title,
+                                           SOURCE_SYNC);
+      }
     }
   },
 
   // This method is oddly structured, but the idea is to return as quickly as
   // possible -- this handler gets called *every time* a bookmark changes, for
   // *each change*.
   onItemChanged: function BMT_onItemChanged(itemId, property, isAnno, value,
                                             lastModified, itemType, parentId,
@@ -1279,26 +1275,22 @@ BookmarksTracker.prototype = {
   },
   onItemVisited: function () {}
 };
 
 // Returns an array of root IDs to recursively query for synced bookmarks.
 // Items in other roots, including tags and organizer queries, will be
 // ignored.
 function getChangeRootIds() {
-  let rootIds = [
+  return [
     PlacesUtils.bookmarksMenuFolderId,
     PlacesUtils.toolbarFolderId,
     PlacesUtils.unfiledBookmarksFolderId,
+    PlacesUtils.mobileFolderId,
   ];
-  let mobileRootId = BookmarkSpecialIds.findMobileRoot(false);
-  if (mobileRootId) {
-    rootIds.push(mobileRootId);
-  }
-  return rootIds;
 }
 
 class BookmarksChangeset extends Changeset {
   getModifiedTimestamp(id) {
     let change = this.changes[id];
     return change ? change.modified : Number.NaN;
   }
 }
--- a/services/sync/tests/unit/test_bookmark_duping.js
+++ b/services/sync/tests/unit/test_bookmark_duping.js
@@ -144,18 +144,18 @@ add_task(function* test_dupe_bookmark() 
 
   try {
     // The parent folder and one bookmark in it.
     let {id: folder1_id, guid: folder1_guid } = createFolder(bms.toolbarFolder, "Folder 1");
     let {id: bmk1_id, guid: bmk1_guid} = createBookmark(folder1_id, "http://getfirefox.com/", "Get Firefox!");
 
     engine.sync();
 
-    // We've added the bookmark, its parent (folder1) plus "menu", "toolbar" and "unfiled"
-    equal(collection.count(), 5);
+    // We've added the bookmark, its parent (folder1) plus "menu", "toolbar", "unfiled", and "mobile".
+    equal(collection.count(), 6);
     equal(getFolderChildrenIDs(folder1_id).length, 1);
 
     // Now create a new incoming record that looks alot like a dupe.
     let newGUID = Utils.makeGUID();
     let to_apply = {
       id: newGUID,
       bmkUri: "http://getfirefox.com/",
       type: "bookmark",
@@ -165,17 +165,17 @@ add_task(function* test_dupe_bookmark() 
     };
 
     collection.insert(newGUID, encryptPayload(to_apply), Date.now() / 1000 + 10);
     _("Syncing so new dupe record is processed");
     engine.lastSync = engine.lastSync - 0.01;
     engine.sync();
 
     // We should have logically deleted the dupe record.
-    equal(collection.count(), 6);
+    equal(collection.count(), 7);
     ok(getServerRecord(collection, bmk1_guid).deleted);
     // and physically removed from the local store.
     yield promiseNoLocalItem(bmk1_guid);
     // Parent should still only have 1 item.
     equal(getFolderChildrenIDs(folder1_id).length, 1);
     // The parent record on the server should now reference the new GUID and not the old.
     let serverRecord = getServerRecord(collection, folder1_guid);
     ok(!serverRecord.children.includes(bmk1_guid));
@@ -199,18 +199,18 @@ add_task(function* test_dupe_reparented_
     let {id: bmk1_id, guid: bmk1_guid} = createBookmark(folder1_id, "http://getfirefox.com/", "Get Firefox!");
     // Another parent folder *with the same name*
     let {id: folder2_id, guid: folder2_guid } = createFolder(bms.toolbarFolder, "Folder 1");
 
     do_print(`folder1_guid=${folder1_guid}, folder2_guid=${folder2_guid}, bmk1_guid=${bmk1_guid}`);
 
     engine.sync();
 
-    // We've added the bookmark, 2 folders plus "menu", "toolbar" and "unfiled"
-    equal(collection.count(), 6);
+    // We've added the bookmark, 2 folders plus "menu", "toolbar", "unfiled", and "mobile".
+    equal(collection.count(), 7);
     equal(getFolderChildrenIDs(folder1_id).length, 1);
     equal(getFolderChildrenIDs(folder2_id).length, 0);
 
     // Now create a new incoming record that looks alot like a dupe of the
     // item in folder1_guid, but with a record that points to folder2_guid.
     let newGUID = Utils.makeGUID();
     let to_apply = {
       id: newGUID,
@@ -223,17 +223,17 @@ add_task(function* test_dupe_reparented_
 
     collection.insert(newGUID, encryptPayload(to_apply), Date.now() / 1000 + 10);
 
     _("Syncing so new dupe record is processed");
     engine.lastSync = engine.lastSync - 0.01;
     engine.sync();
 
     // We should have logically deleted the dupe record.
-    equal(collection.count(), 7);
+    equal(collection.count(), 8);
     ok(getServerRecord(collection, bmk1_guid).deleted);
     // and physically removed from the local store.
     yield promiseNoLocalItem(bmk1_guid);
     // The original folder no longer has the item
     equal(getFolderChildrenIDs(folder1_id).length, 0);
     // But the second dupe folder does.
     equal(getFolderChildrenIDs(folder2_id).length, 1);
 
@@ -265,18 +265,18 @@ add_task(function* test_dupe_reparented_
     let {id: bmk1_id, guid: bmk1_guid} = createBookmark(folder1_id, "http://getfirefox.com/", "Get Firefox!");
     // Another parent folder *with the same name*
     let {id: folder2_id, guid: folder2_guid } = createFolder(bms.toolbarFolder, "Folder 1");
 
     do_print(`folder1_guid=${folder1_guid}, folder2_guid=${folder2_guid}, bmk1_guid=${bmk1_guid}`);
 
     engine.sync();
 
-    // We've added the bookmark, 2 folders plus "menu", "toolbar" and "unfiled"
-    equal(collection.count(), 6);
+    // We've added the bookmark, 2 folders plus "menu", "toolbar", "unfiled", and "mobile".
+    equal(collection.count(), 7);
     equal(getFolderChildrenIDs(folder1_id).length, 1);
     equal(getFolderChildrenIDs(folder2_id).length, 0);
 
     // Now create a new incoming record that looks alot like a dupe of the
     // item in folder1_guid, but with a record that points to folder2_guid.
     let newGUID = Utils.makeGUID();
     let to_apply = {
       id: newGUID,
@@ -295,17 +295,17 @@ add_task(function* test_dupe_reparented_
     // to *not* apply the incoming record.
     engine._tracker.addChangedID(bmk1_guid, Date.now() / 1000 + 60);
 
     _("Syncing so new dupe record is processed");
     engine.lastSync = engine.lastSync - 0.01;
     engine.sync();
 
     // We should have logically deleted the dupe record.
-    equal(collection.count(), 7);
+    equal(collection.count(), 8);
     ok(getServerRecord(collection, bmk1_guid).deleted);
     // and physically removed from the local store.
     yield promiseNoLocalItem(bmk1_guid);
     // The original folder still longer has the item
     equal(getFolderChildrenIDs(folder1_id).length, 1);
     // The second folder does not.
     equal(getFolderChildrenIDs(folder2_id).length, 0);
 
@@ -338,18 +338,18 @@ add_task(function* test_dupe_reparented_
     let {id: bmk1_id, guid: bmk1_guid} = createBookmark(folder1_id, "http://getfirefox.com/", "Get Firefox!");
     // One more folder we'll use later.
     let {id: folder2_id, guid: folder2_guid} = createFolder(bms.toolbarFolder, "A second folder");
 
     do_print(`folder1=${folder1_guid}, bmk1=${bmk1_guid} folder2=${folder2_guid}`);
 
     engine.sync();
 
-    // We've added the bookmark, 2 folders plus "menu", "toolbar" and "unfiled"
-    equal(collection.count(), 6);
+    // We've added the bookmark, 2 folders plus "menu", "toolbar", "unfiled", and "mobile".
+    equal(collection.count(), 7);
     equal(getFolderChildrenIDs(folder1_id).length, 1);
 
     let newGUID = Utils.makeGUID();
     let newParentGUID = Utils.makeGUID();
 
     // Have the new parent appear before the dupe item.
     collection.insert(newParentGUID, encryptPayload({
       id: newParentGUID,
@@ -415,18 +415,18 @@ add_task(function* test_dupe_reparented_
     let {id: bmk1_id, guid: bmk1_guid} = createBookmark(folder1_id, "http://getfirefox.com/", "Get Firefox!");
     // One more folder we'll use later.
     let {id: folder2_id, guid: folder2_guid} = createFolder(bms.toolbarFolder, "A second folder");
 
     do_print(`folder1=${folder1_guid}, bmk1=${bmk1_guid} folder2=${folder2_guid}`);
 
     engine.sync();
 
-    // We've added the bookmark, 2 folders plus "menu", "toolbar" and "unfiled"
-    equal(collection.count(), 6);
+    // We've added the bookmark, 2 folders plus "menu", "toolbar", "unfiled", and "mobile".
+    equal(collection.count(), 7);
     equal(getFolderChildrenIDs(folder1_id).length, 1);
 
     // Now create a new incoming record that looks alot like a dupe of the
     // item in folder1_guid, but with a record that points to a parent with the
     // same name, but a non-existing local ID.
     let newGUID = Utils.makeGUID();
     let newParentGUID = Utils.makeGUID();
 
@@ -492,18 +492,18 @@ add_task(function* test_dupe_reparented_
     let {id: bmk1_id, guid: bmk1_guid} = createBookmark(folder1_id, "http://getfirefox.com/", "Get Firefox!");
     // One more folder we'll use later.
     let {id: folder2_id, guid: folder2_guid} = createFolder(bms.toolbarFolder, "A second folder");
 
     do_print(`folder1=${folder1_guid}, bmk1=${bmk1_guid} folder2=${folder2_guid}`);
 
     engine.sync();
 
-    // We've added the bookmark, 2 folders plus "menu", "toolbar" and "unfiled"
-    equal(collection.count(), 6);
+    // We've added the bookmark, 2 folders plus "menu", "toolbar", "unfiled", and "mobile".
+    equal(collection.count(), 7);
     equal(getFolderChildrenIDs(folder1_id).length, 1);
 
     // Now create a new incoming record that looks alot like a dupe of the
     // item in folder1_guid, but with a record that points to a parent with the
     // same name, but a non-existing local ID.
     let newGUID = Utils.makeGUID();
     let newParentGUID = Utils.makeGUID();
 
@@ -517,17 +517,17 @@ add_task(function* test_dupe_reparented_
       tags: [],
     }), Date.now() / 1000 + 10);
 
     _("Syncing so new dupe record is processed");
     engine.lastSync = engine.lastSync - 0.01;
     engine.sync();
 
     // We should have logically deleted the dupe record.
-    equal(collection.count(), 7);
+    equal(collection.count(), 8);
     ok(getServerRecord(collection, bmk1_guid).deleted);
     // and physically removed from the local store.
     yield promiseNoLocalItem(bmk1_guid);
     // The intended parent doesn't exist, so it remains in the original folder
     equal(getFolderChildrenIDs(folder1_id).length, 1);
 
     // The record for folder1 on the server should reference the new GUID.
     let serverRecord1 = getServerRecord(collection, folder1_guid);
@@ -607,18 +607,18 @@ add_task(function* test_dupe_empty_folde
   let { server, collection } = this.setup();
 
   try {
     // The folder we will end up duping away.
     let {id: folder1_id, guid: folder1_guid } = createFolder(bms.toolbarFolder, "Folder 1");
 
     engine.sync();
 
-    // We've added 1 folder, "menu", "toolbar" and "unfiled"
-    equal(collection.count(), 4);
+    // We've added 1 folder, "menu", "toolbar", "unfiled", and "mobile".
+    equal(collection.count(), 5);
 
     // Now create new incoming records that looks alot like a dupe of "Folder 1".
     let newFolderGUID = Utils.makeGUID();
     collection.insert(newFolderGUID, encryptPayload({
       id: newFolderGUID,
       type: "folder",
       title: "Folder 1",
       parentName: "Bookmarks Toolbar",
@@ -628,17 +628,17 @@ add_task(function* test_dupe_empty_folde
 
     _("Syncing so new dupe records are processed");
     engine.lastSync = engine.lastSync - 0.01;
     engine.sync();
 
     yield validate(collection);
 
     // Collection now has one additional record - the logically deleted dupe.
-    equal(collection.count(), 5);
+    equal(collection.count(), 6);
     // original folder should be logically deleted.
     ok(getServerRecord(collection, folder1_guid).deleted);
     yield promiseNoLocalItem(folder1_guid);
   } finally {
     yield cleanup(server);
   }
 });
 // XXX - TODO - folders with children. Bug 1293163
--- a/services/sync/tests/unit/test_bookmark_engine.js
+++ b/services/sync/tests/unit/test_bookmark_engine.js
@@ -139,60 +139,16 @@ add_task(function* bad_record_allIDs() {
   do_check_true("menu" in all);
   do_check_true("toolbar" in all);
 
   _("Clean up.");
   PlacesUtils.bookmarks.removeItem(badRecordID);
   yield new Promise(r => server.stop(r));
 });
 
-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.");
-  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);
-    _("New mobile ID: " + newMobileID);
-  } catch (ex) {
-    err = ex;
-    _("Error: " + Log.exceptionStr(err));
-  }
-
-  do_check_true(!err);
-
-  // With !noCreate, lookup works, and it's different.
-  newMobileID = store.idForGUID("mobile", false);
-  _("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");
-  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: {}
   });
 }
 
--- a/services/sync/tests/unit/test_bookmark_order.js
+++ b/services/sync/tests/unit/test_bookmark_order.js
@@ -48,16 +48,19 @@ add_task(function* test_bookmark_order()
   }, {
     guid: PlacesUtils.bookmarks.toolbarGuid,
     index: 1,
   }, {
     // Index 2 is the tags root. (Root indices depend on the order of the
     // `CreateRoot` calls in `Database::CreateBookmarkRoots`).
     guid: PlacesUtils.bookmarks.unfiledGuid,
     index: 3,
+  }, {
+    guid: PlacesUtils.bookmarks.mobileGuid,
+    index: 4,
   }], "clean slate");
 
   function bookmark(name, parent) {
     let bookmark = new Bookmark("http://weave.server/my-bookmark");
     bookmark.id = name;
     bookmark.title = name;
     bookmark.bmkUri = "http://uri/";
     bookmark.parentid = parent || "unfiled";
@@ -91,16 +94,19 @@ add_task(function* test_bookmark_order()
     index: 1,
   }, {
     guid: PlacesUtils.bookmarks.unfiledGuid,
     index: 3,
     children: [{
       guid: id10,
       index: 0,
     }],
+  }, {
+    guid: PlacesUtils.bookmarks.mobileGuid,
+    index: 4,
   }], "basic add first bookmark");
   let id20 = "20_aaaaaaaaa";
   _("basic append behind 10");
   apply(bookmark(id20, ""));
   yield check([{
     guid: PlacesUtils.bookmarks.menuGuid,
     index: 0,
   }, {
@@ -111,16 +117,19 @@ add_task(function* test_bookmark_order()
     index: 3,
     children: [{
       guid: id10,
       index: 0,
     }, {
       guid: id20,
       index: 1,
     }],
+  }, {
+    guid: PlacesUtils.bookmarks.mobileGuid,
+    index: 4,
   }], "basic append behind 10");
 
   let id31 = "31_aaaaaaaaa";
   let id30 = "f30_aaaaaaaa";
   _("basic create in folder");
   apply(bookmark(id31, id30));
   let f30 = folder(id30, "", [id31]);
   apply(f30);
@@ -142,16 +151,19 @@ add_task(function* test_bookmark_order()
     }, {
       guid: id30,
       index: 2,
       children: [{
         guid: id31,
         index: 0,
       }],
     }],
+  }, {
+    guid: PlacesUtils.bookmarks.mobileGuid,
+    index: 4,
   }], "basic create in folder");
 
   let id41 = "41_aaaaaaaaa";
   let id40 = "f40_aaaaaaaa";
   _("insert missing parent -> append to unfiled");
   apply(bookmark(id41, id40));
   yield check([{
     guid: PlacesUtils.bookmarks.menuGuid,
@@ -175,16 +187,19 @@ add_task(function* test_bookmark_order()
         guid: id31,
         index: 0,
       }],
     }, {
       guid: id41,
       index: 3,
       requestedParent: id40,
     }],
+  }, {
+    guid: PlacesUtils.bookmarks.mobileGuid,
+    index: 4,
   }], "insert missing parent -> append to unfiled");
 
   let id42 = "42_aaaaaaaaa";
 
   _("insert another missing parent -> append");
   apply(bookmark(id42, id40));
   yield check([{
     guid: PlacesUtils.bookmarks.menuGuid,
@@ -212,16 +227,19 @@ add_task(function* test_bookmark_order()
       guid: id41,
       index: 3,
       requestedParent: id40,
     }, {
       guid: id42,
       index: 4,
       requestedParent: id40,
     }],
+  }, {
+    guid: PlacesUtils.bookmarks.mobileGuid,
+    index: 4,
   }], "insert another missing parent -> append");
 
   _("insert folder -> move children and followers");
   let f40 = folder(id40, "", [id41, id42]);
   apply(f40);
   yield check([{
     guid: PlacesUtils.bookmarks.menuGuid,
     index: 0,
@@ -250,16 +268,19 @@ add_task(function* test_bookmark_order()
       children: [{
         guid: id41,
         index: 0,
       }, {
         guid: id42,
         index: 1,
       }]
     }],
+  }, {
+    guid: PlacesUtils.bookmarks.mobileGuid,
+    index: 4,
   }], "insert folder -> move children and followers");
 
   _("Moving 41 behind 42 -> update f40");
   f40.children = [id42, id41];
   apply(f40);
   yield check([{
     guid: PlacesUtils.bookmarks.menuGuid,
     index: 0,
@@ -288,16 +309,19 @@ add_task(function* test_bookmark_order()
       children: [{
         guid: id42,
         index: 0,
       }, {
         guid: id41,
         index: 1,
       }]
     }],
+  }, {
+    guid: PlacesUtils.bookmarks.mobileGuid,
+    index: 4,
   }], "Moving 41 behind 42 -> update f40");
 
   _("Moving 10 back to front -> update 10, 20");
   f40.children = [id41, id42];
   apply(f40);
   yield check([{
     guid: PlacesUtils.bookmarks.menuGuid,
     index: 0,
@@ -326,16 +350,19 @@ add_task(function* test_bookmark_order()
       children: [{
         guid: id41,
         index: 0,
       }, {
         guid: id42,
         index: 1,
       }]
     }],
+  }, {
+    guid: PlacesUtils.bookmarks.mobileGuid,
+    index: 4,
   }], "Moving 10 back to front -> update 10, 20");
 
   _("Moving 20 behind 42 in f40 -> update 50");
   apply(bookmark(id20, id40));
   yield check([{
     guid: PlacesUtils.bookmarks.menuGuid,
     index: 0,
   }, {
@@ -363,16 +390,19 @@ add_task(function* test_bookmark_order()
       }, {
         guid: id42,
         index: 1,
       }, {
         guid: id20,
         index: 2,
       }]
     }],
+  }, {
+    guid: PlacesUtils.bookmarks.mobileGuid,
+    index: 4,
   }], "Moving 20 behind 42 in f40 -> update 50");
 
   _("Moving 10 in front of 31 in f30 -> update 10, f30");
   apply(bookmark(id10, id30));
   f30.children = [id10, id31];
   apply(f30);
   yield check([{
     guid: PlacesUtils.bookmarks.menuGuid,
@@ -402,16 +432,19 @@ add_task(function* test_bookmark_order()
       }, {
         guid: id42,
         index: 1,
       }, {
         guid: id20,
         index: 2,
       }]
     }],
+  }, {
+    guid: PlacesUtils.bookmarks.mobileGuid,
+    index: 4,
   }], "Moving 10 in front of 31 in f30 -> update 10, f30");
 
   _("Moving 20 from f40 to f30 -> update 20, f30");
   apply(bookmark(id20, id30));
   f30.children = [id10, id20, id31];
   apply(f30);
   yield check([{
     guid: PlacesUtils.bookmarks.menuGuid,
@@ -441,16 +474,19 @@ add_task(function* test_bookmark_order()
       children: [{
         guid: id41,
         index: 0,
       }, {
         guid: id42,
         index: 1,
       }]
     }],
+  }, {
+    guid: PlacesUtils.bookmarks.mobileGuid,
+    index: 4,
   }], "Moving 20 from f40 to f30 -> update 20, f30");
 
   _("Move 20 back to front -> update 20, f30");
   apply(bookmark(id20, ""));
   f30.children = [id10, id31];
   apply(f30);
   yield check([{
     guid: PlacesUtils.bookmarks.menuGuid,
@@ -480,11 +516,14 @@ add_task(function* test_bookmark_order()
       }, {
         guid: id42,
         index: 1,
       }],
     }, {
       guid: id20,
       index: 2,
     }],
+  }, {
+    guid: PlacesUtils.bookmarks.mobileGuid,
+    index: 4,
   }], "Move 20 back to front -> update 20, f30");
 
 });
--- a/services/sync/tests/unit/test_bookmark_tracker.js
+++ b/services/sync/tests/unit/test_bookmark_tracker.js
@@ -1267,29 +1267,26 @@ add_task(function* test_async_onItemDele
 });
 
 add_task(function* test_async_onItemDeleted_eraseEverything() {
   _("Erasing everything should track all deleted items");
 
   try {
     yield stopTracking();
 
-    let mobileRoot = BookmarkSpecialIds.findMobileRoot(true);
-    let mobileGUID = yield PlacesUtils.promiseItemGuid(mobileRoot);
-
     let fxBmk = yield PlacesUtils.bookmarks.insert({
       type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
-      parentGuid: mobileGUID,
+      parentGuid: PlacesUtils.bookmarks.mobileGuid,
       url: "http://getfirefox.com",
       title: "Get Firefox!",
     });
     _(`Firefox GUID: ${fxBmk.guid}`);
     let tbBmk = yield PlacesUtils.bookmarks.insert({
       type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
-      parentGuid: mobileGUID,
+      parentGuid: PlacesUtils.bookmarks.mobileGuid,
       url: "http://getthunderbird.com",
       title: "Get Thunderbird!",
     });
     _(`Thunderbird GUID: ${tbBmk.guid}`);
     let mozBmk = yield PlacesUtils.bookmarks.insert({
       type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
       parentGuid: PlacesUtils.bookmarks.menuGuid,
       url: "https://mozilla.org",
@@ -1335,40 +1332,39 @@ add_task(function* test_async_onItemDele
     yield PlacesUtils.bookmarks.eraseEverything();
 
     // `eraseEverything` removes all items from the database before notifying
     // observers. Because of this, grandchild lookup in the tracker's
     // `onItemRemoved` observer will fail. That means we won't track
     // (bzBmk.guid, bugsGrandChildBmk.guid, bugsChildFolder.guid), even
     // though we should.
     yield verifyTrackedItems(["menu", mozBmk.guid, mdnBmk.guid, "toolbar",
-                              bugsFolder.guid]);
-    do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE * 6);
+                              bugsFolder.guid, "mobile", fxBmk.guid,
+                              tbBmk.guid]);
+    do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE * 10);
   } finally {
     _("Clean up.");
     yield cleanup();
   }
 });
 
 add_task(function* test_onItemDeleted_removeFolderChildren() {
   _("Removing a folder's children should track the folder and its children");
 
   try {
-    let mobileRoot = BookmarkSpecialIds.findMobileRoot(true);
-    let mobileGUID = engine._store.GUIDForId(mobileRoot);
     let fx_id = PlacesUtils.bookmarks.insertBookmark(
-      mobileRoot,
+      PlacesUtils.mobileFolderId,
       Utils.makeURI("http://getfirefox.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Get Firefox!");
     let fx_guid = engine._store.GUIDForId(fx_id);
     _(`Firefox GUID: ${fx_guid}`);
 
     let tb_id = PlacesUtils.bookmarks.insertBookmark(
-      mobileRoot,
+      PlacesUtils.mobileFolderId,
       Utils.makeURI("http://getthunderbird.com"),
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Get Thunderbird!");
     let tb_guid = engine._store.GUIDForId(tb_id);
     _(`Thunderbird GUID: ${tb_guid}`);
 
     let moz_id = PlacesUtils.bookmarks.insertBookmark(
       PlacesUtils.bookmarks.bookmarksMenuFolder,
@@ -1376,18 +1372,18 @@ add_task(function* test_onItemDeleted_re
       PlacesUtils.bookmarks.DEFAULT_INDEX,
       "Mozilla"
     );
     let moz_guid = engine._store.GUIDForId(moz_id);
     _(`Mozilla GUID: ${moz_guid}`);
 
     yield startTracking();
 
-    _(`Mobile root ID: ${mobileRoot}`);
-    PlacesUtils.bookmarks.removeFolderChildren(mobileRoot);
+    _(`Mobile root ID: ${PlacesUtils.mobileFolderId}`);
+    PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.mobileFolderId);
 
     yield verifyTrackedItems(["mobile", fx_guid, tb_guid]);
     do_check_eq(tracker.score, SCORE_INCREMENT_XLARGE * 4);
   } finally {
     _("Clean up.");
     yield cleanup();
   }
 });