Closed
Bug 1394525
Opened 7 years ago
Closed 7 years ago
Don't reset the tracker score after syncing
Categories
(Firefox :: Sync, enhancement, P1)
Firefox
Sync
Tracking
()
RESOLVED
FIXED
Firefox 57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: lina, Assigned: lina)
References
Details
Attachments
(1 file)
I noticed two issues in `test_engine_changes_during_sync.js#test_bookmark_change_during_sync`: * The Bugzilla bookmark is in `menu` locally, but has a `parentid` of `toolbar` remotely. We should either change its `parentid`, or add a remote `toolbar` record. This isn't currently an issue because we treat the `parentid` as canonical, but it's not technically correct, and will cause validation failures once we switch to using the parent's `children` in bug 1305563. * We reset the score after syncing, meaning we'll sometimes clobber increments made during the sync. Since we already set `ignoreAll` before applying changes for the other engines, I think it's safe to do this at the same time we clear the changed IDs. * `_uploadOutgoing` is probably a better place to make changes during the sync than `recordHandler`, especially once we apply merged bookmark trees in a transaction, and pull changes before we're ready to upload.
Assignee | ||
Comment 1•7 years ago
|
||
(In reply to Kit Cambridge (he/him) [:kitcambridge] (UTC-7) from comment #0) > * `_uploadOutgoing` is probably a better place to make changes during the > sync than `recordHandler` s/recordHandler/applyIncomingBatch
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8901924 [details] Bug 1394525 - Reset tracker scores before syncing. https://reviewboard.mozilla.org/r/173350/#review178776 ::: services/sync/tests/unit/test_engine_changes_during_sync.js:384 (Diff revision 1) > await assertChildGuids(folder1.guid, [tbBmk.guid], > "Folder should have 1 child before first sync"); > > let pingsPromise = wait_for_pings(2); > > - let changes = await engine.pullNewChanges(); > + let changes = await PlacesSyncUtils.bookmarks.pullChanges(); These are currently equivalent, but will be different in bug 1305563, hence the change. :-)
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8901924 [details] Bug 1394525 - Reset tracker scores before syncing. https://reviewboard.mozilla.org/r/173350/#review178838 looks great, thanks!
Attachment #8901924 -
Flags: review?(markh) → review+
Comment hidden (mozreview-request) |
Pushed by kcambridge@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/439bc21d3947 Reset tracker scores before syncing. r=markh
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/439bc21d3947
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
You need to log in
before you can comment on or make changes to this bug.
Description
•