More LayoutLocale refactor with additional Chinese support

More LayoutLocale refactor with additional Chinese support

Following the initial LayoutLocale refactoring CL[1], this patch:

1. Support 14 encompassed languages within the Chinese macrolanguage[2].
2. Add "mo" (Macau) as "Traditional by default", as pointed out by W3C
   I18N WG and match to Firefox.
3. Better and more spec conformance to parse BCP-47 language tags[3].
4. Change "und-Zsye" (Emoji) priority from the lowest to the highest.
5. Unify the logic for disambiguation of the Unified Han Ideographs for
   Linux/Android and Windows.
6. Merge duplicated code in AcceptLanguagesResolver to LayoutLocale.
7. Centralize locale-related methods more to LayoutLocale for better
   discoverability, caching, and code sharing.

[1] https://codereview.chromium.org/2161683002
[2] http://www-01.sil.org/iso639-3/documentation.asp?id=zho
[3] https://tools.ietf.org/html/bcp47

BUG=586517, 611817
Committed: https://crrev.com/3ec109dc729a647b433136b2328e9da01ba9c8b8
Cr-Commit-Position: refs/heads/master@{#409157}

[kojii, 2016-07-28 04:46:44]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-28 04:46:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-28 05:58:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-28 05:58:29]

Dry run: This issue passed the CQ dry run.

[kojii, 2016-07-28 06:29:18]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-28 06:29:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kojii, 2016-07-28 06:48:36]

Description was changed from

==========
WIP: locale-refactoring

BUG=
==========

to

==========
More LayoutLocale refactor with additional Chinese support

Following the initial LayoutLocale refactoring CL[1], this patch:

1. Support 14 encompassed languages within the Chinese macrolanguage[2].
2. Add "mo" (Macau) as "Traditional by default", as pointed out by W3C
   I18N WG.
3. Better and more spec conformance to parse BCP-47 language tags[3].
4. Change "und-Zsye" (Emoji) priority from the lowest to the highest.
5. Unify the logic for disambiguation of the Unified Han Ideographs for
   Linux/Android and Windows.
6. Merge duplicated code in AcceptLanguagesResolver to LayoutLocale.
7. Centralize locale-related methods more to LayoutLocale for better
   discoverability, caching, and code sharing.

[1] https://codereview.chromium.org/2161683002
[2] http://www-01.sil.org/iso639-3/documentation.asp?id=zho
[3] https://tools.ietf.org/html/bcp47

BUG=586517, 611817
==========

[kojii, 2016-07-28 06:56:14]

kojii@chromium.org changed reviewers:
+ drott@chromium.org, eae@chromium.org

[kojii, 2016-07-28 06:56:15]

PTAL.

drott@, I included the Emoji locale priority fix we discussed offline in this CL
because it's small, but please let me know if you want to handle, if that should
be in a separate CL, or whatever else.

[kojii, 2016-07-28 07:07:11]

Description was changed from

==========
More LayoutLocale refactor with additional Chinese support

Following the initial LayoutLocale refactoring CL[1], this patch:

1. Support 14 encompassed languages within the Chinese macrolanguage[2].
2. Add "mo" (Macau) as "Traditional by default", as pointed out by W3C
   I18N WG.
3. Better and more spec conformance to parse BCP-47 language tags[3].
4. Change "und-Zsye" (Emoji) priority from the lowest to the highest.
5. Unify the logic for disambiguation of the Unified Han Ideographs for
   Linux/Android and Windows.
6. Merge duplicated code in AcceptLanguagesResolver to LayoutLocale.
7. Centralize locale-related methods more to LayoutLocale for better
   discoverability, caching, and code sharing.

[1] https://codereview.chromium.org/2161683002
[2] http://www-01.sil.org/iso639-3/documentation.asp?id=zho
[3] https://tools.ietf.org/html/bcp47

BUG=586517, 611817
==========

to

==========
More LayoutLocale refactor with additional Chinese support

Following the initial LayoutLocale refactoring CL[1], this patch:

1. Support 14 encompassed languages within the Chinese macrolanguage[2].
2. Add "mo" (Macau) as "Traditional by default", as pointed out by W3C
   I18N WG and match to Firefox.
3. Better and more spec conformance to parse BCP-47 language tags[3].
4. Change "und-Zsye" (Emoji) priority from the lowest to the highest.
5. Unify the logic for disambiguation of the Unified Han Ideographs for
   Linux/Android and Windows.
6. Merge duplicated code in AcceptLanguagesResolver to LayoutLocale.
7. Centralize locale-related methods more to LayoutLocale for better
   discoverability, caching, and code sharing.

[1] https://codereview.chromium.org/2161683002
[2] http://www-01.sil.org/iso639-3/documentation.asp?id=zho
[3] https://tools.ietf.org/html/bcp47

BUG=586517, 611817
==========

[drott, 2016-07-28 08:05:23]

Small remarks, the overall change looks great - happy to see locale usage
getting cleaned up and sharpened.

https://codereview.chromium.org/2192703002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/fonts/AcceptLanguagesResolver.cpp
(right):

https://codereview.chromium.org/2192703002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/fonts/AcceptLanguagesResolver.cpp:14:
LayoutLocale::setLocaleForHan(
This line looks a bit surprising to me. Is this triggering an update on the
object returned from localeForHanFromAcceptLanguages?

https://codereview.chromium.org/2192703002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/fonts/skia/FontCacheSkia.cpp (right):

https://codereview.chromium.org/2192703002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/fonts/skia/FontCacheSkia.cpp:84: // The last
one is the highest.
Perhaps we can rephrase this second line of the comment: Skia expects the
highest array index to be the first priority.

Makes sense to include this change.

Did you test this with mixed text-presentation and emoji-presentation pages on
Android?

[commit-bot: I haz the power, 2016-07-28 09:59:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-28 09:59:26]

Dry run: This issue passed the CQ dry run.

[eae, 2016-07-28 17:08:40]

LGTM

[kojii, 2016-07-29 04:00:52]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-29 04:01:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kojii, 2016-07-29 04:27:40]

drott@, comments inline:

https://codereview.chromium.org/2192703002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/fonts/AcceptLanguagesResolver.cpp
(right):

https://codereview.chromium.org/2192703002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/fonts/AcceptLanguagesResolver.cpp:14:
LayoutLocale::setLocaleForHan(
On 2016/07/28 at 08:05:23, drott wrote:
> This line looks a bit surprising to me. Is this triggering an update on the
object returned from localeForHanFromAcceptLanguages?

No, the returned object is const. The pointer to the returned object is set to a
static variable in LayoutLocale (static const LayoutLocale* s_defaultForHan).

https://codereview.chromium.org/2192703002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/fonts/skia/FontCacheSkia.cpp (right):

https://codereview.chromium.org/2192703002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/fonts/skia/FontCacheSkia.cpp:84: // The last
one is the highest.
On 2016/07/28 at 08:05:23, drott wrote:
> Perhaps we can rephrase this second line of the comment: Skia expects the
highest array index to be the first priority.

Your text looks great, fixed. Wanted to put a link to Skia documents but it
looks like there doesn't exist...

> Makes sense to include this change.
> 
> Did you test this with mixed text-presentation and emoji-presentation pages on
Android?

Tested a few pages on a real device and see no differences.
http://unicode.org/emoji/charts/emoji-zwj-sequences.html
http://apps.timwhitlock.info/emoji/tables/unicode

I now remember we don't run most of LayoutTests on Android. Probably we should
have gtest or make a LayoutTest runnable on Linux that differentiate this line,
but I know it's a bit tricky to build such a test.

At least changing the order doesn't break major cases, and logically this is the
right fix, as we add content locale after the default locale.

[commit-bot: I haz the power, 2016-07-29 06:35:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-29 06:35:51]

Dry run: This issue passed the CQ dry run.

[drott, 2016-08-02 07:14:15]

LGTM, thanks.

https://codereview.chromium.org/2192703002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/fonts/AcceptLanguagesResolver.cpp
(right):

https://codereview.chromium.org/2192703002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/fonts/AcceptLanguagesResolver.cpp:14:
LayoutLocale::setLocaleForHan(
On 2016/07/29 at 04:27:40, kojii wrote:
> On 2016/07/28 at 08:05:23, drott wrote:
> > This line looks a bit surprising to me. Is this triggering an update on the
object returned from localeForHanFromAcceptLanguages?
> 
> No, the returned object is const. The pointer to the returned object is set to
a static variable in LayoutLocale (static const LayoutLocale* s_defaultForHan).

Got it, thanks for the explanation. Now I see that this was in the
acceptLanguagesChanged() and I rememebered what we discussed for when we
introduced this callback from the host process. Makes sense.

[kojii, 2016-08-02 07:15:44]

Thanks!

[kojii, 2016-08-02 07:15:47]

The CQ bit was checked by kojii@chromium.org

[kojii, 2016-08-02 07:15:47]

The patchset sent to the CQ was uploaded after l-g-t-m from eae@chromium.org
Link to the patchset: https://codereview.chromium.org/2192703002/#ps40001
(title: "Comment updated as per drott review")

[commit-bot: I haz the power, 2016-08-02 07:15:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-02 08:27:15]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-02 08:28:32]

Description was changed from

==========
More LayoutLocale refactor with additional Chinese support

Following the initial LayoutLocale refactoring CL[1], this patch:

1. Support 14 encompassed languages within the Chinese macrolanguage[2].
2. Add "mo" (Macau) as "Traditional by default", as pointed out by W3C
   I18N WG and match to Firefox.
3. Better and more spec conformance to parse BCP-47 language tags[3].
4. Change "und-Zsye" (Emoji) priority from the lowest to the highest.
5. Unify the logic for disambiguation of the Unified Han Ideographs for
   Linux/Android and Windows.
6. Merge duplicated code in AcceptLanguagesResolver to LayoutLocale.
7. Centralize locale-related methods more to LayoutLocale for better
   discoverability, caching, and code sharing.

[1] https://codereview.chromium.org/2161683002
[2] http://www-01.sil.org/iso639-3/documentation.asp?id=zho
[3] https://tools.ietf.org/html/bcp47

BUG=586517, 611817
==========

to

==========
More LayoutLocale refactor with additional Chinese support

Following the initial LayoutLocale refactoring CL[1], this patch:

1. Support 14 encompassed languages within the Chinese macrolanguage[2].
2. Add "mo" (Macau) as "Traditional by default", as pointed out by W3C
   I18N WG and match to Firefox.
3. Better and more spec conformance to parse BCP-47 language tags[3].
4. Change "und-Zsye" (Emoji) priority from the lowest to the highest.
5. Unify the logic for disambiguation of the Unified Han Ideographs for
   Linux/Android and Windows.
6. Merge duplicated code in AcceptLanguagesResolver to LayoutLocale.
7. Centralize locale-related methods more to LayoutLocale for better
   discoverability, caching, and code sharing.

[1] https://codereview.chromium.org/2161683002
[2] http://www-01.sil.org/iso639-3/documentation.asp?id=zho
[3] https://tools.ietf.org/html/bcp47

BUG=586517, 611817

Committed: https://crrev.com/3ec109dc729a647b433136b2328e9da01ba9c8b8
Cr-Commit-Position: refs/heads/master@{#409157}
==========

[commit-bot: I haz the power, 2016-08-02 08:28:33]

Patchset 3 (id:??) landed as
https://crrev.com/3ec109dc729a647b433136b2328e9da01ba9c8b8
Cr-Commit-Position: refs/heads/master@{#409157}