Bug 142531 – [Win] 17 different JSC tests started to fail in DST

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Csaba Osztrogonác]

Since daylight saving time is started in CA, 17 different date 
tests started to fail on Windows bots. (Apple Windows + WinCairo)

jsc-layout-tests.yaml/js/script-tests/date-parse-comments-test.js.layout
jsc-layout-tests.yaml/js/script-tests/date-parse-test.js.layout
jsc-layout-tests.yaml/js/script-tests/date-set-to-nan.js.layout
mozilla-tests.yaml/ecma/Date/15.9.3.1-1.js.mozilla
mozilla-tests.yaml/ecma/Date/15.9.3.1-2.js.mozilla
mozilla-tests.yaml/ecma/Date/15.9.3.1-3.js.mozilla
mozilla-tests.yaml/ecma/Date/15.9.3.1-4.js.mozilla
mozilla-tests.yaml/ecma/Date/15.9.3.1-5.js.mozilla
mozilla-tests.yaml/ecma/Date/15.9.3.2-1.js.mozilla
mozilla-tests.yaml/ecma/Date/15.9.3.2-2.js.mozilla
mozilla-tests.yaml/ecma/Date/15.9.3.2-3.js.mozilla
mozilla-tests.yaml/ecma/Date/15.9.3.2-4.js.mozilla
mozilla-tests.yaml/ecma/Date/15.9.3.2-5.js.mozilla
mozilla-tests.yaml/ecma/Date/15.9.4.2-1.js.mozilla
mozilla-tests.yaml/ecma/Date/15.9.5.26-1.js.mozilla
mozilla-tests.yaml/ecma/Date/15.9.5.27-1.js.mozilla
mozilla-tests.yaml/ecma/Date/15.9.5.30-1.js.mozilla

similar bugs: bug138303 and bug139123

This might be wrong, but I get zero stress test failures, with the diff below.
It seems the documentation for GetTimeZoneInformation can be interpreted as the DST bias is already included in the bias member.
Maybe someone can confirm?


Index: DateMath.cpp
===================================================================
--- DateMath.cpp        (revisjon 181279)
+++ DateMath.cpp        (arbeidskopi)
@@ -369,11 +369,7 @@
     if (rc == TIME_ZONE_ID_INVALID)
         return 0;

-    int32_t bias = 0;
-    if (rc == TIME_ZONE_ID_DAYLIGHT)
-        bias = timeZoneInformation.Bias + timeZoneInformation.DaylightBias;
-    else if (rc == TIME_ZONE_ID_STANDARD || rc == TIME_ZONE_ID_UNKNOWN)
-        bias = timeZoneInformation.Bias + timeZoneInformation.StandardBias;
+    int32_t bias = timeZoneInformation.Bias;

     return -bias * 60 * 1000;
 #else

[Alex Christensen]

I think you're right.  Put up a patch and let Brent double check it.  He wrote the code in question about 6 months ago, probably last time daylight savings time changed.

Created attachment 248354 [details]
Patch

[Brent Fulgham]

(In reply to comment #2)
> I think you're right.  Put up a patch and let Brent double check it.  He
> wrote the code in question about 6 months ago, probably last time daylight
> savings time changed.

I don't think this is right. The calculation was working properly while we were in DST, so at the very least I would expect the 'correct' code to still do the Bias calculation including the "DaylightBias" value.

According to <https://msdn.microsoft.com/en-us/library/windows/desktop/ms725481(v=vs.85).aspx>, StandardBias is normally zero. So "timeZoneInformation.Bias + timeZoneInformation.StandardBias" during standard time is equivalent to "timeZoneInformation.Bias".

[Alex Christensen]

Well the current calculation is wrong right now.  I wish there were an easy way to test this without waiting 6 months to see how things change.  We should test this in Arizona, too :)

[Brent Fulgham]

(In reply to comment #5)
> Well the current calculation is wrong right now.  I wish there were an easy
> way to test this without waiting 6 months to see how things change.  We
> should test this in Arizona, too :)

The current calculation is failing because 'GetTimeZoneInformation' is returning TIME_ZONE_ID_DAYLIGHT, instead of TIME_ZONE_ID_STANDARD.

The date that DST ends varies from year-to-year. Apparently this didn't get updated on Windows, or maybe the function I'm calling isn't smart enough to account for this.

According to MSDN, there is another function (in Vista and newer) that should do the right thing. I'll have to 'soft link' to it, since we don't know what OS we are running on, but that might fix it.

I'll give it a try and see.

[Brent Fulgham]

Created attachment 248384 [details]
Patch

[Brent Fulgham]

Created attachment 248385 [details]
Patch

[Chris Dumez]

Comment on attachment 248385 [details]
Patch

r=me

[Brent Fulgham]

Committed r181360: <http://trac.webkit.org/changeset/181360>

(In reply to comment #6)
> (In reply to comment #5)
> > Well the current calculation is wrong right now.  I wish there were an easy
> > way to test this without waiting 6 months to see how things change.  We
> > should test this in Arizona, too :)
> 
> The current calculation is failing because 'GetTimeZoneInformation' is
> returning TIME_ZONE_ID_DAYLIGHT, instead of TIME_ZONE_ID_STANDARD.
> 
> The date that DST ends varies from year-to-year. Apparently this didn't get
> updated on Windows, or maybe the function I'm calling isn't smart enough to
> account for this.
> 
> According to MSDN, there is another function (in Vista and newer) that
> should do the right thing. I'll have to 'soft link' to it, since we don't
> know what OS we are running on, but that might fix it.
> 
> I'll give it a try and see.

Great :) I thought DST already had started in the US (8. of March), but I guess that was not the case :)