1515784 - Consider reuploading inconsistent bookmarks immediately after a sync

Status: RESOLVED DUPLICATE of bug 1482608

(Firefox :: Sync, enhancement, P2)

Description

[Lina Butler [:lina]]

The new bookmarks engine moves orphans to `unfiled`, doesn't check for `parentid` mismatches, and ignores issues like tombstoned GUIDs showing up in a folder's `children`. This was a compromise to match the old Desktop bookmarks engine, which does the same thing.

In theory, this is OK: we'll ignore deleted children, and, when we eventually see the missing parent, we'll move the orphan into it.

In practice, this can backfire if we also have local changes to unfiled. For example, if we've added a new bookmark A to unfiled locally, then see an incoming orphan B, we'll upload a record for unfiled that has A and B as children, even though B's real parent is one we haven't seen yet. B's `parentid` points to the real parent, but the new engine ignores `parentid`, and relies on the parent's `children`...so, when another new Desktop syncs, it'll think that B's real parent is unfiled.

I think Fennec will also try to reupload `unfiled`, but not B, to match its view of the world, but I'm not sure.

Some ideas:

1. Do nothing, at least for now. What we have now might be OK for most cases, and we can think about revisiting this once we have a shared sync implementation.

2. Leave orphans unmerged, keeping them out of the merged tree entirely. This quickly gets hairy: now the orphan doesn't appear to exist at all, we've intentionally diverged from the server, and the user doesn't know why the bookmark they added isn't showing up. Also, what happens if we moved an existing item between two folders, and failed to upload the new parent? That item is now an orphan, but already exists in the tree! Should we delete it and leave it unmerged in the mirror?

3. Flag the bookmark for (weak?) reupload, maybe in a separate batch. (We used to do separate batches, for one notion of weak uploads, but dropped that in bug 1337978, comment 23).

Now we've made the server and other clients consistent, albeit wrong. Batch uploads should make this less common, though my intuition is that it can still happen on a large first sync, where we split records up into multiple batches.

The benefit of (3) is that it's easy for the user to notice and fix, since that order will be wrong _everywhere_, instead of just one or some of their devices. They'll see their bookmarks aren't in the right place, and move them back, after giving Firefox the side-eye. >.>

The danger is getting into a sync loop, where each client thinks its structure is authoritative, and goes back and forth reuploading the same records. This would be even harder for the user to detect and fix. If Sync also undoes their attempt to move their bookmarks back to the right place, we'll be worse off than we are now.

Maybe the right thing to do here is a mix of 1 and 3? Wait until we have a single bookmark syncing implementation, then reupload inconsistent bookmarks, and force everyone to do a full sync to take care of inconsistencies already on the server. The advantage of doing this sooner is that everyone will need to do a full sync when we turn new bookmarks on, anyway. But maybe we can't do this safely yet.

Comment 1

[Lina Butler [:lina]]

Attachment: Bug 1515784 - Fix up inconsistent bookmarks at sync time.

Comment 3

[Lina Butler [:lina]]

Rolling this up into bug 1482608 (see https://github.com/mozilla/dogear/pull/19, too).