Remove many unused ChromeUtils.imports

RESOLVED FIXED in Firefox 63

Status

()

enhancement
RESOLVED FIXED
10 months ago
10 months ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

unspecified
mozilla63
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(4 attachments)

Assignee

Description

10 months ago
In bug 1478305 I am intending on ESLint making `ChromeUtils.import` imports be marked as real variable definitions (where a module exports a single symbol).

With the patch on that bug, there are currently around 237 places where imports are flagged by no-unused-vars as unnecessary.

The patches I'm attaching here will fix most of them (the easy ones). There's a few harder ones that need a bit more investigation / different bugs.
Assignee

Comment 1

10 months ago
Note: Removing most of these should be safe. I've tried to limit them to just jsm files or test files, and not frame scripts etc (I'll deal with frame scripts later). ESLint would pick up if there's anything missing via the no-undef rule.

Also in a few places, I've removed unnecessary export definitions or other ESLint definitions (e.g. duplicate no-unused-vars rules).
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 10

10 months ago
mozreview-review
Comment on attachment 8994789 [details]
Bug 1478308 - Remove unnecessary ChromeUtils.imports in accessible/

https://reviewboard.mozilla.org/r/259328/#review266480
Attachment #8994789 - Flags: review?(surkov.alexander) → review+

Comment 11

10 months ago
mozreview-review
Comment on attachment 8994786 [details]
Bug 1478308 - Remove unnecessary ChromeUtils.imports in browser/

https://reviewboard.mozilla.org/r/259322/#review266484

LGTM, thanks! Anything else than XPCOMUtils.jsm and Services.jsm has the potential to provide perf wins. Regardless,
Attachment #8994786 - Flags: review?(mdeboer) → review+

Comment 12

10 months ago
mozreview-review
Comment on attachment 8994788 [details]
Bug 1478308 - Remove unnecessary ChromeUtils.imports in toolkit/

https://reviewboard.mozilla.org/r/259326/#review266494

This nicely lessens our copy-pasting surface of superfluous code. Thanks!

::: toolkit/components/telemetry/tests/unit/TelemetryArchiveTesting.jsm
(Diff revision 2)
>  ChromeUtils.import("resource://gre/modules/TelemetryArchive.jsm");
> -ChromeUtils.import("resource://testing-common/Assert.jsm");

Guck?! Well, I hope noone actually asserted anything in this file...
Attachment #8994788 - Flags: review?(mdeboer) → review+

Comment 13

10 months ago
mozreview-review
Comment on attachment 8994787 [details]
Bug 1478308 - Remove unnecessary ChromeUtils.imports in services/

https://reviewboard.mozilla.org/r/259324/#review266366

Thanks! (I forgot to hit "publish" earlier :-O).
Attachment #8994787 - Flags: review?(lina) → review+

Comment 14

10 months ago
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/32a27f317a77
Remove unnecessary ChromeUtils.imports in browser/ r=mikedeboer
https://hg.mozilla.org/integration/autoland/rev/0e4ba7a6dc1a
Remove unnecessary ChromeUtils.imports in services/ r=lina
https://hg.mozilla.org/integration/autoland/rev/c68131530742
Remove unnecessary ChromeUtils.imports in toolkit/ r=mikedeboer
https://hg.mozilla.org/integration/autoland/rev/a809b45ff49b
Remove unnecessary ChromeUtils.imports in accessible/ r=surkov
There were also xpcshell failures on AttributionCode.jsm
Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=190206214&repo=autoland&lineNumber=10457
Assignee

Comment 17

10 months ago
Ah, clash with bug 1344771 landing. I'll drop the change for that file.
Flags: needinfo?(standard8)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 22

10 months ago
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ef9af969d02f
Remove unnecessary ChromeUtils.imports in browser/ r=mikedeboer
https://hg.mozilla.org/integration/autoland/rev/111912a07886
Remove unnecessary ChromeUtils.imports in services/ r=lina
https://hg.mozilla.org/integration/autoland/rev/1f452c6be0d7
Remove unnecessary ChromeUtils.imports in toolkit/ r=mikedeboer
https://hg.mozilla.org/integration/autoland/rev/2307557b66b1
Remove unnecessary ChromeUtils.imports in accessible/ r=surkov
You need to log in before you can comment on or make changes to this bug.