1262324 - Two minor intl/locale/ tweaks

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Nicholas Nethercote [inactive]]

I have two minor intl/locale/ tweaks that I stumbled across while looking at the size of Firefox's binary.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Remove "locale.all" prefix from Unix charsets

Currently, every entry in unixcharset.properties starts with "locale.all", and
then when searching them we prepend "locale.all" to the key.

This patch removes the prefixes in both cases, and saves 4 KiB in .text in the
process.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Introduce nsEncodingProp

I'm open to other suggestions for the new type name and its field names.

Comment 3

[Masatoshi Kimura [:emk]]

Comment on attachment 8738337 [details] [diff] [review]
(part 1) - Remove "locale.all" prefix from Unix charsets

Nice cleanup.

Comment 4

[Masatoshi Kimura [:emk]]

Comment on attachment 8738347 [details] [diff] [review]
(part 2) - Introduce nsEncodingProp

Review of attachment 8738347 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with following comments fixed.

::: dom/encoding/EncodingUtils.cpp
@@ +14,5 @@
>  
>  namespace mozilla {
>  namespace dom {
>  
> +static const nsEncodingProp labelsEncodings[] = {

Maybe nsUConvProp is better. Obviously a langGroup is not an encoding.

::: intl/locale/nsUConvPropertySearch.cpp
@@ +33,5 @@
>    size_t index;
>    if (BinarySearchIf(aProperties, 0, aNumberOfProperties,
>                       PropertyComparator(flat), &index)) {
> +    nsDependentCString val(aProperties[index].mValue,
> +                           NS_PTR_TO_UINT32(aProperties[index].mValueLength));

NS_PTR_TO_UINT32 was a trick to store an integer to an element of a pointer array. Now this is unnecessary.

::: intl/locale/nsUConvPropertySearch.h
@@ +10,5 @@
> +struct nsEncodingProp
> +{
> +    const char* const mKey;
> +    const char* const mValue;
> +    const int32_t mValueLength;

uint32_t. length can't be negative.

Comment 5

[Nicholas Nethercote [inactive]]

Thank you for the fast reviews.

Comment 6

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/978884d01e8ef968262e54138bf43acdbbc603b6
Bug 1262324 (part 1) - Remove "locale.all" prefix from Unix charsets. r=emk.

https://hg.mozilla.org/integration/mozilla-inbound/rev/835b1d9e49182f5a91e9dc931c19b836e14085ba
Bug 1262324 (part 2) - Introduce nsUConvProp. r=emk.

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/978884d01e8e
https://hg.mozilla.org/mozilla-central/rev/835b1d9e4918