Closed Bug 1013915 Opened 6 years ago Closed 6 years ago

Clean up module imports/ dependencies and use lazy getters where possible

Categories

(Firefox :: Translation, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 32

People

(Reporter: mikedeboer, Assigned: mikedeboer)

References

Details

(Whiteboard: [translation] p=1 s=it-32c-31a-30b.2 [qa-])

Attachments

(1 file, 1 obsolete file)

It'd be worthwhile to go through the module imports in `browser/components/translation/` to clean up and remove unused dependencies.
Marco, can you add this to the current iteration?
Flags: needinfo?(mmucci)
Flags: firefox-backlog+
Comment on attachment 8426196 [details] [diff] [review]
Patch v1: clean up module imports/ dependencies and use lazy getters where possible

Review of attachment 8426196 [details] [diff] [review]:
-----------------------------------------------------------------

The changes in Translation.jsm and TranslationDocument.jsm seem fine to me, although I doubt the lazy module getters really save us anything here (I would expect Services.jsm to have its compartment created near startup, and CommonUtils to be imported as soon as BingTranslator.jsm is used).

::: browser/components/translation/BingTranslator.jsm
@@ -7,5 @@
>  const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
>  
>  this.EXPORTED_SYMBOLS = [ "BingTranslation" ];
>  
> -Cu.import("resource://gre/modules/Services.jsm");

Services.jsm is indeed unused here.

@@ +12,4 @@
>  Cu.import("resource://gre/modules/Log.jsm");
> +
> +XPCOMUtils.defineLazyModuleGetter(this, "Promise",
> +                                  "resource://gre/modules/Promise.jsm");

I think these lazy getters add overhead here, as BingTranslator.jsm is imported only when we are about to perform a translation; not at startup.
Attachment #8426196 - Flags: review?(florian) → review+
Added to Iteration 32.2
Flags: needinfo?(mmucci)
Carrying over r=florian.

Pushed to fx-team as: https://hg.mozilla.org/integration/fx-team/rev/9f7d9d5c9261
Attachment #8426196 - Attachment is obsolete: true
Attachment #8426315 - Flags: review+
This landed with an incorrect bug number, FWIW.
https://hg.mozilla.org/mozilla-central/rev/9f7d9d5c9261
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
Status: RESOLVED → VERIFIED
Whiteboard: p=1 s=it-32c-31a-30b.2 [qa-] → [translation] p=1 s=it-32c-31a-30b.2 [qa-]
You need to log in before you can comment on or make changes to this bug.