Bug 172592

Summary:

Date should use historical data if it's available.

Product:

WebKit

Reporter:

Keith Miller <keith_miller>

Component:

JavaScriptCore

Assignee:

Keith Miller <keith_miller>

Status:

REOPENED

Severity:

Normal

CC:

alecflett, beidson, benjamin, buildbot, cdumez, cgarcia, cmarcelo, commit-queue, dbates, jsbell, mark.lam, ossy, rniwa, ryanhaddad, sam, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

172637

Bug Blocks:

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Archive of layout-test-results from ews103 for mac-elcapitan	none
Archive of layout-test-results from ews104 for mac-elcapitan-wk2	none
Archive of layout-test-results from ews126 for ios-simulator-wk2	none
Patch for landing	none
Archive of layout-test-results from ews104 for mac-elcapitan-wk2	none
Archive of layout-test-results from ews102 for mac-elcapitan	none
Archive of layout-test-results from ews116 for mac-elcapitan	none
Archive of layout-test-results from ews126 for ios-simulator-wk2	none
Patch for landing	none

Keith Miller

Reported 2017-05-25 08:35:19 PDT

Date should use historical data if it's available.

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
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Keith Miller

Comment 1 2017-05-25 08:53:20 PDT

Created attachment 311223 [details] Patch

Keith Miller

Comment 2 2017-05-25 08:59:40 PDT

rdar://problem/30085692

Sam Weinig

Comment 3 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.

Keith Miller

Comment 4 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.

Keith Miller

Comment 5 2017-05-25 09:23:12 PDT

Created attachment 311227 [details] Patch

Mark Lam

Comment 6 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?

Keith Miller

Comment 7 2017-05-25 09:58:19 PDT

Created attachment 311236 [details] Patch

Mark Lam

Comment 8 2017-05-25 10:01:04 PDT

Comment on attachment 311236 [details] Patch r=me

Keith Miller

Comment 9 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.

Build Bot

Comment 10 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

Build Bot

Comment 11 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

Build Bot

Comment 12 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

Build Bot

Comment 13 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

Keith Miller

Comment 14 2017-05-25 11:03:14 PDT

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

Build Bot

Comment 15 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

Build Bot

Comment 16 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

Keith Miller

Comment 17 2017-05-25 12:14:53 PDT

Created attachment 311260 [details] Patch for landing

Build Bot

Comment 18 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

Build Bot

Comment 19 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

Build Bot

Comment 20 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

Build Bot

Comment 21 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

Build Bot

Comment 22 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

Build Bot

Comment 23 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

Build Bot

Comment 24 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

Build Bot

Comment 25 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

Keith Miller

Comment 26 2017-05-25 16:29:09 PDT

Created attachment 311312 [details] Patch for landing

WebKit Commit Bot

Comment 27 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>

WebKit Commit Bot

Comment 28 2017-05-25 17:06:50 PDT

All reviewed patches have been landed. Closing bug.

Csaba Osztrogonác

Comment 29 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.

Ryan Haddad

Comment 30 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>

Ryan Haddad

Comment 31 2017-05-26 12:29:59 PDT

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

Ryan Haddad

Comment 32 2017-05-26 12:30:16 PDT