Closed Bug 1425690 Opened 5 years ago Closed 5 years ago

Avoid getCharPref in browser-sync.js

Categories

(Firefox :: Sync, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 59
Tracking Status
firefox59 --- fixed

People

(Reporter: tcsc, Assigned: tcsc)

References

Details

Attachments

(1 file)

In bug 1425613 it seems we're one of the worst offenders. My guess is that it's almost entirely due to "services.sync.engine.tabs.filteredUrls", but I guess it doesn't hurt to remove the others. I have a patch that replaces these with properties defined by XPCOMUtils.defineLazyPreferenceGetter, and I'll upload it after double-checking it works.

I'd like to verify it fixes the issue before landing, however.
Comment on attachment 8937285 [details]
Bug 1425690 - Replace getCharPref with lazy pref getters in browser-sync.js

https://reviewboard.mozilla.org/r/207984/#review213802

::: commit-message-defcc:1
(Diff revision 1)
> +Bug 1425690 - Replace getCharPref with lazy pref getters in browser-sync.js r?eoger

Fake issue I should resolve after verifying that removes browser-sync.js from the list in 1425613
We do this regularly inside Sync itself, but that almost certainly isn't going to be picked up by Gijs's script - I guess we should try and do the same there?
> that almost certainly isn't going to be picked up by Gijs's script

Assuming it isn't extremely specific to the tests ran, it might worth doing a similar thing for syncing or sync tests (or maybe TPS?). We use prefs... very heavily.
Comment on attachment 8937285 [details]
Bug 1425690 - Replace getCharPref with lazy pref getters in browser-sync.js

https://reviewboard.mozilla.org/r/207984/#review213962
Attachment #8937285 - Flags: review?(eoger) → review+
Pushed by tchiovoloni@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e8d7f5f691c6
Replace getCharPref with lazy pref getters in browser-sync.js r=eoger
See Also: → 1425960
Priority: -- → P1
https://hg.mozilla.org/mozilla-central/rev/e8d7f5f691c6
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Blocks: 1426480
You need to log in before you can comment on or make changes to this bug.