434676 - XPCOMUtils shouldn't add category entries as persistent by default

Status: RESOLVED INVALID

(Core :: XPConnect, defect)

Description

[Justin Dolske [:Dolske]]

Bug 386533 added support for using alternative storage backends to with the login manager. The login manager checks a category to see if there's an alternative backend available, and if so uses it instead of the default.

I've noticed that, by default, XPCOMUtils "_xpcom_categories" support (implemented in bug 386533) adds categories with the persist flag set to true. This can lead to a problem -- if an extension implementing an alternate backend is uninstalled, the category entry will persist. Currently, it looks like this will completely break the login manager (it assumes if the category exists, it must be used).

I don't think persisting by default is the right thing for XPCOMUtils... As I understand it, this code would generally be adding the category entry each time the application is run anyway.

It might also be nice to support an optional flag to allow the caller to specify if aPersist should be set (ditto for aReplace). EG:

_xpcom_categories : [
    {
        category : "foo",
        persist  : true,
        replace  : false
    }
]

Comment 1

[Justin Dolske [:Dolske]]

Oops, the initial _xpcom_categories implementation was in bug 386536. I wonder if it's coincidence those two bugs were files at nearly the same time. :-)

Comment 2

[Justin Dolske [:Dolske]]

Hmm, looks like XPCOMUtils will automatically remove these categories when the module is unregistered (which maybe shouldn't happen if persist == true?), which I think will limit the scope of this bug for login manager... The persistent category would remain only if browser crashed (so no component unregistration) in the same session as when the user uninstalled the extension.

Comment 3

[Benjamin Smedberg]

I don't understand. Normally category entries are set during component registration, not during startup. Setting a category on every startup is a bad idea anyway. In that case we *should* persist the category entry. Category registrations are blown away when somebody uninstalls or disables an extension (category and component registration are both kept in compreg.dat).

Now that we blow away component registration frequently (every upgrade and every time extensions change) we don't ever call component unregistration hooks... nor, IMO should we.

Comment 4

[Justin Dolske [:Dolske]]

(In reply to comment #3)

> Category
> registrations are blown away when somebody uninstalls or disables an extension
> (category and component registration are both kept in compreg.dat).

Ah, didn't realize that. I guess this shouldn't be a problem, then.

I had also gotten bitten by manually removing a component from my extension's directory, but that's probably a Don't Do That issue, and certainly not something an end-user would run into.