[minimo]implement minimal chrome registry

RESOLVED WONTFIX

Status

Minimo
General
P1
normal
RESOLVED WONTFIX
16 years ago
4 years ago

People

(Reporter: Chris Waterson, Assigned: Alec Flett)

Tracking

(Blocks: 1 bug, {embed, topembed-})

Details

(Whiteboard: chimera)

Attachments

(6 attachments, 2 obsolete attachments)

(Reporter)

Description

16 years ago
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.
(Assignee)

Comment 1

16 years ago
I hope I can get to this for moz 1.0 :)
Target Milestone: --- → mozilla1.0
(Assignee)

Updated

16 years ago
Blocks: 107059

Updated

16 years ago
Keywords: topembed

Comment 2

16 years ago
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.

Whiteboard: chimera
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED
Priority: -- → P2

Comment 3

16 years ago
This is not related to a top priority embedding project at this point.  Minusing
to topembed- and adding embed.
Keywords: topembed → embed, topembed-
(Assignee)

Comment 4

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

Comment 5

16 years ago
Created attachment 77464 [details] [diff] [review]
Split nsIChromeRegistry into 3 interfaces

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?

Comment 6

16 years ago
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.)

Comment 7

16 years ago
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.
(Assignee)

Comment 8

16 years ago
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?

Comment 9

16 years ago
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.

Comment 10

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

Comment 11

16 years ago
Created attachment 77717 [details] [diff] [review]
Split nsIChromeRegistry into 2 interfaces

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?
Attachment #77464 - Attachment is obsolete: true
(Assignee)

Comment 12

16 years ago
Created attachment 77772 [details]
the resulting nsIChromeRegistry.idl

the resulting .idl file, to make things clearer

Comment 13

16 years ago
Yeah, this split looks reasonable.

Comment 14

16 years ago
Comment on attachment 77717 [details] [diff] [review]
Split nsIChromeRegistry into 2 interfaces

sr=hyatt
Attachment #77717 - Flags: superreview+
(Reporter)

Comment 15

16 years ago
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));
+  
(Assignee)

Comment 16

16 years ago
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!)
Attachment #77717 - Flags: review+

Updated

16 years ago
Attachment #77717 - Flags: approval+
(Assignee)

Updated

16 years ago
Blocks: 137039
(Assignee)

Comment 17

16 years ago
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...
(Assignee)

Updated

16 years ago
Keywords: adt1.0.0

Comment 18

16 years ago
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.
Keywords: adt1.0.0 → adt1.0.0-
(Assignee)

Updated

16 years ago
Target Milestone: mozilla1.0 → mozilla1.1beta
mark fixed ?
(Assignee)

Comment 20

16 years ago
I'll do that when I actually fix the bug.
(Assignee)

Comment 21

16 years ago
*** Bug 68832 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 22

16 years ago
moving bugs to mozilla 1.2alpha that missed 1.1beta
Target Milestone: mozilla1.1beta → mozilla1.2alpha
(Assignee)

Comment 23

16 years ago
Created attachment 90977 [details] [diff] [review]
convert CID to ContractID in all consumers

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?
(Assignee)

Comment 24

16 years ago
Created attachment 90978 [details] [diff] [review]
convert CID to ContractID in commercial tree

here's the commercial patch.
(Assignee)

Comment 25

16 years ago
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.
(Assignee)

Updated

16 years ago
Attachment #90977 - Attachment description: contvert CID to ContractID in all consumers → convert CID to ContractID in all consumers

Comment 26

16 years ago
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.
Attachment #90977 - Flags: review+

Updated

16 years ago
Attachment #90978 - Flags: review+

Comment 27

16 years ago
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
Attachment #90978 - Flags: superreview+
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
Attachment #90977 - Flags: superreview+

Comment 30

16 years ago
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
Attachment #90977 - Flags: approval+
(Assignee)

Updated

16 years ago
Priority: P2 → P1
(Assignee)

Comment 31

16 years ago
moving some stuff out to mozilla1.2beta that didn't make the mozilla1.2alpha train
Target Milestone: mozilla1.2alpha → mozilla1.2beta
(Assignee)

Comment 32

15 years ago
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.
Target Milestone: mozilla1.2beta → mozilla1.3beta
(Assignee)

Updated

15 years ago
Target Milestone: mozilla1.3beta → mozilla1.4beta

Updated

15 years ago
Summary: implement minimal chrome registry → [minimo]implement minimal chrome registry
(Assignee)

Comment 33

15 years ago
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.
(Assignee)

Comment 34

15 years ago
Created attachment 121347 [details] [diff] [review]
ability to output to a .properties file

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.
(Assignee)

Comment 35

15 years ago
Created attachment 121349 [details]
sample .properties file

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

Updated

15 years ago
Attachment #121349 - Attachment is patch: false
(Assignee)

Comment 36

15 years ago
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...)
Attachment #121347 - Flags: superreview?(bryner)
Attachment #121347 - Flags: review?(hyatt)
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.
Attachment #121347 - Flags: superreview?(bryner) → superreview+
(Assignee)

Comment 38

15 years ago
Created attachment 121528 [details] [diff] [review]
ability to output to a .properties v1.1

ok, I was missing selectedSkin and selectedLocale because they followed
resources, not literals.

sorry I'll need reviews again :(
Attachment #121347 - Attachment is obsolete: true
(Assignee)

Comment 39

15 years ago
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
Attachment #121528 - Flags: superreview?(bryner)
Attachment #121528 - Flags: review?(hyatt)
(Assignee)

Updated

15 years ago
Attachment #121347 - Flags: review?(hyatt)
(Assignee)

Comment 40

15 years ago
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
Attachment #121528 - Flags: superreview?(bryner) → superreview+

Comment 42

15 years ago
Comment on attachment 121528 [details] [diff] [review]
ability to output to a .properties v1.1

r=hyatt
Attachment #121528 - Flags: review?(hyatt) → review+

Updated

15 years ago
Blocks: 213938
clearing target milestone, since 1.4 is long gone.
Target Milestone: mozilla1.4beta → ---
(Assignee)

Comment 44

14 years ago
just for my own reference, all of the above patches have been checked in.

next step, a chrome registry that can read .properties files.
Target Milestone: --- → mozilla1.7beta

Comment 45

14 years ago
moving minimo bugs to the new bugzilla product.
Component: XP Toolkit/Widgets: XUL → General
Product: Browser → Minimo
Target Milestone: mozilla1.7beta → ---
Version: Trunk → unspecified

Comment 46

13 years ago
ben, any interest in this direction?  alec, any continued interest?

Comment 47

13 years ago
marking as wont fix.  benjamin has done a great job on mozilla/chrome.  that is
what minimo will use.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.