Avoid getCharPref in browser-sync.js

RESOLVED FIXED in Firefox 59

Status

()

enhancement
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: tcsc, Assigned: tcsc)

Tracking

(Blocks 1 bug)

unspecified
Firefox 59
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox59 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

2 years ago
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 hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee

Comment 3

2 years ago
mozreview-review
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?
Assignee

Comment 5

2 years ago
> 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 6

2 years ago
mozreview-review
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+

Comment 7

2 years ago
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
Assignee

Updated

2 years ago
See Also: → 1425960
Priority: -- → P1

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e8d7f5f691c6
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Assignee

Updated

2 years ago
Blocks: 1426480
You need to log in before you can comment on or make changes to this bug.