Bug 172592 – Date should use historical data if it's available.

• Status: REOPENED
• Component: JavaScriptCore

[Keith Miller]

Date should use historical data if it's available.

[Keith Miller]

Created attachment 311223 [details]
Patch

[Keith Miller]

rdar://problem/30085692

[Sam Weinig]

Comment on attachment 311223 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=311223&action=review

> JSTests/mozilla/ecma/Date/15.9.5.31-1.js:96
> -
> +/*
>      addNewTestCase( 946684800000, 1234567, void 0, void 0, void 0,
>                      "TDATE = new Date(946684800000);(TDATE).setUTCHours(1234567);TDATE",
>                      UTCDateFromTime(SetUTCHours(946684800000,1234567)),
>                      LocalDateFromTime(SetUTCHours(946684800000,1234567)) );
> -
> +*/

If you want to keep these commented out, you should at least add an explanation about why you are commenting it out.

> JSTests/mozilla/ecma/Date/15.9.5.35-1.js:64
> +/*
>      addNewTestCase( "TDATE = new Date(0);(TDATE).setUTCMonth(3,4);TDATE",
>                      UTCDateFromTime(SetUTCMonth(0,3,4)),
>                      LocalDateFromTime(SetUTCMonth(0,3,4)) );
> -
> +*/

If you want to keep these commented out, you should at least add an explanation about why you are commenting it out.

[Keith Miller]

Comment on attachment 311223 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=311223&action=review

>> JSTests/mozilla/ecma/Date/15.9.5.31-1.js:96
>> +*/
> 
> If you want to keep these commented out, you should at least add an explanation about why you are commenting it out.

Sounds good.

>> JSTests/mozilla/ecma/Date/15.9.5.35-1.js:64
>> +*/
> 
> If you want to keep these commented out, you should at least add an explanation about why you are commenting it out.

Ditto.

[Keith Miller]

Created attachment 311227 [details]
Patch

[Mark Lam]

Comment on attachment 311227 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=311227&action=review

> Source/WTF/wtf/DateMath.cpp:-514
> -#if HAVE(TM_GMTOFF)
> -    double localToUTCTimeOffset = inputTimeType == LocalTime ? calculateUTCOffset() : 0;
> -#else
> -    double localToUTCTimeOffset = calculateUTCOffset();
> -#endif
>      if (inputTimeType == LocalTime)
> -        ms -= localToUTCTimeOffset;

Why remove the definition of localToUTCTimeOffset here?  The !HAVE(TM_GMTOFF) case below still uses localToUTCTimeOffset.  Removing this now results in a build error.

Also, the HAVE(TM_GMTOFF) case exhibits different behavior from the !HAVE(TM_GMTOFF) case.  Is this no longer needed?  I don't see a correlation between this and the purpose of this patch.  The fixup that you removed below does not use localToUTCTimeOffset.  Can you clarify?

[Keith Miller]

Created attachment 311236 [details]
Patch

[Mark Lam]

Comment on attachment 311236 [details]
Patch

r=me

[Keith Miller]

(In reply to Mark Lam from comment #6)
> Comment on attachment 311227 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=311227&action=review
> 
> > Source/WTF/wtf/DateMath.cpp:-514
> > -#if HAVE(TM_GMTOFF)
> > -    double localToUTCTimeOffset = inputTimeType == LocalTime ? calculateUTCOffset() : 0;
> > -#else
> > -    double localToUTCTimeOffset = calculateUTCOffset();
> > -#endif
> >      if (inputTimeType == LocalTime)
> > -        ms -= localToUTCTimeOffset;
> 
> Why remove the definition of localToUTCTimeOffset here?  The
> !HAVE(TM_GMTOFF) case below still uses localToUTCTimeOffset.  Removing this
> now results in a build error.
> 
> Also, the HAVE(TM_GMTOFF) case exhibits different behavior from the
> !HAVE(TM_GMTOFF) case.  Is this no longer needed?  I don't see a correlation
> between this and the purpose of this patch.  The fixup that you removed
> below does not use localToUTCTimeOffset.  Can you clarify?

Yeah, I missed the other use of localToUTCTimeOffset below. I'll undo that change.

[Build Bot]

Comment on attachment 311236 [details]
Patch

Attachment 311236 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3815111

New failing tests:
storage/indexeddb/modern/get-keyrange-private.html
storage/indexeddb/modern/get-keyrange.html
storage/indexeddb/modern/date-basic.html
js/dom/date-big-setdate.html
storage/indexeddb/modern/date-basic-private.html

[Build Bot]

Created attachment 311244 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 311236 [details]
Patch

Attachment 311236 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3815192

New failing tests:
storage/indexeddb/modern/get-keyrange-private.html
storage/indexeddb/modern/get-keyrange.html
storage/indexeddb/modern/date-basic.html
js/dom/date-big-setdate.html
storage/indexeddb/modern/date-basic-private.html

[Build Bot]

Created attachment 311248 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Keith Miller]

Grumble grumble, I'm not on PST so these tests didn't run...

[Build Bot]

Comment on attachment 311236 [details]
Patch

Attachment 311236 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3815238

New failing tests:
js/dom/date-big-setdate.html

[Build Bot]

Created attachment 311252 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[Keith Miller]

Created attachment 311260 [details]
Patch for landing

[Build Bot]

Comment on attachment 311260 [details]
Patch for landing

Attachment 311260 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3815868

New failing tests:
js/dom/date-big-setdate.html
fetch/closing-while-fetching-blob.html

[Build Bot]

Created attachment 311268 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 311260 [details]
Patch for landing

Attachment 311260 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3815928

New failing tests:
js/dom/date-big-setdate.html

[Build Bot]

Created attachment 311272 [details]
Archive of layout-test-results from ews102 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 311260 [details]
Patch for landing

Attachment 311260 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3815912

New failing tests:
js/dom/date-big-setdate.html

[Build Bot]

Created attachment 311275 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 311260 [details]
Patch for landing

Attachment 311260 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3815920

New failing tests:
js/dom/date-big-setdate.html

[Build Bot]

Created attachment 311277 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[Keith Miller]

Created attachment 311312 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 311312 [details]
Patch for landing

Clearing flags on attachment: 311312

Committed r217458: <http://trac.webkit.org/changeset/217458>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

(In reply to WebKit Commit Bot from comment #27)
> Comment on attachment 311312 [details]
> Patch for landing
> 
> Clearing flags on attachment: 311312
> 
> Committed r217458: <http://trac.webkit.org/changeset/217458>

It made 55 JSC tests fail on Apple Windows and Linux bots, see bug172637 for details.

[Ryan Haddad]

Reverted r217458 for reason:

This change caused 55 JSC test failures.

Committed r217499: <http://trac.webkit.org/changeset/217499>

[Ryan Haddad]

*** Bug 172637 has been marked as a duplicate of this bug. ***

[Ryan Haddad]

See also rdar://problem/32429088