Closed Bug 99154 Opened 19 years ago Closed 18 years ago
These are my propsed changes to this interface: 1. Remove native loader specific code from the idl and move it to the nsNativeComponentLoader header. #include it where needed. 2. removed the "names" of the parameters registryLocation and componentType. I keep the methods in place to avoid compatiblity problems, but I want clients to stop using these as the component manager does not need to expose either where in the registry things are keep nor component type details. I guess for the long term we will have to continue passing what was expected, but we should not advertise what is being passed as "supported". 3. added some doc comments to the methods.
Why did 'registryLocation' and 'componentType' become unused params? These were the key to making the system of arbitrary component loaders work.
Both of these parameters are usually passed backed into the component manager. The registryLocation can be discovered by using the |in nsIFile location| inside of the component manager. The component type can be discovered in the same way autoregistration works today. Maybe this could be somewhat of a performance hit, but it is registration and does not happen frequently.
registryLocation != |in nsIFile location| This system allows for things like multiple components that live in jar files (or databases, or whatever). The loader might open up a file, find 20 components, and pass each its name inside the jar as the 'registryLocation' param. This allows the loader to correctly identify the component when it is later needed. Leaving this system in place allows for this flexibility. Yes, in the normal case 'componentType' is known to the system. But, it is *possible* that one loader might invoke another so that the loader you *think* you called is not the one that is really doing the work and trying to record its actions using the api. I do not see why you want to remove the system we worked out that allows for future flexibility. Is the overhead so great?
I renamed the registryLocation because there was grumbling about exposing the registry path to modules.
Who grumbled? Jband grumbles louder and better, I'm joining him. /be
Mike and I didn't like the idea of passing the explict registry path to the module. I do not have a problem with what Jband suggests and use this as a extra location string, but in the contract we must specify it is not for the module to play with directly.
after talking jband and dp, we came up with the follow.
Attachment #65046 - Attachment is obsolete: true
Comment on attachment 65362 [details] [diff] [review] proposed changes loaderStr - I like it :-) r=dp
Attachment #65362 - Flags: review+
Comment on attachment 65362 [details] [diff] [review] proposed changes sr=jband. I'm good with this. It would be 'nice' to be consistent about the 'aFoo' 'foo' param naming. If you are going to document canUnload you might want to add a dire warning about the dangers of saying 'yes' when *any* references to the module's code/data are outstanding. Though, this is loader dependent; e.g. JS components are immune.
Attachment #65362 - Flags: superreview+
I have fixed what you suggested. I also will add a UNDER_REVIEW status line to the idl.
actually, this needs to be a FROZEN status line. Not that this interface is already being depended on by multiple costumers: Netscape, Sun, Beatnik, ect.
Checked in WITHOUT marking as frozen. Checking in nsIModule.idl; /cvsroot/mozilla/xpcom/components/nsIModule.idl,v <-- nsIModule.idl new revision: 1.13; previous revision: 1.12 done Checking in nsNativeComponentLoader.h; /cvsroot/mozilla/xpcom/components/nsNativeComponentLoader.h,v <-- nsNativeComponentLoader.h new revision: 1.12; previous revision: 1.11 done Checking in nsStaticComponent.h; /cvsroot/mozilla/xpcom/components/nsStaticComponent.h,v <-- nsStaticComponent.h new revision: 3.3; previous revision: 3.2 done Checking in xcDll.cpp; /cvsroot/mozilla/xpcom/components/xcDll.cpp,v <-- xcDll.cpp new revision: 1.55; previous revision: 1.54 done
This checkin broke the static build. Please test the static build if you're gonna change nsNativeComponentLoader, douggy fresh!
chris fixed this by exporting nsNativeComponentLoader.h and xcDll.h. Is all of this because someone needs the NSGetModule symbol defined? Who outside of xpcom/components needs this?
those on the cc' list, please review this last change. It will mark the interface as frozen.
Comment on attachment 66787 [details] [diff] [review] Marks this interface frozen r=dp
Attachment #66787 - Flags: review+
Comment on attachment 66787 [details] [diff] [review] Marks this interface frozen email@example.com
Attachment #66787 - Flags: superreview+
checked in. thanks.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.