Bug 514545 - weave mangles bookmarks

For both folders and separators, use the parent name (not guid) in addition to the item's title/position to determine if it's a dupe. This modifies the bookmark format for folders and separators, so a storage version bump is needed.

services/sync/modules/engines/bookmarks.js

@@ -101,45 +101,61 @@ BookmarksEngine.prototype = {
     SyncEngine.prototype._syncStartup.call(this);
 
     // Lazily create a mapping of folder titles and separator positions to GUID
-    let lazyMap = function() {
-      delete this._folderTitles;
-      delete this._separatorPos;
+    this.__defineGetter__("_lazyMap", function() {
+      delete this._lazyMap;
 
-      let folderTitles = {};
-      let separatorPos = {};
+      let lazyMap = {};
       for (let guid in this._store.getAllIDs()) {
+        // Figure out what key to store the mapping
+        let key;
         let id = idForGUID(guid);
         switch (Svc.Bookmark.getItemType(id)) {
           case Svc.Bookmark.TYPE_FOLDER:
-            // Map the folder name to GUID
-            folderTitles[Svc.Bookmark.getItemTitle(id)] = guid;
+            key = "f" + Svc.Bookmark.getItemTitle(id);
             break;
           case Svc.Bookmark.TYPE_SEPARATOR:
-            // Map the separator position and parent position to GUID
-            let parent = Svc.Bookmark.getFolderIdForItem(id);
-            let pos = [id, parent].map(Svc.Bookmark.getItemIndex);
-            separatorPos[pos] = guid;
+            key = "s" + Svc.Bookmark.getItemIndex(id);
             break;
+          default:
+            continue;
         }
+
+        // The mapping is on a per parent-folder-name basis
+        let parent = Svc.Bookmark.getFolderIdForItem(id);
+        let parentName = Svc.Bookmark.getItemTitle(parent);
+        if (lazyMap[parentName] == null)
+          lazyMap[parentName] = {};
+
+        // Remember this item's guid for its parent-name/key pair
+        lazyMap[parentName][key] = guid;
       }
 
-      this._folderTitles = folderTitles;
-      this._separatorPos = separatorPos;
-    };
+      // Expose a helper function to get a dupe guid for an item
+      return this._lazyMap = function(item) {
+        // Figure out if we have something to key with
+        let key;
+        switch (item.type) {
+          case "folder":
+          case "livemark":
+            key = "f" + item.title;
+            break;
+          case "separator":
+            key = "s" + item.pos;
+            break;
+          default:
+            return;
+        }
 
-    // Make the getters that lazily build the mapping
-    ["_folderTitles", "_separatorPos"].forEach(function(lazy) {
-      this.__defineGetter__(lazy, function() {
-        lazyMap.call(this);
-        return this[lazy];
-      });
-    }, this);
+        // Give the guid if we have the matching pair
+        let parent = lazyMap[item.parentName];
+        return parent && parent[key];
+      };
+    });
   },
 
   _syncFinish: function _syncFinish() {
     SyncEngine.prototype._syncFinish.call(this);
-    delete this._folderTitles;
-    delete this._separatorPos;
+    delete this._lazyMap;
   },
 
   _findDupe: function _findDupe(item) {
@@ -155,9 +171,8 @@ BookmarksEngine.prototype = {
         return GUIDForId(localId);
       case "folder":
       case "livemark":
-        return this._folderTitles[item.title];
       case "separator":
-        return this._separatorPos[item.pos];
+        return this._lazyMap(item);
     }
   },
 
@@ -684,6 +699,7 @@ BookmarksStore.prototype = {
       return record;
     }
 
+    let parent = Svc.Bookmark.getFolderIdForItem(placeId);
     switch (this._bms.getItemType(placeId)) {
     case this._bms.TYPE_BOOKMARK:
       let bmkUri = this._bms.getBookmarkURI(placeId).spec;
@@ -734,14 +750,15 @@ BookmarksStore.prototype = {
         record = new BookmarkFolder();
       }
 
+      record.parentName = Svc.Bookmark.getItemTitle(parent);
       record.title = this._bms.getItemTitle(placeId);
       break;
 
     case this._bms.TYPE_SEPARATOR:
       record = new BookmarkSeparator();
       // Create a positioning identifier for the separator
-      let parent = Svc.Bookmark.getFolderIdForItem(placeId);
-      record.pos = [placeId, parent].map(Svc.Bookmark.getItemIndex);
+      record.parentName = Svc.Bookmark.getItemTitle(parent);
+      record.pos = Svc.Bookmark.getItemIndex(placeId);
       break;
 
     case this._bms.TYPE_DYNAMIC_CONTAINER:

services/sync/modules/type_records/bookmark.js

@@ -93,7 +93,7 @@ PlacesItem.prototype = {
   },
 };
 
-Utils.deferGetSet(PlacesItem, "cleartext", ["predecessorid", "type"]);
+Utils.deferGetSet(PlacesItem, "cleartext", ["parentName", "predecessorid", "type"]);
 
 function Bookmark(uri) {
   this._Bookmark_init(uri);