[static ctor]remove static ctors from nsMathMLChar.cpp

RESOLVED FIXED in mozilla0.8

Status

()

P3
normal
RESOLVED FIXED
18 years ago
18 years ago

People

(Reporter: dbaron, Assigned: rbs)

Tracking

Trunk
mozilla0.8
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

I'm working on a patch to remove static constructors from nsMathMLChar.cpp.

My boilerplate description of why static constructors are bad:
Static constructorsare against the C++ portability guidelines
http://www.mozilla.org/hacking/portable-cpp.html , cause objects
to show up as a leak, and prevent us from running on OpenBSD (bug
49575), and prevents us from doing module unloading (see bug 60709).
Status: NEW → ASSIGNED
OS: Linux → All
Priority: -- → P3
Hardware: PC → All
Target Milestone: --- → mozilla0.8
(Assignee)

Comment 1

18 years ago
Grabbing the bug... In fact, I have already removed them from my tree as part of
a wider work. I would like to land this work and get rid of these static ctors
in my next check-in. Here is the post that I sent to n.p.m.mathml to the effect
under the header "Coming up: MathFont Property Files".

"Roger B. Sidje" wrote:
> 
> Changes in my tree include a revamp of the handling of the Operator
> Dictionary (nsMathMLOperators) and the stretchy characters (nsMathMLChar).
> The changes consist of the total removal of the hard-coding that were
> in these files. With the current code, the Operator Dictionary is there
> sitting in memory even if the user never hits a page with MathML markup.
> With the changes, the Operator Dictionary is now described in an external
> "Mozilla MathFont Property File" (mathfont.properties) which is loaded lazily
> upon occurrence of a first operator. Moreover, since this is not hard-coded
> anymore, users can add their own customizations, such as changing attributes
> to fit their taste, or adding tailor-made operators. The readable format used
> in the dictionnary makes this simple. For example, the parenthesis (U+0028)
> is listed as:
> operator.\u0028.prefix = fence:true stretchy:vertical lspace:0em rspace:0em #
(
> 
> Also out are the static ctors and the hard-coding of the mutable characters
> in nsMathMLChar.cpp. Rather than including the list of stretchy chars in
> nsMathMLCharList.h at compile-time, users can list the variants of larger
> sizes or the partial glyphs needed to build up a character in external Mozilla
> MathFont Property Files. For example, the glyph data for the "Symbol" font and
the
> "MT Extra" font are in mathfontSymbol.properties and
mathfontMTExtra.properties.
> (The format of these files closely follows the scheme used in
nsMathMLCharList.h.)
> 
> These changes also enable to externally specify the overall ordered list of
> fonts to try when stretching a character, as well as the preferred extension
> fonts for a particular character. For example, to specify that the parenthesis
> (U+0028) should only be stretched with partial glyphs from the Mathematica
> Math4 font, the mathfont.properties will include the line:
> extension.\u0028.parts = Math4
> 
> Similarly, to specify that larger variants of the sqrt (U+221A) should
> only be taken from CMEX10, the mathfont.properties will include the line:
> extension.\u221A.variants = CMEX10
> 
> Thus customizations can be made globally as well as down to the character
> level. But unlike authored css files, the customizations are on the user'
> side (user specific). A property is ignored if the given font is not
installed.
> The default properties could be tuned depending on the default shipped fonts.
> 
> Code-wise, the setup needed for these is built lazily, and rely on
> light-weight keys. Thus no overhead is incurred, compared to the current
> setup. On the contrary, since things are now created lazily, a user that
> never hits a MathML-authored page is freed from any unnecessary impact.
> 
> The Mozilla MathFont Property Files will live in the "path-to/bin/res/fonts/"
> directory, accessible under the URI: "resource:/res/fonts/".
> ---
> RBS
Assignee: dbaron → rbs
Status: ASSIGNED → NEW
(Assignee)

Comment 3

18 years ago
Created attachment 23876 [details]
Patch to remove the ctors and to allow extensible glyph tables through property files
(Assignee)

Comment 4

18 years ago
dbaron, you have been quick and pro-active... I have attached some of the work
I was referring to. Thanks anyway. Let me know if you have some comments about
the patch. The work is much involved, but any comments on the style (and
portability issues) will be okay too.
When I attached the patch I hadn't even seen your comment. :-)

One quick comment on your patch - it would probably be better to do the shutdown
based on the layout module destructor (see my patch) rather than an XPCOM
shutdown observer.

It would also be good to put these classes into the leak logs (I was going
to add that to the patch I attached).  This can be done in the following way:

MOZ_DECL_CTOR_COUNTER(nsFoo)

nsFoo::nsFoo()
{
  MOZ_COUNT_CTOR(nsFoo);
}

nsFoo::~nsFoo()
{
  MOZ_COUNT_DTOR(nsFoo);
}
(Assignee)

Comment 6

18 years ago
I was refraining from adding more bindings to core layout since MathML is 
intended to be a "drop-in" component someday. I was even hoping to remove the
nsMathMLOperators::[Add/Release]Table() from the layout module, since I have 
reworked that code to allow lazy instantiation of the Operator Dictionary.
But the problem is yet again with the release. It is a pity that there is no
way to register to layout and be notified of its shutdown. Do you anticipate
any problems if I leave the current xpcom shutdown listener in nsMathMLChar in
view of registering to layout someday.
Not really, although if we ever go to unloading modules when they're not used
(this has been proposed, see bug 45613, then it might be better to use the
module destructor.

Regarding switching MathML to a separate module - pulling the code out of the
factory is just one of the things that will have to happen then.  It always must
be done when modules are split or combined.

Then again, I don't really care, I just prefer module destructors over XPCOM
shutdown observers because it seems like less code and less time spent doing
it...
(Assignee)

Comment 8

18 years ago
dbaron, I have implemented your preference. It is indeed at lot simpler... Just
had to add a oneliner function FreeGlobals() to call gGlyphTableList->Finalize() 
that I already had... However, since some changes are added in layout, I may
spend the rest of the month waiting for layout folks to have a look at it :-)
So I will be check-in what I have (the non-layout dependent xpcom version),
knowing that I can later switch over to the other version, and that there could
be a mathmlStartUp() & mathmlShutDown() that will someday provide single entry
points for the global request and release of math-related resources.
(Assignee)

Comment 9

18 years ago
Code was checked in. Resolving as FIXED.
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.