74803 - Should make global data const where possible

Status: NEW

(Core :: General, defect)

Description

[Simon Fraser [no longer active]]

Making global data const has payoffs in terms of compiler optimizations, and the 
ability of the linker to put global data in read-only sections of the DLL, that 
has consequences for file mapping.

Comment 1

[Simon Fraser [no longer active]]

Attachment: Make kCSSRawProperties data table const

Comment 2

[Simon Fraser [no longer active]]

Other places that could be fixed (const char*'s):
<http://lxr.mozilla.org/seamonkey/search?string=const+char*>
Search in the results for '[]' for the bigger wins. There are some in xpconnect 
and JS, and a few in libpref.

For the benefits of making global data const, read this *excellent*
PDF on Mac OS X performance evaluation.

<http://developer.apple.com/techpubs/macosx/SystemOverview/Performance/
Performance.pdf>

Comment 3

[Simon Fraser [no longer active]]

I filed bug 80329 on the i18n converters, which have huge amounts of non-const 
global data.

Comment 4

[Edward Kandrot]

Sorry to say, but the patch did not compile under Linux.  Failed at line 62,
where it passed kCSSRawProperties into Init.

Comment 5

[Simon Fraser [no longer active]]

Hmm, probably need some const-dust sprinkled around.

Comment 6

[basic]

Changing platform

Comment 7

[Doug Turner (:dougt)]

reassign all kandrot xpcom bug.

Comment 8

[Doug Turner (:dougt)]

performance bug.  dp, cathleen, you might want to try to cash in on this.

Comment 9

[Pierre Saslawsky]

Actually, scc would be a better owner for this bug.

Scott: does this fix apply to all platforms or is it only for OS-X? The URL 
mentioned by sfraser has moved to:
http://developer.apple.com/techpubs/macosx/Essentials/Performance/Performance.pdf
The relevant info is on page 118 and 121.

If it's XP, the patch need to be changed or ifdef'd to compile under Linux and 
possibly other platforms.

Comment 10

[Simon Fraser [no longer active]]

This is XP. Does it not compile on Linux?

Comment 11

[Pierre Saslawsky]

Edward Kandrot [2001-05-14 15:06] says it doesn't.

Comment 12

[Simon Fraser [no longer active]]

Reclaim.

Does anyone know any reasons why the nsModuleComponentInfo of every module cannot 
be |const| (with a fix in nsIGenericFactory.h)?

Comment 13

[Simon Fraser [no longer active]]

Attachment: Change in nsIGenericFactory.h to allow for const moduleInfo data.

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Makes perfect sense to me, although there might be modules doing weird things.

Comment 15

[Suresh Duddi (gone)]

I agree. This should be const.

Comment 16

[Simon Fraser [no longer active]]

Attachment: Spreading the const in ns{I}GenericFactory [Checkin: Comment 19]

Mozilla builds with these conts changes. When this goes in, the components
array of individual modules can be made const one at a time.

Comment 17

[Suresh Duddi (gone)]

Comment on attachment 66205 [details] [diff] [review]
Spreading the const in ns{I}GenericFactory
[Checkin: Comment 19]

r=dp

Comment 18

[Chris Waterson]

Comment on attachment 66205 [details] [diff] [review]
Spreading the const in ns{I}GenericFactory
[Checkin: Comment 19]

sr=waterson

Comment 19

[Simon Fraser [no longer active]]

Comment on attachment 66205 [details] [diff] [review]
Spreading the const in ns{I}GenericFactory
[Checkin: Comment 19]

Patch checked in. Keeping bug open for more constness.

Comment 20

[Simon Fraser [no longer active]]

Results of making gComponents const in libgkcontent.dylib (using pagestuff -p)
Before:

    MP_SECTION (__DATA,__data)
	size = 17340
    MP_SECTION (__DATA,__const)
	size = 130296

After:
    MP_SECTION (__DATA,__data)
	size = 13980
    MP_SECTION (__DATA,__const)
	size = 133656

so this moved ~3.3K of data to the const (i.e. paged once) section.

Comment 21

[Simon Fraser [no longer active]]

I checked in changes to make every 'static nsModuleComponentInfo' in the tree
|const|.

Comment 22

[Simon Fraser [no longer active]]

Ongoing work, so Mozilla 1.2

Comment 23

[Kai Engert (:KaiE:)]

Could the checkin from this bug be the cause why static build on Linux does no
longer work?

../../dist/lib/components/libucvcn.a(nsUCvCnModule.o): In function
`nsUnicodeToCP936Constructor(nsISupports *, nsID const &, void **)':
/home/kaie/current-trunk/mozilla-static/intl/uconv/ucvcn/../../../../mozilla/intl/uconv/ucvcn/nsUnicodeToCP936.h:60:
undefined reference to `nsUnicodeToCP936 virtual table'
../../dist/lib/components/libucvcn.a(nsUCvCnModule.o): In function
`nsUnicodeToCP936Constructor(nsISupports *, nsID const &, void **)':
/home/kaie/current-trunk/mozilla-static/intl/uconv/ucvcn/../../../../mozilla/intl/uconv/ucvcn/nsUCvCnModule.cpp:98:
undefined reference to `nsUnicodeToCP936 virtual table'
../../dist/lib/components/libucvlatin.a(nsUCvLatinModule.o): In function
`nsUnicodeToUTF16Constructor(nsISupports *, nsID const &, void **)':
/home/kaie/current-trunk/mozilla-static/intl/uconv/ucvlatin/../../../../mozilla/intl/uconv/ucvlatin/nsUnicodeToUCS2BE.h:117:
undefined reference to `nsUnicodeToUTF16 virtual table'

...

Comment 24

[Simon Fraser [no longer active]]

I watched the worms tinderbox, which is doing a static build, and it is still 
green.

> undefined reference to `nsUnicodeToCP936 virtual table'

This does not sound related.

Comment 25

[hacker formerly known as [e-mail]]

This checkin is causing beos bustage.

c++  -frtti -fno-exceptions -Wall -Wconversion -Wpointer-arith
-Wbad-function-cast -Wcast-align -Woverloaded-virtual -Wsynth
-Wno-ctor-dtor-privacy -Wno-multichar -pedantic -Wno-long-long -pipe  -DNDEBUG
-DTRIMMED -O -nostart -Wl,-h -Wl,libnecko.so -o libnecko.so  nsNetModule.o     
 -Wl,--whole-archive ../../dist/lib/libneckobase_s.a
../../dist/lib/libneckodns_s.a ../../dist/lib/libneckosocket_s.a
../../dist/lib/libnkconv_s.a ../../dist/lib/libnkcnvts_s.a
../../dist/lib/libnkmime_s.a ../../dist/lib/libnkhttp_s.a
../../dist/lib/libnkfile_s.a ../../dist/lib/libnkdata_s.a
../../dist/lib/libnkjar_s.a ../../dist/lib/libnkres_s.a
../../dist/lib/libnkabout_s.a ../../dist/lib/libnkkwd_s.a 
-Wl,--no-whole-archive -L../../dist/bin -L../../dist/lib  -L../../dist/bin
-lxpcom -L../../dist/bin
-L/boot/home/tinderbox/BeOS_5.0_Depend/mozilla/obj-i686-pc-beos/dist/lib -lplds4
-lplc4 -lnspr4  ../../dist/lib/libunicharutil_s.a -lz    -lbe   
nsNetModule.o: In function `LC60':
nsNetModule.o(.data+0xb38): undefined reference to
`nsIndexedToHTML::Create(nsISupports *, nsID const &, void **)'
collect2: ld returned 1 exit status


If I remove the const from netwerk/build/nsNetModule.cpp's nsModuleComponentInfo
struct, then it links fine.  None of the other modules seem to have this problem.

Comment 26

[Simon Fraser [no longer active]]

What is it about nsIndexedToHTML::Create()?

Comment 27

[hacker formerly known as [e-mail]]

It appears to be because nsIndexedToHTML::Create() is implemented in the header.
 If I move the implementation to nsIndexedToHTML.cpp and restore nsNetModule.cpp
then libnecko.so links fine.  Patch coming up.

Comment 28

[hacker formerly known as [e-mail]]

Attachment: Move nsIndexedToHTML::Create impl to .cpp [Checkin: Comment 84]

Comment 29

[Simon Fraser [no longer active]]

Comment on attachment 67800 [details] [diff] [review]
Move nsIndexedToHTML::Create impl to .cpp
[Checkin: Comment 84]

sr=sfraser. Thanks for fixing this!

Comment 30

[Simon Fraser [no longer active]]

Need some const  loving here:

http://lxr.mozilla.org/seamonkey/source/layout/html/base/src/nsBulletFrame.cpp#525

Comment 31

[Simon Fraser [no longer active]]

Attachment: Make several static data tables const [Checkin: Comment 53]

The patch fixes nsStaticNameTable to take a const char* const [], and fixes the
places that use it to make their data tables const. These are kColorNames in
nsColorNames.cpp (GFX), kCSSRawProperties, kCSSRawKeywords, and a couple of
XSLT tables in unused code.

Comment 32

[Simon Fraser [no longer active]]

Need const loving here too:
http://lxr.mozilla.org/seamonkey/source/content/shared/src/nsBidiUtils.cpp#30

Comment 33

[Alec Flett]

we need an automated way to find this stuff.. codesighs may be helpful here.

Comment 34

[Simon Fraser [no longer active]]

Yeah. What i've been doing is to dump the output of 'nm', and run it though a
perl script that demangles, and parses out class/method names and symbol types.

Comment 35

[Simon Fraser [no longer active]]

More places:
http://lxr.mozilla.org/seamonkey/source/layout/html/base/src/nsTextFrame.cpp#4694
http://lxr.mozilla.org/seamonkey/source/layout/base/src/bidicattable.h#24
http://lxr.mozilla.org/seamonkey/source/content/html/document/src/nsHTMLDocument.cpp#4158
http://lxr.mozilla.org/seamonkey/source/content/html/content/src/nsHTMLOListElement.cpp#166
http://lxr.mozilla.org/seamonkey/source/content/html/content/src/nsGenericHTMLElement.cpp#2938
http://lxr.mozilla.org/seamonkey/source/content/base/src/nsHTMLContentSerializer.cpp#785
http://lxr.mozilla.org/seamonkey/source/content/base/src/nsXMLContentSerializer.cpp#604

Note that I've only looked at the content/layout DLL. (MachO builds).

Comment 36

[Simon Fraser [no longer active]]

Attachment: Huge amount of const goodness in mozilla/intl

Comment 37

[Alec Flett]

Comment on attachment 112574 [details] [diff] [review]
Make several static data tables const
[Checkin: Comment 53]

sr=alecf

Comment 38

[Alec Flett]

Comment on attachment 112831 [details] [diff] [review]
Huge amount of const goodness in mozilla/intl

wow! this is fantastic
sr=alecf - can we get these in for 1.3? I'm really curious how our startup will
be improved, not to mention our footprint.

Comment 39

[Simon Fraser [no longer active]]

I'll try and get some codesighs numbers from my macho build after applying this.

Comment 40

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 112831 [details] [diff] [review]
Huge amount of const goodness in mozilla/intl

>Index: chardet/src/nsPSMDetectors.cpp
>-nsEUCStatistics *gZhTwStatisticsSet[ZHTW_DETECTOR_NUM_VERIFIERS] = {
>+const nsEUCStatistics* gZhTwStatisticsSet[ZHTW_DETECTOR_NUM_VERIFIERS] = {

How about |const nsEUCStatistics*const| (etc.) ?

That would require some of these things to be |const nsEUCStatistics*const*|:

>-nsXPCOMDetector::nsXPCOMDetector(PRUint8 aItems, nsVerifier **aVer, nsEUCStatistics** aStatisticsSet)
>+nsXPCOMDetector::nsXPCOMDetector(PRUint8 aItems, nsVerifier **aVer, const nsEUCStatistics** aStatisticsSet)


>Index: chardet/src/nsPSMDetectors.h
>-extern nsEUCStatistics *gZhTwStatisticsSet[];
>+extern const nsEUCStatistics *gZhTwStatisticsSet[];

ditto. (etc.)

Comment 41

[Simon Fraser [no longer active]]

Attachment: Follow dbaron's suggestion; yet more constness in mozilla/intl [Checkin: Comment 54]

In this patch I typedef'd nsVerifier and nsEUCStatistics to be const, and made
the arrays themselves const. I also made large amounts of data in the
nsFooVerifier.h headers const.

Comment 42

[Simon Fraser [no longer active]]

Applied attachment 112834 [details] [diff] [review] to an opt macho build, and ran codesigh's basesummary
script before and after. Results:

Overall Change in Size
	Total:	      -7792 (+494392/-502184)
	Code:	     -19880 (+4596/-24476)
	Data:	     +12088 (+489796/-477708)

libuconv.dylib
	Total:	      -1440 (+478994/-480434)
	Code:	     -20568 (+3444/-24012)
	Data:	     +19128 (+475550/-456422)

libi18n.dylib
	Total:	      -6352 (+15398/-21750)
	Code:	       +688 (+1152/-464)
	Data:	      -7040 (+14246/-21286)

though I'm not sure how much I trust that data, since the size of
__mh_bundle_header apparently changed, and libuconv.dylib gained:
+19136	std::nothrow
in its S data section.

Comment 43

[Alec Flett]

Comment on attachment 112834 [details] [diff] [review]
Follow dbaron's suggestion; yet more constness in mozilla/intl
[Checkin: Comment 54]

hey, works for me.
sr=alecf

I don't know whats up with that std::throw stuff, but I can't see this making
anything worse...:)

Comment 44

[Simon Montagu :smontagu]

Comment on attachment 112834 [details] [diff] [review]
Follow dbaron's suggestion; yet more constness in mozilla/intl
[Checkin: Comment 54]

r=smontagu

Comment 45

[Alec Flett]

Comment on attachment 112834 [details] [diff] [review]
Follow dbaron's suggestion; yet more constness in mozilla/intl
[Checkin: Comment 54]

dbaron/simon - what are the chances we're going to get this into moz 1.3? Its
had review for a while now, we just need 1.3b approval...

Comment 46

[Alec Flett]

Comment on attachment 112574 [details] [diff] [review]
Make several static data tables const
[Checkin: Comment 53]

same thing here.. (also requesting approval)

Comment 47

[Simon Fraser [no longer active]]

Fine by me  :)  I was waiting for 1.4 to open, but 1.3 will work.

Comment 48

[Asa Dotzler [:asa]]

Comment on attachment 112834 [details] [diff] [review]
Follow dbaron's suggestion; yet more constness in mozilla/intl
[Checkin: Comment 54]

We're already late and trying to get beta out ASAP. This is going to have to
wait.

Comment 49

[Simon Fraser [no longer active]]

Attachment: Make kInlineFrameCID const [Checkin: Comment 55]

Comment 50

[Simon Fraser [no longer active]]

Attachment: Fix nsMathMLContainerFrame not to rely on nsInlineFrame CID [Checkin: Comment 56]

Comment 51

[Alec Flett]

Comment on attachment 114518 [details] [diff] [review]
Make kInlineFrameCID const
[Checkin: Comment 55]

sr=alecf

Comment 52

[Alec Flett]

Comment on attachment 114701 [details] [diff] [review]
Fix nsMathMLContainerFrame not to rely on nsInlineFrame CID
[Checkin: Comment 56]

sr=alecf

Comment 53

[Simon Fraser [no longer active]]

Comment on attachment 112574 [details] [diff] [review]
Make several static data tables const
[Checkin: Comment 53]

Patch checked in.

Comment 54

[Simon Fraser [no longer active]]

Comment on attachment 112834 [details] [diff] [review]
Follow dbaron's suggestion; yet more constness in mozilla/intl
[Checkin: Comment 54]

Patch checked in.

Comment 55

[Simon Fraser [no longer active]]

Comment on attachment 114518 [details] [diff] [review]
Make kInlineFrameCID const
[Checkin: Comment 55]

Patch checked in.

Comment 56

[Simon Fraser [no longer active]]

Comment on attachment 114701 [details] [diff] [review]
Fix nsMathMLContainerFrame not to rely on nsInlineFrame CID
[Checkin: Comment 56]

Patch checked in.
Keeping bug open for additional fixing.

Comment 57

[Boris Zbarsky [:bzbarsky]]

So... looking at the 21k static size increase for libuconv from this checkin,
this looks fishy:

      -124	g_ufJohabJamoMapping
     +21464	g_ufJohabJamoMapping

I suspect codesighs is just confused, since there was no real change here (nor
for nsColorNames::kColors, which supposedly grew by 1440 bytes....)

Comment 58

[Simon Fraser [no longer active]]

Yes, I think codesighs does a poor job of getting accurate symbol sizes. For
example, on Linux:

libuconv.so
    +478136 (+478204/-68)	R (DATA)
        +478136 (+478204/-68)	UNDEF:libuconv.so:R
             +48256	gGBKToUnicodeTable
...
    -456704 (+44/-456748)	D (DATA)
        -456704 (+44/-456748)	UNDEF:libuconv.so:D
             -48160	gGBKToUnicodeTable

but examination of the symbols (on OS X) shows that they are binary identical,
other than some padding at the end.

After my changes, on OS X, the lib actually got smaller:

-rwxr-xr-x    1 smfr  staff  4380320 Feb 26 09:38 libuconv_before.dylib
-rwxr-xr-x    1 smfr  staff  4379232 Feb 26 09:40 libuconv_after.dylib

Comment 59

[Simon Fraser [no longer active]]

Attachment: const goodness in htmlparser

Comment 60

[Simon Fraser [no longer active]]

Attachment: More const goodness in intl/uconv. Really.

Comment 61

[Simon Fraser [no longer active]]

Attachment: Make nsModuleInfo const

Comment 62

[Simon Fraser [no longer active]]

Attachment: Const changes in i18n [Checkin: See comment 72+85]

Comment 63

[Simon Fraser [no longer active]]

Attachment: Const changes in netwerk

Comment 64

[Darin Fisher]

Comment on attachment 115731 [details] [diff] [review]
Const changes in netwerk

>Index: base/src/nsIOService.cpp

>+const PRInt16 gBadPortList[] = { 

looks like this could be static as well.  otherwise, looks great!  r=darin

Comment 65

[Alec Flett]

Comment on attachment 115701 [details] [diff] [review]
const goodness in htmlparser

kick ass!
though personally I prefer the C++ casting syntax:
PRUnichar('_')
r/sr=alecf

Comment 66

[Alec Flett]

Comment on attachment 115715 [details] [diff] [review]
More const goodness in intl/uconv. Really.

r/sr=alecf (looks like a little overlap with the NS_DEFINE_IID work, but that's
fine)

Comment 67

[Alec Flett]

Comment on attachment 115720 [details] [diff] [review]
Make nsModuleInfo const

nice! sr=alecf

Comment 68

[Doug Turner (:dougt)]

Comment on attachment 115720 [details] [diff] [review]
Make nsModuleInfo const

make sure that this doesn't break the static build. r=dougt.

Comment 69

[Alec Flett]

Comment on attachment 115725 [details] [diff] [review]
Const changes in i18n
[Checkin: See comment 72+85]

r/sr=alecf

Comment 70

[Alec Flett]

Comment on attachment 115731 [details] [diff] [review]
Const changes in netwerk

sr=alecf
I kind of feel like I've tried to make some PLDHashTableOps const before, and
it wouldn't build.. but if you've got it, then no complaints here :)

Comment 71

[Simon Fraser [no longer active]]

> I kind of feel like I've tried to make some PLDHashTableOps const before, and
> it wouldn't build.. but if you've got it, then no complaints here :)

You're right, it spits out warnings on Mac OS X. We should get pldhash to take a
const PLDHashTableOps*. I filed bug 195298.

Comment 72

[Simon Fraser [no longer active]]

Comment on attachment 115725 [details] [diff] [review]
Const changes in i18n
[Checkin: See comment 72+85]

This was checked in.

Comment 73

[Simon Fraser [no longer active]]

Attachment: const goodness in extensions/universalchardet

universalchardet had a ton of non-const data (about 10Kb worth) which really needs to be const.

Comment 74

[Simon Fraser [no longer active]]

Comment on attachment 115731 [details] [diff] [review]
Const changes in netwerk

This was never checked in  :(

Comment 75

[Simon Fraser [no longer active]]

Attachment: const goodness in xpconnect [Checkin: Comment 82]

Comment 76

[Simon Fraser [no longer active]]

Attachment: const goodness in intl/chardet

Comment 77

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 204418 [details] [diff] [review]
const goodness in extensions/universalchardet

>Index: src/base/JpCntx.cpp
>-        mRelSample[jp2CharContext[mLastCharOrder][order]]++;
>+        mRelSample[(int)jp2CharContext[mLastCharOrder][order]]++;

Construction-style cast would be much easier to read, i.e.:

mRelSample[int(p2CharContext[mLastCharOrder][order])]++

>Index: src/base/JpCntx.h
>-      mRelSample[jp2CharContext[mLastCharOrder][order]]++;
>+      mRelSample[(int)jp2CharContext[mLastCharOrder][order]]++;

ditto.

Not sure why you're casting, though.  Is it a warning fix?  Should the array be of unsigned char instead of char?

>Index: src/base/nsCodingStateMachine.h
> typedef struct 
> {
>-  nsPkgInt classTable;
>-  PRUint32 classFactor;
>-  nsPkgInt stateTable;
>-  const PRUint32* charLenTable;
>-  const char* name;
>+  const nsPkgInt  classTable;
>+  PRUint32        classFactor;
>+  const nsPkgInt  stateTable;

I don't see the point of the two consts added above.  Isn't that implied when an |SMModel| itself is const?

>+  const PRUint32* const charLenTable;

Likewise for the added second |const| on this line.

>+  const char*     name;
> } SMModel;

r=dbaron.  Sorry for the delay.

Comment 78

[Alfred Kayser]

Who is working on this? The patches look valid still (or at least large chunks of them)

Comment 79

[Simon Fraser [no longer active]]

Sorry, I'm not working on this any more.

Comment 80

[Wayne Mery (:wsmwk)]

Gary, are you able to test your patch applicability magic on this?

note: this is the last bug blocking  Bug 98284 xpcom footprint tracking

Comment 81

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to comment #75)
> Created an attachment (id=204419) [details]
> const goodness in xpconnect

Only attachment 204419 [details] [diff] [review] applies in mozilla-1.9.1 tree.

Status of all non-obsolete patches:
115701 - rotted
115715 - rotted
115720 - rotted
115731 - rotted
204418 - rotted
204419 - still applies
204420 - rotted

===

$ patch -p0 --dry-run < ~/Desktop/204419.txt 
patching file xpcvariant.cpp
Hunk #1 succeeded at 151 (offset 41 lines).

Comment 82

[Serge Gautherie (:sgautherie)]

Comment on attachment 204419 [details] [diff] [review]
const goodness in xpconnect
[Checkin: Comment 82]


http://hg.mozilla.org/mozilla-central/rev/9d8079165852

Comment 83

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 204419 [details] [diff] [review]
const goodness in xpconnect
[Checkin: Comment 82]

Don't think we need this on branch.

Comment 84

[Serge Gautherie (:sgautherie)]

Comment on attachment 67800 [details] [diff] [review]
Move nsIndexedToHTML::Create impl to .cpp
[Checkin: Comment 84]


Move impl of nsIndexedToHTML::Create from .h to .cpp to fix BeOS bustage.
Bug 74803 sr=sfraser
2002-02-04 15:23	seawood%netscape.com 	mozilla/netwerk/streamconv/converters/nsIndexedToHTML.cpp 	1.19 	14/0
2002-02-04 15:23	seawood%netscape.com 	mozilla/netwerk/streamconv/converters/nsIndexedToHTML.h 	1.8 	1/12

Comment 85

[Serge Gautherie (:sgautherie)]

Comment on attachment 115725 [details] [diff] [review]
Const changes in i18n
[Checkin: See comment 72+85]


Bug 207542 checked a part of this in,
but (the rest of) this patch was not actually checked in:
we should check whether we could do more here...

Comment 86

[Navin Lal]

Attachment: const changes to extensions/spellcheck

Comment 87

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 389757 [details] [diff] [review]
const changes to extensions/spellcheck

r=dbaron, although it's probably worth fixing all 10 occurrences in http://mxr.mozilla.org/mozilla-central/search?string=static%20nsmodulecomponentinfo at once rather than doing it one-at-a-time

Comment 88

[Navin Lal]

Attachment: const changes for nsmodulecomponentinfo

Comment 89

[Navin Lal]

Comment on attachment 389801 [details] [diff] [review]
const changes for nsmodulecomponentinfo

>diff --git a/embedding/browser/photon/src/nsUnknownContentTypeHandler.cpp b/embedding/browser/photon/src/nsUnknownContentTypeHandler.cpp
>--- a/embedding/browser/photon/src/nsUnknownContentTypeHandler.cpp
>+++ b/embedding/browser/photon/src/nsUnknownContentTypeHandler.cpp
>@@ -199,17 +199,17 @@ NS_IMETHODIMP className::QueryInterface(
> 		else rv = NS_NOINTERFACE;
> 		} 
> 	return rv; 
> 	}
> 
> NS_GENERIC_FACTORY_CONSTRUCTOR( nsUnknownContentTypeHandler )
> 
> // The list of components we register
>-static nsModuleComponentInfo info[] = {
>+static const nsModuleComponentInfo info[] = {
> 		{
> 			"nsUnknownContentTypeHandler",
> 			NS_IHELPERAPPLAUNCHERDIALOG_IID,
> 			NS_IHELPERAPPLAUNCHERDLG_CONTRACTID,
> 			nsUnknownContentTypeHandlerConstructor
> 		}
> 	};
> 
>diff --git a/extensions/auth/nsAuthFactory.cpp b/extensions/auth/nsAuthFactory.cpp
>--- a/extensions/auth/nsAuthFactory.cpp
>+++ b/extensions/auth/nsAuthFactory.cpp
>@@ -204,17 +204,17 @@ NS_GENERIC_FACTORY_CONSTRUCTOR(nsAuthSSP
>   {0x93, 0x4b, 0x14, 0x8e, 0x33, 0xc2, 0x28, 0xa6} \
> }
> 
> #include "nsAuthSASL.h"
> NS_GENERIC_FACTORY_CONSTRUCTOR(nsAuthSASL)
> 
> //-----------------------------------------------------------------------------
> 
>-static nsModuleComponentInfo components[] = {
>+static const nsModuleComponentInfo components[] = {
>   { "nsAuthKerbGSS", 
>     NS_GSSAUTH_CID,
>     NS_AUTH_MODULE_CONTRACTID_PREFIX "kerb-gss",
>     nsKerbGSSAPIAuthConstructor
>   },
>   { "nsAuthNegoGSSAPI", 
>     NS_NEGOTIATEAUTH_CID,
>     NS_AUTH_MODULE_CONTRACTID_PREFIX "negotiate-gss",
>diff --git a/extensions/spellcheck/src/mozSpellCheckerFactory.cpp b/extensions/spellcheck/src/mozSpellCheckerFactory.cpp
>--- a/extensions/spellcheck/src/mozSpellCheckerFactory.cpp
>+++ b/extensions/spellcheck/src/mozSpellCheckerFactory.cpp
>@@ -112,17 +112,17 @@ mozInlineSpellCheckerConstructor(nsISupp
>   return rv;
> }
> 
> ////////////////////////////////////////////////////////////////////////
> // Define a table of CIDs implemented by this module along with other
> // information like the function to create an instance, contractid, and
> // class name.
> //
>-static nsModuleComponentInfo components[] = {
>+static const nsModuleComponentInfo components[] = {
> #ifdef MOZ_MACBROWSER
>     {
>         "OSX Spell check service",
>         MOZ_OSXSPELL_CID,
>         MOZ_OSXSPELL_CONTRACTID,
>         mozOSXSpellConstructor
>     },
> #else
>diff --git a/intl/build/nsI18nModule.cpp b/intl/build/nsI18nModule.cpp
>--- a/intl/build/nsI18nModule.cpp
>+++ b/intl/build/nsI18nModule.cpp
>@@ -51,17 +51,17 @@
> #include "nsStrBundleConstructors.h"
> 
> // locale
> #include "nsLocaleConstructors.h"
> 
> 
> NS_GENERIC_FACTORY_CONSTRUCTOR(nsSemanticUnitScanner)
> 
>-static nsModuleComponentInfo components[] =
>+static const nsModuleComponentInfo components[] =
> {
>  // lwbrk
>   { "Line Breaker", NS_LBRK_CID, 
>     NS_LBRK_CONTRACTID, nsJISx4051LineBreakerConstructor},
>   { "Word Breaker", NS_WBRK_CID,
>     NS_WBRK_CONTRACTID, nsSampleWordBreakerConstructor},
>   { "Semantic Unit Scanner", NS_SEMANTICUNITSCANNER_CID,
>     NS_SEMANTICUNITSCANNER_CONTRACTID, nsSemanticUnitScannerConstructor},
>diff --git a/intl/chardet/src/nsChardetModule.cpp b/intl/chardet/src/nsChardetModule.cpp
>--- a/intl/chardet/src/nsChardetModule.cpp
>+++ b/intl/chardet/src/nsChardetModule.cpp
>@@ -122,17 +122,17 @@ nsUKProbDetectorRegistrationProc(nsIComp
>                                  const nsModuleComponentInfo *info)
> {
>   return AddCategoryEntry(NS_CHARSET_DETECTOR_CATEGORY,
>                           "ukprob",
>                           info->mContractID);
> }
> 
> 
>-static nsModuleComponentInfo components[] =
>+static const nsModuleComponentInfo components[] =
> {
>  { "Meta Charset", NS_META_CHARSET_CID, 
>     NS_META_CHARSET_CONTRACTID, nsMetaCharsetObserverConstructor, 
>     nsMetaCharsetObserverRegistrationProc, nsMetaCharsetObserverUnegistrationProc,
>     NULL},
>  { "Document Charset Info", NS_DOCUMENTCHARSETINFO_CID, 
>     NS_DOCUMENTCHARSETINFO_CONTRACTID, nsDocumentCharsetInfoConstructor, 
>     NULL, NULL},
>diff --git a/modules/libjar/zipwriter/src/ZipWriterModule.cpp b/modules/libjar/zipwriter/src/ZipWriterModule.cpp
>--- a/modules/libjar/zipwriter/src/ZipWriterModule.cpp
>+++ b/modules/libjar/zipwriter/src/ZipWriterModule.cpp
>@@ -38,17 +38,17 @@
> 
> #include "nsIGenericFactory.h"
> #include "nsDeflateConverter.h"
> #include "nsZipWriter.h"
> 
> NS_GENERIC_FACTORY_CONSTRUCTOR(nsDeflateConverter)
> NS_GENERIC_FACTORY_CONSTRUCTOR(nsZipWriter)
> 
>-static nsModuleComponentInfo components[] =
>+static const nsModuleComponentInfo components[] =
> {
>   {
>     DEFLATECONVERTER_CLASSNAME,
>     DEFLATECONVERTER_CID,
>     "@mozilla.org/streamconv;1?from=uncompressed&to=deflate",
>     nsDeflateConverterConstructor,
>   },
>   {
>diff --git a/toolkit/xre/nsAppRunner.cpp b/toolkit/xre/nsAppRunner.cpp
>--- a/toolkit/xre/nsAppRunner.cpp
>+++ b/toolkit/xre/nsAppRunner.cpp
>@@ -952,17 +952,17 @@ ScopedXPCOMStartup::~ScopedXPCOMStartup(
>     mServiceManager = nsnull;
>   }
> }
> 
> // {95d89e3e-a169-41a3-8e56-719978e15b12}
> #define APPINFO_CID \
>   { 0x95d89e3e, 0xa169, 0x41a3, { 0x8e, 0x56, 0x71, 0x99, 0x78, 0xe1, 0x5b, 0x12 } }
> 
>-static nsModuleComponentInfo kComponents[] =
>+static const nsModuleComponentInfo kComponents[] =
> {
>   {
>     "nsXULAppInfo",
>     APPINFO_CID,
>     XULAPPINFO_SERVICE_CONTRACTID,
>     AppInfoConstructor
>   },
>   {
>diff --git a/widget/src/photon/nsWidgetFactory.cpp b/widget/src/photon/nsWidgetFactory.cpp
>--- a/widget/src/photon/nsWidgetFactory.cpp
>+++ b/widget/src/photon/nsWidgetFactory.cpp
>@@ -74,17 +74,17 @@ NS_GENERIC_FACTORY_CONSTRUCTOR(nsDragSer
> #endif
> NS_GENERIC_FACTORY_CONSTRUCTOR(nsSound)
> NS_GENERIC_FACTORY_CONSTRUCTOR(nsFilePicker)
> 
> #ifdef IBMBIDI
> NS_GENERIC_FACTORY_CONSTRUCTOR(nsBidiKeyboard)
> #endif
> 
>-static nsModuleComponentInfo components[] =
>+static const nsModuleComponentInfo components[] =
> {
>   { "Ph nsWindow",
>     NS_WINDOW_CID,
>     "@mozilla.org/widgets/window/ph;1",
>     nsWindowConstructor },
>   { "Ph Child nsWindow",
>     NS_CHILD_CID,
>     "@mozilla.org/widgets/child_window/ph;1",

Comment 90

[Navin Lal]

Attachment: const changes for nsmodulecomponentinfo [Checkin: Comment 92]

Comment 91

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 389803 [details] [diff] [review]
const changes for nsmodulecomponentinfo
[Checkin: Comment 92]

r=dbaron

Comment 92

[Serge Gautherie (:sgautherie)]

Comment on attachment 389803 [details] [diff] [review]
const changes for nsmodulecomponentinfo
[Checkin: Comment 92]


http://hg.mozilla.org/mozilla-central/rev/112b86848e92

Comment 93

[Mike Hoye [:mhoye]]

Would we any/all of these patches at this point, if a contributor could clean them up?

Comment 94

[(no longer active)]

(In reply to Mike Hoye [:mhoye] from comment #93)
> Would we any/all of these patches at this point, if a contributor could
> clean them up?

Well, someone needs to first verify if comment 0 is (still) true.

Comment 95

[Mike Hoye [:mhoye]]

Dropping good-first-bug status here, on the grounds that this need for this is not currently clear and we don't have a crisp description of a fix.