Bug 1546035 - Remove local and remote livemarks when syncing. r=mak,tcsc
authorLina Cambridge <lina@yakshaving.ninja>
Thu, 02 May 2019 08:03:36 +0000
changeset 472331 76e8b769a7c00797dcaeef19cbaaa564f1f373b6
parent 472330 326391a0e32a33dbf1e459e0684423df860313d4
child 472332 8e41578fce723e0042214478198c65a2daf18e3e
push id84605
push userkcambridge@mozilla.com
push dateThu, 02 May 2019 17:59:36 +0000
treeherderautoland@76e8b769a7c0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak, tcsc
bugs1546035
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1546035 - Remove local and remote livemarks when syncing. r=mak,tcsc This commit exports livemarks before syncing for the first time, to avoid losing livemarks that Sync may have resurrected. As of v0.2.4, Dogear treats livemarks as non-syncable, and deletes them on both sides. This also bumps the mirror schema version, to trigger a first sync. Differential Revision: https://phabricator.services.mozilla.com/D28543
Cargo.lock
services/sync/modules/engines/bookmarks.js
third_party/rust/dogear/.cargo-checksum.json
third_party/rust/dogear/Cargo.toml
third_party/rust/dogear/src/driver.rs
third_party/rust/dogear/src/error.rs
third_party/rust/dogear/src/guid.rs
third_party/rust/dogear/src/lib.rs
third_party/rust/dogear/src/merge.rs
third_party/rust/dogear/src/tests.rs
third_party/rust/dogear/src/tree.rs
toolkit/components/places/SyncedBookmarksMirror.jsm
toolkit/components/places/bookmark_sync/Cargo.toml
toolkit/components/places/tests/bookmarks/test_bookmarks_eraseEverything.js
toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js
toolkit/components/places/tests/bookmarks/test_sync_fields.js
toolkit/components/places/tests/head_common.js
toolkit/components/places/tests/sync/test_bookmark_kinds.js
toolkit/components/places/tests/unit/test_telemetry.js
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -357,17 +357,17 @@ dependencies = [
 ]
 
 [[package]]
 name = "bookmark_sync"
 version = "0.1.0"
 dependencies = [
  "atomic_refcell 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "cstr 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
- "dogear 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)",
+ "dogear 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "libc 0.2.51 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "moz_task 0.1.0",
  "nserror 0.1.0",
  "nsstring 0.1.0",
  "storage 0.1.0",
  "storage_variant 0.1.0",
  "thin-vec 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -914,17 +914,17 @@ dependencies = [
  "regex 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "serde 1.0.88 (registry+https://github.com/rust-lang/crates.io-index)",
  "serde_derive 1.0.88 (git+https://github.com/servo/serde?branch=deserialize_from_enums10)",
  "strsim 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
 name = "dogear"
-version = "0.2.3"
+version = "0.2.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "smallbitvec 2.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
 name = "dtoa"
@@ -3672,17 +3672,17 @@ dependencies = [
 "checksum darling_macro 0.8.6 (registry+https://github.com/rust-lang/crates.io-index)" = "244e8987bd4e174385240cde20a3657f607fb0797563c28255c353b5819a07b1"
 "checksum deflate 0.7.19 (registry+https://github.com/rust-lang/crates.io-index)" = "8a6abb26e16e8d419b5c78662aa9f82857c2386a073da266840e474d5055ec86"
 "checksum derive_more 0.13.0 (registry+https://github.com/rust-lang/crates.io-index)" = "3f57d78cf3bd45270dad4e70c21ec77a960b36c7a841ff9db76aaa775a8fb871"
 "checksum devd-rs 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "e7c9ac481c38baf400d3b732e4a06850dfaa491d1b6379a249d9d40d14c2434c"
 "checksum diff 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)" = "3c2b69f912779fbb121ceb775d74d51e915af17aaebc38d28a592843a2dd0a3a"
 "checksum digest 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "05f47366984d3ad862010e22c7ce81a7dbcaebbdfb37241a620f8b6596ee135c"
 "checksum dirs 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)" = "88972de891f6118092b643d85a0b28e0678e0f948d7f879aa32f2d5aafe97d2a"
 "checksum docopt 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "db2906c2579b5b7207fc1e328796a9a8835dc44e22dbe8e460b1d636f9a7b225"
-"checksum dogear 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)" = "6d54506b6b209740d0a7a35ca5976db1ad2ed1aa168acc3561efc6a84fa95afe"
+"checksum dogear 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)" = "30ac4a8e8f834f02deb2266b1f279aa5494e990c625d8be8f2988a7c708ba1f8"
 "checksum dtoa 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)" = "09c3753c3db574d215cba4ea76018483895d7bff25a31b49ba45db21c48e50ab"
 "checksum dtoa-short 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "068d4026697c1a18f0b0bb8cfcad1b0c151b90d8edb9bf4c235ad68128920d1d"
 "checksum dwrote 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "c31c624339dab99c223a4b26c2e803b7c248adaca91549ce654c76f39a03f5c8"
 "checksum either 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "18785c1ba806c258137c937e44ada9ee7e69a37e3c72077542cd2f069d78562a"
 "checksum ena 0.10.1 (registry+https://github.com/rust-lang/crates.io-index)" = "25b4e5febb25f08c49f1b07dc33a182729a6b21edfb562b5aef95f78e0dbe5bb"
 "checksum encoding_c 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)" = "769ecb8b33323998e482b218c0d13cd64c267609023b4b7ec3ee740714c318ee"
 "checksum encoding_rs 0.8.16 (registry+https://github.com/rust-lang/crates.io-index)" = "0535f350c60aac0b87ccf28319abc749391e912192255b0c00a2c12c6917bd73"
 "checksum env_logger 0.5.6 (registry+https://github.com/rust-lang/crates.io-index)" = "0561146661ae44c579e993456bc76d11ce1e0c7d745e57b2fa7146b6e49fa2ad"
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -11,24 +11,25 @@ const {XPCOMUtils} = ChromeUtils.import(
 const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
 const {Async} = ChromeUtils.import("resource://services-common/async.js");
 const {SCORE_INCREMENT_XLARGE} = ChromeUtils.import("resource://services-sync/constants.js");
 const {Changeset, Store, SyncEngine, Tracker} = ChromeUtils.import("resource://services-sync/engines.js");
 const {CryptoWrapper} = ChromeUtils.import("resource://services-sync/record.js");
 const {Svc, Utils} = ChromeUtils.import("resource://services-sync/util.js");
 
 XPCOMUtils.defineLazyModuleGetters(this, {
-  SyncedBookmarksMirror: "resource://gre/modules/SyncedBookmarksMirror.jsm",
   BookmarkValidator: "resource://services-sync/bookmark_validator.js",
+  LiveBookmarkMigrator: "resource:///modules/LiveBookmarkMigrator.jsm",
   OS: "resource://gre/modules/osfile.jsm",
   PlacesBackups: "resource://gre/modules/PlacesBackups.jsm",
   PlacesDBUtils: "resource://gre/modules/PlacesDBUtils.jsm",
   PlacesSyncUtils: "resource://gre/modules/PlacesSyncUtils.jsm",
   PlacesUtils: "resource://gre/modules/PlacesUtils.jsm",
   Resource: "resource://services-sync/resource.js",
+  SyncedBookmarksMirror: "resource://gre/modules/SyncedBookmarksMirror.jsm",
 });
 
 XPCOMUtils.defineLazyGetter(this, "PlacesBundle", () => {
   return Services.strings.createBundle("chrome://places/locale/places.properties");
 });
 
 XPCOMUtils.defineLazyGetter(this, "ANNOS_TO_TRACK", () => [
   PlacesUtils.LMANNO_FEEDURI,
@@ -358,16 +359,41 @@ BaseBookmarksEngine.prototype = {
 
   async resetLocalSyncID() {
     let newSyncID = await PlacesSyncUtils.bookmarks.resetSyncId();
     this._log.debug("Assigned new sync ID ${newSyncID}", { newSyncID });
     await super.ensureCurrentSyncID(newSyncID); // Remove in bug 1443021.
     return newSyncID;
   },
 
+  async _syncStartup() {
+    await super._syncStartup();
+
+    try {
+      // For first syncs, back up the user's bookmarks and livemarks. Livemarks
+      // are unsupported as of bug 1477671, and syncing deletes them locally and
+      // remotely.
+      let lastSync = await this.getLastSync();
+      if (!lastSync) {
+        this._log.debug("Bookmarks backup starting");
+        await PlacesBackups.create(null, true);
+        this._log.debug("Bookmarks backup done");
+
+        this._log.debug("Livemarks backup starting");
+        await LiveBookmarkMigrator.migrate();
+        this._log.debug("Livemarks backup done");
+      }
+    } catch (ex) {
+      // Failure to create a backup is somewhat bad, but probably not bad
+      // enough to prevent syncing of bookmarks - so just log the error and
+      // continue.
+      this._log.warn("Error while backing up bookmarks, but continuing with sync", ex);
+    }
+  },
+
   async _sync() {
     try {
       await super._sync();
       if (this._ranMaintenanceOnLastSync) {
         // If the last sync failed, we ran maintenance, and this sync succeeded,
         // maintenance likely fixed the issue.
         this._ranMaintenanceOnLastSync = false;
         this.service.recordTelemetryEvent("maintenance", "fix",
@@ -582,33 +608,17 @@ BookmarksEngine.prototype = {
       return dupe;
     }
 
     this._log.trace("No dupe found for key " + key + ".");
     return undefined;
   },
 
   async _syncStartup() {
-    await SyncEngine.prototype._syncStartup.call(this);
-
-    try {
-      // For first-syncs, make a backup for the user to restore
-      let lastSync = await this.getLastSync();
-      if (!lastSync) {
-        this._log.debug("Bookmarks backup starting.");
-        await PlacesBackups.create(null, true);
-        this._log.debug("Bookmarks backup done.");
-      }
-    } catch (ex) {
-      // Failure to create a backup is somewhat bad, but probably not bad
-      // enough to prevent syncing of bookmarks - so just log the error and
-      // continue.
-      this._log.warn("Error while backing up bookmarks, but continuing with sync", ex);
-    }
-
+    await super._syncStartup();
     this._store._childrenToOrder = {};
     this._store.clearPendingDeletions();
   },
 
   async getGuidMap() {
     if (this._guidMap) {
       return this._guidMap;
     }
--- a/third_party/rust/dogear/.cargo-checksum.json
+++ b/third_party/rust/dogear/.cargo-checksum.json
@@ -1,1 +1,1 @@
-{"files":{"CODE_OF_CONDUCT.md":"e85149c44f478f164f7d5f55f6e66c9b5ae236d4a11107d5e2a93fe71dd874b9","Cargo.toml":"9b2fd48cc14073599c7d3a50f74fe6aa9896a862c72651f69e06a1ec37e43c6d","LICENSE":"c71d239df91726fc519c6eb72d318ec65820627232b2f796219e87dcf35d0ab4","README.md":"303ea5ec53d4e86f2c321056e8158e31aa061353a99e52de3d76859d40919efc","src/driver.rs":"10ecce90c6dee4e7b0ecd87f6d4f10c3cb825b544c6413416167248755097ab2","src/error.rs":"c6e661a7b94119dc8770c482681e97e644507e37f0ba32c04cc3a0b43e7b0077","src/guid.rs":"0330e6e893a550e478c8ac678114ebc112add97cb1d5d803d65cda6588ce7ba5","src/lib.rs":"ef42d0d3b234ffb6e459550f36a5f9220a0dd5fd09867affc7f8f9fe0b5430f2","src/merge.rs":"460c6af8ba3b680d072528e9ee27ea62559911b1cce5a511abc3d698bc7b3da3","src/store.rs":"612d90ea0614aa7cc943c4ac0faaee35c155f57b553195ac28518ae7c0b8ebb1","src/tests.rs":"f341d811eb648e8482dd2eb108e110850453e7e0deeccc09610fa234332f3921","src/tree.rs":"b6ba6275716dff398ff81dfcbbc47fa3af59555e452d92b5cb035b73b88f733d"},"package":"6d54506b6b209740d0a7a35ca5976db1ad2ed1aa168acc3561efc6a84fa95afe"}
\ No newline at end of file
+{"files":{"CODE_OF_CONDUCT.md":"e85149c44f478f164f7d5f55f6e66c9b5ae236d4a11107d5e2a93fe71dd874b9","Cargo.toml":"ef36c6d2e8475c91f1a28a4ae1de871f8311f1b0044e6ba20b7c21c0af11f0a1","LICENSE":"c71d239df91726fc519c6eb72d318ec65820627232b2f796219e87dcf35d0ab4","README.md":"303ea5ec53d4e86f2c321056e8158e31aa061353a99e52de3d76859d40919efc","src/driver.rs":"541d0d5a3f87ebafb4294bebc8a08b259b174b2c0607fa7edef570b0d7b52b7f","src/error.rs":"b78609cf0f0a87b2e6d01bcaf565f1ce8723f33f22f36e1847639557bcd49a2e","src/guid.rs":"6185985ca3e416c1bb9b1691b83789f718fd532fc011efd4a28c82f1edd23650","src/lib.rs":"0bdb83959fc75d9ec99108e0c4c0ced4b9a80c08e950ad2ac59d095e74b39f0f","src/merge.rs":"176353b45ce1079e20d705ca82a154a375eaf927e5a6075d1469d490ff8662d3","src/store.rs":"612d90ea0614aa7cc943c4ac0faaee35c155f57b553195ac28518ae7c0b8ebb1","src/tests.rs":"8a12b2d571ca4c59d645879b555c321c7a6fb6445956d41fcb37747ac06b54df","src/tree.rs":"194ccd6642d64347cf79dea3237e6d124aa4a75cad654360d65945617e749afc"},"package":"30ac4a8e8f834f02deb2266b1f279aa5494e990c625d8be8f2988a7c708ba1f8"}
\ No newline at end of file
--- a/third_party/rust/dogear/Cargo.toml
+++ b/third_party/rust/dogear/Cargo.toml
@@ -1,24 +1,24 @@
 # THIS FILE IS AUTOMATICALLY GENERATED BY CARGO
 #
 # When uploading crates to the registry Cargo will automatically
 # "normalize" Cargo.toml files for maximal compatibility
 # with all versions of Cargo and also rewrite `path` dependencies
-# to registry (e.g. crates.io) dependencies
+# to registry (e.g., crates.io) dependencies
 #
 # If you believe there's an error in this file please file an
 # issue against the rust-lang/cargo repository. If you're
 # editing this file be aware that the upstream Cargo.toml
 # will likely look very different (and much more reasonable)
 
 [package]
 edition = "2018"
 name = "dogear"
-version = "0.2.3"
+version = "0.2.4"
 authors = ["Lina Cambridge <lina@mozilla.com>"]
 exclude = ["/.travis/**", ".travis.yml"]
 description = "A library for merging bookmark trees."
 license = "Apache-2.0"
 repository = "https://github.com/mozilla/dogear"
 [dependencies.log]
 version = "0.4"
 
--- a/third_party/rust/dogear/src/driver.rs
+++ b/third_party/rust/dogear/src/driver.rs
@@ -60,17 +60,17 @@ pub trait Driver {
 pub struct DefaultDriver;
 
 impl Driver for DefaultDriver {}
 
 /// Logs a merge message.
 pub fn log<D: Driver>(
     driver: &D,
     level: Level,
-    args: Arguments,
+    args: Arguments<'_>,
     module_path: &'static str,
     file: &'static str,
     line: u32,
 ) {
     let meta = log::Metadata::builder()
         .level(level)
         .target(module_path)
         .build();
--- a/third_party/rust/dogear/src/error.rs
+++ b/third_party/rust/dogear/src/error.rs
@@ -51,17 +51,17 @@ impl From<FromUtf16Error> for Error {
 
 impl From<Utf8Error> for Error {
     fn from(error: Utf8Error) -> Error {
         Error(ErrorKind::MalformedString(error.into()))
     }
 }
 
 impl fmt::Display for Error {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self.kind() {
             ErrorKind::MismatchedItemKind(local_kind, remote_kind) => write!(
                 f,
                 "Can't merge local kind {} and remote kind {}",
                 local_kind, remote_kind
             ),
             ErrorKind::DuplicateItem(guid) => write!(f, "Item {} already exists in tree", guid),
             ErrorKind::MissingItem(guid) => write!(f, "Item {} doesn't exist in tree", guid),
--- a/third_party/rust/dogear/src/guid.rs
+++ b/third_party/rust/dogear/src/guid.rs
@@ -142,22 +142,19 @@ impl IsValidGuid for Guid {
     }
 }
 
 impl<T: Copy + Into<usize>> IsValidGuid for [T] {
     /// Equivalent to `PlacesUtils.isValidGuid`.
     #[inline]
     fn is_valid_guid(&self) -> bool {
         self.len() == 12
-            && self.iter().all(|&byte| {
-                VALID_GUID_BYTES
-                    .get(byte.into())
-                    .map(|&b| b == 1)
-                    .unwrap_or(false)
-            })
+            && self
+                .iter()
+                .all(|&byte| VALID_GUID_BYTES.get(byte.into()).map_or(false, |&b| b == 1))
     }
 }
 
 impl From<String> for Guid {
     #[inline]
     fn from(s: String) -> Guid {
         Guid::from(s.as_str())
     }
@@ -247,23 +244,23 @@ impl Eq for Guid {}
 impl Hash for Guid {
     fn hash<H: Hasher>(&self, state: &mut H) {
         self.as_bytes().hash(state);
     }
 }
 
 // The default Debug impl is pretty unhelpful here.
 impl fmt::Debug for Guid {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "Guid({:?})", self.as_str())
     }
 }
 
 impl fmt::Display for Guid {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.write_str(self.as_str())
     }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
 
--- a/third_party/rust/dogear/src/lib.rs
+++ b/third_party/rust/dogear/src/lib.rs
@@ -7,16 +7,19 @@
 //     http://www.apache.org/licenses/LICENSE-2.0
 //
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#![allow(unknown_lints)]
+#![warn(rust_2018_idioms)]
+
 #[macro_use]
 mod driver;
 mod error;
 mod guid;
 mod merge;
 #[macro_use]
 mod store;
 mod tree;
--- a/third_party/rust/dogear/src/merge.rs
+++ b/third_party/rust/dogear/src/merge.rs
@@ -15,17 +15,17 @@
 use std::{
     collections::{HashMap, HashSet, VecDeque},
     mem,
 };
 
 use crate::driver::{DefaultDriver, Driver};
 use crate::error::{ErrorKind, Result};
 use crate::guid::{Guid, IsValidGuid};
-use crate::tree::{Content, Kind, MergeState, MergedNode, MergedRoot, Node, Tree, Validity};
+use crate::tree::{Content, MergeState, MergedNode, MergedRoot, Node, Tree, Validity};
 
 /// Structure change types, used to indicate if a node on one side is moved
 /// or deleted on the other.
 #[derive(Eq, PartialEq)]
 enum StructureChange {
     /// Node not deleted, or doesn't exist, on the other side.
     Unchanged,
     /// Node moved on the other side.
@@ -201,49 +201,47 @@ impl<'t, D: Driver> Merger<'t, D> {
     /// Checks if the merger merged all GUIDs in the given tree.
     #[inline]
     pub fn subsumes(&self, tree: &Tree) -> bool {
         tree.guids().all(|guid| self.mentions(guid))
     }
 
     /// Returns an iterator for all accepted local and remote deletions.
     #[inline]
-    pub fn deletions(&self) -> impl Iterator<Item = Deletion> {
+    pub fn deletions(&self) -> impl Iterator<Item = Deletion<'_>> {
         self.local_deletions().chain(self.remote_deletions())
     }
 
-    pub(crate) fn local_deletions(&self) -> impl Iterator<Item = Deletion> {
+    pub(crate) fn local_deletions(&self) -> impl Iterator<Item = Deletion<'_>> {
         self.delete_locally.iter().filter_map(move |guid| {
             if self.delete_remotely.contains(guid) {
                 None
             } else {
                 let local_level = self
                     .local_tree
                     .node_for_guid(guid)
-                    .map(|node| node.level())
-                    .unwrap_or(-1);
+                    .map_or(-1, |node| node.level());
                 // Items that should be deleted locally already have tombstones
                 // on the server, so we don't need to upload tombstones for
                 // these deletions.
                 Some(Deletion {
                     guid,
                     local_level,
                     should_upload_tombstone: false,
                 })
             }
         })
     }
 
-    pub(crate) fn remote_deletions(&self) -> impl Iterator<Item = Deletion> {
+    pub(crate) fn remote_deletions(&self) -> impl Iterator<Item = Deletion<'_>> {
         self.delete_remotely.iter().map(move |guid| {
             let local_level = self
                 .local_tree
                 .node_for_guid(guid)
-                .map(|node| node.level())
-                .unwrap_or(-1);
+                .map_or(-1, |node| node.level());
             Deletion {
                 guid,
                 local_level,
                 should_upload_tombstone: true,
             }
         })
     }
 
@@ -1001,17 +999,17 @@ impl<'t, D: Driver> Merger<'t, D> {
     /// This is the inverse of
     /// `check_for_remote_structure_change_of_local_node`.
     fn check_for_local_structure_change_of_remote_node(
         &mut self,
         merged_node: &mut MergedNode<'t>,
         remote_parent_node: Node<'t>,
         remote_node: Node<'t>,
     ) -> Result<StructureChange> {
-        if !remote_node_is_syncable(&remote_node) {
+        if !remote_node.is_syncable() {
             // If the remote node is known to be non-syncable, we unconditionally
             // delete it, even if it's syncable or moved locally.
             return self.delete_remote_node(merged_node, remote_node);
         }
 
         if !self.local_tree.is_deleted(&remote_node.guid) {
             if let Some(local_node) = self.local_tree.node_for_guid(&remote_node.guid) {
                 if !local_node.is_syncable() {
@@ -1105,17 +1103,17 @@ impl<'t, D: Driver> Merger<'t, D> {
         if !local_node.is_syncable() {
             // If the local node is known to be non-syncable, we unconditionally
             // delete it, even if it's syncable or moved remotely.
             return self.delete_local_node(merged_node, local_node);
         }
 
         if !self.remote_tree.is_deleted(&local_node.guid) {
             if let Some(remote_node) = self.remote_tree.node_for_guid(&local_node.guid) {
-                if !remote_node_is_syncable(&remote_node) {
+                if !remote_node.is_syncable() {
                     // The local node is syncable, but the remote node is not.
                     // This can happen if we applied an orphaned left pane
                     // query in a previous sync, and later saw the left pane
                     // root on the server. Since we now have the complete
                     // subtree, we can remove it.
                     return self.delete_local_node(merged_node, local_node);
                 }
                 if remote_node.validity == Validity::Replace
@@ -1536,21 +1534,8 @@ impl<'t, D: Driver> Merger<'t, D> {
                 "Merged node {} doesn't exist locally; no potential dupes for remote child {}",
                 merged_node,
                 remote_child_node
             );
             None
         }
     }
 }
-
-/// Indicates if the tree in the remote node is syncable. This filters out
-/// livemarks (bug 1477671) and orphaned Places queries (bug 1433182).
-fn remote_node_is_syncable(remote_node: &Node) -> bool {
-    if !remote_node.is_syncable() {
-        return false;
-    }
-    match remote_node.kind {
-        Kind::Livemark => false,
-        Kind::Query if remote_node.diverged() => false,
-        _ => true,
-    }
-}
--- a/third_party/rust/dogear/src/tests.rs
+++ b/third_party/rust/dogear/src/tests.rs
@@ -1746,16 +1746,82 @@ fn left_pane_root() {
     let mut deletions = merger.deletions().map(|d| d.guid).collect::<Vec<_>>();
     deletions.sort();
     assert_eq!(deletions, expected_deletions);
 
     assert_eq!(merger.counts(), &expected_telem);
 }
 
 #[test]
+fn livemarks() {
+    before_each();
+
+    let local_tree = nodes!({
+        ("menu________", Folder, {
+            ("livemarkAAAA", Livemark),
+            ("livemarkBBBB", Folder),
+            ("livemarkCCCC", Livemark)
+        }),
+        ("toolbar_____", Folder, {
+            ("livemarkDDDD", Livemark)
+        })
+    })
+    .into_tree()
+    .unwrap();
+
+    let remote_tree = nodes!({
+        ("menu________", Folder, {
+            ("livemarkAAAA", Livemark),
+            ("livemarkBBBB", Livemark),
+            ("livemarkCCCC", Folder)
+        }),
+        ("unfiled_____", Folder, {
+            ("livemarkEEEE", Livemark)
+        })
+    })
+    .into_tree()
+    .unwrap();
+
+    let mut merger = Merger::new(&local_tree, &remote_tree);
+    let merged_root = merger.merge().unwrap();
+    assert!(merger.subsumes(&local_tree));
+    assert!(merger.subsumes(&remote_tree));
+
+    let expected_tree = nodes!({
+        ("menu________", Folder[needs_merge = true]),
+        ("toolbar_____", Folder[needs_merge = true]),
+        ("unfiled_____", Folder[needs_merge = true])
+    })
+    .into_tree()
+    .unwrap();
+    let expected_deletions = vec![
+        "livemarkAAAA",
+        "livemarkBBBB",
+        "livemarkCCCC",
+        "livemarkDDDD",
+        "livemarkEEEE",
+    ];
+    let expected_telem = StructureCounts {
+        merged_nodes: 3,
+        // A, B, and C are counted twice, since they exist on both sides.
+        merged_deletions: 8,
+        ..StructureCounts::default()
+    };
+
+    let merged_tree = merged_root.into_tree().unwrap();
+    assert_eq!(merged_tree, expected_tree);
+
+    let mut deletions = merger.deletions().map(|d| d.guid).collect::<Vec<_>>();
+    deletions.sort();
+    assert_eq!(deletions, expected_deletions);
+
+    assert_eq!(merger.counts(), &expected_telem);
+}
+
+#[test]
 fn non_syncable_items() {
     before_each();
 
     let local_tree = nodes!({
         ("menu________", Folder[needs_merge = true], {
             // A is non-syncable remotely, but B doesn't exist remotely, so
             // we'll remove A from the merged structure, and move B to the
             // menu.
@@ -2562,17 +2628,17 @@ fn cycle() {
         .expect("Should insert B");
 
     match b
         .into_tree()
         .expect_err("Should not build tree with cycles")
         .kind()
     {
         ErrorKind::Cycle(guid) => assert_eq!(guid, &Guid::from("folderAAAAAA")),
-        err => assert!(false, "Wrong error kind for cycle: {:?}", err),
+        err => panic!("Wrong error kind for cycle: {:?}", err),
     }
 }
 
 #[test]
 fn reupload_replace() {
     before_each();
 
     let mut local_tree = nodes!({
--- a/third_party/rust/dogear/src/tree.rs
+++ b/third_party/rust/dogear/src/tree.rs
@@ -64,17 +64,17 @@ impl Tree {
             }],
             entry_index_by_guid,
             reparent_orphans_to: None,
         }
     }
 
     /// Returns the root node.
     #[inline]
-    pub fn root(&self) -> Node {
+    pub fn root(&self) -> Node<'_> {
         Node(self, &self.entries[0])
     }
 
     /// Returns an iterator for all tombstoned GUIDs.
     #[inline]
     pub fn deletions(&self) -> impl Iterator<Item = &Guid> {
         self.deleted_guids.iter()
     }
@@ -99,17 +99,17 @@ impl Tree {
         self.entries
             .iter()
             .map(|entry| &entry.item.guid)
             .chain(self.deleted_guids.iter())
     }
 
     /// Returns the node for a given `guid`, or `None` if a node with the `guid`
     /// doesn't exist in the tree, or was deleted.
-    pub fn node_for_guid(&self, guid: &Guid) -> Option<Node> {
+    pub fn node_for_guid(&self, guid: &Guid) -> Option<Node<'_>> {
         assert_eq!(self.entries.len(), self.entry_index_by_guid.len());
         self.entry_index_by_guid
             .get(guid)
             .map(|&index| Node(self, &self.entries[index]))
     }
 
     /// Returns the structure divergences found when building the tree.
     #[inline]
@@ -121,17 +121,17 @@ impl Tree {
 impl IntoTree for Tree {
     #[inline]
     fn into_tree(self) -> Result<Tree> {
         Ok(self)
     }
 }
 
 impl fmt::Display for Tree {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let root = self.root();
         f.write_str(&root.to_ascii_string())?;
         if !self.deleted_guids.is_empty() {
             f.write_str("\nDeleted: [")?;
             for (i, guid) in self.deleted_guids.iter().enumerate() {
                 if i != 0 {
                     f.write_str(", ")?;
                 }
@@ -256,17 +256,17 @@ impl Builder {
     #[inline]
     pub fn reparent_orphans_to(&mut self, guid: &Guid) -> &mut Builder {
         self.reparent_orphans_to = Some(guid.clone());
         self
     }
 
     /// Inserts an `item` into the tree. Returns an error if the item already
     /// exists.
-    pub fn item(&mut self, item: Item) -> Result<ParentBuilder> {
+    pub fn item(&mut self, item: Item) -> Result<ParentBuilder<'_>> {
         assert_eq!(self.entries.len(), self.entry_index_by_guid.len());
         if self.entry_index_by_guid.contains_key(&item.guid) {
             return Err(ErrorKind::DuplicateItem(item.guid.clone()).into());
         }
         self.entry_index_by_guid
             .insert(item.guid.clone(), self.entries.len());
         let entry_child = BuilderEntryChild::Exists(self.entries.len());
         self.entries.push(BuilderEntry {
@@ -274,17 +274,17 @@ impl Builder {
             parent: BuilderEntryParent::None,
             children: Vec::new(),
         });
         Ok(ParentBuilder(self, entry_child))
     }
 
     /// Sets parents for a `child_guid`. Depending on where the parent comes
     /// from, `child_guid` may not need to exist in the tree.
-    pub fn parent_for(&mut self, child_guid: &Guid) -> ParentBuilder {
+    pub fn parent_for(&mut self, child_guid: &Guid) -> ParentBuilder<'_> {
         assert_eq!(self.entries.len(), self.entry_index_by_guid.len());
         let entry_child = match self.entry_index_by_guid.get(child_guid) {
             Some(&child_index) => BuilderEntryChild::Exists(child_index),
             None => BuilderEntryChild::Missing(child_guid.clone()),
         };
         ParentBuilder(self, entry_child)
     }
 }
@@ -797,17 +797,20 @@ impl<'a> ResolveParent<'a> {
                 // youngest (minimum age).
                 let possible_parents = parents
                     .iter()
                     .map(|parent_by| PossibleParent::new(self.builder, parent_by))
                     .collect::<Vec<_>>();
                 self.problems.note(
                     &self.entry.item.guid,
                     Problem::DivergedParents(
-                        possible_parents.iter().map(|p| p.summarize()).collect(),
+                        possible_parents
+                            .iter()
+                            .map(PossibleParent::summarize)
+                            .collect(),
                     ),
                 );
                 possible_parents
                     .into_iter()
                     .min()
                     .and_then(|p| match p.parent_by {
                         BuilderParentBy::Children(index) => {
                             Some(ResolvedParent::ByChildren(*index))
@@ -884,17 +887,17 @@ impl<'a> PossibleParent<'a> {
     }
 }
 
 impl<'a> Ord for PossibleParent<'a> {
     /// Compares two possible parents to determine which is younger
     /// (`Ordering::Less`). Prefers parents from `children` over `parentid`
     /// (rule 2), and `parentid`s that reference folders over non-folders
     /// (rule 4).
-    fn cmp(&self, other: &PossibleParent) -> Ordering {
+    fn cmp(&self, other: &PossibleParent<'_>) -> Ordering {
         let (index, other_index) = match (&self.parent_by, &other.parent_by) {
             (BuilderParentBy::Children(index), BuilderParentBy::Children(other_index)) => {
                 // Both `self` and `other` mention the item in their `children`.
                 (*index, *other_index)
             }
             (BuilderParentBy::Children(_), BuilderParentBy::KnownItem(_)) => {
                 // `self` mentions the item in its `children`, and the item's
                 // `parentid` is `other`, so prefer `self`.
@@ -931,23 +934,23 @@ impl<'a> Ord for PossibleParent<'a> {
             (false, true) => Ordering::Greater,
             (true, false) => Ordering::Less,
             (false, false) => Ordering::Equal,
         }
     }
 }
 
 impl<'a> PartialOrd for PossibleParent<'a> {
-    fn partial_cmp(&self, other: &PossibleParent) -> Option<Ordering> {
+    fn partial_cmp(&self, other: &PossibleParent<'_>) -> Option<Ordering> {
         Some(self.cmp(other))
     }
 }
 
 impl<'a> PartialEq for PossibleParent<'a> {
-    fn eq(&self, other: &PossibleParent) -> bool {
+    fn eq(&self, other: &PossibleParent<'_>) -> bool {
         self.cmp(other) == Ordering::Equal
     }
 }
 
 impl<'a> Eq for PossibleParent<'a> {}
 
 /// Describes a resolved parent for an item.
 #[derive(Debug)]
@@ -1051,17 +1054,17 @@ pub enum DivergedParent {
 
 impl From<DivergedParentGuid> for DivergedParent {
     fn from(d: DivergedParentGuid) -> DivergedParent {
         DivergedParent::ByParentGuid(d)
     }
 }
 
 impl fmt::Display for DivergedParent {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
             DivergedParent::ByChildren(parent_guid) => {
                 write!(f, "is in children of {}", parent_guid)
             }
             DivergedParent::ByParentGuid(p) => match p {
                 DivergedParentGuid::Folder(parent_guid) => write!(f, "has parent {}", parent_guid),
                 DivergedParentGuid::NonFolder(parent_guid) => {
                     write!(f, "has non-folder parent {}", parent_guid)
@@ -1097,17 +1100,17 @@ impl Problems {
     }
 
     /// Returns `true` if there are no problems.
     pub fn is_empty(&self) -> bool {
         self.0.is_empty()
     }
 
     /// Returns an iterator for all problems.
-    pub fn summarize(&self) -> impl Iterator<Item = ProblemSummary> {
+    pub fn summarize(&self) -> impl Iterator<Item = ProblemSummary<'_>> {
         self.0.iter().flat_map(|(guid, problems)| {
             problems
                 .iter()
                 .map(move |problem| ProblemSummary(guid, problem))
         })
     }
 }
 
@@ -1123,17 +1126,17 @@ impl<'a> ProblemSummary<'a> {
 
     #[inline]
     pub fn problem(&self) -> &Problem {
         &self.1
     }
 }
 
 impl<'a> fmt::Display for ProblemSummary<'a> {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let parents = match self.problem() {
             Problem::Orphan => return write!(f, "{} is an orphan", self.guid()),
             Problem::MisparentedRoot(parents) => {
                 write!(f, "{} is a user content root", self.guid())?;
                 if parents.is_empty() {
                     return Ok(());
                 }
                 f.write_str(", but ")?;
@@ -1180,29 +1183,29 @@ impl<'t> Node<'t> {
         self.1
             .child_indices
             .iter()
             .map(move |&child_index| Node(self.0, &self.0.entries[child_index]))
     }
 
     /// Returns the resolved parent of this node, or `None` if this is the
     /// root node.
-    pub fn parent(&self) -> Option<Node> {
+    pub fn parent(&self) -> Option<Node<'_>> {
         self.1
             .parent_index
             .as_ref()
             .map(|&parent_index| Node(self.0, &self.0.entries[parent_index]))
     }
 
     /// Returns the level of this node in the tree.
     pub fn level(&self) -> i64 {
         if self.is_root() {
             return 0;
         }
-        self.parent().map(|parent| parent.level() + 1).unwrap_or(-1)
+        self.parent().map_or(-1, |parent| parent.level() + 1)
     }
 
     /// Indicates if this node is for a syncable item.
     ///
     /// Syncable items descend from the four user content roots. Any
     /// other roots and their descendants, like the left pane root,
     /// left pane queries, and custom roots, are non-syncable.
     ///
@@ -1216,23 +1219,23 @@ impl<'t> Node<'t> {
     /// remotely.
     pub fn is_syncable(&self) -> bool {
         if self.is_root() {
             return false;
         }
         if self.is_user_content_root() {
             return true;
         }
-        if self.kind == Kind::Query && self.diverged() {
-            // TODO(lina): Flag queries reparented specifically to `unfiled`.
-            return false;
+        match self.kind {
+            // Exclude livemarks (bug 1477671).
+            Kind::Livemark => false,
+            // Exclude orphaned Places queries (bug 1433182).
+            Kind::Query if self.diverged() => false,
+            _ => self.parent().map_or(false, |parent| parent.is_syncable()),
         }
-        self.parent()
-            .map(|parent| parent.is_syncable())
-            .unwrap_or(false)
     }
 
     /// Indicates if this node's structure diverged because it
     /// existed in multiple parents, or was reparented.
     #[inline]
     pub fn diverged(&self) -> bool {
         match &self.1.divergence {
             Divergence::Diverged => true,
@@ -1299,24 +1302,24 @@ impl<'t> Deref for Node<'t> {
     type Target = Item;
 
     fn deref(&self) -> &Item {
         &self.1.item
     }
 }
 
 impl<'t> fmt::Display for Node<'t> {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         self.1.item.fmt(f)
     }
 }
 
 #[cfg(test)]
 impl<'t> PartialEq for Node<'t> {
-    fn eq(&self, other: &Node) -> bool {
+    fn eq(&self, other: &Node<'_>) -> bool {
         match (self.parent(), other.parent()) {
             (Some(parent), Some(other_parent)) => {
                 if parent.1.item != other_parent.1.item {
                     return false;
                 }
             }
             (Some(_), None) | (None, Some(_)) => return false,
             (None, None) => {}
@@ -1368,17 +1371,17 @@ impl Item {
             (Kind::Bookmark, Kind::Query) => true,
             (Kind::Query, Kind::Bookmark) => true,
             (local_kind, remote_kind) => local_kind == remote_kind,
         }
     }
 }
 
 impl fmt::Display for Item {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let kind = match self.validity {
             Validity::Valid => format!("{}", self.kind),
             Validity::Reupload | Validity::Replace => format!("{} ({})", self.kind, self.validity),
         };
         let info = if self.needs_merge {
             format!("{}; Age = {}ms; Unmerged", kind, self.age)
         } else {
             format!("{}; Age = {}ms", kind, self.age)
@@ -1393,17 +1396,17 @@ pub enum Kind {
     Bookmark,
     Query,
     Folder,
     Livemark,
     Separator,
 }
 
 impl fmt::Display for Kind {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         fmt::Debug::fmt(self, f)
     }
 }
 
 /// Synced item validity.
 #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
 pub enum Validity {
     /// The item is valid, and can be applied as-is.
@@ -1415,44 +1418,44 @@ pub enum Validity {
 
     /// The item isn't valid at all, and should either be replaced with a valid
     /// local copy, or deleted if a valid local copy doesn't exist. Places uses
     /// this to flag bookmarks and queries without valid URLs.
     Replace,
 }
 
 impl fmt::Display for Validity {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         fmt::Debug::fmt(self, f)
     }
 }
 
 /// The root of a merged tree, from which all merged nodes descend.
 #[derive(Debug)]
 pub struct MergedRoot<'t> {
     node: MergedNode<'t>,
     size_hint: usize,
 }
 
 impl<'t> MergedRoot<'t> {
     /// Returns a merged root for the given node. `size_hint` indicates the
     /// size of the tree, excluding the root, and is used to avoid extra
     /// allocations for the descendants.
-    pub(crate) fn with_size(node: MergedNode<'t>, size_hint: usize) -> MergedRoot {
+    pub(crate) fn with_size(node: MergedNode<'t>, size_hint: usize) -> MergedRoot<'_> {
         MergedRoot { node, size_hint }
     }
 
     /// Returns the root node.
-    pub fn node(&self) -> &MergedNode {
+    pub fn node(&self) -> &MergedNode<'_> {
         &self.node
     }
 
     /// Returns a flattened `Vec` of the root node's descendants, excluding the
     /// root node itself.
-    pub fn descendants(&self) -> Vec<MergedDescendant> {
+    pub fn descendants(&self) -> Vec<MergedDescendant<'_>> {
         fn accumulate<'t>(
             results: &mut Vec<MergedDescendant<'t>>,
             merged_node: &'t MergedNode<'t>,
             level: usize,
         ) {
             results.reserve(merged_node.merged_children.len());
             for (position, merged_child_node) in merged_node.merged_children.iter().enumerate() {
                 results.push(MergedDescendant {
@@ -1474,17 +1477,17 @@ impl<'t> MergedRoot<'t> {
     pub fn to_ascii_string(&self) -> String {
         self.node.to_ascii_fragment("")
     }
 }
 
 #[cfg(test)]
 impl<'t> IntoTree for MergedRoot<'t> {
     fn into_tree(self) -> Result<Tree> {
-        fn to_item(merged_node: &MergedNode) -> Item {
+        fn to_item(merged_node: &MergedNode<'_>) -> Item {
             let node = merged_node.merge_state.node();
             let mut item = Item::new(merged_node.guid.clone(), node.kind);
             item.age = node.age;
             item.needs_merge = merged_node.merge_state.upload_reason() != UploadReason::None;
             item
         }
 
         let mut b = Tree::with_root(to_item(&self.node));
@@ -1521,18 +1524,17 @@ impl<'t> MergedNode<'t> {
     }
 
     /// Indicates if the merged node exists remotely and has a new GUID. The
     /// merger uses this to flag parents and children of remote nodes with
     /// invalid GUIDs for reupload.
     pub(crate) fn remote_guid_changed(&self) -> bool {
         self.merge_state
             .remote_node()
-            .map(|remote_node| remote_node.guid != self.guid)
-            .unwrap_or(false)
+            .map_or(false, |remote_node| remote_node.guid != self.guid)
     }
 
     fn to_ascii_fragment(&self, prefix: &str) -> String {
         match self.merge_state.node().kind {
             Kind::Folder => {
                 let children_prefix = format!("{}| ", prefix);
                 let children = self
                     .merged_children
@@ -1546,17 +1548,17 @@ impl<'t> MergedNode<'t> {
                 }
             }
             _ => format!("{}🔖 {}", prefix, &self),
         }
     }
 }
 
 impl<'t> fmt::Display for MergedNode<'t> {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "{} {}", self.guid, self.merge_state)
     }
 }
 
 /// A descendant holds a merged node, merged parent node, position in the
 /// merged parent, and level in the merged tree.
 #[derive(Clone, Copy, Debug)]
 pub struct MergedDescendant<'t> {
@@ -1760,17 +1762,17 @@ impl<'t> MergeState<'t> {
             | MergeState::RemoteWithNewStructure { remote_node, .. } => remote_node,
 
             MergeState::Unchanged { local_node, .. } => local_node,
         }
     }
 }
 
 impl<'t> fmt::Display for MergeState<'t> {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.write_str(match self {
             MergeState::LocalOnly(_) | MergeState::Local { .. } => "(Local, Local)",
 
             MergeState::RemoteOnly(_) | MergeState::Remote { .. } => "(Remote, Remote)",
 
             MergeState::RemoteOnlyWithNewStructure(_)
             | MergeState::RemoteWithNewStructure { .. } => "(Remote, New)",
 
--- a/toolkit/components/places/SyncedBookmarksMirror.jsm
+++ b/toolkit/components/places/SyncedBookmarksMirror.jsm
@@ -117,17 +117,17 @@ XPCOMUtils.defineLazyGetter(this, "Local
 // 1375896).
 const DB_URL_LENGTH_MAX = 65536;
 const DB_TITLE_LENGTH_MAX = 4096;
 
 const SQLITE_MAX_VARIABLE_NUMBER = 999;
 
 // The current mirror database schema version. Bump for migrations, then add
 // migration code to `migrateMirrorSchema`.
-const MIRROR_SCHEMA_VERSION = 4;
+const MIRROR_SCHEMA_VERSION = 5;
 
 const DEFAULT_MAX_FRECENCIES_TO_RECALCULATE = 400;
 
 // Use a shared jankYielder in these functions
 XPCOMUtils.defineLazyGetter(this, "yieldState", () => Async.yieldState());
 
 /** Adapts a `Log.jsm` logger to a `mozISyncedBookmarksMirrorLogger`. */
 class MirrorLoggerAdapter {
@@ -1350,18 +1350,18 @@ async function attachAndInitMirrorDataba
  * Migrates the mirror database schema to the latest version.
  *
  * @param {Sqlite.OpenedConnection} db
  *        The mirror database connection.
  * @param {Number} currentSchemaVersion
  *        The current mirror database schema version.
  */
 async function migrateMirrorSchema(db, currentSchemaVersion) {
-  if (currentSchemaVersion < 4) {
-    // The mirror was pref'd off by default for schema versions 1-3.
+  if (currentSchemaVersion < 5) {
+    // The mirror was pref'd off by default for schema versions 1-4.
     throw new DatabaseCorruptError(`Can't migrate from schema version ${
       currentSchemaVersion}; too old`);
   }
 }
 
 /**
  * Initializes a new mirror database, creating persistent tables, indexes, and
  * roots.
--- a/toolkit/components/places/bookmark_sync/Cargo.toml
+++ b/toolkit/components/places/bookmark_sync/Cargo.toml
@@ -1,17 +1,17 @@
 [package]
 name = "bookmark_sync"
 version = "0.1.0"
 authors = ["Lina Cambridge <lina@yakshaving.ninja>"]
 edition = "2018"
 
 [dependencies]
 atomic_refcell = "0.1"
-dogear = "0.2.3"
+dogear = "0.2.4"
 libc = "0.2"
 log = "0.4"
 cstr = "0.1"
 moz_task = { path = "../../../../xpcom/rust/moz_task" }
 nserror = { path = "../../../../xpcom/rust/nserror" }
 nsstring = { path = "../../../../xpcom/rust/nsstring" }
 storage = { path = "../../../../storage/rust" }
 storage_variant = { path = "../../../../storage/variant" }
--- a/toolkit/components/places/tests/bookmarks/test_bookmarks_eraseEverything.js
+++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_eraseEverything.js
@@ -28,48 +28,45 @@ add_task(async function test_eraseEveryt
                                                              type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
                                                              url: "http://example.com/" });
   checkBookmarkObject(unfiledBookmark);
   let unfiledBookmarkInFolder =
     await PlacesUtils.bookmarks.insert({ parentGuid: unfiledFolder.guid,
                                          type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
                                          url: "http://mozilla.org/" });
   checkBookmarkObject(unfiledBookmarkInFolder);
-  PlacesUtils.annotations.setItemAnnotation((await PlacesUtils.promiseItemId(unfiledBookmarkInFolder.guid)),
-                                            "testanno1", "testvalue1", 0, PlacesUtils.annotations.EXPIRE_NEVER);
+  await setItemAnnotation(unfiledBookmarkInFolder.guid, "testanno1", "testvalue1");
 
   let menuFolder = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.menuGuid,
                                                         type: PlacesUtils.bookmarks.TYPE_FOLDER });
   checkBookmarkObject(menuFolder);
   let menuBookmark = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.menuGuid,
                                                           type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
                                                           url: "http://example.com/" });
   checkBookmarkObject(menuBookmark);
   let menuBookmarkInFolder =
     await PlacesUtils.bookmarks.insert({ parentGuid: menuFolder.guid,
                                          type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
                                          url: "http://mozilla.org/" });
   checkBookmarkObject(menuBookmarkInFolder);
-  PlacesUtils.annotations.setItemAnnotation((await PlacesUtils.promiseItemId(menuBookmarkInFolder.guid)),
-                                            "testanno1", "testvalue1", 0, PlacesUtils.annotations.EXPIRE_NEVER);
+  await setItemAnnotation(menuBookmarkInFolder.guid, "testanno1", "testvalue1");
 
   let toolbarFolder = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.toolbarGuid,
                                                            type: PlacesUtils.bookmarks.TYPE_FOLDER });
   checkBookmarkObject(toolbarFolder);
   let toolbarBookmark = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.toolbarGuid,
                                                              type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
                                                              url: "http://example.com/" });
   checkBookmarkObject(toolbarBookmark);
   let toolbarBookmarkInFolder =
     await PlacesUtils.bookmarks.insert({ parentGuid: toolbarFolder.guid,
                                          type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
                                          url: "http://mozilla.org/" });
   checkBookmarkObject(toolbarBookmarkInFolder);
-  PlacesUtils.annotations.setItemAnnotation((await PlacesUtils.promiseItemId(toolbarBookmarkInFolder.guid)),
-                                            "testanno1", "testvalue1", 0, PlacesUtils.annotations.EXPIRE_NEVER);
+  await setItemAnnotation(toolbarBookmarkInFolder.guid, "testanno1", "testvalue1");
 
   await PlacesTestUtils.promiseAsyncUpdates();
   Assert.ok(frecencyForUrl("http://example.com/") > frecencyForExample);
   Assert.ok(frecencyForUrl("http://example.com/") > frecencyForMozilla);
 
   let manyFrecenciesPromise = promiseManyFrecenciesChanged();
 
   await PlacesUtils.bookmarks.eraseEverything();
--- a/toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js
+++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js
@@ -124,18 +124,17 @@ add_task(async function remove_multiple_
 });
 
 add_task(async function remove_bookmark_orphans() {
   let bm1 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
                                                  type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
                                                  url: "http://example.com/",
                                                  title: "a bookmark" });
   checkBookmarkObject(bm1);
-  PlacesUtils.annotations.setItemAnnotation((await PlacesUtils.promiseItemId(bm1.guid)),
-                                            "testanno", "testvalue", 0, PlacesUtils.annotations.EXPIRE_NEVER);
+  await setItemAnnotation(bm1.guid, "testanno", "testvalue");
 
   await PlacesUtils.bookmarks.remove(bm1.guid);
 
   // Check there are no orphan annotations.
   let conn = await PlacesUtils.promiseDBConnection();
   let annoAttrs = await conn.execute(`SELECT id, name FROM moz_anno_attributes`);
   Assert.equal(annoAttrs.length, 0);
   let annos = await conn.execute(`SELECT item_id, anno_attribute_id FROM moz_items_annos`);
--- a/toolkit/components/places/tests/bookmarks/test_sync_fields.js
+++ b/toolkit/components/places/tests/bookmarks/test_sync_fields.js
@@ -243,19 +243,17 @@ class TestCases {
     let tombstones = await PlacesTestUtils.fetchSyncTombstones();
     let tombstoneGuids = sortBy(tombstones, "guid").map(({ guid }) => guid);
     Assert.equal(tombstoneGuids.length, 2);
     Assert.deepEqual(tombstoneGuids, [folder2, child2].sort(compareAscending));
   }
 
   // Annos don't have an async API, so we use the sync API for both tests.
   async setAnno(guid, name, value) {
-    let id = await PlacesUtils.promiseItemId(guid);
-    PlacesUtils.annotations.setItemAnnotation(id, name, value, 0,
-                                              PlacesUtils.annotations.EXPIRE_NEVER);
+    await setItemAnnotation(guid, name, value);
   }
 }
 
 // Exercises the legacy, synchronous `nsINavBookmarksService` calls implemented
 // in C++.
 class SyncTestCases extends TestCases {
   async createFolder(parentGuid, title, index) {
     let parentId = await PlacesUtils.promiseItemId(parentGuid);
--- a/toolkit/components/places/tests/head_common.js
+++ b/toolkit/components/places/tests/head_common.js
@@ -947,16 +947,29 @@ function getItemsWithAnnotation(name) {
       WHERE n.name = :name
     `, {name});
 
     return rows.map(row => row.getResultByName("guid"));
   });
 }
 
 /**
+ * Sets an annotation for an item.
+ *
+ * @param {String} guid The GUID of the item.
+ * @param {String} name The name of the annotation.
+ * @param {Number|String} value The value of the annotation.
+ */
+async function setItemAnnotation(guid, name, value) {
+  let id = await PlacesUtils.promiseItemId(guid);
+  PlacesUtils.annotations.setItemAnnotation(id, name, value, 0,
+                                            PlacesUtils.annotations.EXPIRE_NEVER);
+}
+
+/**
  * Checks there are no orphan page annotations in the database, and no
  * orphan anno attribute names.
  */
 async function assertNoOrphanPageAnnotations() {
   let db = await PlacesUtils.promiseDBConnection();
 
   let rows = await db.execute(`
     SELECT place_id FROM moz_annos
--- a/toolkit/components/places/tests/sync/test_bookmark_kinds.js
+++ b/toolkit/components/places/tests/sync/test_bookmark_kinds.js
@@ -138,50 +138,71 @@ add_task(async function test_mismatched_
 
   info("Set up mirror");
   await PlacesUtils.bookmarks.insertTree({
     guid: PlacesUtils.bookmarks.toolbarGuid,
     children: [{
       guid: "l1nZZXfB8nC7",
       type: PlacesUtils.bookmarks.TYPE_FOLDER,
       title: "Innerst i Sneglehode",
+    }, {
+      guid: "livemarkBBBB",
+      type: PlacesUtils.bookmarks.TYPE_FOLDER,
+      title: "B",
     }],
   });
+  // Livemarks have been removed in bug 1477671, but Sync still checks the anno
+  // to distinguish them from folders.
+  await setItemAnnotation("livemarkBBBB", PlacesUtils.LMANNO_FEEDURI,
+    "http://example.com/b");
   await PlacesTestUtils.markBookmarksAsSynced();
 
   info("Make remote changes");
   await storeRecords(buf, [{
     id: "toolbar",
     parentid: "places",
     type: "folder",
-    children: ["l1nZZXfB8nC7"],
+    children: ["l1nZZXfB8nC7", "livemarkBBBB"],
   }, {
     "id": "l1nZZXfB8nC7",
     "type": "livemark",
     "siteUri": "http://sneglehode.wordpress.com/",
     "feedUri": "http://sneglehode.wordpress.com/feed/",
     "parentName": "Bookmarks Toolbar",
     "title": "Innerst i Sneglehode",
     "description": null,
     "children":
       ["HCRq40Rnxhrd", "YeyWCV1RVsYw", "GCceVZMhvMbP", "sYi2hevdArlF",
        "vjbZlPlSyGY8", "UtjUhVyrpeG6", "rVq8WMG2wfZI", "Lx0tcy43ZKhZ",
        "oT74WwV8_j4P", "IztsItWVSo3-"],
     "parentid": "toolbar",
+  }, {
+    id: "livemarkBBBB",
+    parentid: "toolbar",
+    type: "folder",
+    title: "B",
+    children: [],
   }]);
 
   info("Apply remote");
   let changesToUpload = await buf.apply();
   deepEqual(await buf.fetchUnmergedGuids(), [], "Should merge all items");
 
   let idsToUpload = inspectChangeRecords(changesToUpload);
   deepEqual(idsToUpload, {
     updated: ["toolbar"],
-    deleted: ["l1nZZXfB8nC7"],
-  }, "Legacy livemark should be deleted remotely");
+    deleted: ["l1nZZXfB8nC7", "livemarkBBBB"],
+  }, "Should delete livemarks remotely");
+
+  await assertLocalTree(PlacesUtils.bookmarks.toolbarGuid, {
+    guid: PlacesUtils.bookmarks.toolbarGuid,
+    type: PlacesUtils.bookmarks.TYPE_FOLDER,
+    index: 1,
+    title: BookmarksToolbarTitle,
+  }, "Should delete livemarks locally");
 
   await buf.finalize();
   await PlacesUtils.bookmarks.eraseEverything();
   await PlacesSyncUtils.bookmarks.reset();
 });
 
 add_task(async function test_different_but_compatible_bookmark_types() {
   try {
--- a/toolkit/components/places/tests/unit/test_telemetry.js
+++ b/toolkit/components/places/tests/unit/test_telemetry.js
@@ -81,20 +81,17 @@ add_task(async function test_execute() {
   await PlacesUtils.keywords.insert({ url: uri.spec, keyword: "keyword"});
 
   // Set a large annotation.
   let content = "";
   while (content.length < 1024) {
     content += "0";
   }
 
-  let itemId = await PlacesUtils.promiseItemId(bookmarks[1].guid);
-
-  PlacesUtils.annotations.setItemAnnotation(itemId, "test-anno", content, 0,
-                                            PlacesUtils.annotations.EXPIRE_NEVER);
+  await setItemAnnotation(bookmarks[1].guid, "test-anno", content);
   await PlacesUtils.history.update({
     url: uri,
     annotations: new Map([["test-anno", content]]),
   });
 
   // Request to gather telemetry data.
   Cc["@mozilla.org/places/categoriesStarter;1"]
     .getService(Ci.nsIObserver)