Closed Bug 1385952 Opened 2 years ago Closed 2 months ago

Remove usage of Preferences.jsm from PushService and PushServiceWebsocket

Categories

(Firefox :: General, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 71
Tracking Status
firefox71 --- fixed

People

(Reporter: marco, Assigned: emalysz)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [photon] [fxperf:p2] [overhead:noted])

Attachments

(1 file)

In bug 1357517, we are removing/delaying Preferences.jsm usage until the "before handling user events" phase (the module is now loaded in this phase).

We should try to load it even later, possibly adding it to the "before becoming idle" blaklist.
Blocks: 1385954
The first step would be to move Preferences.jsm to be loaded "before becoming idle", in order to do that, we need to remove its usage from:
dom/push/PushService.jsm
dom/push/PushServiceWebSocket.jsm
browser/extensions/activity-stream/lib/ActivityStreamPrefs.jsm

The last one is a bit problematic, as it exposes a new class extending Preferences which is used by other ActivityStream code. This module is a really small wrapper around Preferences.jsm, I suppose we could get rid of it altogether (which would also help with bug 1350472).
Priority: -- → P3
Whiteboard: [photon]
Whiteboard: [photon] → [photon] [fxperf]
Whiteboard: [photon] [fxperf] → [photon] [fxperf:p2]
Whiteboard: [photon] [fxperf:p2] → [photon] [fxperf:p2] [overhead:noted]
Assignee: nobody → emalysz
See Also: → 1580610

Just documenting some discussion from elsewhere…

ActivityStreamPrefs does extend Preferences.jsm class mainly using .get() and .set() methods and ._prefBranch getter to expose .observeBranch() and .ignoreBranch() helpers similar to Preferences.jsm's .observe() but for all prefs instead of a single pref.

ActivityStream.jsm specifies a prefs object that contains various pref types, so the dynamic int vs string vs bool pref behavior is desired. But potentially this code could be removed in favor of using plain firefox.js prefs in the common case (while still specially changing behavior based on browser.search.region).

Overall, because this is quite a bit more work than simply inlining Preferences.get() -> Services.prefs.getStringPref(), the ActivityStreamPrefs changes probably want to be split out to a separate bug.

Attachment #9092231 - Attachment description: Bug 1385952, load preferences before becoming idle → Bug 1385952, Part 1: removing usage of Preferences.jsm from PushService and PushServiceWebsocket in order to load preferences before becoming idle
Depends on: 1583888
No longer depends on: 1583888
Summary: Stop loading Preferences.jsm module before becoming idle → Remove usage of Preferences.jsm from PushService and PushServiceWebsocket
Depends on: 1583888
Attachment #9092231 - Attachment description: Bug 1385952, Part 1: removing usage of Preferences.jsm from PushService and PushServiceWebsocket in order to load preferences before becoming idle → Bug 1385952, Remove usage of Preferences.jsm from PushService and PushServiceWebsocket in order to load preferences before becoming idle
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d7ab41d7d5f7
Remove usage of Preferences.jsm from PushService and PushServiceWebsocket in order to load preferences before becoming idle r=mconley
Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71
No longer depends on: 1583888
You need to log in before you can comment on or make changes to this bug.