Open Bug 1516525 Opened 5 years ago Updated 2 years ago

Download indicator code should use idle task and/or not load until there are downloads

Categories

(Firefox :: Downloads Panel, enhancement, P5)

enhancement

Tracking

()

Performance Impact medium

People

(Reporter: Gijs, Unassigned, NeedInfo)

References

Details

(Keywords: perf:frontend, perf:startup)

Attachments

(1 file)

Right now we run some indicator.js code onLoad, and some in delayed startup, for every browser window.

If the downloads indicator is autohidden (the default), we can avoid this at least until an idle task from delayed startup, potentially even just not loading things until there are downloads at all.
This avoids loading indicator.js and downloads.js until any of the following are true:
  - the user enters customize mode; or
  - the user has configured the downloads button to always be visible; or
  - a download starts that forces us to show the indicator.

To do this, we make DownloadsCommon expose whether there are any downloads,
and send a notification for the first one via the observer service.

We also allow windows to be aware if they're the first browser window, so that we can
optimize avoiding importing additional jsms on the initial startup and carry over
state for later ones (when the jsm will likely already have been loaded anyway).

P5 for the Downloads panel, but fxperf:p1 from a performance point of view as Gijs is working on it

Priority: -- → P5
Whiteboard: [fxperf] → [fxperf:p1]
No longer blocks: 1522877
See Also: → 1543537
Flags: needinfo?(mconley)

Hey Gijs, it's been a while since I looked at this... can I presume this bug and patch are still relevant? I note that my change requests in my review are what's blocking things here... I don't even remember what AppVariants are - do you know what I was referring to in https://phabricator.services.mozilla.com/D15449#421617 ?

Flags: needinfo?(mconley) → needinfo?(gijskruitbosch+bugs)

(In reply to Mike Conley (:mconley) (:⚙️) from comment #3)

Hey Gijs, it's been a while since I looked at this... can I presume this bug and patch are still relevant? I note that my change requests in my review are what's blocking things here... I don't even remember what AppVariants are - do you know what I was referring to in https://phabricator.services.mozilla.com/D15449#421617 ?

Similar to AppConstants but not constant. Like, stuff relevant to the entire app in one place. Avoiding having to defineLazyPrefGetter all over the shop, and loading a bunch of jsms, esp. when we could initialize the state to something sensible, and then update it once the relevant jsm actually is loaded. Pushed state rather than pulled, in that sense (ie, in this specific case, Downloads.jsm or similar would "push" the number of downloads, and other code could read it without having to pull in all of Downloads.jsm, which avoids doing so on startup).

Other parts of the codebase have done this already, to some extent - e.g. UrlbarPrefs.jsm is a URL bar specific version of this, even specific to prefs only.

I think the note from Paolo that we now load Downloads.jsm on startup when that was previously avoided made me think this area needed a more complete re-evaluation, and I just haven't made time for that because we've had other priorities and it was unclear what kind of wins (if any) we could expect from work here, while it was clear that it'd take a bunch of work and add some complexity to revamp things. Does that answer the question? :-)

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(mconley)

Realistically, not actively working on this at the moment.

Assignee: gijskruitbosch+bugs → nobody
Status: ASSIGNED → NEW
Whiteboard: [fxperf:p1] → [fxperf:p2]
Flags: needinfo?(shmediaproductions)
Performance Impact: --- → P2
Whiteboard: [fxperf:p2]
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: