Closed
Bug 996448
Opened 11 years ago
Closed 11 years ago
Lazify the importing of PluralForm.jsm everywhere
Categories
(Core :: Internationalization, defect)
Core
Internationalization
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: n.nethercote, Assigned: n.nethercote)
References
Details
Attachments
(1 file)
(deleted),
patch
|
mak
:
review+
|
Details | Diff | Splinter Review |
We can import Pluralform.jsm lazily.
Assignee | ||
Comment 1•11 years ago
|
||
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 2•11 years ago
|
||
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+
Assignee | ||
Comment 3•11 years ago
|
||
Comment 4•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in
before you can comment on or make changes to this bug.
Description
•