Investigate if we can load ExtensionContent.jsm lazily

RESOLVED FIXED in Firefox 55

Status

()

Toolkit
WebExtensions: General
P1
normal
RESOLVED FIXED
5 months ago
5 days ago

People

(Reporter: krizsa, Assigned: kmag)

Tracking

(Blocks: 4 bugs)

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [e10s-multi:+] triaged)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(6 attachments, 2 obsolete attachments)

To improve content process startup I'm trying to load a few things lazily at startup if it's possible. I wonder if we need everything from ExtensionContent.jsm right from startup or is there anything we can load lazily.
Blocks: 1317304
Whiteboard: [e10s-multi:?]
(Assignee)

Comment 1

5 months ago
We can probably load most of it lazily, yeah. We'd at least need some skeleton bootstrap code loaded from the start, though.

Updated

5 months ago
Priority: -- → P1
Whiteboard: [e10s-multi:?] → [e10s-multi:?] triaged

Updated

5 months ago
Assignee: nobody → kmaglione+bmo
Blocks: 1336380
Whiteboard: [e10s-multi:?] triaged → [e10s-multi:+] triaged
(Assignee)

Updated

24 days ago
Duplicate of this bug: 1352541
Blocks: 1350472
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

17 days ago
Attachment #8855629 - Attachment is obsolete: true
Attachment #8855629 - Flags: review?(mixedpuppy)
(Assignee)

Comment 21

16 days ago
After these changes, sessionrestore overhead is down to somewhere between 1 and 10ms with a simple content script add-on installed, and about 10ms faster than the previous baseline for no_autorestore, since we previously loaded ExtensionContent.jsm even when no extension was installed. ts_paint overhead is likewise down to ~4-20ms (depending on Linux vs. win32), and about a 10-20ms improvement over the previous baseline in e10s. tp5o is about the same as the previous baseline and tp5o responsiveness is improved by ~2-5%.

https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-inbound&originalRevision=21062cd63dd72565a4f9afc35bb4f6f78726096c&newProject=try&newRevision=d381550636c72215f54f9f1bb35a7df450885b55&framework=1&filter=session&showOnlyImportant=0
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 29

14 days ago
mozreview-review
Comment on attachment 8855627 [details]
Bug 1317697: Fix reference to undefined property warnings.

https://reviewboard.mozilla.org/r/127488/#review131046

logic isn't right
Attachment #8855627 - Flags: review?(mixedpuppy) → review-

Comment 30

14 days ago
mozreview-review
Comment on attachment 8855628 [details]
Bug 1317697: Fix handling of more canceled responses.

https://reviewboard.mozilla.org/r/127490/#review131048

The commit message says this fixes something, but the patch merely adds channelId.
Attachment #8855628 - Flags: review?(mixedpuppy) → review-
(Assignee)

Comment 31

14 days ago
mozreview-review-reply
Comment on attachment 8855628 [details]
Bug 1317697: Fix handling of more canceled responses.

https://reviewboard.mozilla.org/r/127490/#review131048

Correct. Adding the channel ID fixes the canceled response handling for those objects.

Comment 32

14 days ago
mozreview-review
Comment on attachment 8855630 [details]
Bug 1317697: Lazily initialize Schemas.jsm.

https://reviewboard.mozilla.org/r/127494/#review131060
Attachment #8855630 - Flags: review?(mixedpuppy) → review+

Comment 33

14 days ago
mozreview-review
Comment on attachment 8855648 [details]
Bug 1317697: Remove things from ExtensionUtils that don't belong there.

https://reviewboard.mozilla.org/r/127512/#review131064
Attachment #8855648 - Flags: review?(mixedpuppy) → review+

Comment 34

14 days ago
mozreview-review
Comment on attachment 8855951 [details]
Bug 1317697: Update initial process data after permission changes.

https://reviewboard.mozilla.org/r/127828/#review131074

Tests for this are difficult to say the least, r+ with a followup bug to look into that.
Attachment #8855951 - Flags: review?(mixedpuppy) → review+
(In reply to Kris Maglione [:kmag] from comment #31)
> Comment on attachment 8855628 [details]
> Bug 1317697: Fix handling of more canceled responses.
> 
> https://reviewboard.mozilla.org/r/127490/#review131048
> 
> Correct. Adding the channel ID fixes the canceled response handling for
> those objects.

Is there a test that currently covers that (and which one?), if not can we add one?
(Assignee)

Comment 36

14 days ago
(In reply to Shane Caraveo (:mixedpuppy) from comment #35)
> (In reply to Kris Maglione [:kmag] from comment #31)
> > Correct. Adding the channel ID fixes the canceled response handling for
> > those objects.
> 
> Is there a test that currently covers that (and which one?), if not can we
> add one?

Not really. This is mostly meant to reduce confusing noise in tests, so it's not meant to be perfect, and it would be hard to test reliably. I could add a test for it, but if it breaks, people will notice the extra noise in their tests pretty quickly.
(Assignee)

Updated

14 days ago
Blocks: 1355221

Comment 37

14 days ago
mozreview-review
Comment on attachment 8855628 [details]
Bug 1317697: Fix handling of more canceled responses.

https://reviewboard.mozilla.org/r/127490/#review131092

Kris explained on irc, this property is checked has a part of abortedResponses, but was missing in these cases.
Attachment #8855628 - Flags: review- → review+

Comment 38

14 days ago
mozreview-review
Comment on attachment 8855631 [details]
Bug 1317697: Split ExtensionContent.jsm into a stub process script.

https://reviewboard.mozilla.org/r/127496/#review131128

::: toolkit/components/extensions/test/mochitest/test_ext_contentscript_cache.html:70
(Diff revision 5)
>      chromeScript = SpecialPowers.loadChromeScript(() => {
>        addMessageListener("check-script-cache", extensionId => {
>          let {ExtensionManager} = Components.utils.import("resource://gre/modules/ExtensionContent.jsm", {});
>          let ext = ExtensionManager.extensions.get(extensionId);
>  
> +        if (ext) {

Why wouldn't there be an extension here?
(Assignee)

Comment 39

14 days ago
mozreview-review-reply
Comment on attachment 8855631 [details]
Bug 1317697: Split ExtensionContent.jsm into a stub process script.

https://reviewboard.mozilla.org/r/127496/#review131128

> Why wouldn't there be an extension here?

Because we create the extension instance lazily the first time it's needed, and it's not actually needed in the parent process in this test.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

11 days ago
Attachment #8855951 - Attachment is obsolete: true

Comment 46

11 days ago
mozreview-review
Comment on attachment 8855627 [details]
Bug 1317697: Fix reference to undefined property warnings.

https://reviewboard.mozilla.org/r/127488/#review132726
Attachment #8855627 - Flags: review?(mixedpuppy) → review+

Comment 47

10 days ago
mozreview-review
Comment on attachment 8855631 [details]
Bug 1317697: Split ExtensionContent.jsm into a stub process script.

https://reviewboard.mozilla.org/r/127496/#review133018
Attachment #8855631 - Flags: review?(mixedpuppy) → review+

Comment 48

10 days ago
mozreview-review
Comment on attachment 8855632 [details]
Bug 1317697: Split extension page child and content script child code as much as possible.

https://reviewboard.mozilla.org/r/127498/#review133020
Attachment #8855632 - Flags: review?(mixedpuppy) → review+
(Assignee)

Comment 49

10 days ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/9504ce3f418c3df975dcd142b98993448a4e8b1e
Bug 1317697: Fix reference to undefined property warnings. r=mixedpuppy

https://hg.mozilla.org/integration/mozilla-inbound/rev/ae35fa9b3e383a8c83809df8d3a40ba7af259201
Bug 1317697: Fix handling of more canceled responses. r=mixedpuppy

https://hg.mozilla.org/integration/mozilla-inbound/rev/06724365d05824cd380c34f38fccf93dbf7cc9ed
Bug 1317697: Lazily initialize Schemas.jsm. r=mixedpuppy

https://hg.mozilla.org/integration/mozilla-inbound/rev/1267d47eca93f2fc099cdc71ae9ba2bf2b87252f
Bug 1317697: Split ExtensionContent.jsm into a stub process script. r=mixedpuppy

https://hg.mozilla.org/integration/mozilla-inbound/rev/0d80e105e2b5904fe882aaafe6b3f65b2dc04e75
Bug 1317697: Split extension page child and content script child code as much as possible. r=mixedpuppy

https://hg.mozilla.org/integration/mozilla-inbound/rev/895164016ae91086fe04e1f5cafbaa3e46f3805d
Bug 1317697: Remove things from ExtensionUtils that don't belong there. r=mixedpuppy
(Assignee)

Comment 50

9 days ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/05f0e6fea003df5c3d81fd2254486394ef327cf7
Bug 1317697: Follow-up: Fix android test bustage. r=me
https://hg.mozilla.org/mozilla-central/rev/9504ce3f418c
https://hg.mozilla.org/mozilla-central/rev/ae35fa9b3e38
https://hg.mozilla.org/mozilla-central/rev/06724365d058
https://hg.mozilla.org/mozilla-central/rev/1267d47eca93
https://hg.mozilla.org/mozilla-central/rev/0d80e105e2b5
https://hg.mozilla.org/mozilla-central/rev/895164016ae9
https://hg.mozilla.org/mozilla-central/rev/05f0e6fea003
Status: NEW → RESOLVED
Last Resolved: 9 days ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
(Assignee)

Updated

7 days ago
Depends on: 1357137
Blocks: 1355569
You need to log in before you can comment on or make changes to this bug.