Bug 172592 - Date should use historical data if it's available.
Summary: Date should use historical data if it's available.
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Miller
URL:
Keywords: InRadar
: 172637 (view as bug list)
Depends on: 172637
Blocks:
  Show dependency treegraph
 
Reported: 2017-05-25 08:35 PDT by Keith Miller
Modified: 2019-06-16 21:30 PDT (History)
16 users (show)

See Also:


Attachments
Patch (5.73 KB, patch)
2017-05-25 08:53 PDT, Keith Miller
no flags Details | Formatted Diff | Diff
Patch (5.76 KB, patch)
2017-05-25 09:23 PDT, Keith Miller
no flags Details | Formatted Diff | Diff
Patch (5.46 KB, patch)
2017-05-25 09:58 PDT, Keith Miller
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-elcapitan (997.33 KB, application/zip)
2017-05-25 10:45 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 (896.81 KB, application/zip)
2017-05-25 11:01 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews126 for ios-simulator-wk2 (10.96 MB, application/zip)
2017-05-25 11:27 PDT, Build Bot
no flags Details
Patch for landing (14.08 KB, patch)
2017-05-25 12:14 PDT, Keith Miller
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 (1.06 MB, application/zip)
2017-05-25 12:53 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews102 for mac-elcapitan (1.27 MB, application/zip)
2017-05-25 13:01 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews116 for mac-elcapitan (1.72 MB, application/zip)
2017-05-25 13:13 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews126 for ios-simulator-wk2 (11.30 MB, application/zip)
2017-05-25 13:20 PDT, Build Bot
no flags Details
Patch for landing (17.10 KB, patch)
2017-05-25 16:29 PDT, Keith Miller
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Miller 2017-05-25 08:35:19 PDT
Date should use historical data if it's available.
Comment 1 Keith Miller 2017-05-25 08:53:20 PDT
Created attachment 311223 [details]
Patch
Comment 2 Keith Miller 2017-05-25 08:59:40 PDT
rdar://problem/30085692
Comment 3 Sam Weinig 2017-05-25 09:06:34 PDT
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.
Comment 4 Keith Miller 2017-05-25 09:21:02 PDT
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.
Comment 5 Keith Miller 2017-05-25 09:23:12 PDT
Created attachment 311227 [details]
Patch
Comment 6 Mark Lam 2017-05-25 09:54:17 PDT
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?
Comment 7 Keith Miller 2017-05-25 09:58:19 PDT
Created attachment 311236 [details]
Patch
Comment 8 Mark Lam 2017-05-25 10:01:04 PDT
Comment on attachment 311236 [details]
Patch

r=me
Comment 9 Keith Miller 2017-05-25 10:01:19 PDT
(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.
Comment 10 Build Bot 2017-05-25 10:45:50 PDT
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
Comment 11 Build Bot 2017-05-25 10:45:52 PDT
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
Comment 12 Build Bot 2017-05-25 11:01:44 PDT
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
Comment 13 Build Bot 2017-05-25 11:01:45 PDT
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
Comment 14 Keith Miller 2017-05-25 11:03:14 PDT
Grumble grumble, I'm not on PST so these tests didn't run...
Comment 15 Build Bot 2017-05-25 11:27:32 PDT
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
Comment 16 Build Bot 2017-05-25 11:27:34 PDT
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
Comment 17 Keith Miller 2017-05-25 12:14:53 PDT
Created attachment 311260 [details]
Patch for landing
Comment 18 Build Bot 2017-05-25 12:53:39 PDT
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
Comment 19 Build Bot 2017-05-25 12:53:40 PDT
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
Comment 20 Build Bot 2017-05-25 13:01:42 PDT
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
Comment 21 Build Bot 2017-05-25 13:01:43 PDT
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
Comment 22 Build Bot 2017-05-25 13:13:15 PDT
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
Comment 23 Build Bot 2017-05-25 13:13:17 PDT
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
Comment 24 Build Bot 2017-05-25 13:20:55 PDT
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
Comment 25 Build Bot 2017-05-25 13:20:57 PDT
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
Comment 26 Keith Miller 2017-05-25 16:29:09 PDT
Created attachment 311312 [details]
Patch for landing
Comment 27 WebKit Commit Bot 2017-05-25 17:06:48 PDT
Comment on attachment 311312 [details]
Patch for landing

Clearing flags on attachment: 311312

Committed r217458: <http://trac.webkit.org/changeset/217458>
Comment 28 WebKit Commit Bot 2017-05-25 17:06:50 PDT
All reviewed patches have been landed.  Closing bug.
Comment 29 Csaba Osztrogonác 2017-05-26 04:16:08 PDT
(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.
Comment 30 Ryan Haddad 2017-05-26 12:27:46 PDT
Reverted r217458 for reason:

This change caused 55 JSC test failures.

Committed r217499: <http://trac.webkit.org/changeset/217499>
Comment 31 Ryan Haddad 2017-05-26 12:29:59 PDT
*** Bug 172637 has been marked as a duplicate of this bug. ***
Comment 32 Ryan Haddad 2017-05-26 12:30:16 PDT
See also rdar://problem/32429088