617521 - Random bookmark reordering due to dupe handling

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Philipp von Weitershausen [:philikon]]

With the old bookmark GUIDs the bookmark engine preferred "lower" GUIDs for the dupe handling and depending on whether the incoming or existing record had the "lower" GUID, it would either change the local or the remote record. It then went through great lengths to make sure all other references to that GUID were also updated. In bug 615285 I have missed some edge cases in this code that cause random reordering for those dupes.

With the random GUIDs we have now, the current behaviour is pretty much useless. When dealing with bookmark dupes we may as well just always take the upstream GUID. This means that we can we get rid of the aliasing code as well because we accept the upstream GUIDs as the arbiter of truth always.

Comment 1

[Philipp von Weitershausen [:philikon]]

Attachment: v1

Comment 2

[Philipp von Weitershausen [:philikon]]

Comment on attachment 496075 [details] [diff] [review]
v1

I should explain this a bit: This patch makes us always take the incoming GUID for a dupe. This simplifies the code a lot because we don't have to worry about aliases, and we're less likely to run into inconsistencies with ordering.

The patch also fixes an unrelated reference error (Log4Moz).

I created a crossweave test that tests the duping. It fails now and passes with the patch applied.

Comment 3

[Mike Connor [:mconnor]]

Comment on attachment 496075 [details] [diff] [review]
v1

tests ftw!

Comment 4

[Philipp von Weitershausen [:philikon]]

https://hg.mozilla.org/services/fx-sync/rev/006789ffbfa3