We could probably break the dependency of embedding on RDF (and perhaps a few other modules) if we implemented a `minimal' chrome registry. Such an implementation might be a hard-coded list of lookups for scrollbars, etc.

I hope I can get to this for moz 1.0 :)

This is an issue for Chimera. Hopefully no one minds if I adorn the status whiteboard to indicate that. Let me know if you need any help/support with this. IMO you just have to make an in-memory table mapping for the packages, skins, and locales listed in installed-chrome.txt in embedding/config. We should probably also do something like patch NS_InitEmbedding (which presumably isn't called by the main app?) to register the simpler chrome registry automatically. Another note. Make sure not to eliminate the user stylesheet support. That should carry over to a simpler chrome registry.

This is not related to a top priority embedding project at this point. Minusing to topembed- and adding embed.

nsIChromeRegistry is huge. I might cut it up to make implementation simpler (and just force some existing consumers to QI())

ok, looking for an sr=hyatt here :) I've broken out some of the routines from nsIChromeRegistry into 3 interfaces: - nsIChromeInstaller - contains all the runtime-install stuff that xpinstall uses, such as installSkin(), etc - nsIChromePackageRegistry - contains all the per-package stuff, like selectSkinForPackage(), isLocaleSelectedForPackage(), etc - nsIChromeRegistry - everything else While I was there, I also did some big cleanups to switch lots of internal methods from nsCString& to nsACString&, which allowed us to remove a ton of nsCAutoStrings, and use NS_LITERAL_CSTRING instead (which should be faster) None of this has any real effect on the existing chrome registry implementation. I'd like to land this so that the embedding-only registry can simply implement nsIChromeRegistry - that seems to be all we need to have an embedding registry. reviews?

I don't really see the necessity of splitting up nsIChromeInstaller and nsIChromePackageRegistry. Both deal with the installation and selection of skins, and IMO conceptually they belong together. "Package" is also the incorrect term to use here, since the interface contains skin and locale methods. I'd prefer to just see two interfaces: (1) nsIXULChromeRegistry (which contains all the methods of your nsIChromeInstaller and nsIChromePackageRegistry) (2) nsIChromeRegistry (the base interface) nsIXULChromeRegistry should *inherit* from nsIChromeRegistry so that JS code that wishes to use the base chrome registry methods doesn't have to QI. Be aware that there is JS code that uses nsIChromeRegistry, so you'll need to patch it to use nsIXULChromeRegistry instead. (The code that selects skins dynamically probably does this. Although it may be commented out, it should still be patched.)

Also, the code for agent sheets and user sheets needs to be in the minimal chrome registry. That is not part of the XUL chrome registry. Ultimately this code should be broken out into a completely different object (that isn't a chrome registry at all), but that can wait. The point is that it is used by embedding and is part of a minimal registry.

Ok, thanks for the suggestions, I'll reduce this to 2 interfaces. do you have any other suggestions for what goes in nsIXULChromeRegistry? I had a few questionable routines: * boolean allowScriptsForSkin(in nsIURI url); is scriptablity something XUL-specific? * void reloadChrome(); if chrome isn't XUL does this method make any sense? Maybe it does in case chimera or embedding wants a call to reload chrome? * void getAgentSheets(in nsIDocShell docShell, out nsISupportsArray styleSheets); the implementation is very XUL-specific * nsISimpleEnumerator getOverlays(in nsIURI aChromeURL); overlays also seem to be xul-specific. In fact, I'm wondering if any of the "skin" or "locale" related stuff even makes sense in nsIChromeRegistry... if that's the case, it looks like all the basic chrome registry stuff does is deal with chrome URLs. Thoughts?

getAgentSheets *is* relevant for embedding and would be part of a minimal chrome registry, since you need to (at the very least) supply the scrollbar sheet for embedding. User sheets should be part of the base registry as well. Everything dealing with skins and locales should just be part of nsIXULChromeRegistry IMHO. Of the methods you listed, all belong in nsIXULChromeRegistry except for getAgentSheets.

You should sanity-check all the locale stuff. They may have code that asks for the current locale in embedding. I'm not sure.

ok, I've taken hyatt's suggestions and implemented them... nsIChromeRegistry is now split into nsIChromeRegistry and nsIXULChromeRegistry, and I've updated all the C++ and JS I could find (via lxr) that would affect this. hyatt, mind taking a look, and possibly sr'ing?

the resulting .idl file, to make things clearer

Yeah, this split looks reasonable.

Comment on attachment 77717 [details] [diff] [review] Split nsIChromeRegistry into 2 interfaces sr=hyatt

Lose this, and r=waterson. @@ -1188,6 +1187,10 @@ static nsresult main1(int argc, char* argv[], nsISupports *nativeApp ) { nsresult rv; + + printf("sizeof(nsStr) = %d\n", sizeof(nsStr)); + printf("sizeof(nsString) = %d\n", sizeof(nsString)); +

Comment on attachment 77717 [details] [diff] [review] Split nsIChromeRegistry into 2 interfaces oops! printf's removed. I'm adding the has-review flag to make it easier for drivers to see that the patch is ready (also grepped the rest of the patch for printfs, just to be sure!)

ok, with a little trouble, this patch has landed on the trunk. tomorrow's builds will have this, so hopefully all will be well. I tested everything I could think of...

Adding adt1.0.0-. We can ship Mach V beta without this and this is topembed-. If this is needed for Mozilla1.0, please land after Mach V branches.

mark fixed ?

I'll do that when I actually fix the bug.

*** Bug 68832 has been marked as a duplicate of this bug. ***

moving bugs to mozilla 1.2alpha that missed 1.1beta

This converts all consumers of the chrome registry over to using a ContractID, so that I can do a drop-in replacement by just registering under the contractid. It also makes the CID private, so it can only be referenced from the chrome DLL. I'll have a patch for the commercial tree momentarily. Can I get reviews?

here's the commercial patch.

Comment on attachment 90978 [details] [diff] [review] convert CID to ContractID in commercial tree ignore the fact that we're switching from nsIChromeRegistry to nsIXULChromeRegistry - the code is #if 0'ed, and we will need to use nsIXULChromeRegistry if this code is ever turned on.

Comment on attachment 90977 [details] [diff] [review] convert CID to ContractID in all consumers please be consistent in your use of NS_CHROMEREGISTRY_CONTRACTID vs. "@mozilla.org/chrome/chrome-registry;1" in your do_GetService calls. with that... r=bnesse.

Comment on attachment 90978 [details] [diff] [review] convert CID to ContractID in commercial tree r=bnesse.

Comment on attachment 90978 [details] [diff] [review] convert CID to ContractID in commercial tree sr=dveditz

Comment on attachment 90977 [details] [diff] [review] convert CID to ContractID in all consumers >Index: rdf/chrome/public/nsIChromeRegistry.idl >=================================================================== >+#define NS_CHROMEREGISTRY_CONTRACTID \ >+ "@mozilla.org/chrome/chrome-registry;1" > %} When I added the embeddable interfaces for XPInstall I got slapped during super-review for including my contractid define in the .idl file ("the contract id refers to a COMPONENT, the .idl describes an INTERFACE"). I had to include the contract ID as a comment instead. Bah. This way makes more sense to me than having all the consumers define it themselves (or include yet another header file just for that) and I prefer it, but thought I should mention the other line of thought. >+ do_GetService("@mozilla.org/chrome/chrome-registry;1", &rv); You're changing these to the #define, right? sr=dveditz

Comment on attachment 90977 [details] [diff] [review] convert CID to ContractID in all consumers a=asa (on behalf of drivers) for checkin to 1.1

moving some stuff out to mozilla1.2beta that didn't make the mozilla1.2alpha train

unfortunately RDF is too ingrained in the chrome registry right now to be able to fix this for 1.2. Moving out to 1.3, which will require a change to the chrome registry format, probably to some sort of "contents.txt" solution.

ok, I've made some signifigant progress here. Here's how I'm going to tackle this: - regchrome is going to have a new option -s to generated a "simplified" chrome registry given an existing chrome.rdf file. This will basically read the current chrome.rdf and turn it into a chrome.properties which will be a sort of "flat file" version of the chrome registry - a simplified chrome registry implementation will read chrome.properties and will be able to answer all the same questions to get chrome URLs mapped to real URLs. - when you go to build the embedding client, the build system will generate embed.jar, and then sic regchrome on it to generate a chrome.properties for the file. When you distribute the embedding client, all you need to distribute is the chrome.properties file This means we'll still need RDF at BUILD time in order to process all the contents.rdf files that exist in embed.jar, but once they are processed, we won't need rdf.dll at runtime! The other nice thing this means is that the only changes are: - regchrome.cpp - to add this new capability (mostly complete) - build system - to support generation of this new file so no changes are necessary to the core gecko product.

ok, this patch adds a switch "-p" to regchrome which allows someone to specify a flat-file .properties file with all the data in the chrome registry. without the -p, the behavior is the same. I'll attach a sample embed-chrome.properties that I generated from the embedding build.

here is a .properties file that could be read by a bare-bones chrome registry.

Comment on attachment 121347 [details] [diff] [review] ability to output to a .properties file requesting reviews from dougt and bryner - this will allow us to have a lightweight chrome registry (which could be used by embedding and/or camino...)

Comment on attachment 121347 [details] [diff] [review] ability to output to a .properties file >+ >+ // remove the prefix, the provider type, and the trailing ':' >+ // (the compiler will optimize out the -1 + 1 close the parentheses. Looks good. sr=bryner.

ok, I was missing selectedSkin and selectedLocale because they followed resources, not literals. sorry I'll need reviews again :(

Comment on attachment 121528 [details] [diff] [review] ability to output to a .properties v1.1 hyatt/bryner - sorry I need a review on this again - some extra tweaks to handle selectedSkin/selectedLocale

I've also been hacking on nsEmbedChromeRegistry and am almost done implementing nsIChromeRegistry such that it can auto-resolve URLs. the only other trick may be looking at calls to nsIXULChromeRegistry::GetSelectedLocale() to see if they should be converted to something more generic.

Comment on attachment 121528 [details] [diff] [review] ability to output to a .properties v1.1 sr=me

Comment on attachment 121528 [details] [diff] [review] ability to output to a .properties v1.1 r=hyatt

clearing target milestone, since 1.4 is long gone.

just for my own reference, all of the above patches have been checked in. next step, a chrome registry that can read .properties files.

moving minimo bugs to the new bugzilla product.

ben, any interest in this direction? alec, any continued interest?

marking as wont fix. benjamin has done a great job on mozilla/chrome. that is what minimo will use.