Bug 138020 - A bunch of JS dates fail after r175078
Summary: A bunch of JS dates fail after r175078
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-10-23 14:04 PDT by Roger Fong
Modified: 2014-10-24 13:27 PDT (History)
7 users (show)

See Also:


Attachments
Patch (1.41 KB, patch)
2014-10-23 16:57 PDT, Byungseon(Sun) Shin
mark.lam: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Roger Fong 2014-10-23 14:04:38 PDT
Tests failing:
	ecma/Date/15.9.3.1-1.js
	ecma/Date/15.9.3.1-2.js
	ecma/Date/15.9.3.1-3.js
	ecma/Date/15.9.3.1-4.js
	ecma/Date/15.9.3.1-5.js
	ecma/Date/15.9.3.2-1.js
	ecma/Date/15.9.3.2-2.js
	ecma/Date/15.9.3.2-3.js
	ecma/Date/15.9.3.2-4.js
	ecma/Date/15.9.3.2-5.js
	ecma/Date/15.9.4.2-1.js
	ecma/Date/15.9.5.26-1.js
	ecma/Date/15.9.5.27-1.js
	ecma/Date/15.9.5.28-1.js
	ecma/Date/15.9.5.30-1.js

Skipping them here:
http://trac.webkit.org/changeset/175142
Comment 2 Mark Lam 2014-10-23 14:11:53 PDT
From https://build.webkit.org/results/Apple%20Win%207%20Debug%20(Tests)/r175080%20(63295)/results.html:
js/date-parse-comments-test.html
js/date-parse-test.html
js/date-parse-test.html
js/dom/callback-function-with-handle-event.html
js/dom/date-big-setmonth.html

Not sure if these are new to this patch, but should probably take a look into it.
Comment 3 Mark Lam 2014-10-23 14:23:11 PDT
Investigation notes: the following Date files have Windows specific implementations:

    WTF/wtf/DateMath.cpp
    WTF/wtf/GregorianDateTime.cpp
    JavaScriptCore/runtime/DateConversion.cpp
    JavaScriptCore/runtime/DatePrototype.cpp
Comment 4 Byungseon(Sun) Shin 2014-10-23 16:57:34 PDT
Created attachment 240378 [details]
Patch
Comment 5 Mark Lam 2014-10-23 17:01:57 PDT
Comment on attachment 240378 [details]
Patch

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

> Source/WTF/ChangeLog:10
> +        Do not change input time offset in case of Windows operating system.

This describes what you did but not why you did it.  Can you please explain the reasoning behind this?  What’s different about Windows such that this is needed?
Comment 6 Byungseon(Sun) Shin 2014-10-23 17:27:45 PDT
With a preliminary analysis, Widows returns default time milliseconds is based on UTC milliseconds unlike other OSs.
Comment 7 Mark Lam 2014-10-23 17:43:37 PDT
(In reply to comment #6)
> With a preliminary analysis, Widows returns default time milliseconds is
> based on UTC milliseconds unlike other OSs.

That’s nice to know, but it’s still not enough to tell us that this fix is correct and conclusive.  Let’s get more data on why Windows is behaving this way.

Also, if the timeType passed into calculateLocalTimeOffset() says that the incoming ms is in local time, then we would expect it to be in local time.  If this is not the case on Windows, then something upstream is not obeying the contract.  Hence, this is the wrong place to apply this fix.
Comment 8 Mark Lam 2014-10-23 17:45:22 PDT
Comment on attachment 240378 [details]
Patch

Per my previous comment, this is the wrong fix.  If Windows is passing in a UTC time value, then the fix must be somewhere up stream.  It should either pass in a timeType of UTCTime, or ms should be in local time.
Comment 9 Byungseon(Sun) Shin 2014-10-23 18:46:16 PDT
I will keep working on it and update it as soon as I will find a root cause.
Comment 10 Roger Fong 2014-10-24 13:09:15 PDT
If it helps, I also found some layout tests failing as a result:
  js/date-parse-comments-test.html [ Failure ]
  js/date-parse-test.html [ Failure ]
  js/date-set-to-nan.html [ Failure ]
  js/dom/date-big-setmonth.html [ Failure ]

I'll be skipping these as well.
Comment 11 Roger Fong 2014-10-24 13:27:50 PDT
(In reply to comment #10)
> If it helps, I also found some layout tests failing as a result:
>   js/date-parse-comments-test.html [ Failure ]
>   js/date-parse-test.html [ Failure ]
>   js/date-set-to-nan.html [ Failure ]
>   js/dom/date-big-setmonth.html [ Failure ]
> 
> I'll be skipping these as well.

Skipped in a general windows gardening patch here:
http://trac.webkit.org/changeset/175176