Bug 607112 will introduce GUID columns in moz_places and moz_bookmarks proper. Places will do an initial migration that will port over our old GUIDs to new ones. Sync will have to handle this in a sane way that will work on Firefox 3.6 as well as on Firefox 4.0 as well as on profiles that have gone through the migration but users went back to 3.6. We also do not want to do a full reupload when the migration happens (since it could happen multiple times as users go back and forth between Firefox versions on the same profile.) sdwilsh and I have come up with this plan: * The short bookmark GUIDs will be derived from the existing UUIDs. That way Sync can *always* compute the GUID on a 3.6 system. GUIDs will be the first 9 bytes (=18 hex characters) from the UUID in base64url-encoded form. Sync should do this now so that we're forward-compatible with the new Places GUIDs. * In addition we should switch Utils.makeGUID() over to the new GUID format (base64url encoded 9 random bytes = 12 characters) so that the history engine generates these new GUIDs already. Then when Places do their migration we end up with exactly the same GUIDs.

This should go into b8 if we want to time it with the global version bump. I'm also of the opinion that these changes also constitute a version bump for the bookmarks engine because it needs code to deal with the shorter GUIDs on all machines, even if they're on 3.6.

Can you highlight what kind of testing we'll need to be aware of here? Also, unit tests?

Unit tests: * Make sure we generate the same kind of GUIDs for history as Places will do * Make sure we derive the bookmark GUIDs from their UUIDs in exactly the same way as Places will do (will exchange test vectors with sdwilsh) * Make sure Sync recognizes both old and new Places DB schemas and uses the correct queries/APIs in both cases. * Ensure that a Places DB with the new schema continues to work on an older Firefox version. Particularly, ensure that an empty GUID column is dealt with correctly. This is not a problem for bookmarks as we continue to use the UUID and generate the GUID from that, but it needs to be dealt with for history. Q&A testing scenarios (Calling 4.0b8 the version that does NOT have GUIDs-in-places (bug 607112) and 4.0b9 the version that WILL have it... actual numbers might be different. All scenarios assume Sync is set up.) * A 4.0b8 profile is migrated to 4.0b9. Sync should continue to work without having to reupload all bookmarks. Changes to bookmarks should be synced to and from other computers as before, particularly those running Firefox 3.6 * A 4.0b8 profile is migrated to 4.0b9 and then back to 4.0b8. Bookmarks are changed and added under 4.0b8. Sync continues to work. Add more bookmarks, don't sync, upgrade to 4.0b9, sync continues to work and bookmarks are synced to other devices. At no point should all bookmarks be reuploaded.

(In reply to comment #3) > * Make sure we derive the bookmark GUIDs from their UUIDs in exactly the same > way as Places will do (will exchange test vectors with sdwilsh) > > * Make sure Sync recognizes both old and new Places DB schemas and uses the > correct queries/APIs in both cases. Tests are in bug 607112

Marking this as blocking beta8 due to irc conversation with mconnor (I think this is the right bug): (16:44) < mconnor> we want as forward compat for the places GUID stuff

(In reply to comment #5) > Marking this as blocking beta8 due to irc conversation with mconnor (I think > this is the right bug): > > (16:44) < mconnor> we want as forward compat for the places GUID stuff Yes. Thanks!

Making the scope of this bug smaller as the bookmark stuff is more involved and will land separately (will file separate bug for that).

Use a different annotation for history GUIDs so that new GUIDs will be generated for all history records. We'll do the clean up of old annotations in a follow-up.

Comment on attachment 493847 [details] [diff] [review] Part 1 (v1): Make Utils.makeGUID generate new style GUIDs >diff --git a/services/sync/tests/unit/test_utils_makeGUID.js b/services/sync/tests/unit/test_utils_makeGUID.js >+const key = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_"; This is base64url? Can we call it that, so it's clear this is not just a magic const. The comment about intermittent failures raises a question: what happens with collisions when we're adding GUIDs?

(In reply to comment #10) > The comment about intermittent failures raises a question: what happens with > collisions when we're adding GUIDs? Two browser objects will get the same GUID. Depending on the store implementation either one of them wins or they'll alternate randomly. The chance of that happening was always there. It was 1:2^62 before, it'll be 1:2^72 with the new GUIDs, so that's three orders of magnitude (2^10) better than it used to be.

Part 1: https://hg.mozilla.org/services/fx-sync/rev/9c574685c9b7 Part 2: https://hg.mozilla.org/services/fx-sync/rev/361a5337a326