59 bytes, text/x-review-board-request
It's currently 100 bookmarks. We may want to yield to the event loop in the validation code, to minimize jank. Ride this to Aurora.
We may also want to have validation ride all the way to Release.
Bump first, measure, then make a decision to ride the trains later.
Priority: -- → P1
Bumping the pref is a one-line change, but I noticed a bit of jank with my profile of ~2.4k bookmarks. I suspect we'll want to periodically yield to the event loop, just like we discussed at triage.
Comment on attachment 8826705 [details] Bug 1323867 - Bump bookmark validation threshold to 1k. https://reviewboard.mozilla.org/r/104606/#review105984 Looks good, but the conversion of some of those functions to generators will break TPS, and [aboutsync](https://github.com/mhammond/aboutsync)... Not breaking TPS is required IMO for landing this, but keeping aboutsync working is a "nice to have". If you do decide to handle aboutsync, it should be done in a way that won't break on older firefoxes. I believe you can check if a function is a generator via it's constructor property... e.g. `generator.constructor.name === "GeneratorFunction"`, although maybe there's a better way. This kind of sucks, admittedly.
Attachment #8826705 - Flags: review?(tchiovoloni)
Now that we can use async functions in production code, I changed the validator to use them instead of tasks. Still seeing a race in test_bookmark_conflict.js; I'll file a separate bug for that. The Sync ping schema change is also to fix TPS. I submitted a PR for About Sync here: https://github.com/mhammond/aboutsync/pull/23
Comment on attachment 8826705 [details] Bug 1323867 - Bump bookmark validation threshold to 1k. https://reviewboard.mozilla.org/r/104606/#review110430 lgtm
Attachment #8826705 - Flags: review?(tchiovoloni) → review+
Comment on attachment 8826705 [details] Bug 1323867 - Bump bookmark validation threshold to 1k. https://reviewboard.mozilla.org/r/104606/#review110926 ::: services/sync/modules/bookmark_validator.js:8 (Diff revision 2) > * You can obtain one at http://mozilla.org/MPL/2.0/. */ > > "use strict"; > > const Cu = Components.utils; > +const Timer = Components.Constructor("@mozilla.org/timer;1", "nsITimer"); As a meta-comment (as I don't really care!), note there is a Timer.jsm new code tends to use
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/2c3b1968e183 Bump bookmark validation threshold to 1k. r=tcsc
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/074d6b728c5c followup, placate eslint
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
You need to log in before you can comment on or make changes to this bug.