Closed Bug 1478308 Opened 2 years ago Closed 2 years ago

Remove many unused ChromeUtils.imports

Categories

(Toolkit :: General, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: standard8, Assigned: standard8)

References

Details

Attachments

(4 files)

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.
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 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 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 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 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+
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
Ah, clash with bug 1344771 landing. I'll drop the change for that file.
Flags: needinfo?(standard8)
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.