| Summary: | A bunch of JS dates fail after r175078 | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Roger Fong <roger_fong> | ||||
| Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||
| Status: | NEW --- | ||||||
| Severity: | Normal | CC: | benjamin, cmarcelo, commit-queue, mark.lam, roger_fong, sun.shin, xingri | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Roger Fong
2014-10-23 14:04:38 PDT
See also failures here: https://build.webkit.org/results/Apple%20Win%207%20Debug%20(Tests)/r175080%20(63295)/results.html 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. 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
Created attachment 240378 [details]
Patch
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? With a preliminary analysis, Widows returns default time milliseconds is based on UTC milliseconds unlike other OSs. (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 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.
I will keep working on it and update it as soon as I will find a root cause. 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. (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 |