Starting with a fresh profile and then doing something to prompt the initialization of the handler service (f.e. trying to load the URL "foo:bar") causes the HandlerService constructor in nsHandlerService.js to get called ~117 times, chewing up a bunch of time and causing handler service initialization to be the most expensive operation by an order of magnitude in a Tp test run that first creates a profile and then loads a few hundred pages: Inclusive function elapsed times (us), FILE TYPE NAME TOTAL ... browser.js func onLocationChange 27121365 nsSessionStore.js func sss_serializeHistoryEntry 27296999 nsSessionStore.js func sss_getCurrentState 28275317 nsSessionStore.js func sss_saveState 29002207 hp.js@16 func scanTreeForChangeUrls 39065753 nsHandlerService.js func getProtocolHandlerInfo 672815734 nsHandlerService.js func HS__injectNewDefaults 678839771 nsHandlerService.js func HS__updateDB 680224555 nsHandlerService.js func HS__init 680650173 I'm guessing the culprit is the nsIExternalProtocolService::getProtocolHandlerInfo call in injectNewDefaults, which calls nsExternalHelperAppService::GetProtocolHandlerInfo, which retrieves the handler service in order to call its FillHandlerInfo method. Subsequent runs don't have this problem, as they don't prompt injectNewDefaults, so this only affects the first run with a new profile. Nevertheless, it seems pretty serious, given that it is a significant cost that affects the first-run experience for new users, so marking as major severity and requesting blocking-1.9. I'm also marking this as affecting all platforms, although I've only tested on Mac OS X so far, because I expect this to happen the same way across platforms.

I've just confirmed that it happens on Linux as well.

This may not be as severe as I thought at first, given that it doesn't seem to happen on startup but rather the first time the handler service is retrieved. Nevetheless, it seems like it's still worth fixing for Firefox 3.

-'ing. If this can be shown to have a more significant impact, please renom.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2pre) Gecko/2007112118 Minefield/3.0b2pre On Linux (Ubuntu 7.10) when I start with a new profile, I get this error if I try to load foo:bar or open the pref dialog on the Applications tab: Error: too much recursion Source File: file:///home/florian/moz/mozilla/obj-i686-pc-linux-gnu/dist/bin/components/nsHandlerService.js Line: 119 And every time I open the Applications tab, I get: Error: uncaught exception: [Exception... "Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]" nsresult: "0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)" location: "JS frame :: chrome://browser/content/preferences/applications.js :: <TOP_LEVEL> :: line 442" data: no] Error: gApplicationsPane is undefined Source File: chrome://global/content/bindings/preferences.xml Line: 670 The Applications tab is empty.

I tried again with a new nightly: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2pre) Gecko/2007113004 Minefield/3.0b2pre I still get this error: Error: too much recursion Source File: file:///home/florian/Desktop/firefox/components/nsHandlerService.js Line: 119 But I don't have the other errors any more. The Applications tab is not completely broken but it seems like some rows are missing. I only have "webcal" and "Web Feed". I would expect to see "mailto" and maybe others. Renominating for blocking because this issue has a visible effect.

Hmm... Apparently there's no mailto handler in the default list of handlers added the first time the service is started so the bug may not be visible after all. It doesn't seem hard to fix so I still think we should fix it for Firefox 3.

This will ensure _injectNewDefaults is called only once. However there is still a recursion and _init is called twice. I thought we could prevent this by calling _updateDB in a setTimeout from _init but in this case the first time the service is used it would not have the new default values yet. Don't know if this would be acceptable.

Comment on attachment 291270 [details] [diff] [review] patch v1 + this._datastoreDefaultHandlersVersion = this._prefsDefaultHandlersVersion; can you put these on the same line, or if that gets too long at least indent the second line?

Comment on attachment 291270 [details] [diff] [review] patch v1 Looks good; r=dmose.

Checked in.