Split content.js into multiple lazily-loaded modules

RESOLVED FIXED in Firefox 62

Status

()

enhancement
P1
normal
RESOLVED FIXED
Last year
Last year

People

(Reporter: kmag, Assigned: kmag)

Tracking

(Blocks 1 bug)

57 Branch
Firefox 62
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox62 fixed)

Details

Attachments

(4 attachments)

content.js is currently loaded into every content frameloader, and creates tens of KB of memory overhead for a lot of functionality which is rarely needed. This should be split into several modules, which the functionality of each being loaded when first needed.
Apparently some of this has already been done, and landed earlier today, so I am presumably going to conflict with someone else.
Attachment #8980823 - Flags: review?(mconley) → review?(felipc)
Attachment #8980824 - Flags: review?(mconley) → review?(felipc)
Attachment #8980825 - Flags: review?(mconley) → review?(felipc)
Attachment #8980826 - Flags: review?(mconley) → review?(felipc)
So, upon reflection, I think it might be wiser to have felipe look at these. I think he has a better sense of the direction that things are headed (or should be headed) for frame script splitting and lazi-fication.
Comment on attachment 8980823 [details]
Bug 1464552: Part 1 - Split net and cert error utils into separate JSM.

https://reviewboard.mozilla.org/r/247006/#review253870

We put all of the front-end modules in browser/modules, so instead of creating this new file in browser/base/content, please put it there.

Same goes for the other patches in the queue. On the toolkit side, there's the toolkit/modules equivalent

::: browser/base/content/NetErrorContent.jsm:6
(Diff revision 1)
>  /* This content script should work in any browser or iframe and should not
>   * depend on the frame being contained in tabbrowser. */

nit: remove this comment
Attachment #8980823 - Flags: review?(felipc) → review+
Comment on attachment 8980824 [details]
Bug 1464552: Part 2 - Split blocked site handler into separate JSM.

https://reviewboard.mozilla.org/r/247008/#review253874

::: browser/base/content/BlockedSiteContent.jsm:6
(Diff revision 1)
>  /* This content script should work in any browser or iframe and should not
>   * depend on the frame being contained in tabbrowser. */

(same nit, as this comment doesn't apply anymore)
Attachment #8980824 - Flags: review?(felipc) → review+
Comment on attachment 8980825 [details]
Bug 1464552: Part 3 - Split print preview helpers into separate JSM.

https://reviewboard.mozilla.org/r/247010/#review253876
Attachment #8980825 - Flags: review?(felipc) → review+
Comment on attachment 8980826 [details]
Bug 1464552: Part 4 - Split selection source helpers into separate JSM.

https://reviewboard.mozilla.org/r/247012/#review253878

This could use defineLazyProxy to make it even cleaner in browser-content.js, which was built exactly for this use case

`XPCOMUtils.defineLazyProxy(this, "ViewSelectionSource", "resource://...");`
`addMessageListener("ViewSource:GetSelection", ViewSelectionSource);`

And then you can replace the global with `message.target`.

I guess the same goes for the PrintPreview patch, but that has the added trouble of having to extract the mm from the event target.. (which isn't too hard)

up to you to decide on the style
Attachment #8980826 - Flags: review?(felipc) → review+
Priority: -- → P1
Perf wins from either this bug or bug 1464548:

== Change summary for alert #13746 (as of Thu, 07 Jun 2018 21:33:41 GMT) ==

Improvements:

  2%  cpstartup content-process-startup windows7-32 opt e10s stylo     176.83 -> 172.83
  2%  cpstartup content-process-startup windows10-64-qr opt e10s stylo 166.08 -> 162.58
  2%  cpstartup content-process-startup windows10-64 pgo e10s stylo    160.42 -> 157.17
  2%  cpstartup content-process-startup windows10-64 opt e10s stylo    167.00 -> 164.17

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=13746
You need to log in before you can comment on or make changes to this bug.