Allow ICU to normalize time zones

Allow ICU to normalize time zones

There's at least one case of a time zone alias: Asia/Kathmandu aliases
Asia/Katmandu. ICU seems to normalize to the (deprecated) latter choice.
V8 internationalization choked on this change; this patch interprets
ICU's output more precisely and allows it.

BUG=chromium:487322
R=jungshik,adamk
LOG=Y

Committed: https://crrev.com/bff3074d73fdcd946bdb55c94c2f43ef68195fcc
Cr-Commit-Position: refs/heads/master@{#32769}

[Dan Ehrenberg, 2015-12-10 01:00:20]

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

[commit-bot: I haz the power, 2015-12-10 01:00:24]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1509273007/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1509273007/1

[commit-bot: I haz the power, 2015-12-10 04:40:33]

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

[commit-bot: I haz the power, 2015-12-10 04:40:34]

Dry run: This issue passed the CQ dry run.

[adamk, 2015-12-10 17:33:13]

This change seems fine, but I'm a bit curious why we explicitly disallowed any
normalization before.

https://codereview.chromium.org/1509273007/diff/1/src/js/i18n.js
File src/js/i18n.js (left):

https://codereview.chromium.org/1509273007/diff/1/src/js/i18n.js#oldcode1577
src/js/i18n.js:1577: if (!IS_UNDEFINED(tz) && tz !== resolved.timeZone) {
I'd be curious why the old code was specifically checking for a match. Did you
find anything in archeology?

https://codereview.chromium.org/1509273007/diff/1/test/mjsunit/regress/regres...
File test/mjsunit/regress/regress-487322.js (right):

https://codereview.chromium.org/1509273007/diff/1/test/mjsunit/regress/regres...
test/mjsunit/regress/regress-487322.js:13: assertThrows(() =>
Intl.DateTimeFormat(undefined, {timeZone: 'Aurope/Belgrade'}));
Nit: 80 char limit (misspell shorter :) )

[Dan Ehrenberg, 2015-12-10 17:55:36]

https://codereview.chromium.org/1509273007/diff/1/src/js/i18n.js
File src/js/i18n.js (left):

https://codereview.chromium.org/1509273007/diff/1/src/js/i18n.js#oldcode1577
src/js/i18n.js:1577: if (!IS_UNDEFINED(tz) && tz !== resolved.timeZone) {
On 2015/12/10 at 17:33:13, adamk wrote:
> I'd be curious why the old code was specifically checking for a match. Did you
find anything in archeology?

Not sure; it's been there since Jochen's snapshot. I was assuming it's because
this is a way of checking which works 99% of the time and makes intuitive sense
(why should it change, anyway? you'd think).

https://codereview.chromium.org/1509273007/diff/1/test/mjsunit/regress/regres...
File test/mjsunit/regress/regress-487322.js (right):

https://codereview.chromium.org/1509273007/diff/1/test/mjsunit/regress/regres...
test/mjsunit/regress/regress-487322.js:13: assertThrows(() =>
Intl.DateTimeFormat(undefined, {timeZone: 'Aurope/Belgrade'}));
On 2015/12/10 at 17:33:13, adamk wrote:
> Nit: 80 char limit (misspell shorter :) )

Done

[adamk, 2015-12-10 19:34:08]

lgtm

https://codereview.chromium.org/1509273007/diff/1/src/js/i18n.js
File src/js/i18n.js (left):

https://codereview.chromium.org/1509273007/diff/1/src/js/i18n.js#oldcode1577
src/js/i18n.js:1577: if (!IS_UNDEFINED(tz) && tz !== resolved.timeZone) {
On 2015/12/10 17:55:36, Dan Ehrenberg wrote:
> On 2015/12/10 at 17:33:13, adamk wrote:
> > I'd be curious why the old code was specifically checking for a match. Did
you
> find anything in archeology?
> 
> Not sure; it's been there since Jochen's snapshot. I was assuming it's because
> this is a way of checking which works 99% of the time and makes intuitive
sense
> (why should it change, anyway? you'd think).

Given the lack of any test coverage for this if statement previously, I guess
the best we can do is give this a try.

[Dan Ehrenberg, 2015-12-10 19:36:48]

The CQ bit was checked by littledan@chromium.org

[commit-bot: I haz the power, 2015-12-10 19:37:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1509273007/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1509273007/20001

[commit-bot: I haz the power, 2015-12-10 20:13:27]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-12-10 20:13:52]

Description was changed from

==========
Allow ICU to normalize time zones

There's at least one case of a time zone alias: Asia/Kathmandu aliases
Asia/Katmandu. ICU seems to normalize to the (deprecated) latter choice.
V8 internationalization choked on this change; this patch interprets
ICU's output more precisely and allows it.

BUG=chromium:487322
R=jungshik,adamk
LOG=Y
==========

to

==========
Allow ICU to normalize time zones

There's at least one case of a time zone alias: Asia/Kathmandu aliases
Asia/Katmandu. ICU seems to normalize to the (deprecated) latter choice.
V8 internationalization choked on this change; this patch interprets
ICU's output more precisely and allows it.

BUG=chromium:487322
R=jungshik,adamk
LOG=Y

Committed: https://crrev.com/bff3074d73fdcd946bdb55c94c2f43ef68195fcc
Cr-Commit-Position: refs/heads/master@{#32769}
==========

[commit-bot: I haz the power, 2015-12-10 20:13:53]

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

[jungshik at Google, 2015-12-17 20:54:22]

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

[jungshik at Google, 2015-12-17 20:54:23]

Thanks, Dan and sorry that I missed this. 
This CL also fixed a part of the problem for http://crbug.com/364374.