Currently, the cookie service and wallet components are being initialized in apprunner main1(). This is not the correct place to do this; these modules should be lazily initialized the first time they are used. This will help startup performance.

But we can't do that. Every time a page is visited, the sigle-signon code needs to be called to check to see if it is a page that it can prefill with username/password. This can happen only if the wallet component is initialized when the browser starts up. The same applies to the cookie component since we need to check for cookies every time we make an http request. Can you quantify how much time we are adding to the startup performance? If it is in the matter of micro or milli seconds, then we shouldn't even consider making any changes. If it is in the order of seconds, that's a different story.

The first thing that needs to happen is that the code to init these things should be moved out of main1(), for maintenance and readability issues. Second, you should init these things the first time you need them. The user might not be opening a web page first; they might open mailnews, or the address book.

OK, you made a good point. Where is the correct place to put such initializations?

Without knowning more about the pattern of use, I can't say. Talk to necko or layout people to figure this out.

Shaver talked to me about this. Appshell components startup could be used for this. Talk to me about this and I can explain more.

I did talk to dp and we exchanged some e-mail on this topic. I'm putting the hilites of our discussion in this bug report so that we don't lose the thread. FIRST DP SAID: > If you put your hierarchy in the a particular spot in the registry, then > on startup, your component will be created. These are currently called > appshell components as these are a function of the app. > xpinstall and one other appshell component does this. So instead of > adding code to apprunner to just trigger a creation on startup, you can > add this little code that writes your cid into the registry. And you > would need to implement nsIAppshellComponent I think. > You can look at xpinstall/src/nsSoftwareUpdate.cpp for the code that > adds itself to the registry. > This is post beta stuff clearly. MY REPLY TO DP: > Is this really post-beta? I thought that this bug was being filed as a > performance issue so they could improve startup time. DP REPLIED BACK: > I dont see how this would improve startup. Eitherway your dll will > get loaded at startup. Maybe I misread the bug... AND I ANSWERED: > I was against this at first and my initial comments in the bug report so > indicate. But then sfraser added the following comment which made me > change my mind. > >> Second, you should init these things the first time you need them. The >> user might not be opening a web page first; they might open mailnews, >> or the address book. > > So it is a performance issue -- the dll should not get loaded if all you > are doing is going to mailnews. Will the scheme you described avoid > loading the dll if you never open the browser? If so, then your scheme > is the wrong solution. AND DP ACKNOWLEDGED WITH: > Since appshell are needed to display mail also, my guess is it will get > loaded. So yes, wrong solution.

AND dp finally said: Maybe you can invent the right solution. Other components using the old solution may also be doing the wrong thing. They can be switched over too to your solution.

Checked in new version of nsAppRunner.cpp (and its associated makefile) to fix this problem).

could you explain how they get initialized now?

Beats me! But they are getting initialized because everything is working. So apparently that code in nsAppRunner was doing nothing more than loading the dll's too early.

What was I ever thinking. With this change the dll's were not getting loaded. This resulted in bug report 14721. So I had to back out this change to fix that bug. Reopening this report.

actually steve meant bug 15721

See some discussion currently going on in bug 17120 which is relevant to this bug.

Not going to make it for M11. Pushing to M12

*** Bug 18352 has been marked as a duplicate of this bug. ***

Reassigning to dp at his request

I have gotten the nsCookieHTTPNotify object delinked from the internals of cookieservice. Moving that object out into apprunner and creating that instead of the service will delay the load of cookie dll.

Bulk add of "perf" to new keyword field. This will replace the [PERF] we were using in the Status Summary field.

Moving to M14 since this requires a change to jsloader for getting a principal. The way we are fixing this is to move the cookie code into necko.

This needs jband to fix nscategory enumeration. Pushing out to M15.

On behalf of PorkJockeys: putting on beta1 radar, per beta criteria priority #2 - startup performance is not within beta metrics. removing extraneous tags, cc waterson

I would love to fix this. But this aint beta. User benefit of performance is not loading 1 dll and that too only if we start navigator in mail mode say. For navigator this aint a big major win any more than delaying load of a dll.

Putting on PDT- radar for beta1. Will not hold beta for this bug.

Unfortunately I already fixed this. Code fix. Maybe simon or morse can verify this. Cookie get created on first http request using category manager.

Reopening. We are still initializing the wallet service in main1(): http://lxr.mozilla.org/seamonkey/source/xpfe/bootstrap/nsAppRunner.cpp#763 This should happen "sometime later", like when wallet is first used.

Reassign to wallet owner

*** Bug 38611 has been marked as a duplicate of this bug. ***

Putting on [nsbeta2-] radar. Adding relnote keyword. Putting on nsbeta3 radar so we can get done for PR3.

Pushing this post beta2

Adding nsbeta2 keyword to bugs with nsbeta2 triage value in status field so the queries don't get screwed up

Doesn't seem worth the effort just to save one library's worth of loading in the rare case of someone doing mail-only. Open to ideas, but giving [nsbeta3-]

*** Bug 50254 has been marked as a duplicate of this bug. ***

M25 is meaningless -- how about a mozilla0.9 or mozilla1.0 setting, per the http://www.mozilla.org/roadmap.html? /be

Netscape nav triage team: based on Steve Morse's pre-triage recommendation, this is not a beta stopper.

Nominating for mozilla1.0 -- should helpwanted be set too? /be

So, if I read the comments correctly, the CookieService part of this is fixed and now only Wallet remains? If that's true, please update the summary. We are now trying to cut every little piece of time we can out of startup. Loading DLLs is one of the biggest prices we pay at startup and fixing this bug could help. Can we get this before beta?

This is currently an nsbeta1- future bug. That doesn't sound like a beta bug to me.

Renominating.

nav triage team: We don't believe that the instantiation for wallet is that much time. Also, you need to initialize it before you load a page anyway, so we don't think it will affect startup performance much. However, initialization of wallet shouldn't be in main1().

renominating, we need to talk about this as per today's perf meeting discussion.

moving to mozilla0.9.4 as per discussion in perf meeting. I'll talk to Steve about this so that we can scope this out or bury it.

nav triage team: Don't think we'll have time to get around to this for 0.9.4. Pushing out to mozilla1.0.1 to give us time to nail it once and for all (we hope).

I don't think we should ship mozilla 1.0 without better decoupling of the extensions/ stuff. It would be a disservice to many of our downstream code consumers, especially in the embedding space. (mozilla1.0 is already in the keywords, but then so is nsbeta1+...)

I will take this bug. My plan is to load the wallet dll on first form. That should delay loading of the dll up until the first page is parsed (assuming the first page usually has a form)

Ccing morse - module owner of wallet.

r=gagan Steve, can you look at the patch too. Better yet try it out. I need to test it throughly. I tried the password manager senarios. But didn't know enough to try the form manager. NOTES: When the category enumeration fails, should we retry. I am leaning towards no as we could end up trying multiple times for no avail. Let me know if people feel otherwise.

r=morse for the changes in nsWalletService

Of course my r=morse applies to nsWalletFactory as well. I just tried to apply the patch and it did not compile. Probably because my tree is about a week old. I'll pull a new tree and try again.

Patch cannot be automatically applied to latest tree because of recent changes to nsHttpHandler.cpp. But I was able to manually apply the patch for that file. With the patch, all functions of form manager and password manager still appear to be working.

Alright! thanks Steve. I will get super review and start the checkin process.

cc'ing terri, who's the qa person for the passwd and form mgrs.

sr=jband. nits... NS_CreateServicesFromCategory - sigh, yet another linkage point to xpcom. + nsCOMPtr<nsISupports> instance = do_GetService(contractID, &rv); + if (NS_FAILED(rv)) + nFailed++; Why not a continue like the other similar blocks above this? I realize the the do_QueryInterface that follows will just fail nicely, but it is unnecessary. Plus, a good optimizer will just share code with the identical blocks above. Just curious... Any chance of checking a pref and skipping all this for people who don't want to use the feature at all?

cc'ing chak. this createservicesfromcategory things smells like some application startup stuff he banged out. chak, can/should the stuff you did be applied here, or am I off in the weeds?

adding valeski to see if there is any embedding issues with this.

Replied to jband on the concerns via email. For the record, a) Accepted on the "continue". I will add it in while checking it in. b) Pref is not the right way. Plus the whole idea is to delay loading of this dll when the pref is set to true. c) Yeah another symbol exported from xpcom. Atleast this is not changing nsIServiceManager

a=asa on behalf of drivers

Fix checked into 0.9.4 for delaying wallet dll when navigator is started up. Reassigning bug to Steve (owner of wallet) for using similar mechanism on mail, ftp, http passwords.

No longer a perf issue. This is purely a code cleanup issue.

after reading through this, I don't think its clear what the remaining parts of this bug are that need to be fixed. If its just code cleanup, a clean report specifying what needs to be fixed still should be filed as a followup in the appropriate component (which I'm pretty sure isn't Cookies if you read the entire bug). Marking fixed based on the comments throughout as well as the fact that the bug as filed is now fixed. If there is followup work to be done, please file a followup and note the new bug here. Thanks!

V/fixed.