Closed
Bug 414461
Opened 15 years ago
Closed 15 years ago
Delay importing of PluralForm and DownloadUtils for statusbar
Categories
(Toolkit :: Downloads API, defect)
Toolkit
Downloads API
Tracking
()
RESOLVED
FIXED
mozilla1.9beta3
People
(Reporter: Mardak, Assigned: Mardak)
References
Details
(Keywords: perf)
Attachments
(1 file)
1.28 KB,
patch
|
asaf
:
review+
damons
:
approval1.9+
|
Details | Diff | Splinter Review |
This might be the cause of Ts issues because both modules import some strings.
Comment 1•15 years ago
|
||
I thought they weren't loaded until they were needed?
Assignee | ||
Comment 2•15 years ago
|
||
Assignee | ||
Comment 3•15 years ago
|
||
(In reply to comment #1) > I thought they weren't loaded until they were needed? DownloadUtils initializes gStr at load time. I suppose potentially we could delay load.. something like instead of.. let gStr = { strings we want }; /* loadStringsFromBundle(); */ let DownloadUtils = { foo: function() gStr.stringIWant }; We create a new private object (so that we don't pollute DownloadUtils object) with a member variable that loads stuff on first use.. let _ = { get str() { delete this.str; let strings = { strings we want }; /* loadStringsFromBundle(); */ return this.str = strings; } }; let DownloadUtils = { foo: function() _.str.stringIWant };
Comment 4•15 years ago
|
||
you could also just make gStr an object that builds the strings as needed, right?
Assignee | ||
Comment 5•15 years ago
|
||
Right, but that would require programatically creating getters on the gStr object. gStr.__defineGetter("paused", function() { delete gStr.paused; strbundleblahblah; return gStr.paused = realstring; }); So we would need to iterate over each string and create a getter. There's also the overhead of getting the string bundle. I'll try something...
Comment 6•15 years ago
|
||
what about: gStr = { get paused() { delete this.paused; ... this.paused = realstring; } };
Assignee | ||
Comment 7•15 years ago
|
||
Too much duplicate code :p you have that whole code block copy/pasted to each. Don't worry! I've got something ;)
Comment 8•15 years ago
|
||
Comment on attachment 299862 [details] [diff] [review] v1 I think we should fix gStr not to init itself in the global scope anyway. r=mano though.
Attachment #299862 -
Flags: review?(mano) → review+
Assignee | ||
Comment 9•15 years ago
|
||
Comment on attachment 299862 [details] [diff] [review] v1 Hopefully this will help reduce Ts.
Attachment #299862 -
Flags: approval1.9?
Assignee | ||
Comment 10•15 years ago
|
||
(In reply to comment #8) > (From update of attachment 299862 [details] [diff] [review]) > I think we should fix gStr not to init itself in the global scope anyway. Filed bug 414486.
Comment 11•15 years ago
|
||
Comment on attachment 299862 [details] [diff] [review] v1 a1.9+=damons
Attachment #299862 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Comment 12•15 years ago
|
||
Checking in browser/base/content/browser.js; /cvsroot/mozilla/browser/base/content/browser.js,v <-- browser.js new revision: 1.945; previous revision: 1.944 done
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Flags: in-testsuite-
Flags: in-litmus-
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 M11
Updated•15 years ago
|
Flags: blocking-firefox3? → blocking-firefox3+
Updated•15 years ago
|
Product: Firefox → Toolkit
You need to log in
before you can comment on or make changes to this bug.
Description
•