1425690 - Avoid getCharPref in browser-sync.js

Status: RESOLVED FIXED

(Firefox :: Sync, enhancement, P1)

Description

[Thom Chiovoloni [:tcsc] (ex-moco)]

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 1

[Thom Chiovoloni [:tcsc] (ex-moco)]

Attachment: Bug 1425690 - Replace getCharPref with lazy pref getters in browser-sync.js

Review commit: https://reviewboard.mozilla.org/r/207984/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/207984/

Comment 2

[Thom Chiovoloni [:tcsc] (ex-moco)]

Comment on attachment 8937285 [details]
Bug 1425690 - Replace getCharPref with lazy pref getters in browser-sync.js

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/207984/diff/1-2/

Comment 3

[Thom Chiovoloni [:tcsc] (ex-moco)]

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

Comment 4

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

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?

Comment 5

[Thom Chiovoloni [:tcsc] (ex-moco)]

> 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

[Edouard Oger [:eoger]]

Comment on attachment 8937285 [details]
Bug 1425690 - Replace getCharPref with lazy pref getters in browser-sync.js

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

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e8d7f5f691c6
Replace getCharPref with lazy pref getters in browser-sync.js r=eoger

Comment 8

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/e8d7f5f691c6