Reproduction: 1. Disable extension xmlextras 2. Build Actual result: Build fails in dom/src/base/nsDOMClassInfo.cpp, because it #includes several interfaces from extensions/xmlextras. Compiler output below. Expected result: Build succeeds, no traces of xmlextras. Additional Comments: - This is a regression caused by bug 83433. - Beonex Communicator does not ship with xmlextras. We're close to a release. Not sure, what to do. Any idea about a workaround (/hack)? c++ -o nsDOMClassInfo.o -c -DOSTYPE=\"Linux2.2\" -DOSARCH=\"Linux\" -DOJI -D_IMPL_NS_DOM -I../../../dist/include -I../../../dist/include -I/usr/src/beonex-0.7/bin-i686/dist/include/nspr -I/usr/X11R6/include -fPIC -mcpu=pentiumpro -march=pentiumpro -I/usr/X11R6/include -fno-rtti -fno-exceptions -Wall -Wconversion -Wpointer-arith -Wbad-function-cast -Wcast-align -Woverloaded-virtual -Wsynth -pedantic -Wno-long-long -mcpu=pentiumpro -march=pentiumpro -pipe -pthread -O2 -DNDEBUG -DTRIMMED -I../../../../mozilla/dom/src/base/../build -mcpu=pentiumpro -march=pentiumpro -I/usr/X11R6/include -DMOZILLA_CLIENT -include ../../../config-defs.h -Wp,-MD,.deps/nsDOMClassInfo.pp ../../../../mozilla/dom/src/base/nsDOMClassInfo.cpp ../../../../mozilla/dom/src/base/nsDOMClassInfo.cpp:237: nsIXMLHttpRequest.h: No such file or directory ../../../../mozilla/dom/src/base/nsDOMClassInfo.cpp:238: nsIDOMSerializer.h: No such file or directory ../../../../mozilla/dom/src/base/nsDOMClassInfo.cpp:239: nsIDOMParser.h: No such file or directory ../../../../mozilla/dom/src/base/nsDOMClassInfo.cpp: In function `static nsresult nsDOMClassInfo::Init()': ../../../../mozilla/dom/src/base/nsDOMClassInfo.cpp:1505: `nsIXMLHttpRequest' undeclared (first use this function) ../../../../mozilla/dom/src/base/nsDOMClassInfo.cpp:1505: (Each undeclared identifier is reported only once ../../../../mozilla/dom/src/base/nsDOMClassInfo.cpp:1505: for each function it appears in.) ../../../../mozilla/dom/src/base/nsDOMClassInfo.cpp:1505: template argument 1 is invalid ../../../../mozilla/dom/src/base/nsDOMClassInfo.cpp:1506: `nsIJSXMLHttpRequest' undeclared (first use this function) ../../../../mozilla/dom/src/base/nsDOMClassInfo.cpp:1506: template argument 1 is invalid ../../../../mozilla/dom/src/base/nsDOMClassInfo.cpp:1511: `nsIDOMSerializer' undeclared (first use this function) ../../../../mozilla/dom/src/base/nsDOMClassInfo.cpp:1511: template argument 1 is invalid ../../../../mozilla/dom/src/base/nsDOMClassInfo.cpp:1515: `nsIDOMParser' undeclared (first use this function) ../../../../mozilla/dom/src/base/nsDOMClassInfo.cpp:1515: template argument 1 is invalid ../../../../mozilla/dom/src/base/nsDOMClassInfo.cpp: In method `nsresult nsStringArraySH::GetProperty(nsIXPConnectWrappedNative *, JSContext *, JSObject *, long int, jsval *, PRBool *)': ../../../../mozilla/dom/src/base/nsDOMClassInfo.cpp:4429: warning: unused variable `PRInt32 n' make[3]: *** [nsDOMClassInfo.o] Error 1 make[3]: Leaving directory `/mnt/data/compile/beonex-0.7/bin-i686/dom/src/base' make[2]: *** [install] Error 2 make[2]: Leaving directory `/mnt/data/compile/beonex-0.7/bin-i686/dom/src' make[1]: *** [install] Error 2 make[1]: Leaving directory `/mnt/data/compile/beonex-0.7/bin-i686/dom' make: *** [install] Error 2

> Any idea about a workaround (/hack)? Apart from backing out the change :).

Over to peterv who's already working on this...

Any ETA?

No ETA yet, sorry. I'm still trying to find out what would be involved in fixing this.

Peter, well, if this is important to someone we could always temporarily put #ifdef XSLT or whatever around the xslt code in nsDOMClassInfo.cpp...

Yeah, except we don't have a define for XSLT anymore. Ditto for XML Extras (which seems to be the one giving Beonex grieve). I wonder if people could just build with XML Extras/Transformiix and not package them. How would the DOM class info stuff handle that?

> we could always temporarily put > #ifdef XSLT or whatever around the xslt code in nsDOMClassInfo.cpp... Is it just nsDOMClassInfo.cpp? Can I just back out the change in nsDOMClassInfo.[cpp|h] only? (What was giving me grieve was - the large number of files - the fact that other checkins happened after the checkin - the branching .)

You can not back those changes out. If this is such a problem for you then just comment out the XSLT specific code in nsDOMClassInfo.cpp and nsIDOMClassInfo.h and you should be all set.

Welcome to bug 18352 redux. The bits of the main application should not depend upon an extension. Ok, technically, it only depends upon the header. Unfortunately, that still is a no-no because of the way we build. So, it's obvious to me that we need a better separation between church and state...err, interfaces and implementations. This is where bug 59454 comes in.

Apparently Vidur is working on something similar. I'll try to get in touch with him. Ben Bucksch, I'll attach a patch to disable the XML Extras stuff in the DOMClassInfo files. That should get your builds going again for now.

I don't really _want_ this bug, but I think I made the mistake of getting the most work on it done so far, and admitting it in public. When will I learn?

How's it going? We're getting closer to the Feb 1 proposed cutoff. peterv, shouldn't you take this bug back from shaver? /be

Ok, too bad we didn't see the patch from shaver. I'll take this back and start working on it.

A real patch is on its way for this...

Any news? We're past the deadline (01-Feb-2002). /be

I'm going to post later what I am implementing but I still need some time. I could supply a patch which disables DOMClassInfo for the Transformiix and XMLExtras classes but implements regular ClassInfo, which would break the dependency without disabling those modules completely. I'm not completely sure what exactly I would be breaking. I'm currently compiling a tree that has such a patch to see what happens.

This should enable building without extensions. It's a workaround until my real fix to nsDOMClassInfo is ready. I've tried the XMLExtras tests and the Transformiix test and everything seems to work. One part that doesn't work is the XPath NodeSets, I can't supply ClassInfo for them so users from JS will probably have some trouble. I doubt anybody uses those right now and they should be replaced soon by my DOM Level 3 XPath implementation. I still need to test this patch on Linux and Windows, will do that tomorrow. Shall we check in this one and remove the workaround once the real patch goes in? It would avoid disabling Transformiix and XMLExtras completely in the default build.

I wrote a description of what I changed, I'll post it tomorrow after I finish it. Still have to try this out on windows and linux.

New patch clears up an ownership issue for the nsDOMClassInfo we create for external objects.

I think this patch is ready for reviewing. This patch provides three things: - a mechanism for modules other than DOM to register classes by their JS name and have DOMClassInfo and .prototype magic. - a generic scriptable helper object for modules who only need basic DOMClassInfo. - .prototype magic for external constructors with the same name as the classes they construct. Modules other than the DOM module can now register two new categories: JAVASCRIPT_DOM_CLASS, taking a contractid for a 'registrar' class, and JAVASCRIPT_DOM_INTERFACE, taking an IID converted to a string. The DOM module puts these classes and interfaces into its hashtable (hashed by name), and the classes use a new type (eTypeExternalClassInfoCreator). When we try to resolve one of these class names, we look in the hash table and if the struct has type eTypeExternalClassInfoCreator, we try to instantiate the registrar class using the contractid that was supplied and ask it to register the class info with the DOM module. This causes the module that registered the class to get loaded (if it hasn't already been loaded by something else). The registrar then calls back into the DOM module (through the nsDOMSOFactory) and supplies the info to create a nsDOMClassInfo structure which gets put into the hashtable replacing the existing eTypeExternalClassInfoCreator with a eTypeExternalClassInfo. The registrar can supply a constructor function that returns a scriptable helper (as a nsIClassInfo*), if none is supplied we'll create an generic helper (nsDOMGenericSH). The registrar can also supply the CID of the class, if this is a class that needs a constructor with the same name (to make new Foo() work for a class named Foo). Apart from that nsDOMClassInfo hasn't changed much. The module can get a pointer to the scriptable helper by name from nsDOMSOFactory, because it might need it when the class gets QI'ed to nsIClassInfo. Because external modules could be holding on to the generic scriptable helper we created, and that helper needs the nsDOMClassInfoData structure, we can't just blindly delete the structure when the hashtable gets destroyed. So we use the low bit of the mCachedClassInfo member to encode if the scriptable helper is for an internal or an external class. The nsDOMClassInfo destructor can then determine if it should delete the nsDOMClassInfoData structure or not (internal classes have their nsDOMClassInfoData stored in a static table). I've supplied a few helper macros for modules that want the basic functionality and converted Transformiix and XML Extras. I hope that makes sense to anyone else.

Comment on attachment 68865 [details] [diff] [review] v2 I like the patch so far (still reading it -- it's big!), but I'm a little nervous about RegisterExternalClasses, which appears to be on the startup path. Have you run Ts/Tp/Txul numbers on this patch, so we can see if we're going to have stress when it hits the tinderboxes? (I wonder if we'll build xmlextras/transformiix in the default tinderbox performance config after they become optional. Perhaps not, in which case that won't matter much.)

- nsIDOMClassInfo.h uses mixed 2 and 4 space indentation. Stick with 2 space indentation. - In nsDOMClassInfo::~nsDOMClassInfo(), 4 space indentation. - nsDOMClassInfo::RegisterExternalClasses() is an awefully long function, could we split it up and maybe share some of the code...? - In StubConstructor(), the cases where we *might* fail for valid reasons (like components being unregisterd since we started up) we should throw an exception and not just return JS_FALSE. Just returning JS_FALSE is a bit dangerous since it results in silent interruption of the script. - At around line 3300 in nsDOMClassInfo.cpp: + NS_ASSERTION(name_struct->mType == nsGlobalNameStruct::eTypeExternalClassInfo, + "The classinfo data for this class didn't get registered."); + + if (name_struct->mType != nsGlobalNameStruct::eTypeExternalClassInfo) { + return NS_OK; + } How about this in stead: + if (name_struct->mType != nsGlobalNameStruct::eTypeExternalClassInfo) { + NS_ERROR("The classinfo data for this class didn't get registered."); + + return NS_OK; + } - Next hunk: + JSFunction *cfnc; + if (name_struct->mType == nsGlobalNameStruct::eTypeExternalClassInfo && + name_struct->mData->mConstructorCID) { + cfnc = ::JS_DefineFunction(cx, obj, + NS_ConvertUCS2toUTF8(name).get(), + StubConstructor, 0, 0); + } else { + cfnc = ::JS_DefineFunction(cx, obj, + NS_ConvertUCS2toUTF8(name).get(), + NativeConstructor, 0, 0); + } + How about this in stead: + JSFunction *cfnc; + JSNative *native; + if (name_struct->mType == nsGlobalNameStruct::eTypeExternalClassInfo && + name_struct->mData->mConstructorCID) { + native = StubConstructor; + } else { + native = NativeConstructor; + } + + cfnc = ::JS_DefineFunction(cx, obj, + NS_ConvertUCS2toUTF8(name).get(), + native, 0, 0); + } - In nsDOMClassInfo.h, CLEAN_CI_PTR should IMO be renamed to GET_CLEAN_CI_PTR. Also, do we really need both IS_INTERNAL and IS_EXTERNAL? I guess I don't really care, but I would've defined only one of them. - We might want to change the type of the aName argument to nsDOMSOFactory::GetExternalClassInfoInstance() from const char* to a const nsAString& so that we don't need to do the ASCII to UCS2 conversion in that function, the caller can do that conversion much cheaper than we can here. - Also in nsDOMSOFactory::GetExternalClassInfoInstance(), after calling creator->RegusterClassInfo(), you should look up globalStruct again before touching it since it might have been moved around in the pldhash inside the namespace manager. - In GlobalNameHashClearEntry(), put braces around the one line if statement, and use 2 space indentation. - In nsScriptNameSpaceManager::RegisterExternalClassName(), could you not pass the nsCID by reference and not by value? - It should be documented somewhere that the name that's passed to nsScriptNameSpaceManager::RegisterClassInfoData() must be static data, it will not be deleted by the DOM code. Other than that I say sr=jst.

Shaver: I was going to run the performance tests to see if there's a difference. Of course, only having XMLExtras and Transformiix now might minimize it. If other classes would be added later, it might still grow. I don't think we can really avoid it though, we need to register the external classes at some point.

Ok, I got the numbers and they're going up :-(. Without With patch patch Ts Min 2253 2274 Ts Max 2327 2336 Ts Average 2281 2293 Txul 621 627 Tp 1304 1318 I'm going to attach a new patch that resolves jst's comments, and then I'll try to figure out if there's a way to optimize more.

Did those numbers come from a one run only test, or are they best of 3 or something? I'm questioning the accuracy of the tests here...

Thanks for the help Johnny ;-). Txul was run once, but it opens 10 windows. Tp was run once but did 5 iterations. Ts was run 10 times.

I did a review of v2 on the plane, these are the results. I was kinda tired, so be gentle. There are some performance titbits in there, I think. At least, I thought that when I was writing it.

Comments about shaver's comments: Re: the footprint effects of this change: - const nsDOMClassInfoID mID; + const nsDOMClassInfoData* mData; This changes nothing in terms of footprint except for the external class cases where the nsDOMClassInfoData struct is allocated. For the internal classes there is no change, mData will point to the static data in nsDOMClassInfo.cpp... Re: GlobalResolve() growing and performance, we're still down to one single string based pldhash lookup per resolve call (plus the old additional fluff), this patch changes nothing in that respect. The only change here is when resolving a class name for the first time in a scope, which obviously happens very rarely and would hardly impact DHTML performance noticeably. Re: DOM typelibs and constant counts: The reason we do this is to get things like nsIDOMNodeFilter to appear as NodeFilter on all scopes since it's needed for accessing NodeFilter.SKIP_ALL, NodeFilter.SKIP_... The only other possibility here with the current code is to hardcode the names of all the interfaces that have constants in them (and doesn't appear on any class' proto chain) in the nsDOMCI code, and that seems like a really bad and maintenance costly approach to me (and fragile too). Yes, this does pull in a few xpt files that otherwise wouldn't be loaded in a normal browsing session, but I don't know for sure what the lifetime of the heap hit we take for doing that is since once we're done enumerating those interfaces, we'll realease all the xpt related references. Jband would know. Either way, this is nothing new, it's been in the code since the XPCDOM landing, and it was even worse in the early days until jband hacked in more nifty filtering support into the xpt enumeration code, so if we need to do something more about this, we should file a separate bug on this and let this patch in.

I think I fixed all the comments except for the naming problem (nsI* and nsIDOM*). I think solving that will not be easy, I'd like to file it as a separate bug. I redid perf tests: the Txul and Tp numbers are the same (within 2ms difference), the Ts is still slightly up (2330 vs 2313). (Here's me hoping I didn't do anything stupid!)

>- nsIDOMClassInfo.h uses mixed 2 and 4 space indentation. Stick with 2 space >indentation. Done. >- In nsDOMClassInfo::~nsDOMClassInfo(), 4 space indentation. Done. >- nsDOMClassInfo::RegisterExternalClasses() is an awefully long function, could >we split it up and maybe share some of the code...? Moved the interface registration to nsScriptNameSpaceManager::RegisterExternalInterfaces and call that from nsDOMClassInfo::RegisterExternalClasses. >- In StubConstructor(), the cases where we *might* fail for valid reasons (like >components being unregisterd since we started up) we should throw an exception >and not just return JS_FALSE. Just returning JS_FALSE is a bit dangerous since >it results in silent interruption of the script. Made errors throw exeptions. >- At around line 3300 in nsDOMClassInfo.cpp: > >+ NS_ASSERTION(name_struct->mType == nsGlobalNameStruct::eTypeExternalClassInfo, >+ "The classinfo data for this class didn't get registered."); >+ >+ if (name_struct->mType != nsGlobalNameStruct::eTypeExternalClassInfo) { >+ return NS_OK; >+ } > >How about this in stead: > >+ if (name_struct->mType != nsGlobalNameStruct::eTypeExternalClassInfo) { >+ NS_ERROR("The classinfo data for this class didn't get registered."); >+ >+ return NS_OK; >+ } Done. >- Next hunk: > >+ JSFunction *cfnc; >+ if (name_struct->mType == nsGlobalNameStruct::eTypeExternalClassInfo && >+ name_struct->mData->mConstructorCID) { >+ cfnc = ::JS_DefineFunction(cx, obj, >+ NS_ConvertUCS2toUTF8(name).get(), >+ StubConstructor, 0, 0); >+ } else { >+ cfnc = ::JS_DefineFunction(cx, obj, >+ NS_ConvertUCS2toUTF8(name).get(), >+ NativeConstructor, 0, 0); >+ } >+ > >How about this in stead: > >+ JSFunction *cfnc; >+ JSNative *native; >+ if (name_struct->mType == nsGlobalNameStruct::eTypeExternalClassInfo && >+ name_struct->mData->mConstructorCID) { >+ native = StubConstructor; >+ } else { >+ native = NativeConstructor; >+ } >+ >+ cfnc = ::JS_DefineFunction(cx, obj, >+ NS_ConvertUCS2toUTF8(name).get(), >+ native, 0, 0); >+ } Done. >- In nsDOMClassInfo.h, CLEAN_CI_PTR should IMO be renamed to GET_CLEAN_CI_PTR. >Also, do we really need both IS_INTERNAL and IS_EXTERNAL? I guess I don't really >care, but I would've defined only one of them. Done. >- We might want to change the type of the aName argument to >nsDOMSOFactory::GetExternalClassInfoInstance() from const char* to a const >nsAString& so that we don't need to do the ASCII to UCS2 conversion in that >function, the caller can do that conversion much cheaper than we can here. Done. >- Also in nsDOMSOFactory::GetExternalClassInfoInstance(), after calling >creator->RegusterClassInfo(), you should look up globalStruct again before >touching it since it might have been moved around in the pldhash inside the >namespace manager. Done. Also done in the two other places where we call RegisterClassInfo and then use the struct again. >- In GlobalNameHashClearEntry(), put braces around the one line if statement, >and use 2 space indentation. Done. >- In nsScriptNameSpaceManager::RegisterExternalClassName(), could you not pass >the nsCID by reference and not by value? Done. >- It should be documented somewhere that the name that's passed to >nsScriptNameSpaceManager::RegisterClassInfoData() must be static data, it will >not be deleted by the DOM code. Done. Also added the comment to nsIDOMScriptObjectFactory::RegisterDOMClassInfo. >nsIDOMClassInfo.h: > > * the macro impl of RegisterClassInfo exports the CID of the DOM script > object factory -- that should probably use the ContractID. But then, > can the caller provide the nsIDOMScriptObjectFactory more cheaply? It's > not clear if nsWindowSH has easy access to the nsIDSOF handle, but you > guys probably know. Certainly, the call from the nsDOMSOFactory itself > has the pointer around, so the macro could perhaps fetch the service only > if the caller doesn't have it at hand (and therefore passes nsnull for > the hypothetical aDOMSOFactory argument). I've made the callers always pass in the nsIDOMScriptObjectFactory. >(@@ -3201,10 +3320,20 @@ stanza) >+ if (name_struct->mType == nsGlobalNameStruct::eTypeExternalClassInfo && >+ name_struct->mData->mConstructorCID) { >+ cfnc = ::JS_DefineFunction(cx, obj, >+ NS_ConvertUCS2toUTF8(name).get(), >+ StubConstructor, 0, 0); >+ } else { >+ cfnc = ::JS_DefineFunction(cx, obj, >+ NS_ConvertUCS2toUTF8(name).get(), >+ NativeConstructor, 0, 0); >+ } >+ > > * Why do we use UTF8 here? Above (line 2990, pre-patch) we bemoan the > absence of JS_DefineUCFunction, and then deflate to ASCII via > JS_GetStringBytes(str). I think we want to do the same here, until > we remedy this i18n-unfriendly JSAPI lack. Done. > * (Is there a bug filed on the lack of JS_DefineUCFunction?) Jst? (He's the one bemoaning ;-)) >@@ -87,11 +127,12 @@ > static nsresult ThrowJSException(JSContext *cx, nsresult aResult); > > protected: >- const nsDOMClassInfoID mID; >+ const nsDOMClassInfoData* mData; > > * Hrm. What are the footprint effects of this change? It looks like we've > added an additional allocation of 7 words (8 with malloc slop, likely) to > every nsDOMCI object. Do we have a lot of those? Jst answered this one. >+ if (IS_EXTERNAL(mData->mCachedClassInfo)) >+ delete (nsExternalDOMClassInfoData*)mData; > > * NS_STATIC_CAST, please! Done. >+ >+ // Make sure we don't leak this error code to the caller. >+ rv = NS_OK; >+ > > * How could that error code leak to the caller? I don't see an exit path > from that method that doesn't reset rv: either continue runs the loop body > again, and we set rv to the return value of cm->GetData(...), or we're > on the last iteration and we set it to the return value of > cm->EnumerateCategory below. Removed the comment. >+ nsIID *iid = nsnull; >+ >+ if_info->GetIID(&iid); >[...] >+ nsXPIDLCString name; >+ if_info->GetName(getter_Copies(name)); >[...] >+ nsMemory::Free(iid); >+ > > * Can nsIInterfaceInfo::getIIDShared and ::getNameShared save us some > allocations and copies? Switched to non-copying versions. >+ cm->GetCategoryEntry(JAVASCRIPT_DOM_INTERFACE, categoryEntry, >+ getter_Copies(IID_string)); >+ NS_ENSURE_SUCCESS(rv, rv); >+ >+ nsIID primary_iid; >+ if (!primary_iid.Parse(IID_string) || > > * Man, I wish we could store IIDs in category entries, rather than having to > parse every time. Alas. Yeah, alas. > * We were already dealing in nsDOMCIData structures before this patch, of > course, but I'd still like some reassurance that we're not going to get > stung too badly by this part of the switch. (I appreciate that some > space and/or speed cost is inevitable: you're adding a level of indirection, > and indirection isn't free. I just want to hear you say "we only create a > few dozen of these things, and we were already creating these nsDOMCIData > things before, just a little later", or something else equally comforting.) We were already creating these nsDOMCIData things before, we just do it a little later (and on demand!). ;-) See also jst's comments. > * While on that topic, do we share nsDOMCIData structures, or do they otherwise > ever have a lifetime that doesn't match that of the referring nsDOMCI > wrappers? If not, we can nest a structure rather than using an out-of-line > pointer with separate allocation. They don't outlive the wrappers, but we can have a nsDOMCIData struct without a nsDOMCI wrapper. >+ if (nsCRT::strncmp(if_name, NS_DOM_INTERFACE_PREFIX, >+ (PRUint32)strlen(NS_DOM_INTERFACE_PREFIX)) == 0) { > > * nsCRT::strncmp is deprecated (and dangerous, because of possibly-ambiguous > overload resolution, owing to the fact that the two versions differ only > in the signedness of their final parameter), and adds no value. "Bare" > strncmp is to be preferred for |char *| (ASCII, ISO-8859-1, UTF-8, etc.) > strings. Done. >+ rv = RegisterInterface(if_info, if_name.get() + strlen(NS_INTERFACE_PREFIX), &found_old); > > * No need to compute strlen of a constant string at runtime, when we have > the compile-time sizeof. Done. > * But do we really want to force that naming on external DOM entries? It would > certainly make it harder to use this mechanism for implementing standards > prescribed script-thing names: if we're supposed to register "Request" in the > global namespace, we're going to bang heads with the "real" nsIRequest. > How about "domI" or "domextI" as a prefix that is unlikely to cause > collision (unless, of course, two extensions use the same interface...)? > > Or maybe overloading interface name for script-thing name is just the wrong > path, and we need to explicitly parameterize that? I'd like to push this out to a new bug. From what I can see this would involve a second mapping (real interface name to JS name) for when we walk up the interfaces of a class. > * DeBrendan's theorem, or something: > NS_ASSERTION(s->mType == ::eTypeNotInitialized || s->mType ::eTypeInterface, > "Whaaa, ..."); > >+ { "Transformiix DOMCI Registrar", >+ TRANSFORMIIX_DOMCI_REGISTRAR_CID, >+ TRANSFORMIIX_DOMCI_REGISTRAR_CONTRACTID, >+ NS_DOMCI_REGISTRAR_CONSTRUCTOR(Transformiix) } Done. > * Hmm, now I understand jst's objection to the "registrar" name. Unless I've > been living a lie, a registrar is an entity with which one registers > something, and not an entity requesting or performing registration. > "Registerer" is not going to fly, clearly. How about just "extension" or > "hook"? "Transformiix DOMCI Extender"? Extension it is, and Extender. > * We should file a bug on the SVG folks, too, so they can move to the new > mechanism when the dust settles. Yes. So when will we have extensible layout? Drop in the SVG component. Hmmm...

Comment on attachment 70853 [details] [diff] [review] v3 - Document the fact that that the primary interface must be registerd for protos of a class to be accessable. - Ignore attempts at registering classes with non-ASCII characters in the class name in nsScriptNameSpaceManager::RegisterClassName() With that, sr=jst

Comment on attachment 70853 [details] [diff] [review] v3 + NS_ASSERTION(sizeof(PtrBits) == sizeof(void*), + "BAD! You'll need to adjust the size of PtrBits to the size " + "of a pointer on your platform."); + I'd rather see that asserted at compile time: #if sizeof(whatever PtrBits is) != sizeof(void *) #error "PtrBits must have the same size as a pointer" #endif Fix that baby up, and r=shaver. Let's get this baby in.

Forget that. #if can't do sizeof. Bah.

Comment on attachment 70853 [details] [diff] [review] v3 a=asa (on behalf of drivers) for checkin to 0.9.9 and the trunk.

This is the patch I checked in into the branch, containing the last-minute changes that went in to the trunk. Thanks to shaver for late-night bustage fixing.

Fixed in trunk and branch, disable-extensions should work now.

parseFromString is no longer working. This seems to have broken it. See http://bugzilla.mozilla.org/show_bug.cgi?id=129776

Please file a new bug for fallout like that. This bug is huge, old, and about a different thing.

verified.