1464552 - Split content.js into multiple lazily-loaded modules

Status: RESOLVED FIXED

(Firefox :: General, enhancement, P1)

Description

[Kris Maglione [:kmag]]

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.

Comment 1

[Kris Maglione [:kmag]]

Apparently some of this has already been done, and landed earlier today, so I am presumably going to conflict with someone else.

Comment 2

[Kris Maglione [:kmag]]

Attachment: Bug 1464552: Part 1 - Split net and cert error utils into separate JSM.

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

Comment 3

[Kris Maglione [:kmag]]

Attachment: Bug 1464552: Part 2 - Split blocked site handler into separate JSM.

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

Comment 4

[Kris Maglione [:kmag]]

Attachment: Bug 1464552: Part 3 - Split print preview helpers into separate JSM.

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

Comment 5

[Kris Maglione [:kmag]]

Attachment: Bug 1464552: Part 4 - Split selection source helpers into separate JSM.

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

Comment 6

[Mike Conley (:mconley) (:⚙️)]

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 7

[:Felipe Gomes (needinfo for replies!)]

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

Comment 8

[:Felipe Gomes (needinfo for replies!)]

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)

Comment 9

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8980825 [details]
Bug 1464552: Part 3 - Split print preview helpers into separate JSM.

https://reviewboard.mozilla.org/r/247010/#review253876

Comment 10

[:Felipe Gomes (needinfo for replies!)]

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

Comment 11

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fc197074cafbdaf86b4a17e3e2d182675ad22885
Bug 1464552: Part 1 - Split net and cert error utils into separate JSM. r=felipe

https://hg.mozilla.org/integration/mozilla-inbound/rev/885cc804f0e0eae8532dc9cbefb7d919d7c13d44
Bug 1464552: Part 2 - Split blocked site handler into separate JSM. r=felipe

https://hg.mozilla.org/integration/mozilla-inbound/rev/c10662f1a5b4b83752852363ec094fde349e83da
Bug 1464552: Part 3 - Split print preview helpers into separate JSM. r=felipe

https://hg.mozilla.org/integration/mozilla-inbound/rev/fbb3ac47702ca89c99162a9cb9ad74b58a1c8395
Bug 1464552: Part 4 - Split selection source helpers into separate JSM. r=felipe

Comment 12

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/fc197074cafb
https://hg.mozilla.org/mozilla-central/rev/885cc804f0e0
https://hg.mozilla.org/mozilla-central/rev/c10662f1a5b4
https://hg.mozilla.org/mozilla-central/rev/fbb3ac47702c

Comment 13

[Ionuț Goldan [:igoldan]]

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