Closed Bug 414461 Opened 13 years ago Closed 13 years ago

Delay importing of PluralForm and DownloadUtils for statusbar

Categories

(Toolkit :: Downloads API, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9beta3

People

(Reporter: Mardak, Assigned: Mardak)

References

Details

(Keywords: perf)

Attachments

(1 file)

This might be the cause of Ts issues because both modules import some strings.
I thought they weren't loaded until they were needed?
Attached patch v1Splinter Review
Assignee: nobody → edilee
Status: NEW → ASSIGNED
Attachment #299862 - Flags: review?(mano)
(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
};
you could also just make gStr an object that builds the strings as needed, right?
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...
what about:
gStr = {
 get paused() {
  delete this.paused;
  ...
  this.paused = realstring;
  }
};
Too much duplicate code :p you have that whole code block copy/pasted to each. Don't worry! I've got something ;)
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+
Comment on attachment 299862 [details] [diff] [review]
v1

Hopefully this will help reduce Ts.
Attachment #299862 - Flags: approval1.9?
Flags: blocking-firefox3?
Keywords: perf
(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 on attachment 299862 [details] [diff] [review]
v1

a1.9+=damons
Attachment #299862 - Flags: approval1.9? → approval1.9+
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: 13 years ago
Flags: in-testsuite-
Flags: in-litmus-
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 M11
Flags: blocking-firefox3? → blocking-firefox3+
Depends on: 421392
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.