Fix up diverging structure in bookmark trees

src/error.rs

@@ -52,6 +52,7 @@ pub enum ErrorKind {
     MergeConflict,
     UnmergedLocalItems,
     UnmergedRemoteItems,
+    GenerateGuid(Guid),
 }
 
 impl fmt::Display for ErrorKind {
@@ -83,6 +84,9 @@ impl fmt::Display for ErrorKind {
             },
             ErrorKind::UnmergedRemoteItems => {
                 write!(f, "Merged tree doesn't mention all items from remote tree")
+            },
+            ErrorKind::GenerateGuid(invalid_guid) => {
+                write!(f, "Failed to generate new GUID for {}", invalid_guid)
             }
         }
     }

src/guid.rs

@@ -30,12 +30,15 @@ enum Repr {
 /// The Places root GUID, used to root all items in a bookmark tree.
 pub const ROOT_GUID: Guid = Guid(Repr::Fast(*b"root________"));
 
+/// The "Other Bookmarks" GUID, used to hold items without a parent.
+pub const UNFILED_GUID: Guid = Guid(Repr::Fast(*b"unfiled_____"));
+
 /// The syncable Places roots. All synced items should descend from these
 /// roots.
 pub const USER_CONTENT_ROOTS: [Guid; 4] = [
     Guid(Repr::Fast(*b"toolbar_____")),
     Guid(Repr::Fast(*b"menu________")),
-    Guid(Repr::Fast(*b"unfiled_____")),
+    UNFILED_GUID,
     Guid(Repr::Fast(*b"mobile______")),
 ];
 
@@ -51,18 +54,6 @@ const VALID_GUID_BYTES: [u8; 255] =
      0, 0, 0, 0, 0, 0, 0];
 
 impl Guid {
-    pub fn new(s: &str) -> Guid {
-        let repr = if Guid::is_valid(s.as_bytes()) {
-            assert!(s.is_char_boundary(12));
-            let mut bytes = [0u8; 12];
-            bytes.copy_from_slice(s.as_bytes());
-            Repr::Fast(bytes)
-        } else {
-            Repr::Slow(s.into())
-        };
-        Guid(repr)
-    }
-
     pub fn from_utf8(b: &[u8]) -> Option<Guid> {
         let repr = if Guid::is_valid(b) {
             let mut bytes = [0u8; 12];
@@ -117,6 +108,13 @@ impl Guid {
         }
     }
 
+    pub fn valid(&self) -> bool {
+        match self.0 {
+            Repr::Fast(_) => true,
+            Repr::Slow(_) => false,
+        }
+    }
+
     /// Equivalent to `PlacesUtils.isValidGuid`.
     #[inline]
     fn is_valid<T: Copy + IntoByte>(bytes: &[T]) -> bool {
@@ -129,7 +127,15 @@ impl Guid {
 impl<'a> From<&'a str> for Guid {
     #[inline]
     fn from(s: &'a str) -> Guid {
-        Guid::new(s)
+        let repr = if Guid::is_valid(s.as_bytes()) {
+            assert!(s.is_char_boundary(12));
+            let mut bytes = [0u8; 12];
+            bytes.copy_from_slice(s.as_bytes());
+            Repr::Fast(bytes)
+        } else {
+            Repr::Slow(s.into())
+        };
+        Guid(repr)
     }
 }