Bug 1432425 - Remove synchronous Bookmarks::RemoveFolderChildren. r=standard8,kitcambridge draft
authorMarco Bonardo <mbonardo@mozilla.com>
Sun, 25 Feb 2018 21:44:41 +0100
changeset 760356 1ab8bd3a9935e25679044cbaac59e58ed9082e6e
parent 759647 1b2bbd46ee3e7c374b172553ebb9ed9da59def70
push id100608
push usermak77@bonardo.net
push dateTue, 27 Feb 2018 11:38:35 +0000
reviewersstandard8, kitcambridge
bugs1432425
milestone60.0a1
Bug 1432425 - Remove synchronous Bookmarks::RemoveFolderChildren. r=standard8,kitcambridge MozReview-Commit-ID: GPrNcmEIgpZ
browser/base/content/test/general/browser_star_hsts.js
browser/components/places/tests/browser/browser_copy_query_without_tree.js
browser/components/places/tests/browser/browser_library_batch_delete.js
services/sync/tests/unit/test_bookmark_tracker.js
toolkit/components/places/nsINavBookmarksService.idl
toolkit/components/places/nsNavBookmarks.cpp
toolkit/components/places/nsNavBookmarks.h
toolkit/components/places/tests/bookmarks/test_417228-other-roots.js
toolkit/components/places/tests/bookmarks/test_818587_compress-bookmarks-backups.js
toolkit/components/places/tests/legacy/test_418643_removeFolderChildren.js
toolkit/components/places/tests/legacy/test_bookmarks.js
toolkit/components/places/tests/legacy/test_protectRoots.js
toolkit/components/places/tests/legacy/xpcshell.ini
toolkit/components/places/tests/sync/head_sync.js
toolkit/components/places/tests/sync/sync_utils_bookmarks.html
toolkit/components/places/tests/sync/sync_utils_bookmarks.json
toolkit/components/places/tests/sync/test_sync_utils.js
toolkit/components/places/tests/sync/xpcshell.ini
toolkit/components/places/tests/unit/sync_utils_bookmarks.html
toolkit/components/places/tests/unit/sync_utils_bookmarks.json
toolkit/components/places/tests/unit/test_sync_utils.js
toolkit/components/places/tests/unit/xpcshell.ini
--- a/browser/base/content/test/general/browser_star_hsts.js
+++ b/browser/base/content/test/general/browser_star_hsts.js
@@ -2,23 +2,23 @@
  * http://creativecommons.org/publicdomain/zero/1.0/
  */
 /* eslint-disable mozilla/no-arbitrary-setTimeout */
 
 var secureURL = "https://example.com/browser/browser/base/content/test/general/browser_star_hsts.sjs";
 var unsecureURL = "http://example.com/browser/browser/base/content/test/general/browser_star_hsts.sjs";
 
 add_task(async function test_star_redirect() {
-  registerCleanupFunction(function() {
+  registerCleanupFunction(async () => {
     // Ensure to remove example.com from the HSTS list.
     let sss = Cc["@mozilla.org/ssservice;1"]
                 .getService(Ci.nsISiteSecurityService);
     sss.removeState(Ci.nsISiteSecurityService.HEADER_HSTS,
                     NetUtil.newURI("http://example.com/"), 0);
-    PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.unfiledBookmarksFolderId);
+    await PlacesUtils.bookmarks.eraseEverything();
     gBrowser.removeCurrentTab();
   });
 
   let tab = gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser);
   // This will add the page to the HSTS cache.
   await promiseTabLoadEvent(tab, secureURL, secureURL);
   // This should transparently be redirected to the secure page.
   await promiseTabLoadEvent(tab, unsecureURL, secureURL);
--- a/browser/components/places/tests/browser/browser_copy_query_without_tree.js
+++ b/browser/components/places/tests/browser/browser_copy_query_without_tree.js
@@ -7,19 +7,19 @@
 const SHORTCUT_URL = "place:folder=2";
 const QUERY_URL = "place:sort=8&maxResults=10";
 
 add_task(async function copy_toolbar_shortcut() {
   await promisePlacesInitComplete();
 
   let library = await promiseLibrary();
 
-  registerCleanupFunction(function() {
+  registerCleanupFunction(async () => {
     library.close();
-    PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.unfiledBookmarksFolderId);
+    await PlacesUtils.bookmarks.eraseEverything();
   });
 
   library.PlacesOrganizer.selectLeftPaneBuiltIn("BookmarksToolbar");
 
   await promiseClipboard(function() { library.PlacesOrganizer._places.controller.copy(); },
                          PlacesUtils.TYPE_X_MOZ_PLACE);
 
   library.PlacesOrganizer.selectLeftPaneBuiltIn("UnfiledBookmarks");
--- a/browser/components/places/tests/browser/browser_library_batch_delete.js
+++ b/browser/components/places/tests/browser/browser_library_batch_delete.js
@@ -7,19 +7,18 @@
 
 const TEST_URL = "http://www.batch.delete.me/";
 
 var gLibrary;
 
 add_task(async function test_setup() {
   gLibrary = await promiseLibrary();
 
-  registerCleanupFunction(function() {
-    PlacesUtils.bookmarks
-               .removeFolderChildren(PlacesUtils.unfiledBookmarksFolderId);
+  registerCleanupFunction(async () => {
+    await PlacesUtils.bookmarks.eraseEverything();
     // Close Library window.
     gLibrary.close();
   });
 });
 
 add_task(async function test_create_and_batch_remove_bookmarks() {
   let testURI = makeURI(TEST_URL);
   PlacesUtils.history.runInBatchMode({
--- a/services/sync/tests/unit/test_bookmark_tracker.js
+++ b/services/sync/tests/unit/test_bookmark_tracker.js
@@ -399,39 +399,42 @@ add_task(async function test_nested_batc
   await cleanup();
 });
 
 add_task(async function test_tracker_sql_batching() {
   _("Test tracker does the correct thing when it is forced to batch SQL queries");
 
   const SQLITE_MAX_VARIABLE_NUMBER = 999;
   let numItems = SQLITE_MAX_VARIABLE_NUMBER * 2 + 10;
-  let createdIDs = [];
 
   await startTracking();
 
-  PlacesUtils.bookmarks.runInBatchMode({
-    runBatched() {
-      for (let i = 0; i < numItems; i++) {
-        let syncBmkID = PlacesUtils.bookmarks.insertBookmark(
-                          PlacesUtils.bookmarks.unfiledBookmarksFolder,
-                          CommonUtils.makeURI("https://example.org/" + i),
-                          PlacesUtils.bookmarks.DEFAULT_INDEX,
-                          "Sync Bookmark " + i);
-        createdIDs.push(syncBmkID);
-      }
-    }
-  }, null);
+  let children = [];
+  for (let i = 0; i < numItems; i++) {
+    children.push({
+      url: "https://example.org/" + i,
+      title: "Sync Bookmark " + i
+    });
+  }
+  let inserted = await PlacesUtils.bookmarks.insertTree({
+    guid: PlacesUtils.bookmarks.unfiledGuid,
+    children: [{
+      type: PlacesUtils.bookmarks.TYPE_FOLDER,
+      children
+    }]
+  });
 
-  Assert.equal(createdIDs.length, numItems);
-  await verifyTrackedCount(numItems + 1); // the folder is also tracked.
+
+  Assert.equal(children.length, numItems);
+  Assert.equal(inserted.length, numItems + 1);
+  await verifyTrackedCount(numItems + 2); // The parent and grandparent are also tracked.
   await resetTracker();
 
-  PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.bookmarks.unfiledBookmarksFolder);
-  await verifyTrackedCount(numItems + 1);
+  await PlacesUtils.bookmarks.remove(inserted[0]);
+  await verifyTrackedCount(numItems + 2);
 
   await cleanup();
 });
 
 add_task(async function test_onItemAdded() {
   _("Items inserted via the synchronous bookmarks API should be tracked");
 
   try {
@@ -1404,58 +1407,16 @@ add_task(async function test_async_onIte
                               bugsGrandChildBmk.guid]);
     Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE * 8);
   } finally {
     _("Clean up.");
     await cleanup();
   }
 });
 
-add_task(async function test_onItemDeleted_removeFolderChildren() {
-  _("Removing a folder's children should track the folder and its children");
-
-  try {
-    let fx_id = PlacesUtils.bookmarks.insertBookmark(
-      PlacesUtils.mobileFolderId,
-      CommonUtils.makeURI("http://getfirefox.com"),
-      PlacesUtils.bookmarks.DEFAULT_INDEX,
-      "Get Firefox!");
-    let fx_guid = await PlacesUtils.promiseItemGuid(fx_id);
-    _(`Firefox GUID: ${fx_guid}`);
-
-    let tb_id = PlacesUtils.bookmarks.insertBookmark(
-      PlacesUtils.mobileFolderId,
-      CommonUtils.makeURI("http://getthunderbird.com"),
-      PlacesUtils.bookmarks.DEFAULT_INDEX,
-      "Get Thunderbird!");
-    let tb_guid = await PlacesUtils.promiseItemGuid(tb_id);
-    _(`Thunderbird GUID: ${tb_guid}`);
-
-    let moz_id = PlacesUtils.bookmarks.insertBookmark(
-      PlacesUtils.bookmarks.bookmarksMenuFolder,
-      CommonUtils.makeURI("https://mozilla.org"),
-      PlacesUtils.bookmarks.DEFAULT_INDEX,
-      "Mozilla"
-    );
-    let moz_guid = await PlacesUtils.promiseItemGuid(moz_id);
-    _(`Mozilla GUID: ${moz_guid}`);
-
-    await startTracking();
-
-    _(`Mobile root ID: ${PlacesUtils.mobileFolderId}`);
-    PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.mobileFolderId);
-
-    await verifyTrackedItems(["mobile", fx_guid, tb_guid]);
-    Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE * 2);
-  } finally {
-    _("Clean up.");
-    await cleanup();
-  }
-});
-
 add_task(async function test_onItemDeleted_tree() {
   _("Deleting a tree of bookmarks should track all items");
 
   try {
     // Create a couple of parent folders.
     let folder1_id = PlacesUtils.bookmarks.createFolder(
       PlacesUtils.bookmarks.bookmarksMenuFolder,
       "First test folder",
--- a/toolkit/components/places/nsINavBookmarksService.idl
+++ b/toolkit/components/places/nsINavBookmarksService.idl
@@ -397,28 +397,16 @@ interface nsINavBookmarksService : nsISu
    *  @throws if aGuid is malformed.
    */
   long long createFolder(in long long aParentFolder, in AUTF8String name,
                          in long index,
                          [optional] in ACString aGuid,
                          [optional] in unsigned short aSource);
 
   /**
-   * Convenience function for container services.  Removes
-   * all children of the given folder.
-   *  @param aItemId
-   *         The id of the folder to remove children from.
-   *  @param [optional] aSource
-   *         The change source, forwarded to all bookmark observers. Defaults
-   *         to SOURCE_DEFAULT.
-   */
-  void removeFolderChildren(in long long aItemId,
-                            [optional] in unsigned short aSource);
-
-  /**
    * Set the title for an item.
    *  @param aItemId
    *         The id of the item whose title should be updated.
    *  @param aTitle
    *         The new title for the bookmark.
    *  @param [optional] aSource
    *         The change source, forwarded to all bookmark observers. Defaults
    *         to SOURCE_DEFAULT.
--- a/toolkit/components/places/nsNavBookmarks.cpp
+++ b/toolkit/components/places/nsNavBookmarks.cpp
@@ -981,17 +981,17 @@ nsNavBookmarks::GetDescendantChildren(in
                             aFolderChildrenArray);
     }
   }
 
   return NS_OK;
 }
 
 
-NS_IMETHODIMP
+nsresult
 nsNavBookmarks::RemoveFolderChildren(int64_t aFolderId, uint16_t aSource)
 {
   AUTO_PROFILER_LABEL("nsNavBookmarks::RemoveFolderChilder", OTHER);
 
   NS_ENSURE_ARG_MIN(aFolderId, 1);
   int64_t rootId = -1;
   nsresult rv = GetPlacesRoot(&rootId);
   NS_ENSURE_SUCCESS(rv, rv);
--- a/toolkit/components/places/nsNavBookmarks.h
+++ b/toolkit/components/places/nsNavBookmarks.h
@@ -317,16 +317,18 @@ private:
            aFolderId == mToolbarRoot || aFolderId == mMobileRoot;
   }
 
   nsresult SetItemDateInternal(enum mozilla::places::BookmarkDate aDateType,
                                int64_t aSyncChangeDelta,
                                int64_t aItemId,
                                PRTime aValue);
 
+  nsresult RemoveFolderChildren(int64_t aFolderId, uint16_t aSource);
+
   // Recursive method to build an array of folder's children
   nsresult GetDescendantChildren(int64_t aFolderId,
                                  const nsACString& aFolderGuid,
                                  int64_t aGrandParentId,
                                  nsTArray<BookmarkData>& aFolderChildrenArray);
 
   enum ItemType {
     BOOKMARK = TYPE_BOOKMARK,
--- a/toolkit/components/places/tests/bookmarks/test_417228-other-roots.js
+++ b/toolkit/components/places/tests/bookmarks/test_417228-other-roots.js
@@ -18,17 +18,17 @@ var myTest = {
 this.push(myTest);
 
 */
 
 tests.push({
   // Initialise something to avoid undefined property warnings in validate.
   _litterTitle: "",
 
-  populate: function populate() {
+  async populate() {
     // check initial size
     var rootNode = PlacesUtils.getFolderContents(PlacesUtils.placesRootId,
                                                  false, false).root;
     Assert.equal(rootNode.childCount, 5);
 
     // create a test root
     this._folderTitle = "test folder";
     this._folderId =
@@ -38,26 +38,26 @@ tests.push({
     Assert.equal(rootNode.childCount, 6);
 
     // add a tag
     this._testURI = Services.io.newURI("http://test");
     this._tags = ["a", "b"];
     PlacesUtils.tagging.tagURI(this._testURI, this._tags);
 
     // add a child to each root, including our test root
+    await PlacesUtils.bookmarks.eraseEverything();
     this._roots = [PlacesUtils.bookmarksMenuFolderId, PlacesUtils.toolbarFolderId,
                    PlacesUtils.unfiledBookmarksFolderId, PlacesUtils.mobileFolderId,
                    this._folderId];
-    this._roots.forEach(function(aRootId) {
-      // clean slate
-      PlacesUtils.bookmarks.removeFolderChildren(aRootId);
-      // add a test bookmark
+
+    this._roots.forEach(aRootId => {
+          // add a test bookmark
       PlacesUtils.bookmarks.insertBookmark(aRootId, this._testURI,
                                            PlacesUtils.bookmarks.DEFAULT_INDEX, "test");
-    }, this);
+    });
 
     // add a folder to exclude from replacing during restore
     // this will still be present post-restore
     var excludedFolderId =
       PlacesUtils.bookmarks.createFolder(PlacesUtils.placesRootId,
                                          "excluded",
                                          PlacesUtils.bookmarks.DEFAULT_INDEX);
     Assert.equal(rootNode.childCount, 7);
@@ -122,21 +122,21 @@ tests.push({
   }
 });
 
 add_task(async function() {
   // make json file
   let jsonFile = OS.Path.join(OS.Constants.Path.profileDir, "bookmarks.json");
 
   // populate db
-  tests.forEach(function(aTest) {
-    aTest.populate();
+  for (let test of tests) {
+    await test.populate();
     // sanity
-    aTest.validate();
-  });
+    test.validate();
+  }
 
   await BookmarkJSONUtils.exportToFile(jsonFile);
 
   tests.forEach(function(aTest) {
     aTest.inbetween();
   });
 
   // restore json file
--- a/toolkit/components/places/tests/bookmarks/test_818587_compress-bookmarks-backups.js
+++ b/toolkit/components/places/tests/bookmarks/test_818587_compress-bookmarks-backups.js
@@ -42,13 +42,13 @@ add_task(async function compress_bookmar
   let recentBackup = await PlacesBackups.getMostRecentBackup();
   await PlacesUtils.bookmarks.remove(bm);
   await BookmarkJSONUtils.importFromFile(recentBackup, true);
   let root = PlacesUtils.getFolderContents(PlacesUtils.unfiledBookmarksFolderId).root;
   let node = root.getChild(0);
   Assert.equal(node.uri, url);
 
   root.containerOpen = false;
-  PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.unfiledBookmarksFolderId);
+  await PlacesUtils.bookmarks.eraseEverything();
 
   // Cleanup.
   await OS.File.remove(jsonFile);
 });
deleted file mode 100644
--- a/toolkit/components/places/tests/legacy/test_418643_removeFolderChildren.js
+++ /dev/null
@@ -1,177 +0,0 @@
-/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
-/* vim:set ts=2 sw=2 sts=2 et: */
-/* 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/. */
-
-// Get services.
-try {
-  var histSvc = Cc["@mozilla.org/browser/nav-history-service;1"].
-                getService(Ci.nsINavHistoryService);
-  var annoSvc = Cc["@mozilla.org/browser/annotation-service;1"]
-                  .getService(Ci.nsIAnnotationService);
-} catch (ex) {
-  do_throw("Could not get services\n");
-}
-
-var validAnnoName = "validAnno";
-var validItemName = "validItem";
-var deletedAnnoName = "deletedAnno";
-var deletedItemName = "deletedItem";
-var bookmarkedURI = "http://www.mozilla.org/";
-// set lastModified to the past to prevent VM timing bugs
-var pastDate = new Date(Date.now() - 1000);
-var deletedBookmarkGuids = [];
-
-// bookmarks observer
-var observer = {
-  // cached ordered array of notified items
-  _onItemRemovedItemGuids: [],
-  onItemRemoved(itemId, parentId, index, type, uri, guid, parentGuid) {
-    // We should first get notifications for children, then for their parent
-    Assert.equal(this._onItemRemovedItemGuids.indexOf(parentGuid), -1,
-      "should first get notifications for children, then their parents.");
-    // Ensure we are not wrongly removing 1 level up
-    Assert.notEqual(parentGuid, PlacesUtils.bookmarks.toolbarGuid,
-      "should not be removing from the level above");
-    // Removed item must be one of those we have manually deleted
-    Assert.notEqual(deletedBookmarkGuids.indexOf(guid), -1,
-      "should be one of the deleted items.");
-    this._onItemRemovedItemGuids.push(itemId);
-  },
-
-  QueryInterface: XPCOMUtils.generateQI([Ci.nsINavBookmarkObserver])
-};
-
-PlacesUtils.bookmarks.addObserver(observer);
-
-async function add_bookmarks() {
-  // This is the folder we will cleanup
-  let validFolder = await PlacesUtils.bookmarks.insert({
-    parentGuid: PlacesUtils.bookmarks.toolbarGuid,
-    title: validItemName,
-    type: PlacesUtils.bookmarks.TYPE_FOLDER
-  });
-
-  let validFolderId = await PlacesUtils.promiseItemId(validFolder.guid);
-
-  annoSvc.setItemAnnotation(validFolderId, validAnnoName,
-                            "annotation", 0,
-                            annoSvc.EXPIRE_NEVER);
-  await PlacesUtils.bookmarks.update({
-    guid: validFolder.guid,
-    dateAdded: pastDate,
-    lastModified: pastDate,
-  });
-
-  // This bookmark should not be deleted
-  let bookmark = await PlacesUtils.bookmarks.insert({
-    parentGuid: PlacesUtils.bookmarks.toolbarGuid,
-    title: validItemName,
-    url: bookmarkedURI,
-  });
-
-  let validItemId = await PlacesUtils.promiseItemId(bookmark.guid);
-  annoSvc.setItemAnnotation(validItemId, validAnnoName,
-                            "annotation", 0, annoSvc.EXPIRE_NEVER);
-
-  // The following contents should be deleted
-  bookmark = await PlacesUtils.bookmarks.insert({
-    parentGuid: validFolder.guid,
-    title: deletedItemName,
-    url: bookmarkedURI,
-  });
-
-  let deletedItemId = await PlacesUtils.promiseItemId(bookmark.guid);
-
-  annoSvc.setItemAnnotation(deletedItemId, deletedAnnoName,
-                            "annotation", 0, annoSvc.EXPIRE_NEVER);
-  deletedBookmarkGuids.push(bookmark.guid);
-
-  let internalFolder = await PlacesUtils.bookmarks.insert({
-    parentGuid: validFolder.guid,
-    title: deletedItemName,
-    type: PlacesUtils.bookmarks.TYPE_FOLDER,
-  });
-
-  let internalFolderId = await PlacesUtils.promiseItemId(internalFolder.guid);
-
-  annoSvc.setItemAnnotation(internalFolderId, deletedAnnoName,
-                            "annotation", 0, annoSvc.EXPIRE_NEVER);
-  deletedBookmarkGuids.push(internalFolder.guid);
-
-  bookmark = await PlacesUtils.bookmarks.insert({
-    parentGuid: internalFolder.guid,
-    title: deletedItemName,
-    url: bookmarkedURI,
-  });
-
-  deletedItemId = await PlacesUtils.promiseItemId(bookmark.guid);
-
-  annoSvc.setItemAnnotation(deletedItemId, deletedAnnoName,
-                            "annotation", 0, annoSvc.EXPIRE_NEVER);
-  deletedBookmarkGuids.push(bookmark.guid);
-
-  return [validFolder.guid, validFolderId];
-}
-
-async function check_bookmarks(folderGuid) {
-  // check that we still have valid bookmarks
-  let bookmarks = [];
-  await PlacesUtils.bookmarks.fetch({url: bookmarkedURI}, bookmark => bookmarks.push(bookmark));
-
-  Assert.equal(bookmarks.length, 1, "Should have the correct amount of bookmarks.");
-
-  Assert.equal(bookmarks[0].title, validItemName);
-  let id = await PlacesUtils.promiseItemId(bookmarks[0].guid);
-  Assert.ok(annoSvc.itemHasAnnotation(id, validAnnoName));
-
-  // check that folder exists and has still its annotation
-  let folder = await PlacesUtils.bookmarks.fetch(folderGuid);
-  Assert.equal(folder.title, validItemName, "should have the correct name");
-  let folderId = await PlacesUtils.promiseItemId(folderGuid);
-  Assert.ok(annoSvc.itemHasAnnotation(folderId, validAnnoName),
-    "should still have an annotation");
-
-  // test that lastModified got updated
-  Assert.ok(pastDate < folder.lastModified, "lastModified date should have been updated");
-
-  // check that folder is empty
-  var options = histSvc.getNewQueryOptions();
-  var query = histSvc.getNewQuery();
-  query.setFolders([folderId], 1);
-  var result = histSvc.executeQuery(query, options);
-  var root = result.root;
-  root.containerOpen = true;
-  Assert.equal(root.childCount, 0);
-  root.containerOpen = false;
-
-  // test that all children have been deleted, we use annos for that
-  var deletedItems = annoSvc.getItemsWithAnnotation(deletedAnnoName);
-  Assert.equal(deletedItems.length, 0);
-
-  // test that observer has been called for (and only for) deleted items
-  Assert.equal(observer._onItemRemovedItemGuids.length, deletedBookmarkGuids.length);
-
-  // Sanity check: all roots should be intact.
-  folder = await PlacesUtils.bookmarks.fetch(PlacesUtils.bookmarks.rootGuid);
-  Assert.equal(folder.parentGuid, undefined, "root shouldn't have a parent.");
-  folder = await PlacesUtils.bookmarks.fetch(PlacesUtils.bookmarks.menuGuid);
-  Assert.equal(folder.parentGuid, PlacesUtils.bookmarks.rootGuid,
-    "bookmarks menu should still be a child of root.");
-  folder = await PlacesUtils.bookmarks.fetch(PlacesUtils.bookmarks.tagsGuid);
-  Assert.equal(folder.parentGuid, PlacesUtils.bookmarks.rootGuid,
-    "tags folder should still be a child of root.");
-  folder = await PlacesUtils.bookmarks.fetch(PlacesUtils.bookmarks.unfiledGuid);
-  Assert.equal(folder.parentGuid, PlacesUtils.bookmarks.rootGuid,
-    "unfiled folder should still be a child of root.");
-  folder = await PlacesUtils.bookmarks.fetch(PlacesUtils.bookmarks.toolbarGuid);
-  Assert.equal(folder.parentGuid, PlacesUtils.bookmarks.rootGuid,
-    "toolbar folder should still be a child of root.");
-}
-
-add_task(async function test_removing_folder_children() {
-  let [folderGuid, folderId] = await add_bookmarks();
-  PlacesUtils.bookmarks.removeFolderChildren(folderId);
-  await check_bookmarks(folderGuid);
-});
--- a/toolkit/components/places/tests/legacy/test_bookmarks.js
+++ b/toolkit/components/places/tests/legacy/test_bookmarks.js
@@ -241,57 +241,20 @@ add_task(async function test_bookmarks()
   let newId6 = bs.insertBookmark(testRoot, uri6, bs.DEFAULT_INDEX, "");
   Assert.equal(bookmarksObserver._itemAddedParent, testRoot);
   Assert.equal(bookmarksObserver._itemAddedIndex, 3);
 
   // change item
   bs.setItemTitle(newId6, "Google Sites");
   Assert.equal(bookmarksObserver._itemChangedProperty, "title");
 
-  // test get folder's index
-  let tmpFolder = bs.createFolder(testRoot, "tmp", 2);
-
-  // test removeFolderChildren
-  // 1) add/remove each child type (bookmark, folder)
-  tmpFolder = bs.createFolder(testRoot, "removeFolderChildren",
-                              bs.DEFAULT_INDEX);
-  bs.insertBookmark(tmpFolder, uri("http://foo9.com/"), bs.DEFAULT_INDEX, "");
-  bs.createFolder(tmpFolder, "subfolder", bs.DEFAULT_INDEX);
-  // 2) confirm that folder has 2 children
-  let options = hs.getNewQueryOptions();
-  let query = hs.getNewQuery();
-  query.setFolders([tmpFolder], 1);
-  try {
-    let result = hs.executeQuery(query, options);
-    let rootNode = result.root;
-    rootNode.containerOpen = true;
-    Assert.equal(rootNode.childCount, 2);
-    rootNode.containerOpen = false;
-  } catch (ex) {
-    do_throw("test removeFolderChildren() - querying for children failed: " + ex);
-  }
-  // 3) remove all children
-  bs.removeFolderChildren(tmpFolder);
-  // 4) confirm that folder has 0 children
-  try {
-    let result = hs.executeQuery(query, options);
-    let rootNode = result.root;
-    rootNode.containerOpen = true;
-    Assert.equal(rootNode.childCount, 0);
-    rootNode.containerOpen = false;
-  } catch (ex) {
-    do_throw("removeFolderChildren(): " + ex);
-  }
-
-  // XXX - test folderReadOnly
-
   // test bookmark id in query output
   try {
-    options = hs.getNewQueryOptions();
-    query = hs.getNewQuery();
+    let options = hs.getNewQueryOptions();
+    let query = hs.getNewQuery();
     query.setFolders([testRoot], 1);
     let result = hs.executeQuery(query, options);
     let rootNode = result.root;
     rootNode.containerOpen = true;
     let cc = rootNode.childCount;
     info("bookmark itemId test: CC = " + cc);
     Assert.ok(cc > 0);
     for (let i = 0; i < cc; ++i) {
@@ -317,18 +280,18 @@ add_task(async function test_bookmarks()
     let mURI = uri("http://multiple.uris.in.query");
 
     let testFolder = bs.createFolder(testRoot, "test Folder", bs.DEFAULT_INDEX);
     // add 2 bookmarks
     bs.insertBookmark(testFolder, mURI, bs.DEFAULT_INDEX, "title 1");
     bs.insertBookmark(testFolder, mURI, bs.DEFAULT_INDEX, "title 2");
 
     // query
-    options = hs.getNewQueryOptions();
-    query = hs.getNewQuery();
+    let options = hs.getNewQueryOptions();
+    let query = hs.getNewQuery();
     query.setFolders([testFolder], 1);
     let result = hs.executeQuery(query, options);
     let rootNode = result.root;
     rootNode.containerOpen = true;
     let cc = rootNode.childCount;
     Assert.equal(cc, 2);
     Assert.equal(rootNode.getChild(0).title, "title 1");
     Assert.equal(rootNode.getChild(1).title, "title 2");
@@ -361,17 +324,17 @@ add_task(async function test_bookmarks()
   // insert a bookmark with title ZZZXXXYYY and then search for it.
   // this test confirms that we can find bookmarks that we haven't visited
   // (which are "hidden") and that we can find by title.
   // see bug #369887 for more details
   let newId13 = bs.insertBookmark(testRoot, uri("http://foobarcheese.com/"),
                                   bs.DEFAULT_INDEX, "");
   Assert.equal(bookmarksObserver._itemAddedId, newId13);
   Assert.equal(bookmarksObserver._itemAddedParent, testRoot);
-  Assert.equal(bookmarksObserver._itemAddedIndex, 9);
+  Assert.equal(bookmarksObserver._itemAddedIndex, 7);
 
   // set bookmark title
   bs.setItemTitle(newId13, "ZZZXXXYYY");
   Assert.equal(bookmarksObserver._itemChangedId, newId13);
   Assert.equal(bookmarksObserver._itemChangedProperty, "title");
   Assert.equal(bookmarksObserver._itemChangedValue, "ZZZXXXYYY");
 
   // check if setting an item annotation triggers onItemChanged
@@ -379,20 +342,20 @@ add_task(async function test_bookmarks()
   anno.setItemAnnotation(newId3, "test-annotation", "foo", 0, 0);
   Assert.equal(bookmarksObserver._itemChangedId, newId3);
   Assert.equal(bookmarksObserver._itemChangedProperty, "test-annotation");
   Assert.ok(bookmarksObserver._itemChanged_isAnnotationProperty);
   Assert.equal(bookmarksObserver._itemChangedValue, "");
 
   // test search on bookmark title ZZZXXXYYY
   try {
-    options = hs.getNewQueryOptions();
+    let options = hs.getNewQueryOptions();
     options.excludeQueries = 1;
     options.queryType = Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS;
-    query = hs.getNewQuery();
+    let query = hs.getNewQuery();
     query.searchTerms = "ZZZXXXYYY";
     let result = hs.executeQuery(query, options);
     let rootNode = result.root;
     rootNode.containerOpen = true;
     let cc = rootNode.childCount;
     Assert.equal(cc, 1);
     let node = rootNode.getChild(0);
     Assert.equal(node.title, "ZZZXXXYYY");
@@ -400,20 +363,20 @@ add_task(async function test_bookmarks()
     rootNode.containerOpen = false;
   } catch (ex) {
     do_throw("bookmarks query: " + ex);
   }
 
   // test dateAdded and lastModified properties
   // for a search query
   try {
-    options = hs.getNewQueryOptions();
+    let options = hs.getNewQueryOptions();
     options.excludeQueries = 1;
     options.queryType = Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS;
-    query = hs.getNewQuery();
+    let query = hs.getNewQuery();
     query.searchTerms = "ZZZXXXYYY";
     let result = hs.executeQuery(query, options);
     let rootNode = result.root;
     rootNode.containerOpen = true;
     let cc = rootNode.childCount;
     Assert.equal(cc, 1);
     let node = rootNode.getChild(0);
 
@@ -426,18 +389,18 @@ add_task(async function test_bookmarks()
     rootNode.containerOpen = false;
   } catch (ex) {
     do_throw("bookmarks query: " + ex);
   }
 
   // test dateAdded and lastModified properties
   // for a folder query
   try {
-    options = hs.getNewQueryOptions();
-    query = hs.getNewQuery();
+    let options = hs.getNewQueryOptions();
+    let query = hs.getNewQuery();
     query.setFolders([testRoot], 1);
     let result = hs.executeQuery(query, options);
     let rootNode = result.root;
     rootNode.containerOpen = true;
     let cc = rootNode.childCount;
     Assert.ok(cc > 0);
     for (let i = 0; i < cc; i++) {
       let node = rootNode.getChild(i);
--- a/toolkit/components/places/tests/legacy/test_protectRoots.js
+++ b/toolkit/components/places/tests/legacy/test_protectRoots.js
@@ -13,19 +13,10 @@ function run_test() {
 
   for (let root of ROOTS) {
     Assert.ok(PlacesUtils.isRootItem(root));
 
     try {
       PlacesUtils.bookmarks.removeItem(root);
       do_throw("Trying to remove a root should throw");
     } catch (ex) {}
-
-    try {
-      PlacesUtils.bookmarks.removeFolderChildren(root);
-      if (root == PlacesUtils.placesRootId)
-        do_throw("Trying to remove children of the main root should throw");
-    } catch (ex) {
-      if (root != PlacesUtils.placesRootId)
-        do_throw("Trying to remove children of other roots should not throw");
-    }
   }
 }
--- a/toolkit/components/places/tests/legacy/xpcshell.ini
+++ b/toolkit/components/places/tests/legacy/xpcshell.ini
@@ -1,11 +1,10 @@
 # This directory is for tests for the legacy, sync APIs as somewhere to put them
 # until we remove the APIs themselves.
 
 [DEFAULT]
 head = head_legacy.js
 firefox-appdir = browser
 
-[test_418643_removeFolderChildren.js]
 [test_bookmarks.js]
 [test_bookmarks_setNullTitle.js]
 [test_protectRoots.js]
--- a/toolkit/components/places/tests/sync/head_sync.js
+++ b/toolkit/components/places/tests/sync/head_sync.js
@@ -1,18 +1,27 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+ChromeUtils.import("resource://gre/modules/Services.jsm");
+
+// Import common head.
+{
+  /* import-globals-from ../head_common.js */
+  let commonFile = do_get_file("../head_common.js", false);
+  let uri = Services.io.newFileURI(commonFile);
+  Services.scriptloader.loadSubScript(uri.spec, this);
+}
+
+// Put any other stuff relative to this test folder below.
+
 ChromeUtils.import("resource://gre/modules/Log.jsm");
 ChromeUtils.import("resource://gre/modules/ObjectUtils.jsm");
-ChromeUtils.import("resource://gre/modules/osfile.jsm");
-ChromeUtils.import("resource://gre/modules/PlacesUtils.jsm");
 ChromeUtils.import("resource://gre/modules/PlacesSyncUtils.jsm");
-ChromeUtils.import("resource://gre/modules/Services.jsm");
 ChromeUtils.import("resource://gre/modules/SyncedBookmarksMirror.jsm");
-ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
-
-ChromeUtils.import("resource://testing-common/PlacesTestUtils.jsm");
 ChromeUtils.import("resource://testing-common/httpd.js");
 
 // These titles are defined in Database::CreateBookmarkRoots
 const BookmarksMenuTitle = "menu";
 const BookmarksToolbarTitle = "toolbar";
 const UnfiledBookmarksTitle = "unfiled";
 const MobileBookmarksTitle = "mobile";
 
rename from toolkit/components/places/tests/unit/sync_utils_bookmarks.html
rename to toolkit/components/places/tests/sync/sync_utils_bookmarks.html
rename from toolkit/components/places/tests/unit/sync_utils_bookmarks.json
rename to toolkit/components/places/tests/sync/sync_utils_bookmarks.json
rename from toolkit/components/places/tests/unit/test_sync_utils.js
rename to toolkit/components/places/tests/sync/test_sync_utils.js
--- a/toolkit/components/places/tests/unit/test_sync_utils.js
+++ b/toolkit/components/places/tests/sync/test_sync_utils.js
@@ -731,29 +731,16 @@ add_task(async function test_pullChanges
     deepEqual(Object.keys(changes).sort(),
       [untaggedItem.recordId].sort(),
       "Should include tagged bookmarks after changing tag entry URL");
     assertTagForURLs("tricky", ["https://example.com/", "https://mozilla.org/"],
       "Should remove tag entry for old URL");
     await setChangesSynced(changes);
   }
 
-  info("Remove all tag folders");
-  {
-    deepEqual(PlacesUtils.tagging.allTags, ["tricky"], "Should have existing tags");
-
-    PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.tagsFolderId);
-    let changes = await PlacesSyncUtils.bookmarks.pullChanges();
-    deepEqual(Object.keys(changes).sort(), [taggedItem.recordId].sort(),
-      "Should include tagged bookmarks after removing all tags");
-
-    deepEqual(PlacesUtils.tagging.allTags, [], "Should remove all tags from tag service");
-    await setChangesSynced(changes);
-  }
-
   await PlacesUtils.bookmarks.eraseEverything();
   await PlacesSyncUtils.bookmarks.reset();
 });
 
 add_task(async function test_update_keyword() {
   info("Insert item without keyword");
   let item = await PlacesSyncUtils.bookmarks.insert({
     kind: "bookmark",
@@ -2274,24 +2261,16 @@ add_task(async function test_pullChanges
       parentGuid: unsyncedGuids.rootFolder,
       url: "https://example.club",
     });
     let changes = await PlacesSyncUtils.bookmarks.pullChanges();
     deepEqual(changes, {}, `Pulling changes should ignore unsynced sibling ${
       unsyncedSibling.guid}`);
   }
 
-  info("Clear custom root using old API");
-  {
-    let unsyncedRootId = await PlacesUtils.promiseItemId(unsyncedGuids.rootFolder);
-    PlacesUtils.bookmarks.removeFolderChildren(unsyncedRootId);
-    let changes = await PlacesSyncUtils.bookmarks.pullChanges();
-    deepEqual(changes, {}, "Clearing custom root should not write tombstones for children");
-  }
-
   info("Remove custom root");
   {
     await PlacesUtils.bookmarks.remove(unsyncedGuids.rootFolder);
     let changes = await PlacesSyncUtils.bookmarks.pullChanges();
     deepEqual(changes, {}, "Removing custom root should not write tombstone");
   }
 
   info("Append sibling to menu");
--- a/toolkit/components/places/tests/sync/xpcshell.ini
+++ b/toolkit/components/places/tests/sync/xpcshell.ini
@@ -1,12 +1,15 @@
 [DEFAULT]
 head = head_sync.js
 support-files =
   livemark.xml
+  sync_utils_bookmarks.html
+  sync_utils_bookmarks.json
 
 [test_bookmark_corruption.js]
 [test_bookmark_deduping.js]
 [test_bookmark_deletion.js]
 [test_bookmark_haschanges.js]
 [test_bookmark_kinds.js]
 [test_bookmark_structure_changes.js]
 [test_bookmark_value_changes.js]
+[test_sync_utils.js]
--- a/toolkit/components/places/tests/unit/xpcshell.ini
+++ b/toolkit/components/places/tests/unit/xpcshell.ini
@@ -10,18 +10,16 @@ support-files =
   corruptDB.sqlite
   livemark.xml
   mobile_bookmarks_folder_import.json
   mobile_bookmarks_folder_merge.json
   mobile_bookmarks_multiple_folders.json
   mobile_bookmarks_root_import.json
   mobile_bookmarks_root_merge.json
   places.sparse.sqlite
-  sync_utils_bookmarks.html
-  sync_utils_bookmarks.json
 
 [test_000_frecency.js]
 [test_317472.js]
 [test_331487.js]
 [test_384370.js]
 skip-if = (os == 'win' && ccov) # Bug 1423667
 [test_385397.js]
 [test_399264_query_to_string.js]
@@ -108,17 +106,16 @@ skip-if = true
 [test_preventive_maintenance.js]
 [test_preventive_maintenance_checkAndFixDatabase.js]
 [test_preventive_maintenance_runTasks.js]
 [test_promiseBookmarksTree.js]
 [test_resolveNullBookmarkTitles.js]
 [test_result_sort.js]
 [test_resultsAsVisit_details.js]
 [test_sql_guid_functions.js]
-[test_sync_utils.js]
 [test_tag_autocomplete_search.js]
 [test_tagging.js]
 [test_telemetry.js]
 [test_update_frecency_after_delete.js]
 [test_utils_backups_create.js]
 [test_utils_getURLsForContainerNode.js]
 [test_utils_setAnnotationsForItem.js]
 [test_visitsInDB.js]