[builtins] Apply ES2016 conditional default timezone semantics

[builtins] Apply ES2016 conditional default timezone semantics

ES2016 changed the default timezone of dates to be conditional on
whether a time is included. The semantics were a compromise approach
based on web compatibility feedback from V8, but until now, we have been
shipping ES5.1 default timezone semantics. This patch implements the
new semantics, following ChakraCore and SpiderMonkey (though JSC
implements V8's previous semantics).

BUG=chromium:589858
Review-Url: https://codereview.chromium.org/2648603002
Cr-Commit-Position: refs/heads/master@{#43239}
Committed: https://chromium.googlesource.com/v8/v8/+/d31c5410c4fdfc5eb66582892d5e3ecd3706bd58

[Dan Ehrenberg, 2017-02-15 12:05:13]

Description was changed from

==========
WIP [builtins] Apply ES2016 conditional default timezone semantics

ES2016 changed the default timezone of dates to be conditional on
whether a time is included. The semantics were a compromise approach
based on web compatibility feedback from V8, but until now, we have been
shipping ES5.1 default timezone semantics. This patch implements the
new semantics.

Tests in progress.

BUG=chromium:589858
==========

to

==========
[builtins] Apply ES2016 conditional default timezone semantics

ES2016 changed the default timezone of dates to be conditional on
whether a time is included. The semantics were a compromise approach
based on web compatibility feedback from V8, but until now, we have been
shipping ES5.1 default timezone semantics. This patch implements the
new semantics, following ChakraCore and SpiderMonkey (though JSC
implements V8's semantics).

BUG=chromium:589858
==========

[Dan Ehrenberg, 2017-02-15 12:05:13]

littledan@chromium.org changed reviewers:
+ adamk@chromium.org, ulan@chromium.org

[Dan Ehrenberg, 2017-02-15 12:05:26]

Description was changed from

==========
[builtins] Apply ES2016 conditional default timezone semantics

ES2016 changed the default timezone of dates to be conditional on
whether a time is included. The semantics were a compromise approach
based on web compatibility feedback from V8, but until now, we have been
shipping ES5.1 default timezone semantics. This patch implements the
new semantics, following ChakraCore and SpiderMonkey (though JSC
implements V8's semantics).

BUG=chromium:589858
==========

to

==========
[builtins] Apply ES2016 conditional default timezone semantics

ES2016 changed the default timezone of dates to be conditional on
whether a time is included. The semantics were a compromise approach
based on web compatibility feedback from V8, but until now, we have been
shipping ES5.1 default timezone semantics. This patch implements the
new semantics, following ChakraCore and SpiderMonkey (though JSC
implements V8's previous semantics).

BUG=chromium:589858
==========

[Dan Ehrenberg, 2017-02-15 12:07:40]

What do you think? This patch has some compat risk; should it be developed
behind a flag and shipped that way?

[adamk, 2017-02-15 19:12:44]

Do you know whether the ChakraCore and SpiderMonkey semantics have been in
shipping versions of the respective browsers? If so, I'd be comfortable with
landing this (and don't think a flag is much of a win).

[Dan Ehrenberg, 2017-02-15 19:45:36]

On 2017/02/15 19:12:44, adamk wrote:
> Do you know whether the ChakraCore and SpiderMonkey semantics have been in
> shipping versions of the respective browsers? If so, I'd be comfortable with
> landing this (and don't think a flag is much of a win).

It seems to be on in Firefox 51.

I don't currently have a setup for testing Edge, but by my reading of the code,
it seems like the logic to implement this was in the original ChakraCore
check-in from last year.

[adamk, 2017-02-15 21:29:34]

Edge 13 has the same behavior as Firefox 51. This lgtm (though I'd be interested
if ulan has any input, given the history here).

[ulan, 2017-02-15 21:35:04]

Note that the last time we changed this, Firefox and IE were already supporting
the new semantics (https://bugs.chromium.org/p/v8/issues/detail?id=4242#c5), but
we had to revert anyway due to breakage.
Do you think this time we'll have more luck?

In any case lgtm from my side since this is what the spec defines. [although I
think that Date.parse('2000-01-01') !== Date.parse('2000-01-01T00:00:00') is
super-confusing].

https://codereview.chromium.org/2648603002/diff/40001/test/mjsunit/date-parse.js
File test/mjsunit/date-parse.js (right):

https://codereview.chromium.org/2648603002/diff/40001/test/mjsunit/date-parse...
test/mjsunit/date-parse.js:262: // If the timezone is absent and time is
present, use local time
The array is named "testCasesES5Misc" but this is testing ES2016 semantics.

Should we rename the array or move these test to separate array?

[Dan Ehrenberg, 2017-02-15 22:22:39]

On 2017/02/15 21:35:04, ulan wrote:
> Note that the last time we changed this, Firefox and IE were already
supporting
> the new semantics 

If I'm reading the history right, last time we tried to always make the change,
and this time the scope is more limited. The spec was changed in response to our
complaint about compatibility.

(https://bugs.chromium.org/p/v8/issues/detail?id=4242#c5), but
> we had to revert anyway due to breakage.
> Do you think this time we'll have more luck?
> 
> In any case lgtm from my side since this is what the spec defines. [although I
> think that Date.parse('2000-01-01') !== Date.parse('2000-01-01T00:00:00') is
> super-confusing].
> 

I agree personally, but I think the logic was that it would be better to change
to the 'right' answer sometimes than not at all.
>
https://codereview.chromium.org/2648603002/diff/40001/test/mjsunit/date-parse.js
> File test/mjsunit/date-parse.js (right):
> 
>
https://codereview.chromium.org/2648603002/diff/40001/test/mjsunit/date-parse...
> test/mjsunit/date-parse.js:262: // If the timezone is absent and time is
> present, use local time
> The array is named "testCasesES5Misc" but this is testing ES2016 semantics.
> 
> Should we rename the array or move these test to separate array?

You're right, I'll clean that up before landing.

[Dan Ehrenberg, 2017-02-16 09:35:16]

The CQ bit was checked by littledan@chromium.org

[Dan Ehrenberg, 2017-02-16 09:35:17]

The patchset sent to the CQ was uploaded after l-g-t-m from ulan@chromium.org,
adamk@chromium.org
Link to the patchset: https://codereview.chromium.org/2648603002/#ps60001
(title: "Clean up tests")

[commit-bot: I haz the power, 2017-02-16 09:35:25]

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 10:02:01]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1487237716935500,
"parent_rev": "d21621cf35cfcbc4e18ffa4dd45f69ab283b4669", "commit_rev":
"d31c5410c4fdfc5eb66582892d5e3ecd3706bd58"}

[commit-bot: I haz the power, 2017-02-16 10:02:06]

Description was changed from

==========
[builtins] Apply ES2016 conditional default timezone semantics

ES2016 changed the default timezone of dates to be conditional on
whether a time is included. The semantics were a compromise approach
based on web compatibility feedback from V8, but until now, we have been
shipping ES5.1 default timezone semantics. This patch implements the
new semantics, following ChakraCore and SpiderMonkey (though JSC
implements V8's previous semantics).

BUG=chromium:589858
==========

to

==========
[builtins] Apply ES2016 conditional default timezone semantics

ES2016 changed the default timezone of dates to be conditional on
whether a time is included. The semantics were a compromise approach
based on web compatibility feedback from V8, but until now, we have been
shipping ES5.1 default timezone semantics. This patch implements the
new semantics, following ChakraCore and SpiderMonkey (though JSC
implements V8's previous semantics).

BUG=chromium:589858

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

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

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/v8/v8/+/d31c5410c4fdfc5eb66582892d5e3ecd370...