1380474 - Separator deduping is non-deterministic

Status: RESOLVED DUPLICATE of bug 1305563

(Firefox :: Sync, enhancement, P2)

Description

[Lina Butler [:lina]]

We currently use the position and parent title to dedupe separators. This has unpredictable results depending on when we build the GUID map.

If we build the map the first time we insert a bookmark we don't have locally, and the containing folder has changed in some way (for example, new or reordered local items that haven't been uploaded yet, or items moved between folders)...the positions will be off, and so we don't dedupe. Or, if another separator exists at the new position, we might dedupe, but then we'll wrongly smush the separators together. Changing the folder during a sync will also affect positions, as will interrupting a sync that partially changed the folder's contents.

I think we're saved from the worst of this because we: 1) always add new or moved synced items to the end of the folder, and 2) order children at the end of the sync. Even in the case of a partial download, items earlier in the folder will retain their positions, so they'll still line up when we build the GUID map on the next sync. Also, since most syncs don't change large portions of the tree, deduping likely won't be involved at all, and I suspect separators apart from the default ones aren't that common.

The approach in bug 1366888 has the same problem. It's fine as long as the separator we're deduping hasn't changed position. Once it has, we won't flag it as a dupe because the local position and the `pos` in the incoming record will differ.

Without two-phase application, this is tricky. One idea is to apply all separators first, then pare duplicates before uploading. If a "NEW" separator is directly adjacent to a "NORMAL" separator, we assume the "NEW" one is a duplicate and delete it.