1368608 - Bookmark de-duping behaves differently on a full sync compared to partial sync.

Status: RESOLVED DUPLICATE of bug 1305563

(Firefox :: Sync, enhancement, P3)

Description

[Mark Hammond [:markh] [:mhammond]]

If you change http://searchfox.org/mozilla-central/rev/31eec6f59eb303adf3318058cb30bd6a883115a5/services/sync/tests/unit/test_bookmark_duping.js#305 to read |engine.lastSync = engine.lastSync - 5000;|, the test fails.

It fails because it causes us to read all records rather than just that new one. The reason this causes failure is because our own existing item (which is marked as modified) gets reconciled, which removes that ID from the modified map (at http://searchfox.org/mozilla-central/rev/31eec6f59eb303adf3318058cb30bd6a883115a5/services/sync/modules/engines.js#1558)

Then, when we end up processing the duplicate record, we check to see if the local item is modified and find that it isn't - so we *do* apply the incoming record.

Without the changes mentioned above, the sync only sees 1 incoming record (the dupe), so correctly sees our local record as modified, so does not apply the incoming record (which is the whole point of the test).

So while that test is certainly suspect with how it mangles lastSync etc, it seems that we shouldn't behave differently - if it just so-happened that we hit this case after a node reassignment we'd do the wrong thing.

Interestingly/Suspiciously, if you comment out the line at engines.js#1558 (ie, don't remove the item from the changeset), then this test, and all other tests pass. So it's not clear why that line is necessary. Without that line, I think we *would* end up re-uploading our record, but it seems like we should anyway - the last-modified date changed.

Comment 1

[Lina Butler [:lina]]

Flagging myself to add a buffer test for this.

Comment 2

[Lina Butler [:lina]]

Done and landed. :-) I think it makes sense to press on with the mirror, and have it block older Desktops from applying their deduping via `hasDupe: true`, instead of figuring out how to make our current deduping less brittle.