[intl] Fall back on an invalid default locale to "und"

[intl] Fall back on an invalid default locale to "und"

The default locale can be changed in some environments with environment
variables. These environment variables used to allow the system to get
into an invalid state, where the default locale was unsupported. This
patch detects that case and falls back to "und" as the default locale
if there is an Intl service which does not support the locale that ICU
reports as the default. It also has a slight cleanup of surrounding code.

I haven't gone through the work to set up an automated test, as triggering
the case requires setting environment variables, which our tests don't tend
to do, but I tested interactively as follows:

dehrenberg@dehrenberg:~/v8/v8$ LC_ALL="tlh-FR" rlwrap out/Release/d8
V8 version 5.7.0 (candidate)
d8> new Intl.NumberFormat("foo").resolvedOptions().locale
"und"
d8> new Intl.NumberFormat().resolvedOptions().locale
"und"
d8>

dehrenberg@dehrenberg:~/v8/v8$ LC_ALL="de" rlwrap out/Release/d8
V8 version 5.7.0 (candidate)
d8> new Intl.NumberFormat().resolvedOptions().locale
"de"
d8> new Intl.NumberFormat("foo").resolvedOptions().locale
"de"
d8>

BUG=v8:4216
Review-Url: https://codereview.chromium.org/2646593002
Cr-Commit-Position: refs/heads/master@{#43253}
Committed: https://chromium.googlesource.com/v8/v8/+/3059138b20bb56918cce24003832442c0ad701fa

[Dan Ehrenberg, 2017-01-18 20:28:56]

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

[commit-bot: I haz the power, 2017-01-18 20:29:04]

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

[Dan Ehrenberg, 2017-01-18 20:29:25]

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

[commit-bot: I haz the power, 2017-01-18 21:07:01]

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

[commit-bot: I haz the power, 2017-01-18 21:07:02]

Dry run: This issue passed the CQ dry run.

[jungshik at Google, 2017-01-19 19:16:22]

I have to think about this a bit more. Below is my initial response.

https://codereview.chromium.org/2646593002/diff/1/src/js/i18n.js
File src/js/i18n.js (right):

https://codereview.chromium.org/2646593002/diff/1/src/js/i18n.js#newcode159
src/js/i18n.js:159: DEFAULT_ICU_LOCALE = "en";
Hmm... I'm not sure of falling back to 'en'. How about 'und'?

[Dan Ehrenberg, 2017-02-10 08:53:39]

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

[commit-bot: I haz the power, 2017-02-10 08:53:50]

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

[Dan Ehrenberg, 2017-02-10 08:54:00]

Description was changed from

==========
[intl] Fall back on an invalid default locale to "en"

The default locale can be changed in some environments with environment
variables. These environment variables used to allow the system to get
into an invalid state, where the default locale was unsupported. This
patch detects that case and falls back to "en" as the default locale
if there is an Intl service which does not support the locale that ICU
reports as the default. It also has a slight cleanup of surrounding code.

I haven't gone through the work to set up an automated test, as triggering
the case requires setting environment variables, which our tests don't tend
to do, but I tested interactively as follows:

dehrenberg@dehrenberg:~/v8/v8$ LC_ALL="tlh-FR" rlwrap out/Release/d8
V8 version 5.7.0 (candidate)
d8> new Intl.NumberFormat("foo").resolvedOptions().locale
"en"
d8> new Intl.NumberFormat().resolvedOptions().locale
"en"
d8>

dehrenberg@dehrenberg:~/v8/v8$ LC_ALL="de" rlwrap out/Release/d8
V8 version 5.7.0 (candidate)
d8> new Intl.NumberFormat().resolvedOptions().locale
"de"
d8> new Intl.NumberFormat("foo").resolvedOptions().locale
"de"
d8>

BUG=v8:4216
==========

to

==========
[intl] Fall back on an invalid default locale to "und"

The default locale can be changed in some environments with environment
variables. These environment variables used to allow the system to get
into an invalid state, where the default locale was unsupported. This
patch detects that case and falls back to "und" as the default locale
if there is an Intl service which does not support the locale that ICU
reports as the default. It also has a slight cleanup of surrounding code.

I haven't gone through the work to set up an automated test, as triggering
the case requires setting environment variables, which our tests don't tend
to do, but I tested interactively as follows:

dehrenberg@dehrenberg:~/v8/v8$ LC_ALL="tlh-FR" rlwrap out/Release/d8
V8 version 5.7.0 (candidate)
d8> new Intl.NumberFormat("foo").resolvedOptions().locale
"und"
d8> new Intl.NumberFormat().resolvedOptions().locale
"und"
d8>

dehrenberg@dehrenberg:~/v8/v8$ LC_ALL="de" rlwrap out/Release/d8
V8 version 5.7.0 (candidate)
d8> new Intl.NumberFormat().resolvedOptions().locale
"de"
d8> new Intl.NumberFormat("foo").resolvedOptions().locale
"de"
d8>

BUG=v8:4216
==========

[Dan Ehrenberg, 2017-02-10 08:54:28]

https://codereview.chromium.org/2646593002/diff/1/src/js/i18n.js
File src/js/i18n.js (right):

https://codereview.chromium.org/2646593002/diff/1/src/js/i18n.js#newcode159
src/js/i18n.js:159: DEFAULT_ICU_LOCALE = "en";
On 2017/01/19 19:16:22, jungshik at Google wrote:
> Hmm... I'm not sure of falling back to 'en'. How about 'und'? 

Done.

[commit-bot: I haz the power, 2017-02-10 09:15:10]

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

[commit-bot: I haz the power, 2017-02-10 09:15:11]

Dry run: This issue passed the CQ dry run.

[jungshik at Google, 2017-02-15 18:37:58]

LGTM. Thanks.

Just to make sure, have you tried formatting numbers and date when the locale is
'und' because service does not support the default locale (e.g. 'tlh-FR').

[Dan Ehrenberg, 2017-02-15 18:47:45]

On 2017/02/15 18:37:58, jungshik at Google wrote:
> LGTM. Thanks.
> 
> Just to make sure, have you tried formatting numbers and date when the locale
is
> 'und' because service does not support the default locale (e.g. 'tlh-FR').

I just checked, and it seems to fall back to something, and NumberFormat and
DateTimeFormat do something with the "und" locale.

[Dan Ehrenberg, 2017-02-15 18:47:53]

The CQ bit was checked by littledan@chromium.org

[commit-bot: I haz the power, 2017-02-15 18:47:58]

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-02-15 18:47:59]

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

[commit-bot: I haz the power, 2017-02-15 18:48:00]

No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
Committers are members of the group "project-v8-committers".
Note that this has nothing to do with OWNERS files.

[Dan Ehrenberg, 2017-02-15 18:51:22]

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

[Dan Ehrenberg, 2017-02-15 18:52:57]

Yang, could you take a look?

[Yang, 2017-02-16 14:13:01]

On 2017/02/15 18:52:57, Dan Ehrenberg wrote:
> Yang, could you take a look?

lgtm.

[Dan Ehrenberg, 2017-02-16 14:15:24]

The CQ bit was checked by littledan@chromium.org

[commit-bot: I haz the power, 2017-02-16 14:15:36]

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-02-16 14:41:02]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1487254524577460,
"parent_rev": "ae8f28208f2bd41cee49a4db5a7b5b5b92114842", "commit_rev":
"3059138b20bb56918cce24003832442c0ad701fa"}

[commit-bot: I haz the power, 2017-02-16 14:41:07]

Description was changed from

==========
[intl] Fall back on an invalid default locale to "und"

The default locale can be changed in some environments with environment
variables. These environment variables used to allow the system to get
into an invalid state, where the default locale was unsupported. This
patch detects that case and falls back to "und" as the default locale
if there is an Intl service which does not support the locale that ICU
reports as the default. It also has a slight cleanup of surrounding code.

I haven't gone through the work to set up an automated test, as triggering
the case requires setting environment variables, which our tests don't tend
to do, but I tested interactively as follows:

dehrenberg@dehrenberg:~/v8/v8$ LC_ALL="tlh-FR" rlwrap out/Release/d8
V8 version 5.7.0 (candidate)
d8> new Intl.NumberFormat("foo").resolvedOptions().locale
"und"
d8> new Intl.NumberFormat().resolvedOptions().locale
"und"
d8>

dehrenberg@dehrenberg:~/v8/v8$ LC_ALL="de" rlwrap out/Release/d8
V8 version 5.7.0 (candidate)
d8> new Intl.NumberFormat().resolvedOptions().locale
"de"
d8> new Intl.NumberFormat("foo").resolvedOptions().locale
"de"
d8>

BUG=v8:4216
==========

to

==========
[intl] Fall back on an invalid default locale to "und"

The default locale can be changed in some environments with environment
variables. These environment variables used to allow the system to get
into an invalid state, where the default locale was unsupported. This
patch detects that case and falls back to "und" as the default locale
if there is an Intl service which does not support the locale that ICU
reports as the default. It also has a slight cleanup of surrounding code.

I haven't gone through the work to set up an automated test, as triggering
the case requires setting environment variables, which our tests don't tend
to do, but I tested interactively as follows:

dehrenberg@dehrenberg:~/v8/v8$ LC_ALL="tlh-FR" rlwrap out/Release/d8
V8 version 5.7.0 (candidate)
d8> new Intl.NumberFormat("foo").resolvedOptions().locale
"und"
d8> new Intl.NumberFormat().resolvedOptions().locale
"und"
d8>

dehrenberg@dehrenberg:~/v8/v8$ LC_ALL="de" rlwrap out/Release/d8
V8 version 5.7.0 (candidate)
d8> new Intl.NumberFormat().resolvedOptions().locale
"de"
d8> new Intl.NumberFormat("foo").resolvedOptions().locale
"de"
d8>

BUG=v8:4216

Review-Url: https://codereview.chromium.org/2646593002
Cr-Commit-Position: refs/heads/master@{#43253}
Committed:
https://chromium.googlesource.com/v8/v8/+/3059138b20bb56918cce24003832442c0ad...
==========

[commit-bot: I haz the power, 2017-02-16 14:41:09]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/v8/v8/+/3059138b20bb56918cce24003832442c0ad...