Don't import gDevTools.jsm until needed

RESOLVED FIXED in Firefox 53

Status

()

Toolkit
WebExtensions: Developer Tools
RESOLVED FIXED
11 months ago
11 months ago

People

(Reporter: kmag, Assigned: kmag)

Tracking

unspecified
mozilla54
Points:
---

Firefox Tracking Flags

(firefox53 fixed, firefox54 fixed)

Details

MozReview Requests

()

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

Attachments

(1 attachment)

This causes a non-trivial startup performance hit.
Comment hidden (mozreview-request)

Comment 2

11 months ago
mozreview-review
Comment on attachment 8829625 [details]
Bug 1333201: Don't import gDevTools.jsm before necessary.

https://reviewboard.mozilla.org/r/106646/#review107842

r+

Thanks, I agree, it makes completely sense to don't import anything from the devtools internals until a devtools_page is actually used (and I'm going to take a second look to the other patches related to the devtools to be sure that we are going to do the same where needed).

We should also create an uplift request for this patch, this part is already landed in Firefox 53 and it would be better to fix this there too.
Attachment #8829625 - Flags: review?(lgreco) → review+

Comment 3

11 months ago
Pushed by maglione.k@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/386d5a9d293f
Don't import gDevTools.jsm before necessary. r=rpl
Comment on attachment 8829625 [details]
Bug 1333201: Don't import gDevTools.jsm before necessary.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1291737.

[User impact if declined]: This adds non-trivial performance overhead to startup when loading WebExtensions that don't use the devtools API.

[Is this code covered by automated tests?]: Yes.

[Has the fix been verified in Nightly?]: N/A
[Needs manual test from QE? If yes, steps to reproduce]: N/A

[List of other uplifts needed for the feature/fix]: None.

[Is the change risky?]: Very low-risk.
[Why is the change risky/not risky?]: This change simply delays running certain initialization code until it's necessary.

[String changes made/needed]: None.
Attachment #8829625 - Flags: approval-mozilla-aurora?

Comment 5

11 months ago
I had to back this out for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=71613738&repo=autoland

https://hg.mozilla.org/integration/autoland/rev/37ab612ff969708fe1c18c89bcfc387452343db6
Flags: needinfo?(kmaglione+bmo)
https://hg.mozilla.org/integration/mozilla-inbound/rev/d7ad91f903facea2a0ff088c14d6ca4a501fb509
Bug 1333201: Don't import gDevTools.jsm before necessary. r=rpl
Flags: needinfo?(kmaglione+bmo)

Comment 7

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d7ad91f903fa
Status: NEW → RESOLVED
Last Resolved: 11 months ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54

Updated

11 months ago
status-firefox53: --- → affected

Comment 8

11 months ago
Comment on attachment 8829625 [details]
Bug 1333201: Don't import gDevTools.jsm before necessary.

Fix a startup perf. Aurora53+.
Attachment #8829625 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 9

11 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/3f9f37f1bcf8
status-firefox53: affected → fixed
You need to log in before you can comment on or make changes to this bug.