Fix two DCHECK failures in ICU case mapping code

Fix two DCHECK failures in ICU case mapping code

1.
DCHECK in runtime-i18n.cc for case mapping was wrong to
assume that the longest primary language tag is 3 characters.
BCP 47 actually allows up to 8 characters.

2. GetFlatContent() was called to a string without flattening it first.

BUG=680314, 680464
TEST=intl/general/case-mapping (see also the bugs)

Review-Url: https://codereview.chromium.org/2629763003
Cr-Commit-Position: refs/heads/master@{#42343}
Committed: https://chromium.googlesource.com/v8/v8/+/ac9e628539eec41ebdd7142eebb324683dfe46b7

[jungshik at Google, 2017-01-13 00:50:26]

jshin@chromium.org changed reviewers:
+ littledan@chromium.org

[jungshik at Google, 2017-01-13 00:50:27]

Dan, can you take a look? Thanks

[jungshik at Google, 2017-01-13 01:24:39]

Description was changed from

==========
Fix DCHECK to allow up to 8 chars long lang tag

DCHECK in runtime-i18n.cc for case mapping was wrong to
assume that the longest primary language tag is 3 characters.
BCP 47 actually allows up to 8 characters.

BUG=680314
TEST=intl/general/case-mapping
==========

to

==========
Fix two DCHECK failures in ICU case mapping code

1.
DCHECK in runtime-i18n.cc for case mapping was wrong to
assume that the longest primary language tag is 3 characters.
BCP 47 actually allows up to 8 characters.

2. GetFlatContent() was called to a string without flattening it first.

BUG=680314,680464
TEST=intl/general/case-mapping (see also the bugs)
==========

[Dan Ehrenberg, 2017-01-13 02:24:00]

On 2017/01/13 00:50:27, jungshik at Google (ooo) wrote:
> Dan, can you take a look? Thanks

Is it possible to write a test to hit the non-flat case? E.g., does
("x"+"y").toUpperCase() hit this non-flat case?

[jungshik at Google, 2017-01-13 17:56:40]

On 2017/01/13 02:24:00, Dan Ehrenberg wrote:
> On 2017/01/13 00:50:27, jungshik at Google (ooo) wrote:
> > Dan, can you take a look? Thanks
> 
> Is it possible to write a test to hit the non-flat case? E.g., does
> ("x"+"y").toUpperCase() hit this non-flat case?

Lines 71 ~ 79 also deal with cons strings. However, neither they nor your case
would
hit this particular path (that led to a crash) because it'd only hit when
'locale/langId' is 3 characters or longer. 
Lines 198-199 does that. Let me move it to the 'cons string' block (lines 71 ~
79). I'll also add a simpler case (yours).

[jungshik at Google, 2017-01-13 18:35:37]

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

[commit-bot: I haz the power, 2017-01-13 18:35:48]

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, 2017-01-13 19:01:59]

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

[commit-bot: I haz the power, 2017-01-13 19:02:00]

Dry run: This issue passed the CQ dry run.

[jungshik at Google, 2017-01-13 20:12:13]

On 2017/01/13 17:56:40, jungshik at Google (ooo) wrote:
> On 2017/01/13 02:24:00, Dan Ehrenberg wrote:
> > On 2017/01/13 00:50:27, jungshik at Google (ooo) wrote:
> > > Dan, can you take a look? Thanks
> > 
> > Is it possible to write a test to hit the non-flat case? E.g., does
> > ("x"+"y").toUpperCase() hit this non-flat case?
> 
> Lines 71 ~ 79 also deal with cons strings. However, neither they nor your case
> would
> hit this particular path (that led to a crash) because it'd only hit when
> 'locale/langId' is 3 characters or longer. 
> Lines 198-199 does that. Let me move it to the 'cons string' block (lines 71 ~
> 79). I'll also add a simpler case (yours).

Updated the CL with more tests. Can you take another look? Thanks !

[jungshik at Google, 2017-01-13 20:24:43]

Description was changed from

==========
Fix two DCHECK failures in ICU case mapping code

1.
DCHECK in runtime-i18n.cc for case mapping was wrong to
assume that the longest primary language tag is 3 characters.
BCP 47 actually allows up to 8 characters.

2. GetFlatContent() was called to a string without flattening it first.

BUG=680314,680464
TEST=intl/general/case-mapping (see also the bugs)
==========

to

==========
Fix two DCHECK failures in ICU case mapping code

1.
DCHECK in runtime-i18n.cc for case mapping was wrong to
assume that the longest primary language tag is 3 characters.
BCP 47 actually allows up to 8 characters.

2. GetFlatContent() was called to a string without flattening it first.

BUG=680314,680464
TEST=intl/general/case-mapping (see also the bugs)
==========

[Dan Ehrenberg, 2017-01-13 20:52:56]

lgtm

[jungshik at Google, 2017-01-13 23:10:41]

The CQ bit was checked by jshin@chromium.org

[commit-bot: I haz the power, 2017-01-13 23:10:48]

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, 2017-01-13 23:12:44]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484349041840360,
"parent_rev": "f8fd6ec3dd89cf1278c0ac00d5ae9e0f480e4d4a", "commit_rev":
"ac9e628539eec41ebdd7142eebb324683dfe46b7"}

[commit-bot: I haz the power, 2017-01-13 23:12:48]

Description was changed from

==========
Fix two DCHECK failures in ICU case mapping code

1.
DCHECK in runtime-i18n.cc for case mapping was wrong to
assume that the longest primary language tag is 3 characters.
BCP 47 actually allows up to 8 characters.

2. GetFlatContent() was called to a string without flattening it first.

BUG=680314,680464
TEST=intl/general/case-mapping (see also the bugs)
==========

to

==========
Fix two DCHECK failures in ICU case mapping code

1.
DCHECK in runtime-i18n.cc for case mapping was wrong to
assume that the longest primary language tag is 3 characters.
BCP 47 actually allows up to 8 characters.

2. GetFlatContent() was called to a string without flattening it first.

BUG=680314,680464
TEST=intl/general/case-mapping (see also the bugs)

Review-Url: https://codereview.chromium.org/2629763003
Cr-Commit-Position: refs/heads/master@{#42343}
Committed:
https://chromium.googlesource.com/v8/v8/+/ac9e628539eec41ebdd7142eebb324683df...
==========

[commit-bot: I haz the power, 2017-01-13 23:12:49]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/v8/v8/+/ac9e628539eec41ebdd7142eebb324683df...