Closed Bug 996448 Opened 10 years ago Closed 10 years ago

Lazify the importing of PluralForm.jsm everywhere

Categories

(Core :: Internationalization, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: n.nethercote, Assigned: n.nethercote)

References

Details

Attachments

(1 file)

We can import Pluralform.jsm lazily.
Blocks: 986323
This patch makes all the imports of PluralForm.js lazy. I've confirmed that it
removes the creation of a compartment at start-up. (It was the webrtcUI.jsm one
that was responsible for the initialization at start-up.)

Also, I replaced some ad hoc lazy getters for PluralForm in 
toolkit/mozapps/downloads/DownloadUtils.jsm and 
browser/base/content/browser.js.

mak, please pass on to another reviewer if you can think of someone more suitable.
Attachment #8406666 - Flags: review?(mak77)
Comment on attachment 8406666 [details] [diff] [review]
Lazify the loading of PluralForm.jsm everywhere

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

::: browser/base/content/browser.js
@@ +79,5 @@
>    return this.AddonManager = val;
>  });
>  
> +XPCOMUtils.defineLazyModuleGetter(this, "PluralForm",
> +  "resource://gre/modules/PluralForm.jsm");

the __defineSetter__ was added originally in bug 394759 for add-ons compatibility, cause __defineGetter__ was causing any call to Cu.import for the same property to throw. Though, I just tried in Scratchpad and looks like I can't reproduce any kind of failure anymore, especially with defineLazyModuleGetter, so I think the behavior changed from 2009 to now (I found some related bug in 2012 but not directly the one changing behavior or explaining this). I think it's fine to remove it, as you did.

::: browser/devtools/styleeditor/StyleEditorUI.jsm
@@ +26,5 @@
>  const require = Cu.import("resource://gre/modules/devtools/Loader.jsm", {}).devtools.require;
>  const { PrefObserver, PREF_ORIG_SOURCES } = require("devtools/styleeditor/utils");
>  
> +XPCOMUtils.defineLazyModuleGetter(this, "PluralForm",
> +                                  "resource://gre/modules/PluralForm.jsm");

nit: I'd probably move this above the "const require" line, cause that's another "standard" for importing, different from Cu.import (defineLazyModuleGetter is just about the same as Cu.import, just lazy, so keeping them close each other seems right).
Attachment #8406666 - Flags: review?(mak77) → review+
https://hg.mozilla.org/mozilla-central/rev/b747c35ac54b
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.