Currently xpti only scan's the components directory rooted in the bin directory. This limits plugin installation by requiring the the xpt files that plugins need for scriptability to be installed in the components dir, while the plugins themselves are installed in the plugins directory. Furthermore, applications sharing a plugin directory, will actually not be able to share scriptable plugins, because the applications installed *after* the plugin was installed, will not have the necessary xpt files in their components directory. This problem is solved by xpti scanning multiple directories. Once we have a dynamic directory dermination model (probably keys hanging off of nsIDirectoryServiceProvider; this still needs to be hashed out), xpti needs to be able to scan those directories as well. The work to support multiple directories (this bug) can be done independently of directory resolution.

> The work to support multiple directories (this bug) can be done independently > of directory resolution. I'd prefix that with "Most of...". I'm not clear on who might be working on the bigger picture. In what bug? Bug 14923 is billed as a "tracking bug". There is one issue external to xpti I really want to work out up front... How is the list of directories communicated to xpti and the other users of the list? In bug 100368 dougt said: ............................................................................ spoke to conrad about this a bit. We concluded that it would be best to just have 3 defined directory properties: NS_XPCOM_COMPONENT_DIR NS_XPCOM_COMPONENT_USER_DIR NS_XPCOM_COMPONENT_GLOBAL_DIR Then have the embedding API call nsComponentManager::AutoRegister as it sees fit. ............................................................................ I'm not thrilled about that. Hardcoding the "variable names" (and implicit order) of the items to go into the search list is shortsighted. We have at least two subsystems that are going to be scanning these directories (maybe 3 if plugins needs to also?). What if some embedding wants a 4th directory? Bah! I say that xpcom should have acces to an ordered set of nsIFiles that constitutes the component search path. I don't really care what service does this, but it should be there when anyone asks. Also, (perhaps stating the obvious) we need to rememeber that this is not just a startup issue and control does not always flow from the core xpcom to the other autoreg'ers. Each of these subsystems can independently be asked to re-autoreg at anytime. If it was up to me, I'd add an additional interface to the object that implments nsIDirectoryServiceProvider (since that interface is FROZEN). This new interface would supply the ordered component search path - perhaps as an nsISimpleEnumerator or nsISupportsArray whose elements are nsIFiles. If that new interface is not present, then only NS_XPCOM_COMPONENT_DIR is used like it is now. If we really feel the need to only use the existing nsIDirectoryServiceProvider interface, then I'd still vote for something like asking it for NS_XPCOM_COMPONENT_DIR and then the series... "AdditionalComponentDir1" then "AdditonalComponentDir2", and so on, until it returns null. This is butt ugly, but at least it's flexible.

Because nsIDirectoryServiceProvider is frozen, we'd have to use a new iface and because this seems so tightly coupled to nsIDirectoryServiceProvider, I'd be inclined to call it nsIDirectoryServiceProvider2, just rev'ing the existing iface. So, you're saying it's up to the nsIDirectoryServiceProvider impl to define the precedence.? That seems vulnerable to me. Don't we want to distinguish between "core"/"app" stuff and "add-ons", so XPCOM can ensure some level of control over what's registered?

If any subsystem had to obtain a list of directories which could be scanned, it could be slow. Imagine if this list grew to some large number in the future. Would a component have to scan the whole list of directories? If each of the additional directories is explicitly defined, and rules are made as to which clients can scan where, it would avoid excessive directory scanning. If we need to add another named directory in the future, we can - unless we are saying that, along with the nsIDirectoryService iface, the list of locations is frozen as well. I don't think it is.

The patch I attached does almost all of what needs to be done in the typelib loader to fix this bug. xptiInterfaceInfoManager::BuildFileSearchPath is the method that will need to be updated to get the search path from wherever the search path is supposed to come from. In this patch that code gets the components directory and the plugins directory (using NS_XPCOM_COMPONENT_DIR and NS_APP_PLUGINS_DIR). We can change change this to whatever. xptiInterfaceInfoManager::GetCloneOfManifestDir *could* be changed if we want to store xpti.dat in some place other than the components directory. xpti figures out the search path and manifest directory at xpcom startup and caches the result. Whatever any given embedding wants to use for those directories *must* be available at that time. Any changes after that will be ignored. As things stand now, this precludes using anything based on the profile directory (at least in the mozilla browser embedding) because that is set later. I've only tested this stuff on NT, but I think I got it right (using the PersistentDescriptor stuff to make Mac happy) The patch was made against the 0.9.4 branch. I would expect it to apply to the trunk too.

The patch I just attached is mostly the same as the one before. It adds a fix for bug 105042. And it disables the scanning of the plugins directory. So, this means that there should be not detectable a difference between this and current behavior. The patch works on the trunk too. The only additional change needed for the trunk is to add the string module to the REQUIRES lists in makefile.win and Makefile.in. I'd like to get this reviewed as is and get it checked in to both the 0.9.4 branch and the trunk. After that only a small change will be needed to support scanning whatever directories we decide to scan.

- You might wanto use the more typesafe CallQueryInterface() in xptiCloneLocalFile() in stead of calling QueryInterface() directly. - Do you really need the void** cast in xptiCloneElementAsLocalFile() when calling QueryInterfaceAt()? Same in xptiWorkingSet::FindDirectory(). Either way, sr=jst

Overall, looks good. r=ccarlen. One thing though: + xptiFile& file = mFileArray[i]; + if(file.GetDirectory() == dir && + 0 == PL_strcasecmp(name, file.GetName())) Is this right on file systems which are case-sensitive as well as those that aren't?

I updated the patch based on both reviewers' suggestions and checked it into the trunk. I'm going to leave this bug open pending approval for checkin to the 0.9.4 branch. I'll attach the current patch to ensure that it does not get lost in the shuffle. Thanks!

Is this a regression, or was this issue on Netscape 6.1?

not a regression.

Can this wait to be checked in on the trunk, until next wednesday? If not, pls call into the PDT at noon. Gonna PDT- for now, until we here this has to be in branch before 10.23.

looks ready to be checked-in (once the branch is owned by the embedding team)

So I guess the plugin-side must now use nsIDirectoryService when doing scanning so both will look in the same locations. Is there already a bug filed on this?

Yes. bug 77231.

Closing. Now also checked into 0.9.4 branch. a=valeski

Marking Verified -