Closed
Bug 414461
Opened 18 years ago
Closed 18 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•18 years ago
|
||
I thought they weren't loaded until they were needed?
| Assignee | ||
Comment 2•18 years ago
|
||
| Assignee | ||
Comment 3•18 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•18 years ago
|
||
you could also just make gStr an object that builds the strings as needed, right?
| Assignee | ||
Comment 5•18 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•18 years ago
|
||
what about:
gStr = {
get paused() {
delete this.paused;
...
this.paused = realstring;
}
};
| Assignee | ||
Comment 7•18 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•18 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•18 years ago
|
||
Comment on attachment 299862 [details] [diff] [review]
v1
Hopefully this will help reduce Ts.
Attachment #299862 -
Flags: approval1.9?
| Assignee | ||
Comment 10•18 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•18 years ago
|
||
Comment on attachment 299862 [details] [diff] [review]
v1
a1.9+=damons
Attachment #299862 -
Flags: approval1.9? → approval1.9+
| Assignee | ||
Comment 12•18 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: 18 years ago
Flags: in-testsuite-
Flags: in-litmus-
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 M11
Updated•18 years ago
|
Flags: blocking-firefox3? → blocking-firefox3+
Updated•17 years ago
|
Product: Firefox → Toolkit
You need to log in
before you can comment on or make changes to this bug.
Description
•