386533 - Mechanism for registering alternative nsILoginManagerStorage implementations

Status: RESOLVED FIXED

(Toolkit :: Password Manager, defect)

Description

[Sylvain Pasche]

Attachment: looks for a category to find cid, v1

This patch uses a category for finding an alternative cid for the nsILoginManagerStorage component.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Please use "contractID" and not "cid", cid is the commonly used acronym for "class/component ID".

Comment 2

[Sylvain Pasche]

Attachment: v2

Thanks for your comment, I've renamed it.

Comment 3

[Justin Dolske [:Dolske]]

While this works for having *one* alternative replace the default, what if there are multiple alternative storage modules?

Another flavor of this is that it would probably be a good idea to have all storage modules be detected through the same code path (including storage-Legacy.js). That makes it less likely that we'll accidently break something.

Comment 4

[Sylvain Pasche]

(In reply to comment #3)
> While this works for having *one* alternative replace the default, what if
> there are multiple alternative storage modules?

Actually, the alternative storage module that will be used is the last one to perform the addCategoryEntry() call before nsLoginManager initializes the storage module. This more or less assumes that user won't have several storage module providers. 

> Another flavor of this is that it would probably be a good idea to have all
> storage modules be detected through the same code path (including
> storage-Legacy.js). That makes it less likely that we'll accidently break
> something.

That would be a good idea. For this to work using the current usage of the category manager, it means we need to add the default storage-Legacy category before any other components do. However, there may be timing issues for components doing it when they are registered (like I did for gnome-keyring), as I think we can't control that a given component is registered before all others.

Comment 5

[Justin Dolske [:Dolske]]

Comment on attachment 270609 [details] [diff] [review]
v2

>Index: nsLoginManager.js
>===================================================================
>+            this.__storage = Cc[contractID]
>+                               .createInstance(Ci.nsILoginManagerStorage);

Nit, make it:

+            this.__storage = Cc[contractID].
+                             createInstance(Ci.nsILoginManagerStorage);

I'm ok with with this, although I would sort of prefer to not have this checked in until there's a finished Gnome / OS X nsILoginManagerStorage component ready to make use of it.

Comment 6

[Sylvain Pasche]

Attachment: v3

I've unassigned me from bug 309807, so this is going to stay in stand-by mode until someone takes one of the dependent bug.

Comment 7

[Justin Dolske [:Dolske]]

Taking to help push this in for Beta 4. We really ought to have a way to replace the storage module (even if imperfect), so that extensions for native Gnome/OSX storage are possible in FF3.

Comment 8

[Sylvain Pasche]

Attachment: unbitrot

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 304702 [details] [diff] [review]
unbitrot

a=beltzner for 1.9

Comment 10

[Justin Dolske [:Dolske]]

Yay!

Checking in toolkit/components/passwordmgr/src/nsLoginManager.js;
  new revision: 1.27; previous revision: 1.26

Comment 11

[Eric Shepherd [:sheppy]]

Can anyone summarize how you use this?  I've looked at it for a while and am not sure I could describe it adequately in docs.

Comment 12

[Sylvain Pasche]

Hi Eric, I can write some explanations and sample code and let you proofread it if you point me on devmo where it should go.

Comment 13

[Eric Shepherd [:sheppy]]

Let's try putting an article here:

http://developer.mozilla.org/en/docs/Creating_a_Login_Manager_storage_module

Once you get something put in there, I'll clean it up as needed.  Thanks!

Comment 14

[Sylvain Pasche]

I've created that page. I think this should provide enough information for extension authors to get started. (nsILoginStorage page is not created but the .idl is well documented).

Comment 15

[Eric Shepherd [:sheppy]]

Where is nsILoginStorage?  I can't find it searching mxr.

Comment 16

[Sylvain Pasche]

Sorry, that was nsILoginManagerStorage. I've fixed the page. Thanks for the cleanups.

Comment 17

[Justin Dolske [:Dolske]]

(In reply to comment #14)
>  (nsILoginManagerStorage page is not created but the .idl is well documented).

It's functionally equivalent to the interfaces in nsILoginManger (which is mostly just a wrapper), so any page for that should probably just note that and not repeat detail from nsILoginManager.

Comment 18

[Justin Dolske [:Dolske]]

Oh, and thanks bunches for writing up the "Creating a Login Manager storage module" wiki page!