Bug 1433180 - Ensure the bookmark mirror handles a child which is a tombstone. r?kitcambridge draft
authorMark Hammond <mhammond@skippinet.com.au>
Thu, 15 Mar 2018 17:54:18 +1100
changeset 771295 1e732c36f49a5a5370d1a4f86fe7833d036e6038
parent 771261 8bf380faae74e4921be6000496ca09d4a2c44e8d
push id103651
push userbmo:markh@mozilla.com
push dateThu, 22 Mar 2018 21:50:36 +0000
reviewerskitcambridge
bugs1433180
milestone61.0a1
Bug 1433180 - Ensure the bookmark mirror handles a child which is a tombstone. r?kitcambridge MozReview-Commit-ID: AJJv9y8Py94
toolkit/components/places/tests/sync/head_sync.js
toolkit/components/places/tests/sync/test_bookmark_corruption.js
--- a/toolkit/components/places/tests/sync/head_sync.js
+++ b/toolkit/components/places/tests/sync/head_sync.js
@@ -1,12 +1,13 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 ChromeUtils.import("resource://gre/modules/Services.jsm");
+ChromeUtils.import("resource://gre/modules/CanonicalJSON.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);
 }
@@ -83,18 +84,18 @@ async function fetchLocalTree(rootGuid) 
   }
   let root = await PlacesUtils.promiseBookmarksTree(rootGuid);
   return bookmarkNodeToInfo(root);
 }
 
 async function assertLocalTree(rootGuid, expected, message) {
   let actual = await fetchLocalTree(rootGuid);
   if (!ObjectUtils.deepEqual(actual, expected)) {
-    info(`Expected structure for ${rootGuid}: ${JSON.stringify(expected)}`);
-    info(`Actual structure for ${rootGuid}: ${JSON.stringify(actual)}`);
+    info(`Expected structure for ${rootGuid}: ${CanonicalJSON.stringify(expected)}`);
+    info(`Actual structure for ${rootGuid}:   ${CanonicalJSON.stringify(actual)}`);
     throw new Assert.constructor.AssertionError({ actual, expected, message });
   }
 }
 
 function makeLivemarkServer() {
   let server = new HttpServer();
   server.registerPrefixHandler("/feed/", do_get_file("./livemark.xml"));
   server.start(-1);
--- a/toolkit/components/places/tests/sync/test_bookmark_corruption.js
+++ b/toolkit/components/places/tests/sync/test_bookmark_corruption.js
@@ -789,18 +789,105 @@ add_task(async function test_new_orphan_
   }, "Should update child positions once A exists in mirror");
 
   await buf.finalize();
   await PlacesUtils.bookmarks.eraseEverything();
   await PlacesSyncUtils.bookmarks.reset();
 });
 
 add_task(async function test_tombstone_as_child() {
-  // TODO (Bug 1433180): Add a folder that mentions a tombstone in its
-  // `children`.
+  await PlacesTestUtils.markBookmarksAsSynced();
+
+  let buf = await openMirror("tombstone_as_child");
+  // Setup the mirror such that an incoming folder references a tombstone
+  // as a child.
+  await buf.store(shuffle([{
+    id: "menu",
+    type: "folder",
+    children: ["folderAAAAAA"],
+  }, {
+    id: "folderAAAAAA",
+    type: "folder",
+    title: "A",
+    children: ["bookmarkAAAA", "bookmarkTTTT", "bookmarkBBBB"],
+  }, {
+    id: "bookmarkAAAA",
+    type: "bookmark",
+    title: "Bookmark A",
+    bmkUri: "http://example.com/a",
+  }, {
+    id: "bookmarkBBBB",
+    type: "bookmark",
+    title: "Bookmark B",
+    bmkUri: "http://example.com/b",
+  }, {
+    id: "bookmarkTTTT",
+    deleted: true,
+  }]), { needsMerge: true });
+
+  let changesToUpload = await buf.apply();
+  let idsToUpload = inspectChangeRecords(changesToUpload);
+  deepEqual(idsToUpload.deleted, [], "no new tombstones were created.");
+  // Note that we do not attempt to re-upload the folder with the correct
+  // list of children - but we might take some action in the future around
+  // this.
+  deepEqual(idsToUpload.updated, [], "parent is not re-uploaded");
+
+  await assertLocalTree(PlacesUtils.bookmarks.rootGuid, {
+    guid: PlacesUtils.bookmarks.rootGuid,
+    type: PlacesUtils.bookmarks.TYPE_FOLDER,
+    index: 0,
+    title: "",
+    children: [{
+      guid: PlacesUtils.bookmarks.menuGuid,
+      type: PlacesUtils.bookmarks.TYPE_FOLDER,
+      index: 0,
+      title: BookmarksMenuTitle,
+      children: [{
+        guid: "folderAAAAAA",
+        type: PlacesUtils.bookmarks.TYPE_FOLDER,
+        index: 0,
+        title: "A",
+        children: [{
+          guid: "bookmarkAAAA",
+          type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+          url: "http://example.com/a",
+          index: 0,
+          title: "Bookmark A",
+        }, {
+          // Note that this was the 3rd child specified on the server record,
+          // but we we've correctly moved it back to being the second after
+          // ignoring the tombstone.
+          guid: "bookmarkBBBB",
+          type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+          url: "http://example.com/b",
+          index: 1,
+          title: "Bookmark B",
+        }],
+      }],
+    }, {
+      guid: PlacesUtils.bookmarks.toolbarGuid,
+      type: PlacesUtils.bookmarks.TYPE_FOLDER,
+      index: 1,
+      title: BookmarksToolbarTitle,
+    }, {
+      guid: PlacesUtils.bookmarks.unfiledGuid,
+      type: PlacesUtils.bookmarks.TYPE_FOLDER,
+      index: 3,
+      title: UnfiledBookmarksTitle,
+    }, {
+      guid: PlacesUtils.bookmarks.mobileGuid,
+      type: PlacesUtils.bookmarks.TYPE_FOLDER,
+      index: 4,
+      title: MobileBookmarksTitle,
+    }],
+  }, "Should have ignored tombstone record");
+  await buf.finalize();
+  await PlacesUtils.bookmarks.eraseEverything();
+  await PlacesSyncUtils.bookmarks.reset();
 });
 
 // See what happens when a left-pane root and a left-pane query are on the server
 add_task(async function test_left_pane_root() {
   let buf = await openMirror("lpr");
 
   let initialTree = await fetchLocalTree(PlacesUtils.bookmarks.rootGuid);